diff --git a/pkg/skaffold/build/cache.go b/pkg/skaffold/build/cache.go index fe7e743560c..b18ba9d4071 100644 --- a/pkg/skaffold/build/cache.go +++ b/pkg/skaffold/build/cache.go @@ -97,8 +97,7 @@ func NewCache(ctx context.Context, builder Builder, opts *skafconfig.SkaffoldOpt } client, err := newDockerCilent() if err != nil { - logrus.Warnf("Error retrieving local daemon client, not using skaffold cache: %v", err) - return noCache + logrus.Warnf("Error retrieving local daemon client; local daemon will not be used as a cache: %v", err) } imageList, err := client.ImageList(ctx, types.ImageListOptions{}) if err != nil { @@ -230,7 +229,7 @@ func (c *Cache) retrieveCachedArtifactDetails(ctx context.Context, a *latest.Art needsRebuild: true, }, nil } - hashTag := fmt.Sprintf("%s:%s", a.ImageName, hash) + hashTag := HashTag(a) // Check if we are using a local cluster var existsRemotely bool @@ -239,8 +238,14 @@ func (c *Cache) retrieveCachedArtifactDetails(ctx context.Context, a *latest.Art existsRemotely = imageExistsRemotely(hashTag, imageDetails.Digest) } + if existsRemotely { + return &cachedArtifactDetails{ + hashTag: hashTag, + }, nil + } + // See if this image exists in the local daemon - if c.client.ImageExists(ctx, hashTag) { + if c.client != nil && c.client.ImageExists(ctx, hashTag) { return &cachedArtifactDetails{ needsPush: (!existsRemotely && !localCluster) || (localCluster && c.needsPush), hashTag: hashTag, @@ -325,9 +330,12 @@ func (c *Cache) CacheArtifacts(ctx context.Context, artifacts []*latest.Artifact if digest == "" { logrus.Debugf("couldn't get image digest for %s, will try to cache just image id (expected with a local cluster)", tags[a.ImageName]) } - id, err := c.client.ImageID(ctx, tags[a.ImageName]) - if err != nil { - logrus.Debugf("couldn't get image id for %s", tags[a.ImageName]) + var id string + if c.client != nil { + id, err = c.client.ImageID(ctx, tags[a.ImageName]) + if err != nil { + logrus.Debugf("couldn't get image id for %s", tags[a.ImageName]) + } } if id == "" && digest == "" { logrus.Debugf("both image id and digest are empty for %s, skipping caching", tags[a.ImageName]) @@ -343,7 +351,7 @@ func (c *Cache) CacheArtifacts(ctx context.Context, artifacts []*latest.Artifact // Retag retags newly built images in the format [imageName:workspaceHash] and pushes them if using a remote cluster func (c *Cache) Retag(ctx context.Context, out io.Writer, artifactsToBuild []*latest.Artifact, buildArtifacts []Artifact) { - if !c.useCache || len(artifactsToBuild) == 0 { + if !c.useCache || len(artifactsToBuild) == 0 || c.client == nil { return } tags := map[string]string{} @@ -369,8 +377,12 @@ func (c *Cache) Retag(ctx context.Context, out io.Writer, artifactsToBuild []*la } } -// Check local daemon for img digest +// Check local daemon for img digest, check remote digest if that doesn't work func (c *Cache) retrieveImageDigest(ctx context.Context, img string) (string, error) { + if c.client == nil { + // Check for remote digest + return docker.RemoteDigest(img) + } repoDigest, err := c.client.RepoDigest(ctx, img) if err != nil { return docker.RemoteDigest(img) @@ -430,3 +442,7 @@ func cacheHasher(p string) (string, error) { } return hex.EncodeToString(h.Sum(nil)), nil } + +func HashTag(a *latest.Artifact) string { + return fmt.Sprintf("%s:%s", a.ImageName, a.WorkspaceHash) +} diff --git a/pkg/skaffold/build/cache_test.go b/pkg/skaffold/build/cache_test.go index d9850a0aefa..6bf36117d9c 100644 --- a/pkg/skaffold/build/cache_test.go +++ b/pkg/skaffold/build/cache_test.go @@ -272,7 +272,7 @@ func TestRetrieveCachedArtifactDetails(t *testing.T) { artifact *latest.Artifact hashes map[string]string digest string - api testutil.FakeAPIClient + api *testutil.FakeAPIClient cache *Cache expected *cachedArtifactDetails }{ @@ -299,7 +299,7 @@ func TestRetrieveCachedArtifactDetails(t *testing.T) { name: "image in cache and exists remotely, remote cluster", artifact: &latest.Artifact{ImageName: "image"}, hashes: map[string]string{"image": "hash"}, - api: testutil.FakeAPIClient{ + api: &testutil.FakeAPIClient{ TagToImageID: map[string]string{"image:hash": "image:tag"}, ImageSummaries: []types.ImageSummary{ { @@ -322,7 +322,7 @@ func TestRetrieveCachedArtifactDetails(t *testing.T) { artifact: &latest.Artifact{ImageName: "image"}, hashes: map[string]string{"image": "hash"}, localCluster: true, - api: testutil.FakeAPIClient{ + api: &testutil.FakeAPIClient{ TagToImageID: map[string]string{"image:hash": "image:tag"}, ImageSummaries: []types.ImageSummary{ { @@ -356,10 +356,7 @@ func TestRetrieveCachedArtifactDetails(t *testing.T) { }, digest: digest, expected: &cachedArtifactDetails{ - needsRetag: true, - needsPush: true, - prebuiltImage: "anotherimage:hash", - hashTag: "image:hash", + hashTag: "image:hash", }, }, { @@ -408,6 +405,20 @@ func TestRetrieveCachedArtifactDetails(t *testing.T) { hashTag: "image:hash", }, }, + { + name: "no local daemon, image exists remotely", + artifact: &latest.Artifact{ImageName: "image"}, + hashes: map[string]string{"image": "hash"}, + cache: &Cache{ + useCache: true, + needsPush: true, + artifactCache: ArtifactCache{"hash": ImageDetails{Digest: digest}}, + }, + digest: digest, + expected: &cachedArtifactDetails{ + hashTag: "image:hash", + }, + }, } for _, test := range tests { @@ -434,7 +445,9 @@ func TestRetrieveCachedArtifactDetails(t *testing.T) { remoteDigest = originalRemoteDigest }() - test.cache.client = docker.NewLocalDaemon(&test.api, nil) + if test.api != nil { + test.cache.client = docker.NewLocalDaemon(test.api, nil) + } actual, err := test.cache.retrieveCachedArtifactDetails(context.Background(), test.artifact) if err != nil { t.Fatalf("error retrieving artifact details: %v", err) diff --git a/pkg/skaffold/build/kaniko/run.go b/pkg/skaffold/build/kaniko/run.go index 69feb27b2b9..ba797a71ed5 100644 --- a/pkg/skaffold/build/kaniko/run.go +++ b/pkg/skaffold/build/kaniko/run.go @@ -21,6 +21,7 @@ import ( "fmt" "io" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/kaniko/sources" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" @@ -57,6 +58,11 @@ func (b *Builder) run(ctx context.Context, out io.Writer, artifact *latest.Artif args = append(args, b.AdditionalFlags...) args = append(args, docker.GetBuildArgs(artifact.DockerArtifact)...) + if artifact.WorkspaceHash != "" { + hashTag := build.HashTag(artifact) + args = append(args, []string{"--destination", hashTag}...) + } + if b.Cache != nil { args = append(args, "--cache=true") if b.Cache.Repo != "" { diff --git a/pkg/skaffold/plugin/environments/gcb/desc.go b/pkg/skaffold/plugin/environments/gcb/desc.go index be9878c10de..b2a594aad17 100644 --- a/pkg/skaffold/plugin/environments/gcb/desc.go +++ b/pkg/skaffold/plugin/environments/gcb/desc.go @@ -19,6 +19,7 @@ package gcb import ( "fmt" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" @@ -28,7 +29,12 @@ import ( ) func (b *Builder) buildDescription(artifact *latest.Artifact, tag, bucket, object string) (*cloudbuild.Build, error) { - steps, err := b.buildSteps(artifact, tag) + tags := []string{tag} + if artifact.WorkspaceHash != "" { + tags = append(tags, build.HashTag(artifact)) + } + + steps, err := b.buildSteps(artifact, tags) if err != nil { return nil, err } @@ -42,7 +48,7 @@ func (b *Builder) buildDescription(artifact *latest.Artifact, tag, bucket, objec }, }, Steps: steps, - Images: []string{tag}, + Images: tags, Options: &cloudbuild.BuildOptions{ DiskSizeGb: b.DiskSizeGb, MachineType: b.MachineType, @@ -51,28 +57,29 @@ func (b *Builder) buildDescription(artifact *latest.Artifact, tag, bucket, objec }, nil } -func (b *Builder) buildSteps(artifact *latest.Artifact, tag string) ([]*cloudbuild.BuildStep, error) { +func (b *Builder) buildSteps(artifact *latest.Artifact, tags []string) ([]*cloudbuild.BuildStep, error) { switch { case artifact.BuilderPlugin != nil: - return b.pluginBuildSteps(artifact, tag) + return b.pluginBuildSteps(artifact, tags) case artifact.DockerArtifact != nil: - return b.dockerBuildSteps(artifact.DockerArtifact, tag), nil + return b.dockerBuildSteps(artifact.DockerArtifact, tags), nil case artifact.BazelArtifact != nil: return nil, errors.New("skaffold can't build a bazel artifact with Google Cloud Build") + // TODO: build multiple tagged images with jib in GCB (priyawadhwa@) case artifact.JibMavenArtifact != nil: - return b.jibMavenBuildSteps(artifact.JibMavenArtifact, tag), nil + return b.jibMavenBuildSteps(artifact.JibMavenArtifact, tags[0]), nil case artifact.JibGradleArtifact != nil: - return b.jibGradleBuildSteps(artifact.JibGradleArtifact, tag), nil + return b.jibGradleBuildSteps(artifact.JibGradleArtifact, tags[0]), nil default: return nil, fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType) } } -func (b *Builder) pluginBuildSteps(artifact *latest.Artifact, tag string) ([]*cloudbuild.BuildStep, error) { +func (b *Builder) pluginBuildSteps(artifact *latest.Artifact, tags []string) ([]*cloudbuild.BuildStep, error) { switch artifact.BuilderPlugin.Name { case constants.DockerBuilderPluginName: var da *latest.DockerArtifact @@ -83,7 +90,7 @@ func (b *Builder) pluginBuildSteps(artifact *latest.Artifact, tag string) ([]*cl da = &latest.DockerArtifact{} } defaults.SetDefaultDockerArtifact(da) - return b.dockerBuildSteps(da, tag), nil + return b.dockerBuildSteps(da, tags), nil default: return nil, errors.Errorf("the '%s' builder is not supported", artifact.BuilderPlugin.Name) } diff --git a/pkg/skaffold/plugin/environments/gcb/docker.go b/pkg/skaffold/plugin/environments/gcb/docker.go index b7dadd39337..cba70191ddc 100644 --- a/pkg/skaffold/plugin/environments/gcb/docker.go +++ b/pkg/skaffold/plugin/environments/gcb/docker.go @@ -24,7 +24,7 @@ import ( cloudbuild "google.golang.org/api/cloudbuild/v1" ) -func (b *Builder) dockerBuildSteps(artifact *latest.DockerArtifact, tag string) []*cloudbuild.BuildStep { +func (b *Builder) dockerBuildSteps(artifact *latest.DockerArtifact, tags []string) []*cloudbuild.BuildStep { var steps []*cloudbuild.BuildStep for _, cacheFrom := range artifact.CacheFrom { @@ -35,7 +35,11 @@ func (b *Builder) dockerBuildSteps(artifact *latest.DockerArtifact, tag string) }) } - args := append([]string{"build", "--tag", tag, "-f", artifact.DockerfilePath}) + args := []string{"build"} + for _, t := range tags { + args = append(args, []string{"--tag", t}...) + } + args = append(args, []string{"-f", artifact.DockerfilePath}...) args = append(args, docker.GetBuildArgs(artifact)...) args = append(args, ".") diff --git a/pkg/skaffold/plugin/environments/gcb/docker_test.go b/pkg/skaffold/plugin/environments/gcb/docker_test.go index 10db31f558c..9ac672e5b0d 100644 --- a/pkg/skaffold/plugin/environments/gcb/docker_test.go +++ b/pkg/skaffold/plugin/environments/gcb/docker_test.go @@ -82,7 +82,7 @@ func TestPullCacheFrom(t *testing.T) { DockerImage: "docker/docker", }, } - steps := builder.dockerBuildSteps(artifact, "nginx2") + steps := builder.dockerBuildSteps(artifact, []string{"nginx2"}) expected := []*cloudbuild.BuildStep{{ Name: "docker/docker",