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

Add a new flag to cleanup the filesystem at the end #370

Merged
merged 1 commit into from
Sep 28, 2018

Conversation

vbehar
Copy link
Contributor

@vbehar vbehar commented Sep 27, 2018

Currently, kaniko can only build a single image per container run, because the filesystem is full of the content of the first image.
When running kaniko in Jenkins, where we need to start the container "doing nothing" first (using the debug kaniko container), and then exec /kaniko/executor, this is a limitation because it means that if we want to build multiple images, we need to start multiple containers - see https://groups.google.com/forum/#!topic/kaniko-users/_7LivHdMdy0 for more details

A solution to fix this issue is to add a new flag to cleanup the filesystem at the end - the same way it is done between stages when building a multi-stages image. This way, the same (debug) container can be used to build multiple images.

@container-tools-bot
Copy link
Collaborator

Hi @vbehar. Thanks for your PR.

I'm waiting for a GoogleContainerTools member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@svalo
Copy link

svalo commented Sep 27, 2018

This would be awesome! I'm using GitLab CI, I need to build 2 images, the second depending on the first. To be able to use kaniko I had to create 2 stages because GitLab's jobs can't be sequential. If this gets merged everything could be achieved in a single job

Copy link
Collaborator

@priyawadhwa priyawadhwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing this feature! Just had a few comments around organization.

@@ -37,11 +37,13 @@ var (
opts = &config.KanikoOptions{}
logLevel string
force bool
cleanup bool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a new variable here, could we add this to KanikoOptions? This should help with executing the comment below.

@@ -77,6 +79,11 @@ var RootCmd = &cobra.Command{
if err := executor.DoPush(image, opts); err != nil {
exit(errors.Wrap(err, "error pushing image"))
}
if cleanup {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having this in root.go, I think it would be better placed in this if statement in build.go

If opts.Cleanup is true, then delete the filesystem before returning the image.

Currently, kaniko can only build a single image per container run, because the filesystem is full of the content of the first image.
When running kaniko in Jenkins, where we need to start the container "doing nothing" first (using the debug kaniko container), and then exec /kaniko/executor, this is a limitation because it means that if we want to build multiple images, we need to start multiple containers - see https://groups.google.com/forum/#!topic/kaniko-users/_7LivHdMdy0 for more details

A solution to fix this issue is to add a new flag to cleanup the filesystem at the end - the same way it is done between stages when building a multi-stages image. This way, the same (debug) container can be used to build multiple images.
@vbehar
Copy link
Contributor Author

vbehar commented Sep 28, 2018

@priyawadhwa ok, thanks for the review! I changed the implementation and squashed.

Copy link
Collaborator

@priyawadhwa priyawadhwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for contributing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants