Add mount-buildkite-agent patch#2486
Conversation
🦋 Changeset detectedLatest commit: d5e4059 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…/skuba into add-mount-buildkite-agent-patch
| 'skuba': patch | ||
| --- | ||
|
|
||
| lint: Add patch to migrate Docker Compose Buildkite plugins to `mount-buildkite-agent` |
There was a problem hiding this comment.
Do we want to fix up Docker Buildkite plugin usage too?
steps:
- plugins:
- *docker-ecr-cache
- docker#v5.13.0:
- # Disable SEEK BuildAgency's wrapped agent that requires Bash.
- mount-buildkite-agent: false
+ mount-buildkite-agent: true
propagate-environment: true
volumes:
- # Mount agent for Buildkite annotations.
- - /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
- # Mount cached dependencies.
- /workdir/node_modules
KSHITIZ6341
left a comment
KSHITIZ6341
left a comment
There was a problem hiding this comment.
One migration edge case that looks worth fixing before merge.
| !firstOption || | ||
| !lastOption || | ||
| options.some( | ||
| (option) => option.field('key')?.text() === MOUNT_BUILDKITE_AGENT, |
There was a problem hiding this comment.
I think this may need attention before merge because the Docker Compose path treats any existing mount-buildkite-agent key as done, even when it is explicitly false.
addMountBuildkiteAgentEdits returns no edits as soon as the option exists. If a project currently has both a docker-compose#... plugin with mount-buildkite-agent: false and the old /usr/bin/buildkite-agent:/usr/bin/buildkite-agent bind mount in docker-compose.yml, this migration can remove the bind mount while leaving the plugin opted out. The Docker Compose plugin defaults this option to false, so the agent would remain unavailable inside the container.
Suggested direction:
Mirror the docker plugin handling here: when the existing value is false, flip it to true, and add a test for the Docker Compose plugin false case.
This reverts commit b638a84.
Co-authored-by: KSHITIZ6341 <kshitizsuwal@gmail.com>
5 participants
Resolves #1945