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

mirror mount-buildkite-agent implementation from docker-buildkite-plugin #285

Merged

Conversation

jmchuster
Copy link
Contributor

Per this discussion here #158

This is implemented the same way that docker-buildkite-plugin does. https://github.com/buildkite-plugins/docker-buildkite-plugin/blob/c3acef02fa694b8be6951de36e7c34725dadf6c6/hooks/command#L210-L230

As such, it suffers from all the same strengths and weaknesses. It also means we can easily improve it in the same conceptual manner if docker-buildkite-plugin improves its implementation, and vice versa.

Copy link
Contributor

@toolmantim toolmantim left a comment

Choose a reason for hiding this comment

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

Thanks so much @jmchuster! Looks great. All it needs is a test, and we should be good to merge and release this.

I couldn't seem to push a test commit onto your branch, so here's a passing test for this new flag:

diff --git a/tests/run.bats b/tests/run.bats
index 9541358..32cb576 100644
--- a/tests/run.bats
+++ b/tests/run.bats
@@ -803,6 +803,33 @@ export BUILDKITE_JOB_ID=1111
   unstub buildkite-agent
 }
 
+@test "Run with mount-buildkite-agent enabled" {
+  export BUILDKITE_JOB_ID=1111
+  export BUILDKITE_PLUGIN_DOCKER_COMPOSE_RUN=myservice
+  export BUILDKITE_PIPELINE_SLUG=test
+  export BUILDKITE_BUILD_NUMBER=1
+  export BUILDKITE_COMMAND=""
+  export BUILDKITE_PLUGIN_DOCKER_COMPOSE_CHECK_LINKED_CONTAINERS=false
+  export BUILDKITE_PLUGIN_DOCKER_COMPOSE_CLEANUP=false
+  export BUILDKITE_PLUGIN_DOCKER_COMPOSE_MOUNT_BUILDKITE_AGENT=true
+
+  stub docker-compose \
+    "-f docker-compose.yml -p buildkite1111 build --pull myservice : echo built myservice" \
+    "-f docker-compose.yml -p buildkite1111 up -d --scale myservice=0 : echo ran myservice dependencies" \
+    "-f docker-compose.yml -p buildkite1111 run --name buildkite1111_myservice_build_1 --rm -e BUILDKITE_JOB_ID -e BUILDKITE_BUILD_ID -e BUILDKITE_AGENT_ACCESS_TOKEN -v $BATS_MOCK_TMPDIR/bin/buildkite-agent:/usr/bin/buildkite-agent myservice : echo ran myservice"
+
+  stub buildkite-agent \
+    "meta-data exists docker-compose-plugin-built-image-tag-myservice : exit 1"
+
+  run $PWD/hooks/command
+
+  assert_success
+  assert_output --partial "built myservice"
+  assert_output --partial "ran myservice"
+  unstub docker-compose
+  unstub buildkite-agent
+}
+
 @test "Run with various build arguments" {
   export BUILDKITE_JOB_ID=1111
   export BUILDKITE_PLUGIN_DOCKER_COMPOSE_RUN=myservice

If you could add, we can merge and release this. Thanks again!

@jmchuster
Copy link
Contributor Author

Oh, nice. I had put this together real quick just for my job, and wanted to just open up this PR to see if it might be something you guys wanted.

Also, wanted to ask, how do the tests work? Is it running the hook and then asserting that the docker-compose command that gets run by the hook is equivalent to the stub?

@jayco
Copy link
Member

jayco commented Sep 22, 2020

Thanks for updating this @jmchuster!

Yup, you are correct on the tests! We run them using the plugin tester https://github.com/buildkite-plugins/buildkite-plugin-tester which makes it easy to run locally without needing to install bats.

@jayco jayco merged commit b332edb into buildkite-plugins:master Sep 22, 2020
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.

None yet

3 participants