CircleCI-Public / aws-ecr-orb

CircleCI orb for interacting with Amazon's Elastic Container Registry (ECR)
https://circleci.com/orbs/registry/orb/circleci/aws-ecr
MIT License
78 stars 139 forks source link

Improve performance with --cache-from #4

Closed bessey closed 4 years ago

bessey commented 5 years ago

If there was an option to use docker build's --cache-from here: https://github.com/CircleCI-Public/aws-ecr-orb/blob/master/src/orb.yml#L73 It would presumably cut build time significantly in the same way Circle's proprietary layer caching does, but more reliably

Reference: https://semaphoreci.com/docs/docker/docker-layer-caching.html https://docs.docker.com/edge/engine/reference/commandline/build/

KyleTryon commented 4 years ago

I may be incorrect here, attempting to look into this. I do not believe this will have any effect as (i think) the docker image you are sourcing for as a cache would need to be in the container in the first place, meaning you would have to first pull it and then use it as a cache.

If that is incorrect I would love to be wrong!

KyleTryon commented 4 years ago

That does make sense. Thank you for sharing that explanation.

@RanadeepPolavarapu @vsoch have both submitted PRs to this issue. Though both are slightly different. @vsoch was first as well.

https://github.com/CircleCI-Public/docker-orb/pull/21 https://github.com/CircleCI-Public/docker-orb/pull/16

However, I am more interested in merging whichever we believe makes the most sense.

They are very similar so this is a tough call. With #16 being first and including a useful separate pull command I think I am going to hand the merge over to #16. Both are good pr's and are valid for #hacktoberfest or #orbtoberfest. Thank you both!

KyleTryon commented 4 years ago

Actually going to reopen this issue for comment. Sorry about that. 1 concern.

https://github.com/CircleCI-Public/docker-orb/pull/21/files#diff-097aec5fcf12af1d49792323e01f8b6fR86

Automatically pulling the images when using the Cache from parameter seems important.

@vsoch one possible concern I have with your approach is if someone uses the cache from parameter in a custom job but has not also used your pull command in conjunction.

Perhaps we should split the difference in these approaches?

vsoch commented 4 years ago

Definitely! I might next some help to get the command right. What (I thought) I had done was add a proper when condition so that if cache_from is used, it will always pull first. E.g.,:

  - when:
      name: Pull containers for cache_from
      condition: <<parameters.cache_from>>
      steps:
        - pull:
            containers: <<parameters.cache_from>>

I'm a bit confused, because the statement above is defined in "publish" and the step "pull" is what is implementing the core bits for cache_from (which is just a parameter). You are saying this approach won't work, because cache_from can be used in some custom way? Could you give an example of how cache_from (a parameter) could be used without pull, or without publish? It's akin to extra_args - if that helps.

vsoch commented 4 years ago

@RanadeepPolavarapu do you want to help me on #16 to achieve what @KyleTryon is asking? I'm relatively new with the configs here and would really appreciate it!

vsoch commented 4 years ago

If @KyleTryon can confirm this is the best approach to take - I'm down! And I don't think you can push directly, so ideally you can PR to https://github.com/CircleCI-Public/docker-orb/pull/16 (and that way I can also see the changes, and learn from them). I'd like for us both to be commit authors, if that's okay with you.

vsoch commented 4 years ago

Awesome! I love Hacktoberfest <3

KyleTryon commented 4 years ago

@RanadeepPolavarapu & @vsoch Awwwwwwww yeeeesss. Let's do this.

vsoch commented 4 years ago

okay let me give that a try! I expected you to open a PR to https://github.com/CircleCI-Public/docker-orb/pull/16 but this works too :)

vsoch commented 4 years ago

@RanadeepPolavarapu I don't think you provided full instructions, since your remote won't be added to my git config?

vsoch commented 4 years ago

@RanadeepPolavarapu anyone can open a pull request to another individual's repository - you don't need to be a contributor. I'll look over the changes and then merge into my branch. I'd prefer doing this over the cherry pick kung fu.

vsoch commented 4 years ago

Once you do the PR and we merge to the branch add/docker-cache it will update the PR automatically - this seems like the cleanest approach to take.

vsoch commented 4 years ago

Why are there conflicts?

vsoch commented 4 years ago

If you start cleanly from the branch, make changes, and commit, there shouldn't be.

vsoch commented 4 years ago

okay we're combined! https://github.com/CircleCI-Public/docker-orb/pull/16 Take it away @KyleTryon

eddiewebb commented 4 years ago

https://github.com/CircleCI-Public/docker-orb/pull/27 addresses this.