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

Fix composite cache key for multi-stage copy command #1735

Merged

Conversation

gilbsgilbs
Copy link
Contributor

@gilbsgilbs gilbsgilbs commented Sep 9, 2021

Fixes #1706

Description

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.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Adds integration tests if needed.

Author note: I did not include an integration test because you'd have to build two different images and check that the two digests differ, which is a quite different paradigm from the existing caching integration tests that always check that the digests match. I thought it would be overkill to design a new integration test system to support such scenario given the use-case is rather specific and the bug itself is kind of dummy. I don't think it'd be worth it.

See the contribution guide for more details.

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

Release Notes

Describe any changes here so maintainer can include it in the release notes, or delete this block.

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

@google-cla google-cla bot added the cla: yes CLA signed by all commit authors label Sep 9, 2021
…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.
@google-cla
Copy link

google-cla bot commented Oct 19, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no CLA not signed by all commit authors and removed cla: yes CLA signed by all commit authors labels Oct 19, 2021
@tejal29 tejal29 added cla: yes CLA signed by all commit authors and removed cla: no CLA not signed by all commit authors labels Oct 19, 2021
@tejal29 tejal29 merged commit a42adb9 into GoogleContainerTools:master Oct 19, 2021
@Cave-Johnson
Copy link

Has this regressed in the latest version of kaniko? Since the release of 1.8 I have started to experience the cache layers not detecting repository code changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed by all commit authors kokoro:run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caching problem with multistage build
3 participants