Merge pull request #888 from cvgw/u/cvgw/caching_unit_tests

add unit tests for caching run and copy

pkg/commands/copy.go

@@ -17,6 +17,7 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 
@@ -156,8 +157,9 @@ func (c *CopyCommand) ShouldCacheOutput() bool {
 func (c *CopyCommand) CacheCommand(img v1.Image) DockerCommand {
 
 	return &CachingCopyCommand{
-		img: img,
-		cmd: c.cmd,
+		img:       img,
+		cmd:       c.cmd,
+		extractFn: util.ExtractFile,
 	}
 }
 
@@ -170,16 +172,23 @@ type CachingCopyCommand struct {
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.CopyCommand
+	extractFn      util.ExtractFunction
 }
 
 func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	logrus.Infof("Found cached layer, extracting to filesystem")
 	var err error
-	cr.extractedFiles, err = util.GetFSFromImage(RootDir, cr.img)
+
+	if cr.img == nil {
+		return errors.New(fmt.Sprintf("cached command image is nil %v", cr.String()))
+	}
+	cr.extractedFiles, err = util.GetFSFromImage(RootDir, cr.img, cr.extractFn)
+
 	logrus.Infof("extractedFiles: %s", cr.extractedFiles)
 	if err != nil {
 		return errors.Wrap(err, "extracting fs from image")
 	}
+
 	return nil
 }
 
@@ -188,6 +197,9 @@ func (cr *CachingCopyCommand) FilesToSnapshot() []string {
 }
 
 func (cr *CachingCopyCommand) String() string {
+	if cr.cmd == nil {
+		return "nil command"
+	}
 	return cr.cmd.String()
 }
 

pkg/commands/copy_test.go

@@ -16,6 +16,7 @@ limitations under the License.
 package commands
 
 import (
+	"archive/tar"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -215,3 +216,143 @@ func Test_resolveIfSymlink(t *testing.T) {
 		})
 	}
 }
