Skip to content

Commit

Permalink
pruning implemented for normal builders. need to figure out plugins
Browse files Browse the repository at this point in the history
  • Loading branch information
nkubala committed Mar 6, 2019
1 parent d7b71c9 commit 3e067ee
Show file tree
Hide file tree
Showing 13 changed files with 97 additions and 24 deletions.
2 changes: 2 additions & 0 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ var rootCmd = &cobra.Command{

func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
// only prune if flag is set, AND we're not caching
opts.Prune = !(opts.NoPrune || opts.CacheArtifacts)
if err := SetUpLogs(err, v); err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions cmd/skaffold/app/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,17 @@ func dev(out io.Writer, ui bool) error {
catchCtrlC(cancel)
}

prune := func() {}
if !opts.NoPrune {
cleanup := func() {}
if opts.Cleanup {
defer func() {
prune()
cleanup()
}()
}

cleanup := func() {}
if opts.Cleanup {
prune := func() {}
if opts.Prune {
defer func() {
cleanup()
prune()
}()
}

Expand Down
7 changes: 6 additions & 1 deletion pkg/skaffold/build/bazel/bazel.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (b *Builder) local(ctx context.Context, out io.Writer, tags tag.ImageTags,
if err != nil {
return nil, errors.Wrap(err, "getting current cluster context")
}
builder, err := local.NewBuilder(l, kubeContext, b.opts.SkipTests)
builder, err := local.NewBuilder(l, kubeContext, b.opts.Prune, b.opts.SkipTests)
if err != nil {
return nil, errors.Wrap(err, "getting local builder")
}
Expand All @@ -114,6 +114,11 @@ func (b *Builder) local(ctx context.Context, out io.Writer, tags tag.ImageTags,
return builder.Build(ctx, out, tags, artifacts)
}

func (b *Builder) Prune(ctx context.Context, out io.Writer) error {
// TODO(nkubala): implement
return nil
}

func setArtifact(artifact *latest.Artifact) error {
if artifact.ArtifactType.BazelArtifact != nil {
return nil
Expand Down
33 changes: 31 additions & 2 deletions pkg/skaffold/build/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/docker/docker/api/types"
"github.com/google/go-containerregistry/pkg/name"
homedir "github.com/mitchellh/go-homedir"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

yaml "gopkg.in/yaml.v2"
)

Expand All @@ -61,6 +63,7 @@ type Cache struct {
cacheFile string
useCache bool
needsPush bool
prune bool
}

var (
Expand All @@ -86,7 +89,7 @@ func NewCache(builder Builder, opts *skafconfig.SkaffoldOptions, needsPush bool)
logrus.Warnf("Error retrieving artifact cache, not using skaffold cache: %v", err)
return noCache
}
client, err := docker.NewAPIClient(!opts.NoPrune)
client, err := docker.NewAPIClient(opts.Prune)
if err != nil {
logrus.Warnf("Error retrieving local daemon client, not using skaffold cache: %v", err)
return noCache
Expand All @@ -98,6 +101,7 @@ func NewCache(builder Builder, opts *skafconfig.SkaffoldOptions, needsPush bool)
client: client,
builder: builder,
needsPush: needsPush,
prune: opts.Prune,
}
}

Expand Down Expand Up @@ -139,7 +143,7 @@ func (c *Cache) RetrieveCachedArtifacts(ctx context.Context, out io.Writer, arti
artifact, err := c.resolveCachedArtifact(ctx, out, a)
if err != nil {
logrus.Debugf("error retrieving cached artifact for %s: %v\n", a.ImageName, err)
color.Red.Fprintf(out, "Unable to retrieve %s from cache; this image will be rebuilt.", a.ImageName)
color.Red.Fprintln(out, "Unable to retrieve %s from cache; this image will be rebuilt.", a.ImageName)
needToBuild = append(needToBuild, a)
continue
}
Expand Down Expand Up @@ -297,6 +301,12 @@ func (c *Cache) CacheArtifacts(ctx context.Context, artifacts []*latest.Artifact
logrus.Debugf("both image id and digest are empty for %s, skipping caching", tags[a.ImageName])
continue
}
entry, ok := c.artifactCache[hash]
if ok && c.prune {
if err := c.Prune(ctx, entry.ID); err != nil {
logrus.Warnf("error pruning evicted cache entry %s: %s", entry.ID, err.Error())
}
}
c.artifactCache[hash] = ImageDetails{
Digest: digest,
ID: id,
Expand Down Expand Up @@ -352,6 +362,25 @@ func (c *Cache) save() error {
return ioutil.WriteFile(c.cacheFile, data, 0755)
}

func (c *Cache) Prune(ctx context.Context, imageID string) error {
resp, err := c.client.ImageRemove(ctx, imageID, types.ImageRemoveOptions{
Force: true,
PruneChildren: true,
})
if err != nil {
return errors.Wrap(err, "pruning images")
}
for _, r := range resp {
if r.Deleted != "" {
logrus.Infof("deleted image %s\n", r.Deleted)
}
if r.Untagged != "" {
logrus.Infof("untagged image %s\n", r.Untagged)
}
}
return nil
}

func getHashForArtifact(ctx context.Context, builder Builder, a *latest.Artifact) (string, error) {
deps, err := builder.DependenciesForArtifact(ctx, a)
if err != nil {
Expand Down
14 changes: 11 additions & 3 deletions pkg/skaffold/build/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import (
type Builder struct {
opts *config.SkaffoldOptions
env *latest.ExecutionEnvironment

builder build.Builder
}

// NewBuilder creates a new Builder that builds artifacts with Docker.
Expand Down Expand Up @@ -102,7 +104,8 @@ func (b *Builder) local(ctx context.Context, out io.Writer, tags tag.ImageTags,
if err != nil {
return nil, errors.Wrap(err, "getting current cluster context")
}
builder, err := local.NewBuilder(l, kubeContext, b.opts.SkipTests)
builder, err := local.NewBuilder(l, kubeContext, b.opts.Prune, b.opts.SkipTests)
b.builder = builder
if err != nil {
return nil, errors.Wrap(err, "getting local builder")
}
Expand All @@ -111,7 +114,7 @@ func (b *Builder) local(ctx context.Context, out io.Writer, tags tag.ImageTags,
return nil, err
}
}
return builder.Build(ctx, out, tags, artifacts)
return b.builder.Build(ctx, out, tags, artifacts)
}

// googleCloudBuild sets any necessary defaults and then builds artifacts with docker in GCB
Expand All @@ -126,7 +129,12 @@ func (b *Builder) googleCloudBuild(ctx context.Context, out io.Writer, tags tag.
return nil, err
}
}
return gcb.NewBuilder(g, b.opts.SkipTests).Build(ctx, out, tags, artifacts)
b.builder = gcb.NewBuilder(g, b.opts.SkipTests)
return b.builder.Build(ctx, out, tags, artifacts)
}

func (b *Builder) Prune(ctx context.Context, out io.Writer) error {
return b.builder.Prune(ctx, out)
}

func setArtifact(artifact *latest.Artifact) error {
Expand Down
8 changes: 4 additions & 4 deletions pkg/skaffold/build/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *la
}

if b.pushImages {
digest := digestOrImageID
imageID, err := b.getImageIDForDigest(ctx, digest)
imageID, err := b.getImageIDForTag(ctx, tag)
if err != nil {
logrus.Warnf("unable to inspect image: built images may not be cleaned up correctly by skaffold")
}
b.builtImages = append(b.builtImages, imageID)
digest := digestOrImageID
return tag + "@" + digest, nil
}

Expand Down Expand Up @@ -132,8 +132,8 @@ func (b *Builder) DependenciesForArtifact(ctx context.Context, a *latest.Artifac
return util.AbsolutePaths(a.Workspace, paths), nil
}

func (b *Builder) getImageIDForDigest(ctx context.Context, digest string) (string, error) {
insp, _, err := b.api.ImageInspectWithRaw(ctx, digest)
func (b *Builder) getImageIDForTag(ctx context.Context, tag string) (string, error) {
insp, _, err := b.localDocker.ImageInspectWithRaw(ctx, tag)
if err != nil {
return "", errors.Wrap(err, "inspecting image")
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ type Builder struct {
}

// NewBuilder returns an new instance of a local Builder.
func NewBuilder(cfg *latest.LocalBuild, kubeContext string, noPrune bool, skipTests bool) (*Builder, error) {
api, err := docker.NewAPIClient(!noPrune)
func NewBuilder(cfg *latest.LocalBuild, kubeContext string, prune bool, skipTests bool) (*Builder, error) {
localDocker, err := docker.NewAPIClient(prune)
if err != nil {
return nil, errors.Wrap(err, "getting docker client")
}
Expand All @@ -70,7 +70,7 @@ func NewBuilder(cfg *latest.LocalBuild, kubeContext string, noPrune bool, skipTe
localCluster: localCluster,
pushImages: pushImages,
skipTests: skipTests,
prune: !noPrune,
prune: prune,
}, nil
}

Expand All @@ -91,7 +91,7 @@ func (b *Builder) Labels() map[string]string {
// Prune uses the docker API client to remove all images built with Skaffold
func (b *Builder) Prune(ctx context.Context, out io.Writer) error {
for _, id := range b.builtImages {
resp, err := b.api.ImageRemove(ctx, id, types.ImageRemoveOptions{
resp, err := b.localDocker.ImageRemove(ctx, id, types.ImageRemoveOptions{
Force: true,
PruneChildren: true,
})
Expand Down
9 changes: 9 additions & 0 deletions pkg/skaffold/build/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,15 @@ func (b *Builder) DependenciesForArtifact(ctx context.Context, artifact *latest.
return nil, errors.New("couldn't find plugin builder to get dependencies for artifact")
}

func (b *Builder) Prune(ctx context.Context, out io.Writer) error {
for name, builder := range b.Builders {
if err := builder.Prune(ctx, out); err != nil {
return errors.Wrapf(err, "pruning images for builder %s", name)
}
}
return nil
}

func retrieveArtifactsByPlugin(artifacts []*latest.Artifact) map[string][]*latest.Artifact {
m := map[string][]*latest.Artifact{}
for _, a := range artifacts {
Expand Down
1 change: 1 addition & 0 deletions pkg/skaffold/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type SkaffoldOptions struct {
ExperimentalGUI bool
EnableRPC bool
NoPrune bool
Prune bool // this is a logical combination of NoPrune and CacheArtifacts
Profiles []string
CustomTag string
Namespace string
Expand Down
10 changes: 10 additions & 0 deletions pkg/skaffold/docker/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ type LocalDaemon interface {
Load(ctx context.Context, out io.Writer, input io.Reader, ref string) (string, error)
Tag(ctx context.Context, image, ref string) error
ImageID(ctx context.Context, ref string) (string, error)
ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error)
ImageRemove(ctx context.Context, image string, opts types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error)
RepoDigest(ctx context.Context, ref string) (string, error)
FindTaggedImageByDigest(ctx context.Context, id string) (string, error)
FindImageByID(ctx context.Context, id string) (string, error)
Expand Down Expand Up @@ -345,6 +347,14 @@ func (l *localDaemon) FindTaggedImageByDigest(ctx context.Context, digest string
return "", nil
}

func (l *localDaemon) ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) {
return l.apiClient.ImageInspectWithRaw(ctx, image)
}

func (l *localDaemon) ImageRemove(ctx context.Context, image string, opts types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error) {
return l.apiClient.ImageRemove(ctx, image, opts)
}

func getDigest(img string) string {
ref, _ := name.NewDigest(img, name.WeakValidation)
return ref.DigestStr()
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/plugin/environments/gcb/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,5 @@ func (b *Builder) DependenciesForArtifact(ctx context.Context, a *latest.Artifac
}

func (b *Builder) Prune(ctx context.Context, out io.Writer) error {
return nil
return nil // noop
}
12 changes: 10 additions & 2 deletions pkg/skaffold/plugin/shared/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (b *BuilderRPC) Init(opts *config.SkaffoldOptions, env *latest.ExecutionEnv
b.client.Call("Plugin.Init", args, &resp)
}

func (b *BuilderRPC) DependenciesForArtifact(ctx context.Context, artifact *latest.Artifact) ([]string, error) {
func (b *BuilderRPC) DependenciesForArtifact(_ context.Context, artifact *latest.Artifact) ([]string, error) {
var resp []string
if err := convertPropertiesToBytes([]*latest.Artifact{artifact}); err != nil {
return nil, errors.Wrapf(err, "converting properties to bytes")
Expand All @@ -70,7 +70,7 @@ func (b *BuilderRPC) Labels() map[string]string {
return resp
}

func (b *BuilderRPC) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) {
func (b *BuilderRPC) Build(_ context.Context, _ io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) {
var resp []build.Artifact
if err := convertPropertiesToBytes(artifacts); err != nil {
return nil, errors.Wrapf(err, "converting properties to bytes")
Expand All @@ -86,6 +86,14 @@ func (b *BuilderRPC) Build(ctx context.Context, out io.Writer, tags tag.ImageTag
return resp, nil
}

func (b *BuilderRPC) Prune(_ context.Context, _ io.Writer) error {
var resp error
if err := b.client.Call("Plugin.Prune", new(interface{}), &resp); err != nil {
return err
}
return resp
}

func convertPropertiesToBytes(artifacts []*latest.Artifact) error {
for _, a := range artifacts {
if a.BuilderPlugin.Properties == nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func getBuilder(cfg *latest.BuildConfig, kubeContext string, opts *config.Skaffo

case cfg.LocalBuild != nil:
logrus.Debugln("Using builder: local")
return local.NewBuilder(cfg.LocalBuild, kubeContext, opts.NoPrune, opts.SkipTests)
return local.NewBuilder(cfg.LocalBuild, kubeContext, opts.Prune, opts.SkipTests)

case cfg.GoogleCloudBuild != nil:
logrus.Debugln("Using builder: google cloud")
Expand Down Expand Up @@ -331,6 +331,7 @@ func (r *SkaffoldRunner) BuildAndTest(ctx context.Context, out io.Writer, artifa
if err != nil {
return nil, errors.Wrap(err, "build failed")
}
// TODO(nkubala): prune evicted cache entries here
artifactCache.Retag(ctx, out, artifactsToBuild, bRes)
bRes = append(bRes, res...)
if err := artifactCache.CacheArtifacts(ctx, artifacts, bRes); err != nil {
Expand Down

0 comments on commit 3e067ee

Please sign in to comment.