[ci] add more docker groups to work with buildkite amis#53640
Merged
[ci] add more docker groups to work with buildkite amis#53640
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates Docker configurations to support multiple Buildkite AMI group IDs across different eras by adding and mapping new group entries.
- Add separate
docker0,docker1, and updateddockergroup GIDs for various AMI versions. - Ensure the
forgeuser is added to all relevant Docker groups. - Mirror these changes in the release-automation Dockerfile for x86_64.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ci/docker/forge.Dockerfile | Added docker0, docker1, and updated docker group GIDs and usermods. |
| .buildkite/release-automation/forge_x86_64.Dockerfile | Added docker1 group and usermod for the release automation container. |
Comments suppressed due to low confidence (2)
ci/docker/forge.Dockerfile:49
- [nitpick] The group name
docker1is ambiguous. Consider using a more descriptive name (e.g.,docker_legacy_993) or expand the comment to clearly explain its purpose.
addgroup --gid 993 docker1
ci/docker/forge.Dockerfile:50
- Adding the
dockergroup may fail if a group with that name already exists in the base image. Consider checking for its existence (e.g.,getent group docker || addgroup ...) or usinggroupmodto adjust its GID.
addgroup --gid 992 docker # buildkite AMI as of 2025-06-07
| # Needs to be synchronized to the host group id as we map /var/run/docker.sock | ||
| # into the container. | ||
| addgroup --gid 993 docker | ||
| addgroup --gid 992 docker1 # docker group on buildkite AMI as of 2025-06-07 |
There was a problem hiding this comment.
[nitpick] These group definitions are duplicated in multiple Dockerfiles. Consider extracting them into a shared script or parameterizing the GIDs via build arguments to reduce duplication and ease future updates.
different group names for buildkite amis from different eras Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
5b62439 to
1229b3d
Compare
Collaborator
Author
|
merging to fix CI builds. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
different group names for buildkite amis from different eras