Skip to content

fix(unikontainers): preserve slice order when removing element#678

Merged
cmainas merged 1 commit into
urunc-dev:main-pr678from
mdryaan:fix/remove-preserves-order
Jun 2, 2026
Merged

fix(unikontainers): preserve slice order when removing element#678
cmainas merged 1 commit into
urunc-dev:main-pr678from
mdryaan:fix/remove-preserves-order

Conversation

@mdryaan
Copy link
Copy Markdown
Contributor

@mdryaan mdryaan commented May 14, 2026

Description

remove() in pkg/unikontainers/utils.go removed an element by swapping it with the last element before truncating. This destroyed the ordering of all remaining elements. Both call sites are affected: handleQueueProxy (removes SERVING_READINESS_PROBE from spec.Process.Env) and the vAccel env cleanup in unikontainers.go (removes VACCEL_RPC_ADDRESS). Replace the swap-and-truncate with append(s[:i], s[i+1:]...) to preserve order.

Related issues

How was this tested?

Added TestRemovePreservesOrder to pkg/unikontainers/utils_test.go. The test
calls remove([]string{"a", "b", "c", "d"}, 0) and asserts the result is
["b", "c", "d"].

Before fix:

Screenshot 2026-05-14 083407

After fix:

Screenshot 2026-05-14 083605

Run with:
go test ./pkg/unikontainers/ -v -run TestRemovePreservesOrder

golangci-lint run passes with 0 issues. make builds successfully.

LLM usage

N/A

Checklist

  • I have read the contribution guide.
  • The linter passes locally (make lint).
  • The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
  • If LLMs were used: I have read the llm policy.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 14, 2026

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit 5a99b19
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a1ebdbfcbb63a0008273d95

@mdryaan
Copy link
Copy Markdown
Contributor Author

mdryaan commented May 14, 2026

Hey @cmainas , Could you please have a look at this PR whenever you have time? Thanks!

@cmainas
Copy link
Copy Markdown
Contributor

cmainas commented Jun 2, 2026

Hello @mdryaan ,

the PR looks good. Can you rebase over main?

Signed-off-by: mdryaan <alikhurshid842001@gmail.com>
@mdryaan mdryaan force-pushed the fix/remove-preserves-order branch from 05846d1 to 5a99b19 Compare June 2, 2026 11:25
@mdryaan
Copy link
Copy Markdown
Contributor Author

mdryaan commented Jun 2, 2026

Hello @mdryaan ,

the PR looks good. Can you rebase over main?

@cmainas rebased over main

@urunc-bot urunc-bot Bot changed the base branch from main to main-pr678 June 2, 2026 13:22
Copy link
Copy Markdown
Contributor

@cmainas cmainas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mdryaan for the fix.

@cmainas cmainas merged commit fb0b282 into urunc-dev:main-pr678 Jun 2, 2026
35 of 36 checks passed
github-actions Bot pushed a commit that referenced this pull request Jun 2, 2026
PR: #678
Signed-off-by: mdryaan <alikhurshid842001@gmail.com>
Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
urunc-bot Bot pushed a commit that referenced this pull request Jun 2, 2026
PR: #678
Signed-off-by: mdryaan <alikhurshid842001@gmail.com>
Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
@mdryaan
Copy link
Copy Markdown
Contributor Author

mdryaan commented Jun 2, 2026

Thanks for the review and merge @cmainas , could you please have a look on #727 #728 whenever you get a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve slice order when removing environment variable

2 participants