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

Caching / tagging of docker images in CI causes errant build failures #3030

Closed
heartsucker opened this issue Feb 19, 2018 · 5 comments
Closed

Comments

@heartsucker
Copy link
Contributor

heartsucker commented Feb 19, 2018

Bug

Description

As noted in this comment from @kushaldas (#2948 (comment)) and some chat in gitter, it seems like the caching of Docker images and using tags based off the SHA1 hashes of git commits is causing things to cache when they shouldn't.

Steps to Reproduce

Honestly, I can't think of a guaranteed way to reproduce this. None of the builds for #2948 succeed in CI, but on multiple people's personal machines they succeed.

Expected Behavior

Caching should invalidate correctly and rebuild images?

Actual Behavior

It does not?

Comments

We shouldn't use tags for the docker images. We should just use latest for everything and let docker handle the caching of layers itself. I would rather have some builds take longer than have the cache behave badly.

@ghost
Copy link

ghost commented Feb 19, 2018

Did this happen in CI on a pull request other than #2948 ? That would be undeniable proof it is not caused by the commits found in this PR.

@ghost
Copy link

ghost commented Feb 19, 2018

We should just use latest for everything and let docker handle the caching of layers itself.

This has been in place for some time and saves between 1mn and 2mn in the build process. It also reduces the chance of a failure due to environmental reasons because it shares the layer than does "apt-get update && apt-get install" between pull requests.

I'm also inclined to prefer stability over speed, every time. But in this case @tmc carefully tested this and it worked very reliably over the past months. I would even go as far as to say it significantly contributes to the CI stability we've enjoyed in the past two weeks (yeah !). Reason why we should have proof this optimization did the wrong thing before considering removing it.

@heartsucker
Copy link
Contributor Author

heartsucker commented Feb 19, 2018

The following disproves that we need to tag the images with a sha1.

mkdir /tmp/securedrop-test
cd /tmp/securedrop-test
cp $sdroot/securedrop/Dockerfile .
cp -r $sdroot/securedrop/requirements .
docker build --tag securedrop-test:latest .
docker build --tag securedrop-test:latest .

The second build happens in under a second. We don't need our own tags if we're correctly snagging the docker build cache and saving it between circleCI builds.

@ghost ghost changed the title Cahcing / tagging of docker images in CI causes errant build failures Caching / tagging of docker images in CI causes errant build failures Feb 19, 2018
@ghost
Copy link

ghost commented Feb 19, 2018

We don't need our own tags if we're correctly snagging the docker build cache and saving it between circleCI builds.

I don't see how that would work. I suggest you propose a pull request against .circleci/config.yml that does what you think is better than what is currently implemented. As far as I can see using latest only will have one PR use the image from another PR and it will lead to massive confusion.

@heartsucker
Copy link
Contributor Author

Having poked around in the build, I think the current setup is doing things right in the sense that we shouldn't be seeing caching issues. I'm closing this and am going to try to debug CI from another angle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant