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.

Sign up for GitHub

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

Jump to bottom

add caching copy layers back #1518

Merged
merged 7 commits into from
Dec 11, 2020
Merged

add caching copy layers back #1518

merged 7 commits into from
Dec 11, 2020

Conversation

tejal29
Copy link
Member

@tejal29 tejal29 commented Dec 9, 2020

Fixes: #1489

Revert changes from #1408

Introduce a new flag --cache-copy-layers

@google-cla google-cla bot added the cla: yes CLA signed by all commit authors label Dec 9, 2020
@tejal29 tejal29 force-pushed the add_copy_cache_back branch 4 times, most recently from c4fff02 to 6066c03 Compare December 10, 2020 22:53
@tejal29
Copy link
Member Author

tejal29 commented Dec 11, 2020

Manual testing notes

/ # /kaniko/executor -f dockerfiles/Dockerfile_test_copy --context=dir://workspace --destination=gcr.io/tejal-test/test1:test --cache-copy-layers=true --cache
WARN[0000] 
Skip running docker-credential-gcr as user provided docker configuration exists at /kaniko/.docker/config.json 
INFO[0000] Using dockerignore file: /workspace/.dockerignore 
INFO[0000] Retrieving image manifest alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a 
INFO[0000] Retrieving image alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a from registry index.docker.io 
INFO[0001] Built cross stage deps: map[]                
INFO[0001] Retrieving image manifest alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a 
INFO[0001] Retrieving image alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a from registry index.docker.io 
INFO[0002] Executing 0 build triggers                   
INFO[0002] Checking for cached layer gcr.io/tejal-test/test1/cache:d01be001644cf1aaa8bffa23a594f0d57924ad87b7fe0a1ddf82e6a032cf7f20... 
INFO[0002] Using caching version of cmd: COPY context/foo foo 
INFO[0002] Checking for cached layer gcr.io/tejal-test/test1/cache:0816f40595cee98f17f244d0115f933b7584813cd192560f6bfcda2d4f9e5d0c... 
INFO[0003] Using caching version of cmd: COPY context/foo /foodir/ 
INFO[0014] Skipping unpacking as no commands require it. 
INFO[0014] COPY context/foo foo                         
INFO[0014] Found cached layer, extracting to filesystem

without the flag

/kaniko/executor -f dockerfiles/Dockerfile_test_copy --context=dir://workspace --destination=gcr.io/tejal-test/test1:test --cache-copy-layers=tr
ue
WARN[0000] 
Skip running docker-credential-gcr as user provided docker configuration exists at /kaniko/.docker/config.json 
INFO[0000] Using dockerignore file: /workspace/.dockerignore 
INFO[0000] Retrieving image manifest alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a 
INFO[0000] Retrieving image alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a from registry index.docker.io 
INFO[0001] Built cross stage deps: map[]                
INFO[0001] Retrieving image manifest alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a 
INFO[0001] Retrieving image alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a from registry index.docker.io 
INFO[0002] Executing 0 build triggers                   
INFO[0002] Unpacking rootfs as cmd COPY context/foo foo requires it. 
INFO[0002] COPY context/foo foo                         
INFO[0002] Taking snapshot of files...

@tejal29 tejal29 merged commit b04399e into GoogleContainerTools:master Dec 11, 2020
@paprickar
Copy link

Hey @tejal29

thank you for adding this back.

Do you know when this will be released?

@kerthcet
Copy link

waiting for the feature too.

@tejal29 tejal29 deleted the add_copy_cache_back branch April 26, 2021 16:43
gilbsgilbs added a commit to gilbsgilbs/kaniko that referenced this pull request Sep 9, 2021
@gilbsgilbs
Fix composite cache key for multi-stage copy command (GoogleContainer…
180af46 
…Tools#1706)

PR GoogleContainerTools#1518 reintroduced COPY layers caching using the `--cache-copy-layers`
flag. Unfortunately, this PR also introduced a bug by not including the
stage digest into the caching key of the COPY command when the
`--cache-copy-layers` flag was not set. As a result, kaniko would use
any previous (possibly stalled) layer from the cache because the digest
of the "COPY --from" command would never change.

PR author probably expected Go to fallthrough in the switch just like C
does. However, this is not the case. Go does not fallthrough in
switch-statements by default and requires the fallthrough keyword to be
used. Note that this keyword is not available in type-switches though,
because it wouldn't work properly with typings.
gilbsgilbs added a commit to gilbsgilbs/kaniko that referenced this pull request Sep 9, 2021
@gilbsgilbs
Fix composite cache key for multi-stage copy command (GoogleContainer…
a6e0f2f 
…Tools#1706)

PR GoogleContainerTools#1518 reintroduced COPY layers caching using the `--cache-copy-layers`
flag. Unfortunately, this PR also introduced a bug by not including the
stage digest into the caching key of the COPY command when the
`--cache-copy-layers` flag was not set. As a result, kaniko would use
any previous (possibly stalled) layer from the cache because the digest
of the "COPY --from" command would never change.

PR author probably expected Go to fallthrough in the switch just like C
does. However, this is not the case. Go does not fallthrough in
switch-statements by default and requires the fallthrough keyword to be
used. Note that this keyword is not available in type-switches though,
because it wouldn't work properly with typings.
tejal29 pushed a commit to gilbsgilbs/kaniko that referenced this pull request Oct 19, 2021
@gilbsgilbs @tejal29
Fix composite cache key for multi-stage copy command (GoogleContainer…
442c059 
…Tools#1706)

PR GoogleContainerTools#1518 reintroduced COPY layers caching using the `--cache-copy-layers`
flag. Unfortunately, this PR also introduced a bug by not including the
stage digest into the caching key of the COPY command when the
`--cache-copy-layers` flag was not set. As a result, kaniko would use
any previous (possibly stalled) layer from the cache because the digest
of the "COPY --from" command would never change.

PR author probably expected Go to fallthrough in the switch just like C
does. However, this is not the case. Go does not fallthrough in
switch-statements by default and requires the fallthrough keyword to be
used. Note that this keyword is not available in type-switches though,
because it wouldn't work properly with typings.
tejal29 added a commit that referenced this pull request Oct 19, 2021
@gilbsgilbs @tejal29
Fix composite cache key for multi-stage copy command (#1735)
a42adb9 
* chore: add workflows for pr tests

* fix unit tests

* fix formatting

* chore: fix gobuild

* change minikube script

* chore: fix lint install script

* chore: ignore and fix tests

* fix lint and run gofmt

* lint fixes

* k8s executor image only

* fix Makefile

* fix travis env variables

* more info on k8s tests

* fix travis run

* fix

* fix

* fix

* fix log

* some more changes

* increase timeout

* delete travis.yml and fix multiple copy tests

* fix registry mirror

* fix lint

* add concurency

* last attemot to fix k8 integrations

* diff id for diff workflows

* Fix composite cache key for multi-stage copy command (#1706)

PR #1518 reintroduced COPY layers caching using the `--cache-copy-layers`
flag. Unfortunately, this PR also introduced a bug by not including the
stage digest into the caching key of the COPY command when the
`--cache-copy-layers` flag was not set. As a result, kaniko would use
any previous (possibly stalled) layer from the cache because the digest
of the "COPY --from" command would never change.

PR author probably expected Go to fallthrough in the switch just like C
does. However, this is not the case. Go does not fallthrough in
switch-statements by default and requires the fallthrough keyword to be
used. Note that this keyword is not available in type-switches though,
because it wouldn't work properly with typings.

* refactor: add an abstract copy command interface to avoid code duplication

* fix typo in error message

Co-authored-by: Tejal Desai <tejal29@gmail.com>
This was referenced Apr 27, 2022
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes CLA signed by all commit authors
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Return caching COPY layers
3 participants
@tejal29 @paprickar @kerthcet