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

Calculate docker instance label based on the hash of the config #2683

Merged
merged 2 commits into from Sep 18, 2023

Conversation

nikola-jokic
Copy link
Member

Fixes #2647

The issue can occur when multiple runners are sharing the same docker socket, and they are executing within a container.
The well known directory is going to be the same for both runners, assuming they are executing the same image.

So when docker instance is calculated, the paths within the container are going to be exactly the same, one runner can remove resources owned by the other runner.

The fix uses the .runner file, which is unique per runner to calculate instance label, which should eliminate collision issues.

@nikola-jokic nikola-jokic requested a review from a team as a code owner July 7, 2023 13:25
Copy link
Collaborator

@fhammerl fhammerl left a comment

Choose a reason for hiding this comment

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

LGTM

This will make the docker instance label unique even if the runners sharing the same docker socket through docker-from-docker are under the same path in their respective containers.

Hashing the entire .runner file means that changes to this file (e.g. runnergroupname) will be reflected as a change in docker instance label as well - we found this acceptable as this change would happen outside of a running job, which means the docker network created based on this label in a previous job has been disposed.

@nikola-jokic nikola-jokic enabled auto-merge (squash) September 18, 2023 09:50
@nikola-jokic nikola-jokic merged commit 16834ed into main Sep 18, 2023
10 checks passed
@nikola-jokic nikola-jokic deleted the nikola-jokic/docker-instance-label-fix branch September 18, 2023 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

network label same for each workflows
2 participants