Disable cache after first miss

[cvgw]

Fixes #870
Description

Disable the cache after the first miss as following layers may depend on it. Kaniko already did this within a single stage, but not across stages.

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.

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.

Disable the cache for following stages after the first cache miss

[cvgw]

I think this will also fix #845 and #589

[mcfedr]

Not quite sure if this is what its doing already - but i guess ideally the cache will be disabled from the point where the previous stage is included.

FROM a as a
RUN break-the-cache
FROM b as b
RUN something  <-- this will still be cached
COPY --from a a/file   <--here the cache is disabled

[mcfedr]

Also is this much the same as #853 ?

[cvgw]

Not quite sure if this is what its doing already - but i guess ideally the cache will be disabled from the point where the previous stage is included.

FROM a as a
RUN break-the-cache
FROM b as b
RUN something  <-- this will still be cached
COPY --from a a/file   <--here the cache is disabled

@mcfedr unfortunately that is not the outcome this PR would produce (though I think what you are suggesting may be the ideal behavior). This PR would result in the following behavior

  FROM a as a
  RUN break-the-cache
  FROM b as b <--here the cache is disabled
  RUN something
  COPY --from a a/file   

Without computing a full dependency tree or having behavior specific to COPY --from commands I think the behavior produced by this PR would be the best option. Computing a full dependency tree to produce the behavior that you illustrated would not be unreasonable but certainly a nontrivial undertaking.

Open to other thoughts

As a side note I'm not sure I will be submitting this PR as I have an idea for what I think is a better implementation.

[abergmeier]

Computing a full dependency tree to produce the behavior that you illustrated would not be unreasonable but certainly a nontrivial undertaking.

Somewhat surprised that Kaniko doesn't have a dependency DAG internally.

[abergmeier]

@mcfedr unfortunately that is not the outcome this PR would produce (though I think what you are suggesting may be the ideal behavior).

Doesn't everything else defeat the purpose of multi-stage Dockerfiles? At least for us it is to a certain degree as bad as disabling caching completely.

[cvgw]

@mcfedr unfortunately that is not the outcome this PR would produce (though I think what you are suggesting may be the ideal behavior).

Doesn't everything else defeat the purpose of multi-stage Dockerfiles? At least for us it is to a certain degree as bad as disabling caching completely.

In your opinion is it preferable to leave caching broken (as it currently is) until a true dependency tree is implemented vs making an incremental fix that would prevent stale data from being retrieved from the cache though not producing ideal caching behavior?

[abergmeier]

In your opinion is it preferable to leave caching broken (as it currently is) until a true dependency tree is implemented vs making an incremental fix that would prevent stale data from being retrieved from the cache though not producing ideal caching behavior?

Not arguing against that.
For us we will probably (in Cloud Build) replace Kaniko with Docker since the USP of Kaniko just went out the window.
In other words, being cache-compatible with Docker is a Prio 1 for us.

[cvgw]

In your opinion is it preferable to leave caching broken (as it currently is) until a true dependency tree is implemented vs making an incremental fix that would prevent stale data from being retrieved from the cache though not producing ideal caching behavior?

Not arguing against that.
For us we will probably (in Cloud Build) replace Kaniko with Docker since the USP of Kaniko just went out the window.
In other words, being cache-compatible with Docker is a Prio 1 for us.

Thank you @abergmeier . I appreciate that feedback and I will share it with the rest of the team to help prioritize work.

[cvgw]

Going in a different direction to fix this one

[mcfedr]

In your opinion is it preferable to leave caching broken (as it currently is) until a true dependency tree is implemented vs making an incremental fix that would prevent stale data from being retrieved from the cache though not producing ideal caching behavior?

broken caching is clearly worse - but no caching isnt good, anyway it looks like you already found a better fix in #883