Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker-container: avoid fail if container conflict #2000

Merged
merged 1 commit into from Aug 21, 2023

Conversation

jedevc
Copy link
Collaborator

@jedevc jedevc commented Aug 10, 2023

Fixes the race condition where two boots are executed simultaneously across multiple processes.

Additionally, this would probably also fix #1868.

We initially check to see if the container exists, but if during container creation we get a name conflict, we don't treat this error as a hard failure, and instead move immediately into waiting for the node to boot.

Not 100% sure about this, we could alternatively attempt to get a file lock when attempting to create the docker container (@crazy-max's suggestion).

Fixes the race condition where two boots are executed simultaneously
across multiple processes.

We initially check to see if the container exists, but if during
container creation we get a name conflict, we don't treat this error as
a hard failure, and instead move immediately into waiting for the node
to boot.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM. What I was thinking with locking the instance file was to avoid race cond when bootstrapping but that should not be an issue for other drivers.

@jedevc
Copy link
Collaborator Author

jedevc commented Aug 10, 2023

I guess the file lock approach also doesn't work very well if the daemon is remote. e.g. the daemon is remote on a server, and multiple clients are accessing it.

@crazy-max
Copy link
Member

I guess the file lock approach also doesn't work very well if the daemon is remote. e.g. the daemon is remote on a server, and multiple clients are accessing it.

Yes indeed. I wonder if we should have smth similar to what you have done here for k8s 🤔 (cc @AkihiroSuda)

@jedevc jedevc added this to the v0.12.0 milestone Aug 14, 2023
@crazy-max crazy-max merged commit a97e164 into docker:master Aug 21, 2023
59 checks passed
@jedevc jedevc deleted the fix-race-container-creation branch August 21, 2023 14:13
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.

docker-container boot fails with "container conflict" for multiple simultaneous builds
2 participants