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

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

Open
code-asher opened this issue Oct 16, 2023 · 5 comments · Fixed by #2943
Open

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

code-asher opened this issue Oct 16, 2023 · 5 comments · Fixed by #2943
Labels
area/snapshotting feat/use-new-run kind/bug Something isn't working priority/p2 High impact feature/bug. Will get a lot of users happy
Milestone

Comments

@code-asher
Copy link
Contributor

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:

if s.opts.SingleSnapshot || s.opts.RunV2 {

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!

@aaron-prindle aaron-prindle added kind/bug Something isn't working feat/use-new-run priority/p2 High impact feature/bug. Will get a lot of users happy area/snapshotting needs-follow-up and removed needs-follow-up labels Oct 19, 2023
@liambennett
Copy link

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
Copy link
Contributor Author

code-asher commented Jan 5, 2024

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

@aaron-prindle
Copy link
Collaborator

@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
Copy link
Contributor Author

Thank you for the confirmation! PR here: #2943

@aaron-prindle
Copy link
Collaborator

I believe the removal of this snapshot might cause issues related to which files persist in the resulting image. See - #2958

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/snapshotting feat/use-new-run kind/bug Something isn't working priority/p2 High impact feature/bug. Will get a lot of users happy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants