From 6b26b8b30da7542163d51bf7ee7347abaa7c380b Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 11 Jul 2019 18:17:32 -0700 Subject: [PATCH] Simplify caching This PR simplifies caching to the following rules, described in my second design doc for artifact caching: If remote cluster or push:true : 1. Check if image exists remotely. 2. If not, check if same digest exists locally. If it does, retag and repush. 3. If not, rebuild and repush. If local cluster, and push:false: 1. Check if image exists locally 2. If not, check if same digest exists locally. If it does, retag. 3. If not, rebuild --- pkg/skaffold/build/cache/retrieve.go | 230 ++++++-------- pkg/skaffold/build/cache/retrieve_test.go | 371 ++++++++-------------- testutil/fake_image_api.go | 3 +- 3 files changed, 242 insertions(+), 362 deletions(-) diff --git a/pkg/skaffold/build/cache/retrieve.go b/pkg/skaffold/build/cache/retrieve.go index a9bbb1d09e2..2f2387ee130 100644 --- a/pkg/skaffold/build/cache/retrieve.go +++ b/pkg/skaffold/build/cache/retrieve.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "sync" "time" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" @@ -42,11 +43,6 @@ type ImageDetails struct { ID string `yaml:"id,omitempty"` } -type detailsErr struct { - details *cachedArtifactDetails - err error -} - // RetrieveCachedArtifacts checks to see if artifacts are cached, and returns tags for cached images, otherwise a list of images to be built func (c *Cache) RetrieveCachedArtifacts(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) ([]*latest.Artifact, []build.Artifact, error) { if !c.useCache { @@ -54,173 +50,147 @@ func (c *Cache) RetrieveCachedArtifacts(ctx context.Context, out io.Writer, arti } start := time.Now() - color.Default.Fprintln(out, "Checking cache...") - detailsErrs := make([]chan detailsErr, len(artifacts)) + wg := sync.WaitGroup{} + builtImages := make([]bool, len(artifacts)) + needToBuildImages := make([]bool, len(artifacts)) for i := range artifacts { - detailsErrs[i] = make(chan detailsErr, 1) - + wg.Add(1) i := i go func() { - details, err := c.retrieveCachedArtifactDetails(ctx, artifacts[i]) - detailsErrs[i] <- detailsErr{details: details, err: err} + defer wg.Done() + ok, err := c.retrieveCachedArtifact(ctx, out, artifacts[i]) + if ok { + builtImages[i] = true + return + } + if err != nil { + logrus.Debugf("error finding cached artifact for %s: %v", artifacts[i].ImageName, err) + } + color.Default.Fprintf(out, " - %s: ", artifacts[i].ImageName) + color.Red.Fprintf(out, "Not Found. Rebuilding.\n") + needToBuildImages[i] = true }() } + wg.Wait() + var ( needToBuild []*latest.Artifact built []build.Artifact ) - for i, artifact := range artifacts { - color.Default.Fprintf(out, " - %s: ", artifact.ImageName) - - select { - case <-ctx.Done(): - return nil, nil, context.Canceled - - case d := <-detailsErrs[i]: - details := d.details - err := d.err - if err != nil || details.needsRebuild { - color.Red.Fprintln(out, "Not found. Rebuilding.") - needToBuild = append(needToBuild, artifact) - continue - } - - color.Green.Fprint(out, "Found") - if details.needsRetag { - color.Green.Fprint(out, ". Retagging") - } - if details.needsPush { - color.Green.Fprint(out, ". Pushing.") - } - color.Default.Fprintln(out) - - if details.needsRetag { - if err := c.client.Tag(ctx, details.prebuiltImage, details.hashTag); err != nil { - return nil, nil, errors.Wrap(err, "retagging image") - } - } - if details.needsPush { - if _, err := c.client.Push(ctx, out, details.hashTag); err != nil { - return nil, nil, errors.Wrap(err, "pushing image") - } - } - + for i, imageBuilt := range builtImages { + if imageBuilt { built = append(built, build.Artifact{ - ImageName: artifact.ImageName, - Tag: details.hashTag, + ImageName: artifacts[i].ImageName, + Tag: HashTag(artifacts[i]), }) } } + for i, imageBuilt := range needToBuildImages { + if imageBuilt { + needToBuild = append(needToBuild, artifacts[i]) + } + } + color.Default.Fprintln(out, "Cache check complete in", time.Since(start)) return needToBuild, built, nil } -type cachedArtifactDetails struct { - needsRebuild bool - needsRetag bool - needsPush bool - prebuiltImage string - hashTag string -} - -func (c *Cache) retrieveCachedArtifactDetails(ctx context.Context, a *latest.Artifact) (*cachedArtifactDetails, error) { +// retrieveCachedArtifact tries to retrieve a cached artifact. +// If it cannot, it returns false. +// If it can, it returns true. +func (c *Cache) retrieveCachedArtifact(ctx context.Context, out io.Writer, a *latest.Artifact) (bool, error) { hash, err := hashForArtifact(ctx, c.builder, a) if err != nil { - return nil, errors.Wrapf(err, "getting hash for artifact %s", a.ImageName) + return false, errors.Wrapf(err, "getting hash for artifact %s", a.ImageName) } a.WorkspaceHash = hash imageDetails, cacheHit := c.artifactCache[hash] if !cacheHit { - return &cachedArtifactDetails{ - needsRebuild: true, - }, nil + return false, nil } - hashTag := HashTag(a) - il, err := c.imageLocation(ctx, imageDetails, hashTag) - if err != nil { - return nil, errors.Wrapf(err, "getting artifact details for %s", a.ImageName) + if c.localCluster && !c.pushImages { + return c.artifactExistsLocally(ctx, out, a, imageDetails) } - return &cachedArtifactDetails{ - needsRebuild: needsRebuild(il, c.localCluster), - needsRetag: needsRetag(il), - needsPush: needsPush(il, c.localCluster, c.pushImages), - prebuiltImage: il.prebuiltImage, - hashTag: hashTag, - }, nil + return c.artifactExistsRemotely(ctx, out, a, imageDetails) } -// imageLocation holds information about where the image currently is -type imageLocation struct { - existsRemotely bool - existsLocally bool - prebuiltImage string -} - -func (c *Cache) imageLocation(ctx context.Context, imageDetails ImageDetails, tag string) (*imageLocation, error) { - // Check if tagged image exists remotely with the same digest - existsRemotely := imgExistsRemotely(tag, imageDetails.Digest, c.insecureRegistries) - existsLocally := false - if c.client != nil { - // See if this image exists in the local daemon - if c.client.ImageExists(ctx, tag) { - existsLocally = true - } - } - if existsLocally { - return &imageLocation{ - existsLocally: existsLocally, - existsRemotely: existsRemotely, - prebuiltImage: tag, - }, nil +// artifactExistsLocally assumes the artifact must exist locally. +// This is called when using a local cluster and push:false. +// 1. Check if image exists locally +// 2. If not, check if same digest exists locally. If it does, retag. +// 3. If not, rebuild +// It returns true if the artifact exists locally, and false if not. +func (c *Cache) artifactExistsLocally(ctx context.Context, out io.Writer, a *latest.Artifact, imageDetails ImageDetails) (bool, error) { + hashTag := HashTag(a) + if c.client.ImageExists(ctx, hashTag) { + color.Default.Fprintf(out, " - %s: ", a.ImageName) + color.Green.Fprintf(out, "Found\n") + return true, nil } // Check for a local image with the same digest as the image we want to build prebuiltImage, err := c.retrievePrebuiltImage(imageDetails) if err != nil { - return nil, errors.Wrapf(err, "getting prebuilt image") - } - return &imageLocation{ - existsRemotely: existsRemotely, - existsLocally: existsLocally, - prebuiltImage: prebuiltImage, - }, nil -} - -func needsRebuild(d *imageLocation, localCluster bool) bool { - // If using local cluster, rebuild if all of the following are true: - // 1. does not exist locally - // 2. can't retag a prebuilt image - if localCluster { - return !d.existsLocally && d.prebuiltImage == "" + return false, errors.Wrapf(err, "getting prebuilt image") + } + // If prebuilt image exists, tag it. Otherwise, return false, as artifact doesn't exist locally. + if prebuiltImage != "" { + color.Default.Fprintf(out, " - %s: ", a.ImageName) + color.Green.Fprintf(out, "Found. Retagging.\n") + if err := c.client.Tag(ctx, prebuiltImage, hashTag); err != nil { + return false, errors.Wrap(err, "retagging image") + } + return true, nil } - // If using remote cluster, only rebuild image if all of the following are true: - // 1. does not exist locally - // 2. does not exist remotely - // 3. can't retag a prebuilt image - return !d.existsLocally && !d.existsRemotely && d.prebuiltImage == "" + return false, nil } -func needsPush(d *imageLocation, localCluster, push bool) bool { - // If using local cluster... - if localCluster { - // ... only push if specified and image does not exist remotely - return push && !d.existsRemotely +// artifactExistsRemotely assumes the artifact must exist locally. +// this is used when running a remote cluster, or when push:true +// 1. Check if image exists remotely. +// 2. If not, check if same digest exists locally. If it does, retag and repush. +// 3. If not, rebuild. +// It returns true if the artifact exists remotely, and false if not. +func (c *Cache) artifactExistsRemotely(ctx context.Context, out io.Writer, a *latest.Artifact, imageDetails ImageDetails) (bool, error) { + hashTag := HashTag(a) + if imgExistsRemotely(hashTag, imageDetails.Digest, c.insecureRegistries) { + color.Default.Fprintf(out, " - %s: ", a.ImageName) + color.Green.Fprintf(out, "Found\n") + return true, nil + } + + // Check if image exists locally. + if c.client.ImageExists(ctx, hashTag) { + color.Default.Fprintf(out, " - %s: ", a.ImageName) + color.Green.Fprintf(out, "Found Locally. Pushing.\n") + if _, err := c.client.Push(ctx, out, hashTag); err != nil { + return false, errors.Wrap(err, "retagging image") + } + return true, nil } - // If using remote cluster, push if image does not exist remotely - return !d.existsRemotely -} -func needsRetag(d *imageLocation) bool { - // Don't need a retag if image already exists locally - if d.existsLocally { - return false + // Check for a local image with the same digest as the image we want to build + prebuiltImage, err := c.retrievePrebuiltImage(imageDetails) + if err != nil { + return false, errors.Wrapf(err, "getting prebuilt image") + } + // If prebuilt image exists, tag it and push it. Otherwise, return false, as artifact doesn't exist locally. + if prebuiltImage != "" { + color.Default.Fprintf(out, " - %s: ", a.ImageName) + color.Green.Fprintf(out, "Found Locally. Retagging and Pushing.\n") + if err := c.client.Tag(ctx, prebuiltImage, hashTag); err != nil { + return false, errors.Wrap(err, "retagging image") + } + if _, err := c.client.Push(ctx, out, hashTag); err != nil { + return false, errors.Wrap(err, "retagging image") + } + return true, nil } - // If a prebuilt image is found locally, retag the image - return d.prebuiltImage != "" + return false, nil } func (c *Cache) retrievePrebuiltImage(details ImageDetails) (string, error) { diff --git a/pkg/skaffold/build/cache/retrieve_test.go b/pkg/skaffold/build/cache/retrieve_test.go index 53c5ecc0f8b..a9e3a78136a 100644 --- a/pkg/skaffold/build/cache/retrieve_test.go +++ b/pkg/skaffold/build/cache/retrieve_test.go @@ -18,10 +18,7 @@ package cache import ( "context" - "fmt" "os" - "reflect" - "sort" "testing" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" @@ -31,298 +28,210 @@ import ( "github.com/docker/docker/api/types" ) -// artifactSorter joins a By function and a slice of Planets to be sorted. -type artifactSorter struct { - artifacts []*latest.Artifact -} - -// Len is part of sort.Interface. -func (s *artifactSorter) Len() int { - return len(s.artifacts) -} - -// Swap is part of sort.Interface. -func (s *artifactSorter) Swap(i, j int) { - s.artifacts[i], s.artifacts[j] = s.artifacts[j], s.artifacts[i] -} - -// Less is part of sort.Interface. It is implemented by calling the "by" closure in the sorter. -func (s *artifactSorter) Less(i, j int) bool { - return s.artifacts[i].ImageName < s.artifacts[j].ImageName -} - -func Test_RetrieveCachedArtifacts(t *testing.T) { +func TestRetrieveCachedArtifacts_Local(t *testing.T) { tests := []struct { - description string - cache *Cache - hashes map[string]string - artifacts []*latest.Artifact - expectedArtifacts []*latest.Artifact - api testutil.FakeAPIClient - expectedBuildResults []build.Artifact + description string + cache *Cache + api testutil.FakeAPIClient + hashes map[string]string + artifacts []*latest.Artifact + expectedArtifactsToBuild []*latest.Artifact + expectedBuildResults []build.Artifact + expectedTaggedImages []string }{ { - description: "useCache is false, return all artifacts", - cache: &Cache{}, - artifacts: []*latest.Artifact{{ImageName: "image1"}}, - expectedArtifacts: []*latest.Artifact{{ImageName: "image1"}}, - }, - { - description: "no artifacts in cache", - cache: &Cache{useCache: true}, - hashes: map[string]string{"image1": "hash", "image2": "hash2"}, - artifacts: []*latest.Artifact{{ImageName: "image1"}, {ImageName: "image2"}}, - expectedArtifacts: []*latest.Artifact{{ImageName: "image1", WorkspaceHash: "hash"}, {ImageName: "image2", WorkspaceHash: "hash2"}}, - }, - { - description: "one artifact in cache", + description: "no cache", + cache: &Cache{}, + }, { + description: "artifact exists locally", cache: &Cache{ - useCache: true, - artifactCache: ArtifactCache{"workspace-hash": ImageDetails{ + artifactCache: ArtifactCache{"hash": ImageDetails{ Digest: "sha256@digest", }}, }, - hashes: map[string]string{"image1": "workspace-hash", "image2": "workspace-hash-2"}, api: testutil.FakeAPIClient{ - TagToImageID: map[string]string{"image1:workspace-hash": "image1:tag"}, + TagToImageID: map[string]string{"image:hash": "image1:tag"}, ImageSummaries: []types.ImageSummary{ { RepoDigests: []string{"sha256@digest"}, - RepoTags: []string{"image1:workspace-hash"}, + RepoTags: []string{"image:hash"}, }, }, }, - artifacts: []*latest.Artifact{{ImageName: "image1"}, {ImageName: "image2"}}, - expectedBuildResults: []build.Artifact{{ImageName: "image1", Tag: "image1:workspace-hash"}}, - expectedArtifacts: []*latest.Artifact{{ImageName: "image2", WorkspaceHash: "workspace-hash-2"}}, - }, - { - description: "both artifacts in cache, but only one exists locally", - cache: &Cache{ - useCache: true, - artifactCache: ArtifactCache{ - "hash": ImageDetails{Digest: "sha256@digest1"}, - "hash2": ImageDetails{Digest: "sha256@digest2"}, + hashes: map[string]string{"image": "hash"}, + artifacts: []*latest.Artifact{{ImageName: "image"}}, + expectedBuildResults: []build.Artifact{ + { + ImageName: "image", + Tag: "image:hash", }, }, + }, { + description: "artifact exists under a different tag", + cache: &Cache{ + artifactCache: ArtifactCache{"hash": ImageDetails{ + ID: "imageID", + }}, + imageList: []types.ImageSummary{{ + ID: "imageID", + RepoTags: []string{"image:anothertag"}, + }}}, api: testutil.FakeAPIClient{ - TagToImageID: map[string]string{"image1:hash": "image1:tag"}, - ImageSummaries: []types.ImageSummary{ - { - ID: "id", - RepoDigests: []string{"sha256@digest1"}, - RepoTags: []string{"image1:hash"}, - }, + TagToImageID: map[string]string{"image:anothertag": "imageID"}, + }, + hashes: map[string]string{"image": "hash"}, + artifacts: []*latest.Artifact{{ImageName: "image"}}, + expectedBuildResults: []build.Artifact{ + { + ImageName: "image", + Tag: "image:hash", }, }, - hashes: map[string]string{"image1": "hash", "image2": "hash2"}, - artifacts: []*latest.Artifact{{ImageName: "image1"}, {ImageName: "image2"}}, - expectedArtifacts: []*latest.Artifact{{ImageName: "image2", WorkspaceHash: "hash2"}}, - expectedBuildResults: []build.Artifact{{ImageName: "image1", Tag: "image1:hash"}}, + expectedTaggedImages: []string{"image:hash"}, + }, { + description: "artifact doesn't exist", + cache: &Cache{ + artifactCache: ArtifactCache{"hash": ImageDetails{ + ID: "imageID", + }}}, + api: testutil.FakeAPIClient{}, + hashes: map[string]string{"image": "hash"}, + artifacts: []*latest.Artifact{{ImageName: "image"}}, + expectedArtifactsToBuild: []*latest.Artifact{{ImageName: "image", WorkspaceHash: "hash"}}, }, } + for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { + test.cache.localCluster = true + test.cache.useCache = true t.Override(&hashForArtifact, mockHashForArtifact(test.hashes)) test.cache.client = docker.NewLocalDaemon(&test.api, nil, false, map[string]bool{}) - actualArtifacts, actualBuildResults, err := test.cache.RetrieveCachedArtifacts(context.Background(), os.Stdout, test.artifacts) - - sort.Sort(&artifactSorter{artifacts: actualArtifacts}) - - t.CheckNoError(err) - t.CheckDeepEqual(test.expectedArtifacts, actualArtifacts) - t.CheckDeepEqual(test.expectedBuildResults, actualBuildResults) + actualArtifactsToBuild, actualBuildResults, err := test.cache.RetrieveCachedArtifacts(context.Background(), os.Stdout, test.artifacts) + t.CheckError(false, err) + t.CheckDeepEqual(actualArtifactsToBuild, test.expectedArtifactsToBuild) + t.CheckDeepEqual(actualBuildResults, test.expectedBuildResults) + t.CheckDeepEqual(test.expectedTaggedImages, test.api.Tagged) }) } } -func TestRetrieveCachedArtifactDetails(t *testing.T) { +func TestRetrieveCachedArtifacts_Remote(t *testing.T) { tests := []struct { description string - targetImageExistsRemotely bool - artifact *latest.Artifact - hashes map[string]string - digest string - api *testutil.FakeAPIClient cache *Cache - expected *cachedArtifactDetails + api testutil.FakeAPIClient + hashes map[string]string + artifacts []*latest.Artifact + targetImageExistsRemotely bool + expectedArtifactsToBuild []*latest.Artifact + expectedBuildResults []build.Artifact + expectedTaggedImages []string + expectedPushedImages []string }{ { - description: "image doesn't exist in cache, remote cluster", - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, - cache: noCache, - expected: &cachedArtifactDetails{ - needsRebuild: true, - }, - }, - { - description: "image doesn't exist in cache, local cluster", - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, - cache: noCache, - expected: &cachedArtifactDetails{ - needsRebuild: true, + description: "artifact exists remotely", + cache: &Cache{ + artifactCache: ArtifactCache{"hash": ImageDetails{ + Digest: "sha256@digest", + }}, }, - }, - { - description: "image in cache and exists remotely, remote cluster", targetImageExistsRemotely: true, - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, - api: &testutil.FakeAPIClient{ - TagToImageID: map[string]string{"image:hash": "image:tag"}, - ImageSummaries: []types.ImageSummary{ - { - RepoDigests: []string{"digest"}, - RepoTags: []string{"image:hash"}, - }, + hashes: map[string]string{"image": "hash"}, + artifacts: []*latest.Artifact{{ImageName: "image"}}, + expectedBuildResults: []build.Artifact{ + { + ImageName: "image", + Tag: "image:hash", }, }, + }, { + description: "artifact exists locally", cache: &Cache{ - useCache: true, - artifactCache: ArtifactCache{"hash": ImageDetails{Digest: "digest"}}, - }, - digest: "digest", - expected: &cachedArtifactDetails{ - hashTag: "image:hash", - prebuiltImage: "image:hash", - }, - }, - { - description: "image in cache and exists in daemon, local cluster", - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, - api: &testutil.FakeAPIClient{ - TagToImageID: map[string]string{"image:hash": "image:tag"}, - ImageSummaries: []types.ImageSummary{ + artifactCache: ArtifactCache{"hash": ImageDetails{ + Digest: "sha256@digest", + }}, + imageList: []types.ImageSummary{ { - RepoDigests: []string{"digest"}, + RepoDigests: []string{"sha256@digest"}, RepoTags: []string{"image:hash"}, }, }, }, - cache: &Cache{ - useCache: true, - localCluster: true, - artifactCache: ArtifactCache{"hash": ImageDetails{Digest: "digest"}}, - }, - digest: "digest", - expected: &cachedArtifactDetails{ - hashTag: "image:hash", - prebuiltImage: "image:hash", - }, - }, - { - description: "image in cache, prebuilt image exists, remote cluster", - targetImageExistsRemotely: true, - api: &testutil.FakeAPIClient{}, - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, - cache: &Cache{ - useCache: true, - artifactCache: ArtifactCache{"hash": ImageDetails{Digest: digest}}, - imageList: []types.ImageSummary{ + api: testutil.FakeAPIClient{ + TagToImageID: map[string]string{"image:hash": "image1:tag"}, + ImageSummaries: []types.ImageSummary{ { - RepoDigests: []string{fmt.Sprintf("image@%s", digest)}, - RepoTags: []string{"anotherimage:hash"}, + RepoDigests: []string{"sha256@digest"}, + RepoTags: []string{"image:hash"}, }, }, }, - digest: digest, - expected: &cachedArtifactDetails{ - hashTag: "image:hash", - prebuiltImage: "anotherimage:hash", - needsRetag: true, - }, - }, - { - description: "image in cache, prebuilt image exists, local cluster", - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, - api: &testutil.FakeAPIClient{}, - cache: &Cache{ - useCache: true, - localCluster: true, - artifactCache: ArtifactCache{"hash": ImageDetails{Digest: digest}}, - imageList: []types.ImageSummary{ - { - RepoDigests: []string{fmt.Sprintf("image@%s", digest)}, - RepoTags: []string{"anotherimage:hash"}, - }, + hashes: map[string]string{"image": "hash"}, + artifacts: []*latest.Artifact{{ImageName: "image"}}, + expectedBuildResults: []build.Artifact{ + { + ImageName: "image", + Tag: "image:hash", }, }, - digest: digest, - expected: &cachedArtifactDetails{ - needsRetag: true, - prebuiltImage: "anotherimage:hash", - hashTag: "image:hash", - }, - }, - { - description: "push specified, local cluster, image exists remotely", - targetImageExistsRemotely: true, - api: &testutil.FakeAPIClient{}, - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, + expectedPushedImages: []string{"image:hash"}, + }, { + description: "artifact exists locally under a different tag", cache: &Cache{ - useCache: true, - pushImages: true, - localCluster: true, - artifactCache: ArtifactCache{"hash": ImageDetails{Digest: digest}}, - imageList: []types.ImageSummary{ - { - RepoDigests: []string{fmt.Sprintf("image@%s", digest)}, - RepoTags: []string{"anotherimage:hash"}, - }, - }, + artifactCache: ArtifactCache{"hash": ImageDetails{ + ID: "imageID", + }}, + imageList: []types.ImageSummary{{ + ID: "imageID", + RepoTags: []string{"image:anothertag"}, + }}}, + api: testutil.FakeAPIClient{ + TagToImageID: map[string]string{"image:anothertag": "imageID"}, }, - digest: digest, - expected: &cachedArtifactDetails{ - needsRetag: true, - prebuiltImage: "anotherimage:hash", - hashTag: "image:hash", + hashes: map[string]string{"image": "hash"}, + artifacts: []*latest.Artifact{{ImageName: "image"}}, + expectedBuildResults: []build.Artifact{ + { + ImageName: "image", + Tag: "image:hash", + }, }, - }, - { - description: "no local daemon, image exists remotely", - artifact: &latest.Artifact{ImageName: "image"}, - hashes: map[string]string{"image": "hash"}, - targetImageExistsRemotely: true, + expectedTaggedImages: []string{"image:hash"}, + expectedPushedImages: []string{"image:hash"}, + }, { + description: "artifact doesn't exist", cache: &Cache{ - useCache: true, - pushImages: true, - artifactCache: ArtifactCache{"hash": ImageDetails{Digest: digest}}, - }, - digest: digest, - expected: &cachedArtifactDetails{ - hashTag: "image:hash", - }, + artifactCache: ArtifactCache{"hash": ImageDetails{ + ID: "imageID", + }}, + imageList: []types.ImageSummary{{ + ID: "imageID", + RepoTags: []string{"image:anothertag"}, + }}}, + api: testutil.FakeAPIClient{}, + hashes: map[string]string{"image": "hash"}, + artifacts: []*latest.Artifact{{ImageName: "image"}}, + expectedArtifactsToBuild: []*latest.Artifact{{ImageName: "image", WorkspaceHash: "hash"}}, }, } + for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { + test.cache.useCache = true t.Override(&hashForArtifact, mockHashForArtifact(test.hashes)) - - t.Override(&remoteDigest, func(string, map[string]bool) (string, error) { - return test.digest, nil - }) - t.Override(&imgExistsRemotely, func(string, string, map[string]bool) bool { return test.targetImageExistsRemotely }) - if test.api != nil { - test.cache.client = docker.NewLocalDaemon(test.api, nil, false, map[string]bool{}) - } - actual, err := test.cache.retrieveCachedArtifactDetails(context.Background(), test.artifact) - t.CheckNoError(err) - - // cmp.Diff cannot access unexported fields, so use reflect.DeepEqual here directly - if !reflect.DeepEqual(test.expected, actual) { - t.Errorf("Expected: %v, Actual: %v", test.expected, actual) - } + test.cache.client = docker.NewLocalDaemon(&test.api, nil, false, map[string]bool{}) + actualArtifactsToBuild, actualBuildResults, err := test.cache.RetrieveCachedArtifacts(context.Background(), os.Stdout, test.artifacts) + t.CheckError(false, err) + t.CheckDeepEqual(actualArtifactsToBuild, test.expectedArtifactsToBuild) + t.CheckDeepEqual(actualBuildResults, test.expectedBuildResults) + t.CheckDeepEqual(test.expectedTaggedImages, test.api.Tagged) + t.CheckDeepEqual(test.expectedPushedImages, test.api.PushedImages) }) } } diff --git a/testutil/fake_image_api.go b/testutil/fake_image_api.go index 06401f8f6ef..338c3e43e7d 100644 --- a/testutil/fake_image_api.go +++ b/testutil/fake_image_api.go @@ -43,6 +43,7 @@ type FakeAPIClient struct { ErrStream bool nextImageID int + Tagged []string Pushed []string Built []types.ImageBuildOptions PushedImages []string @@ -119,7 +120,7 @@ func (f *FakeAPIClient) ImageTag(_ context.Context, image, ref string) error { f.TagToImageID = make(map[string]string) } f.TagToImageID[ref] = imageID - + f.Tagged = append(f.Tagged, ref) return nil }