+
+func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
+	tarContent, err := prepareTarFixture([]string{"foo.txt"})
+	if err != nil {
+		t.Errorf("couldn't prepare tar fixture %v", err)
+	}
+
+	config := &v1.Config{}
+	buildArgs := &dockerfile.BuildArgs{}
+
+	type testCase struct {
+		desctiption    string
+		expectLayer    bool
+		expectErr      bool
+		count          *int
+		expectedCount  int
+		command        *CachingCopyCommand
+		extractedFiles []string
+		contextFiles   []string
+	}
+	testCases := []testCase{
+		func() testCase {
+			c := &CachingCopyCommand{
+				img: fakeImage{
+					ImageLayers: []v1.Layer{
+						fakeLayer{TarContent: tarContent},
+					},
+				},
+				cmd: &instructions.CopyCommand{
+					SourcesAndDest: []string{
+						"foo.txt", "foo.txt",
+					},
+				},
+			}
+			count := 0
+			tc := testCase{
+				desctiption:    "with valid image and valid layer",
+				count:          &count,
+				expectedCount:  1,
+				expectLayer:    true,
+				extractedFiles: []string{"/foo.txt"},
+				contextFiles:   []string{"foo.txt"},
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				*tc.count++
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			c := &CachingCopyCommand{}
+			tc := testCase{
+				desctiption: "with no image",
+				expectErr:   true,
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			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
+		}(),
+		func() testCase {
+			c := &CachingCopyCommand{
+				img: fakeImage{
+					ImageLayers: []v1.Layer{
+						fakeLayer{},
+					},
+				},
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc := testCase{
+				desctiption: "with image one layer which has no tar content",
+				expectErr:   false, // this one probably should fail but doesn't because of how ExecuteCommand and util.GetFSFromLayers are implemented - cvgw- 2019-11-25
+				expectLayer: true,
+			}
+			tc.command = c
+			return tc
+		}(),
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desctiption, func(t *testing.T) {
+			c := tc.command
+			err := c.ExecuteCommand(config, buildArgs)
+			if !tc.expectErr && err != nil {
+				t.Errorf("Expected err to be nil but was %v", err)
+			} else if tc.expectErr && err == nil {
+				t.Error("Expected err but was nil")
+			}
+
+			if tc.count != nil {
+				if *tc.count != tc.expectedCount {
+					t.Errorf("Expected extractFn to be called %v times but was called %v times", tc.expectedCount, *tc.count)
+				}
+				for _, file := range tc.extractedFiles {
+					match := false
+					cFiles := c.FilesToSnapshot()
+					for _, cFile := range cFiles {
+						if file == cFile {
+							match = true
+							break
+						}
+					}
+					if !match {
+						t.Errorf("Expected extracted files to include %v but did not %v", file, cFiles)
+					}
+				}
+				// CachingCopyCommand does not override BaseCommand
+				// FilesUseFromContext so this will always return an empty slice and no error
+				// This seems like it might be a bug as it results in CopyCommands and CachingCopyCommands generating different cache keys - cvgw - 2019-11-27
+				cmdFiles, err := c.FilesUsedFromContext(
+					config, buildArgs,
+				)
+				if err != nil {
+					t.Errorf("failed to get files used from context from command")
+				}
+				if len(cmdFiles) != 0 {
+					t.Errorf("expected files used from context to be empty but was not")
+				}
+			}
+
+		})
+	}
+}

pkg/commands/fake_commands.go

@@ -0,0 +1,88 @@
+/*
+Copyright 2018 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.
+*/
+
+// used for testing in the commands package
+package commands
+
+import (
+	"bytes"
+	"io"
+	"io/ioutil"
+
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/types"
+)
+
+type fakeLayer struct {
+	TarContent []byte
+}
+
+func (f fakeLayer) Digest() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeLayer) DiffID() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeLayer) Compressed() (io.ReadCloser, error) {
+	return nil, nil
+}
+func (f fakeLayer) Uncompressed() (io.ReadCloser, error) {
+	return ioutil.NopCloser(bytes.NewReader(f.TarContent)), nil
+}
+func (f fakeLayer) Size() (int64, error) {
+	return 0, nil
+}
+func (f fakeLayer) MediaType() (types.MediaType, error) {
+	return "", nil
+}
+
+type fakeImage struct {
+	ImageLayers []v1.Layer
+}
+
+func (f fakeImage) Layers() ([]v1.Layer, error) {
+	return f.ImageLayers, nil
+}
+func (f fakeImage) MediaType() (types.MediaType, error) {
+	return "", nil
+}
+func (f fakeImage) Size() (int64, error) {
+	return 0, nil
+}
+func (f fakeImage) ConfigName() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeImage) ConfigFile() (*v1.ConfigFile, error) {
+	return &v1.ConfigFile{}, nil
+}
+func (f fakeImage) RawConfigFile() ([]byte, error) {
+	return []byte{}, nil
+}
+func (f fakeImage) Digest() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeImage) Manifest() (*v1.Manifest, error) {
+	return &v1.Manifest{}, nil
+}
+func (f fakeImage) RawManifest() ([]byte, error) {
+	return []byte{}, nil
+}
+func (f fakeImage) LayerByDigest(v1.Hash) (v1.Layer, error) {
+	return fakeLayer{}, nil
+}
+func (f fakeImage) LayerByDiffID(v1.Hash) (v1.Layer, error) {
+	return fakeLayer{}, nil
+}

pkg/commands/run.go

@@ -164,8 +164,9 @@ func (r *RunCommand) FilesToSnapshot() []string {
 func (r *RunCommand) CacheCommand(img v1.Image) DockerCommand {
 
 	return &CachingRunCommand{
-		img: img,
-		cmd: r.cmd,
+		img:       img,
+		cmd:       r.cmd,
+		extractFn: util.ExtractFile,
 	}
 }
 
@@ -186,15 +187,21 @@ type CachingRunCommand struct {
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.RunCommand
+	extractFn      util.ExtractFunction
 }
 
 func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	logrus.Infof("Found cached layer, extracting to filesystem")
 	var err error
-	cr.extractedFiles, err = util.GetFSFromImage(constants.RootDir, cr.img)
+
+	if cr.img == nil {
+		return errors.New(fmt.Sprintf("command image is nil %v", cr.String()))
+	}
+	cr.extractedFiles, err = util.GetFSFromImage(constants.RootDir, cr.img, cr.extractFn)
 	if err != nil {
 		return errors.Wrap(err, "extracting fs from image")
 	}
+
 	return nil
 }
 
@@ -203,5 +210,8 @@ func (cr *CachingRunCommand) FilesToSnapshot() []string {
 }
 
 func (cr *CachingRunCommand) String() string {
+	if cr.cmd == nil {
+		return "nil command"
+	}
 	return cr.cmd.String()
 }