Be specific about which Conjur image targeting during build/test/push [CON-4493] #494
Conversation
ghost
assigned 
| @@ -1,4 +1,4 @@ | |||
| FROM conjur | |||
| ARG VERSION=latest | |||
| FROM conjur:${VERSION} | |||
There was a problem hiding this comment.
Oh, hey, can docker on our executor's handle this now? That'd be very exciting....
There was a problem hiding this comment.
Yeah, I had been wondering this about it since we upgraded Docker on the executors, and it works! I think we can simplify some of our bash/sed-fu that does the same thing in some of our projects.
Right now only ARG can come before FROM
https://docs.docker.com/engine/reference/builder/#understand-how-arg-and-from-interact
| @@ -47,5 +49,5 @@ rm -rf spec/reports/* | |||
| rm -rf cucumber/api/features/reports/* | |||
| rm -rf cucumber/policy/features/reports/* | |||
|
|
|||
| bundle exec rake jenkins || true | |||
| bundle exec rake jenkins | |||
There was a problem hiding this comment.
I know you hate || true. But, the way this script and the pipeline are currently written, no test results will get collected when the tests fail.
There was a problem hiding this comment.
Good eye. I don't hate it, just wish this wasn't the way it worked :) I'll update this PR so it's working again.
There was a problem hiding this comment.
I think I've found a better way to do this, without requiring us to hijack exit codes. Since this is a Jenkins-specific workaround I think it's better to handle this in Jenkins logic, not in our application code.
Since each stage can also have a post block, I think it's better to use
post { always { junit ... } } to capture the reports.
This pattern is mentioned here: https://jenkins.io/doc/book/pipeline/jenkinsfile/#handling-failures. See the updated Jenkinsfile in this PR.
What do you think Alan?
There was a problem hiding this comment.
In general, this seems like a good approach to me. The only thing that seems a little weird is using always in the post block. But, I guess, if this is doc'ed as the current best practice, it's a good way to go.
|
I also added coverage report collection to this PR: https://jenkins.conjur.net/job/cyberark--conjur/job/prevent-image-collision/Coverage_Report/ |
Closes CON-4493.
What does this pull request do?
This PR resolves image congestion when multiple CI pipelines are run at once to build/test/push the Conjur image. With this merge it is now impossible to accidentally pick up the wrong image.
What background context can you provide?
Previously we were naively taking the
conjur:latestimage and retagging it for push. This worked fine most of the time, but a race condition exists where anotherconjur:latestimage is created before the retag and push to repositories. That newer image would be picked up, even if it didn't pass tests or was on an incompatible branch.Where should the reviewer start?
View the jenkins job, see how it is correctly passing the TAG through build/test/push stages.
How should this be manually tested?
Not required. You can run build.sh and test.sh locally if you like.
Screenshots (if appropriate)
N/A
Link to build in Jenkins (if appropriate)
https://jenkins.conjur.net/job/cyberark--conjur/job/prevent-image-collision/
Questions: