Stop caching COPY layers

[isker]

Resolves #1357

Description

Cached COPY layers are expensive in that they both need to be retrieved over the network and occupy space in the layer cache.

They are unnecessary in that we already have all resources needed to execute the COPY locally, and doing so is a trivial file-system operation. This is in contrast to RUN layers, which can do arbitrary and unbounded work.

The end result is that cached COPY commands were more expensive when cached, not less. Remove them.

Submitter Checklist

• Includes unit tests
• Adds integration tests if needed.

Reviewer Notes

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

Release Notes

• kaniko no longer caches COPY commands in its layer cache as the associated network and storage costs outweigh the costs of executing them locally.

[isker]

Sorry, couldn't get all of the tests to run on my macbook so I decided to "test in CI". Clearly not working. The tests run as expected on a Linux box, will fix them there.

[isker]

Unit tests are good to go now, I removed some obsolete test cases and added a new one.

I think that integration test that failed is flakey? It was not failing previously.

[tejal29]

Unit tests are good to go now, I removed some obsolete test cases and added a new one.

I think that integration test that failed is flakey? It was not failing previously.

Thanks @isker i restarted the failed integration test. I opened a issue to look into flaky test #1416

[tejal29]

@isker If a file in the copy command changes, then the Run command will be invalidated since the composite key for the previous COPY layer will be different.
e.g. in the below dockefile,

FROM ...

COPY package.json package-lock.json ./
RUN npm ci 

The cache key for RUN npm ci depends on contents of file copied COPY package.json package-lock.json ./
If contents change the cache will change and hence Run npm ci will be executed locally.

[tejal29]

code change looks good.
Pending Manual Verification

[isker]

Hi @tejal29, anything else you need me to do here?

[tejal29]

Verified this is working on my build.

[tejal29]

Sorry for the delay @isker
This will be part of v1.2.0 release of kaniko.

[isker]

No worries. Thank you!

…

On Wed, Sep 30, 2020 at 8:19 PM Tejal Desai ***@***.***> wrote: Merged #1408 <#1408> into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1408 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABH3ZNKEPTL5XX6GBUFULQTSIPDHRANCNFSM4QQ7O5RA> .

[tejal29]

@isker did you end up testing v1.2.0?

[isker]

@tejal29 have not had time yet to do so but I will let you know the results when I do.

[tejal29]

sounds good. no one has reported issues yet!

[isker]

@tejal29 with Kaniko 1.2.0, we're seeing the improvements we expect. Before, it took the same amount of time to build our image with caching on as with it off, because we were stuck downloading and uploading large COPY layers, with the added cost of maintaining the layer cache with caching on.

Afterward, we see a 30-40% improvement in build time with caching on, and of course our layer cache is smaller.

Thanks for your support on getting this merged and released!

[tejal29]

Thanks @isker