fix: uniquify container names #746
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
replaces #651 |
mikemccracken
force-pushed
the
2025.11.24/main/uniquify-container-names-2
branch
from
450b031 to
69f86db
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #746 +/- ##
==========================================
+ Coverage 51.67% 51.83% +0.16%
==========================================
Files 59 59
Lines 6491 6492 +1
==========================================
+ Hits 3354 3365 +11
+ Misses 2489 2483 -6
+ Partials 648 644 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mikemccracken
requested review from
hallyn,
raharper,
rchamarthy,
rchincha,
smoser and
tych0
as code owners
mikemccracken
changed the title
rchincha
approved these changes
Dec 10, 2025
Contributor
|
@mikemccracken there are some copilot reviews, pls take a look. |
mikemccracken
force-pushed
the
2025.11.24/main/uniquify-container-names-2
branch
from
69f86db to
5544203
Compare
Currently, if: concurrent stacker runs are happening on a system, and they are building containers with the same name, and they are being done inside a mount namespace, and the path name given as the roots dir is the same, but the actual mounted volume is different, then both stackers will be able to acquire the file lock at $rootdir/.lock, and will go ahead and start containers named $name, which will then race to set up the lxc control socket, which is named after the container name and the rootfs path, which are both the same here. This fix adds a UUID to the container name, which ensures the sockets do not clash. Adds a test 'concurrent.bats' to test this, and includes some other test changes to make it easier to run tests locally with a cached copy of the base images. Signed-off-by: Michael McCracken <[email protected]>
should be able to re-run 'make download-tools' and not have it fail Signed-off-by: Michael McCracken <[email protected]>
When debugging tests, if you comment out the cleanup in helpers, you end up with a bunch of stackertest- dirs which really gum up the output of search tools while you work, so ignore those dirs. Signed-off-by: Michael McCracken <[email protected]>
We do this elsewhere, should have had this in from the start Signed-off-by: Michael McCracken <[email protected]>
mikemccracken
force-pushed
the
2025.11.24/main/uniquify-container-names-2
branch
from
5544203 to
cf95fd0
Compare
Contributor
|
lgtm Will this need a minor release after merge? |
Contributor
Author
|
thanks - yes I'd like to get a release with this change, we can stabilize some internal builds that way
… On Dec 11, 2025, at 4:45 PM, Ramkumar Chinchani ***@***.***> wrote:
rchincha left a comment (project-stacker/stacker#746)
lgtm
Will this need a minor release after merge?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
bug... fix!
Which issue does this PR fix:
#645
In situations where concurrent stacker runs are happening on a system, and they are building containers with the same name, and they are being done inside a mount namespace,
and the path name given as the roots dir is the same, but the actual mounted volume is different,
then both stackers will be able to acquire the file lock at $rootdir/.lock, and will go ahead and start containers named $name, which will then race to set up the lxc control socket, which is named after the container name and the rootfs path, which are both the same here.
What does this PR do / Why do we need it:
The fix is to add some randomness to the lxc container name, which ensures that the socket won't clash. This does not affect other uses of the image name in the code, which will still use the un-randomized name.
Testing done on this change:
Manual testing, plus the PR adds
concurrent.batsto catch regressions.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.