Skip to content
This repository has been archived by the owner on Nov 15, 2022. It is now read-only.

Delete tarball after image builds #496

Merged
merged 1 commit into from
Dec 4, 2017

Conversation

surajnarwade
Copy link
Collaborator

Resolves #488, This PR will delete temporary files created in /tmp
for building images with kedge build

Copy link
Member

@kadel kadel left a comment

Choose a reason for hiding this comment

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

have you tried to run you code?

return errors.Wrap(err, "unable to build image")
}

log.Infof("Image '%s' from directory '%s' built successfully", image, path.Base(context))
log.Debugf("Image %s build output:\n%s", image, outputBuffer)

log.Debugf("Deleted temporary file %v", os.Remove(tmpFile.Name()))
Copy link
Member

Choose a reason for hiding this comment

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

This will show Deleted temporary file <nil>

os.Remove returns error. You should check it, not printing it.

Copy link
Member

Choose a reason for hiding this comment

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

This is also nice case for using defer to avoid having to call in two places in case of erro

@surajnarwade
Copy link
Collaborator Author

@kadel , got it, updated the code :)

@@ -99,14 +99,15 @@ func (c *Build) BuildImage(dockerfile, image, context string) error {
Dockerfile: dockerfile,
}

// Delete tarball after creating image
defer os.Remove(tmpFile.Name())
Copy link
Member

Choose a reason for hiding this comment

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

defer should be placed right after tmpFile is created. Otherwise, it won't be run before for any return between this and tmpFile, err := ioutil.TempFile("/tmp", "kedge-image-build-")

// Build it!
if err := c.Client.BuildImage(opts); err != nil {
return errors.Wrap(err, "unable to build image")
}

Copy link
Member

Choose a reason for hiding this comment

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

@surajnarwade : unrelated changes!

Resolves kedgeproject#488, This PR will delete temporary files created in `/tmp`
for building images with `kedge build`
@surajnarwade
Copy link
Collaborator Author

@kadel @surajssd, updated PR with suggested changes, please review

Copy link
Member

@surajssd surajssd left a comment

Choose a reason for hiding this comment

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

This LGTM!

@surajssd surajssd merged commit eb77cb3 into kedgeproject:master Dec 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants