Merge pull request #882 from cvgw/u/cvgw/reuse-cached-layer

Do not recompute layers retrieved from cache

pkg/commands/cache.go

@@ -0,0 +1,37 @@
+/*
+Copyright 2020 Google LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package commands
+
+import v1 "github.com/google/go-containerregistry/pkg/v1"
+
+type Cached interface {
+	Layer() v1.Layer
+	ReadSuccess() bool
+}
+
+type caching struct {
+	layer       v1.Layer
+	readSuccess bool
+}
+
+func (c caching) Layer() v1.Layer {
+	return c.layer
+}
+
+func (c caching) ReadSuccess() bool {
+	return c.readSuccess
+}

pkg/commands/cache_test.go

@@ -0,0 +1,36 @@
+/*
+Copyright 2020 Google LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package commands
+
+import (
+	"testing"
+)
+
+func Test_caching(t *testing.T) {
+	c := caching{layer: fakeLayer{}, readSuccess: true}
+
+	actual := c.Layer().(fakeLayer)
+	expected := fakeLayer{}
+	actualLen, expectedLen := len(actual.TarContent), len(expected.TarContent)
+	if actualLen != expectedLen {
+		t.Errorf("expected layer tar content to be %v but was %v", expectedLen, actualLen)
+	}
+
+	if !c.ReadSuccess() {
+		t.Errorf("expected ReadSuccess to be %v but was %v", true, c.ReadSuccess())
+	}
+}

pkg/commands/copy.go

@@ -169,6 +169,7 @@ func (c *CopyCommand) From() string {
 
 type CachingCopyCommand struct {
 	BaseCommand
+	caching
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.CopyCommand
@@ -189,6 +190,13 @@ func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *docke
 		return errors.Wrapf(err, "retrieve image layers")
 	}
 
+	if len(layers) != 1 {
+		return errors.New(fmt.Sprintf("expected %d layers but got %d", 1, len(layers)))
+	}
+
+	cr.layer = layers[0]
+	cr.readSuccess = true
+
 	cr.extractedFiles, err = util.GetFSFromLayers(RootDir, layers, util.ExtractFunc(cr.extractFn), util.IncludeWhiteout())
 
 	logrus.Debugf("extractedFiles: %s", cr.extractedFiles)

pkg/commands/copy_test.go

@@ -305,14 +305,14 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
 			c := &CachingCopyCommand{
 				img: fakeImage{},
 			}
-			tc := testCase{
-				desctiption: "with image containing no layers",
-			}
 			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
 				return nil
 			}
-			tc.command = c
-			return tc
+			return testCase{
+				desctiption: "with image containing no layers",
+				expectErr:   true,
+				command:     c,
+			}
 		}(),
 		func() testCase {
 			c := &CachingCopyCommand{
@@ -375,6 +375,15 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
 				}
 			}
 
+			if c.layer == nil && tc.expectLayer {
+				t.Error("expected the command to have a layer set but instead was nil")
+			} else if c.layer != nil && !tc.expectLayer {
+				t.Error("expected the command to have no layer set but instead found a layer")
+			}
+
+			if c.readSuccess != tc.expectLayer {
+				t.Errorf("expected read success to be %v but was %v", tc.expectLayer, c.readSuccess)
+			}
 		})
 	}
 }

pkg/commands/run.go

@@ -161,6 +161,7 @@ func (r *RunCommand) ShouldCacheOutput() bool {
 
 type CachingRunCommand struct {
 	BaseCommand
+	caching
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.RunCommand
@@ -180,6 +181,13 @@ func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *docker
 		return errors.Wrap(err, "retrieving image layers")
 	}
 
+	if len(layers) != 1 {
+		return errors.New(fmt.Sprintf("expected %d layers but got %d", 1, len(layers)))
+	}
+
+	cr.layer = layers[0]
+	cr.readSuccess = true
+
 	cr.extractedFiles, err = util.GetFSFromLayers(
 		constants.RootDir,
 		layers,

pkg/commands/run_test.go

@@ -238,14 +238,16 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
 			c := &CachingRunCommand{
 				img: fakeImage{},
 			}
-			tc := testCase{
-				desctiption: "with image containing no layers",
-			}
+
 			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
 				return nil
 			}
-			tc.command = c
-			return tc
+
+			return testCase{
+				desctiption: "with image containing no layers",
+				expectErr:   true,
+				command:     c,
+			}
 		}(),
 		func() testCase {
 			c := &CachingRunCommand{
@@ -310,6 +312,16 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
 					t.Errorf("expected files used from context to be empty but was not")
 				}
 			}
+
+			if c.layer == nil && tc.expectLayer {
+				t.Error("expected the command to have a layer set but instead was nil")
+			} else if c.layer != nil && !tc.expectLayer {
+				t.Error("expected the command to have no layer set but instead found a layer")
+			}
+
+			if c.readSuccess != tc.expectLayer {
+				t.Errorf("expected read success to be %v but was %v", tc.expectLayer, c.readSuccess)
+			}
 		})
 	}
 }

pkg/executor/build.go

@@ -326,29 +326,47 @@ func (s *stageBuilder) build() error {
 			continue
 		}
 
-		tarPath, err := s.takeSnapshot(files)
-		if err != nil {
-			return errors.Wrap(err, "failed to take snapshot")
+		fn := func() bool {
+			switch v := command.(type) {
+			case commands.Cached:
+				return v.ReadSuccess()
+			default:
+				return false
+			}
 		}
 
-		logrus.Debugf("build: composite key for command %v %v", command.String(), compositeKey)
-		ck, err := compositeKey.Hash()
-		if err != nil {
-			return errors.Wrap(err, "failed to hash composite key")
-		}
+		if fn() {
+			v := command.(commands.Cached)
+			layer := v.Layer()
+			if err := s.saveLayerToImage(layer, command.String()); err != nil {
+				return err
+			}
+		} else {
+			tarPath, err := s.takeSnapshot(files)
+			if err != nil {
+				return errors.Wrap(err, "failed to take snapshot")
+			}
+
+			logrus.Debugf("build: composite key for command %v %v", command.String(), compositeKey)
+			ck, err := compositeKey.Hash()
+			if err != nil {
+				return errors.Wrap(err, "failed to hash composite key")
+			}
 
-		logrus.Debugf("build: cache key for command %v %v", command.String(), ck)
+			logrus.Debugf("build: cache key for command %v %v", command.String(), ck)
 
-		// Push layer to cache (in parallel) now along with new config file
-		if s.opts.Cache && command.ShouldCacheOutput() {
-			cacheGroup.Go(func() error {
-				return s.pushCache(s.opts, ck, tarPath, command.String())
-			})
-		}
-		if err := s.saveSnapshotToImage(command.String(), tarPath); err != nil {
-			return errors.Wrap(err, "failed to save snapshot to image")
+			// Push layer to cache (in parallel) now along with new config file
+			if s.opts.Cache && command.ShouldCacheOutput() {
+				cacheGroup.Go(func() error {
+					return s.pushCache(s.opts, ck, tarPath, command.String())
+				})
+			}
+			if err := s.saveSnapshotToImage(command.String(), tarPath); err != nil {
+				return errors.Wrap(err, "failed to save snapshot to image")
+			}
 		}
 	}
+
 	if err := cacheGroup.Wait(); err != nil {
 		logrus.Warnf("error uploading layer to cache: %s", err)
 	}
@@ -398,22 +416,40 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool {
 }
 
 func (s *stageBuilder) saveSnapshotToImage(createdBy string, tarPath string) error {
-	if tarPath == "" {
+	layer, err := s.saveSnapshotToLayer(tarPath)
+	if err != nil {
+		return err
+	}
+
+	if layer == nil {
 		return nil
 	}
+
+	return s.saveLayerToImage(layer, createdBy)
+}
+
+func (s *stageBuilder) saveSnapshotToLayer(tarPath string) (v1.Layer, error) {
+	if tarPath == "" {
+		return nil, nil
+	}
 	fi, err := os.Stat(tarPath)
 	if err != nil {
-		return errors.Wrap(err, "tar file path does not exist")
+		return nil, errors.Wrap(err, "tar file path does not exist")
 	}
 	if fi.Size() <= emptyTarSize {
 		logrus.Info("No files were changed, appending empty layer to config. No layer added to image.")
-		return nil
+		return nil, nil
 	}
 
 	layer, err := tarball.LayerFromFile(tarPath)
 	if err != nil {
-		return err
+		return nil, err
 	}
+
+	return layer, nil
+}
+func (s *stageBuilder) saveLayerToImage(layer v1.Layer, createdBy string) error {
+	var err error
 	s.image, err = mutate.Append(s.image,
 		mutate.Addendum{
 			Layer: layer,