GoogleContainerTools / kaniko

Build Container Images In Kubernetes
Apache License 2.0
14.88k stars 1.44k forks source link

Redundant snapshot with --use-new-run #2800

Open code-asher opened 1 year ago

code-asher commented 1 year ago

Actual behavior

When I pass --use-new-run I see the following toward the end of the logs:

INFO[0000] Initializing snapshotter ...                 
INFO[0000] Taking snapshot of full filesystem...

But I am not sure why a snapshot would be required, or if it makes sense that the behavior differs when running without --use-new-run.

Expected behavior

I would expect not to see any snapshotting. There is no snapshot if I omit --use-new-run. In some cases we are seeing a pretty long delay for the snapshot (12 seconds in one case) so avoiding it would speed up our builds.

To Reproduce

  1. Create an empty directory.
  2. Create a Dockerfile in that directory with only FROM codercom/oss-dogfood (or any other image, I believe).
  3. Change into that directory and run something like:
    docker run --rm -v $PWD:/workspace gcr.io/kaniko-project/executor:latest --no-push --use-new-run

Additional Information

I can prevent the snapshot by removing the s.opts.RunV2 check found here:

https://github.com/GoogleContainerTools/kaniko/blob/b6f14ae676b92ba5e27943c5a873c476d3e9a0f8/pkg/executor/build.go#L347

However, I am unsure if there are negative consequences to doing so. I see the commit that added the check but it did not shed any light for me.

I tested my own build and ran go test without seeing any related issues though.

If this does seem like something that can be removed, please let me know if it would be helpful for me to send a PR or if this is tiny enough that it is easier for someone else to just push it.

Triage Notes for the Maintainers

Description Yes/No
Please check if this a new feature you are proposing
  • - [ ]
Please check if the build works in docker but not in kaniko
  • - [ ]
Please check if this error is seen when you use --cache flag
  • - [ ]
Please check if your dockerfile is a multistage dockerfile
  • - [ ]

Thank you!

liambennett commented 10 months ago

This appears to have been resolved in a fork, is there a reason this can't be updated in this repo? Really keen to switch to the use-new-run if it offers this kind of speed benefit.

code-asher commented 10 months ago

I would be happy to open a PR here, just wanted to get confirmation on whether the change makes sense.

aaron-prindle commented 10 months ago

@code-asher, thanks for flagging this with context and providing a fix here. I believe that the additional snapshot check that was added in the mentioned commit is likely not necessary. If you can provide a PR we should be able to get this fix in and improve performance here. Thanks!

code-asher commented 10 months ago

Thank you for the confirmation! PR here: https://github.com/GoogleContainerTools/kaniko/pull/2943

aaron-prindle commented 10 months ago

I believe the removal of this snapshot might cause issues related to which files persist in the resulting image. See - https://github.com/GoogleContainerTools/kaniko/issues/2958