Added integration test, minor changes to files

executor/cmd/root.go

@@ -91,7 +91,7 @@ func execute() error {
 	}
 
 	// Execute commands here
-	if err := image.SetEnvVariables(); err != nil {
+	if err := image.SetEnvVariables(sourceImage); err != nil {
 		return err
 	}
 
@@ -109,36 +109,23 @@ func execute() error {
 			// If this is nil, snapshot the entire filesystem
 			// Else take a snapshot of the specific files
 			snapshotFiles := dockerCommand.FilesToSnapshot()
+			var contents []byte
 			if snapshotFiles == nil {
-				logrus.Info("Taking snapshot of full filesystem...")
+				contents, err = snapshotter.TakeSnapshot()
-				contents, filesAdded, err := snapshotter.TakeSnapshot()
-				if err != nil {
-					return err
-				}
-				if !filesAdded {
-					logrus.Info("No files were changed, appending empty layer to config.")
-					image.AppendConfigHistory(dockerCommand.Author(), true)
-					continue
-				}
-				// Append the layer to the image
-				if err := image.AppendLayer(contents, dockerCommand.Author()); err != nil {
-					return err
-				}
 			} else {
-				logrus.Infof("Taking snapshot of files %v...", snapshotFiles)
+				contents, err = snapshotter.TakeSnapshotOfFiles(snapshotFiles)
-				contents, err := snapshotter.TakeSnapshotOfFiles(snapshotFiles)
+			}
-				if err != nil {
+			if err != nil {
-					return err
+				return err
-				}
+			}
-				if contents == nil {
+			if contents == nil {
-					logrus.Info("No files were changed, appending empty layer to config.")
+				logrus.Info("No files were changed, appending empty layer to config.")
-					image.AppendConfigHistory(dockerCommand.Author(), true)
+				sourceImage.AppendConfigHistory(dockerCommand.Author(), true)
-					continue
+				continue
-				}
+			}
-				// Append the layer to the image
+			// Append the layer to the image
-				if err := image.AppendLayer(contents, dockerCommand.Author()); err != nil {
+			if err := sourceImage.AppendLayer(contents, dockerCommand.Author()); err != nil {
-					return err
+				return err
-				}
 			}
 		}
 	}

integration_tests/dockerfiles/Dockerfile_test_run

@@ -1,3 +1,17 @@
+# Copyright 2018 Google, Inc. All rights reserved.
+#
+# 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.
+
 FROM gcr.io/google-appengine/debian9
 RUN echo "hey" > /etc/foo
 RUN apt-get update && apt-get install -y \

integration_tests/dockerfiles/Dockerfile_test_run_2

@@ -0,0 +1,19 @@
+# Copyright 2018 Google, Inc. All rights reserved.
+#
+# 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.
+
+# Test to make sure the executor builds an image correctly
+# when no files are changed
+
+FROM gcr.io/google-appengine/debian9
+RUN echo "hey" 

integration_tests/dockerfiles/config_test_run.json

@@ -45,4 +45,4 @@
       ]
     }
   }
-]
+]

integration_tests/dockerfiles/config_test_run_2.json

@@ -0,0 +1,12 @@
+[
+  {
+    "Image1": "gcr.io/kbuild-test/docker-test-run-2:latest",
+    "Image2": "gcr.io/kbuild-test/kbuild-test-run-2:latest",
+    "DiffType": "File",
+    "Diff": {
+      "Adds": null,
+      "Dels": null,
+      "Mods": null
+    }
+  }
+]

integration_tests/integration_test_yaml.go

@@ -42,6 +42,13 @@ var tests = []struct {
 		context:        "integration_tests/dockerfiles/",
 		repo:           "test-run",
 	},
+	{
+		description:    "test run no files changed",
+		dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run_2",
+		configPath:     "/workspace/integration_tests/dockerfiles/config_test_run_2.json",
+		context:        "integration_tests/dockerfiles/",
+		repo:           "test-run-2",
+	},
 }
 
 type step struct {

pkg/commands/run.go

@@ -17,6 +17,7 @@ limitations under the License.
 package commands
 
 import (
+	"github.com/GoogleCloudPlatform/k8s-container-builder/pkg/constants"
 	"github.com/docker/docker/builder/dockerfile/instructions"
 	"github.com/sirupsen/logrus"
 	"os"
@@ -54,5 +55,13 @@ func (r *RunCommand) FilesToSnapshot() []string {
 
 // Author returns some information about the command for the image config
 func (r *RunCommand) Author() string {
-	return "kbuild /bin/sh -c" + strings.Join(r.cmd.CmdLine, " ")
+	author := []string{constants.Author}
+	cmdLine := strings.Join(r.cmd.CmdLine, " ")
+	if r.cmd.PrependShell {
+		shell := []string{"/bin/sh", "-c"}
+		author = append(author, shell...)
+		return strings.Join(author, " ") + " " + cmdLine
+	}
+	author = append(author, cmdLine)
+	return strings.Join(author, " ")
 }

pkg/constants/constants.go

@@ -27,4 +27,6 @@ const (
 	WorkspaceDir = "/workspace"
 
 	WhitelistPath = "/proc/self/mountinfo"
+
+	Author = "kbuild"
 )

pkg/image/image.go

@@ -56,14 +56,9 @@ func PushImage(ms *img.MutableSource, destImg string) error {
 	return copy.Image(policyContext, destRef, srcRef, nil)
 }
 
-// AppendConfigHistory appends to the source image config history
-func AppendConfigHistory(author string, emptyLayer bool) {
-	sourceImage.AppendConfigHistory(author, emptyLayer)
-}
-
 // SetEnvVariables sets environment variables as specified in the image
-func SetEnvVariables() error {
+func SetEnvVariables(ms *img.MutableSource) error {
-	envVars := sourceImage.Env()
+	envVars := ms.Env()
 	for key, val := range envVars {
 		if err := os.Setenv(key, val); err != nil {
 			return err

pkg/snapshot/snapshot.go

@@ -49,23 +49,27 @@ func (s *Snapshotter) Init() error {
 
 // TakeSnapshot takes a snapshot of the filesystem, avoiding directories in the whitelist, and creates
 // a tarball of the changed files. Return contents of the tarball, and whether or not any files were changed
-func (s *Snapshotter) TakeSnapshot() ([]byte, bool, error) {
+func (s *Snapshotter) TakeSnapshot() ([]byte, error) {
+	logrus.Info("Taking snapshot of full filesystem...")
 	buf := bytes.NewBuffer([]byte{})
 	filesAdded, err := s.snapShotFS(buf)
 	if err != nil {
-		return nil, filesAdded, err
+		return nil, err
 	}
 	contents, err := ioutil.ReadAll(buf)
 	if err != nil {
-		return nil, filesAdded, err
+		return nil, err
 	}
-	return contents, filesAdded, err
+	if !filesAdded {
+		return nil, nil
+	}
+	return contents, err
 }
 
 // TakeSnapshotOfFiles takes a snapshot of specific files
 // Used for ADD/COPY commands, when we know which files have changed
 func (s *Snapshotter) TakeSnapshotOfFiles(files []string) ([]byte, error) {
-	logrus.Infof("Taking snapshot of files %s", files)
+	logrus.Infof("Taking snapshot of files %v...", files)
 	s.l.Snapshot()
 	if len(files) == 0 {
 		logrus.Info("No files changed in this command, skipping snapshotting.")

pkg/snapshot/snapshot_test.go

@@ -45,11 +45,11 @@ func TestSnapshotFileChange(t *testing.T) {
 		t.Fatalf("Error setting up fs: %s", err)
 	}
 	// Take another snapshot
-	contents, filesAdded, err := snapshotter.TakeSnapshot()
+	contents, err := snapshotter.TakeSnapshot()
 	if err != nil {
 		t.Fatalf("Error taking snapshot of fs: %s", err)
 	}
-	if !filesAdded {
+	if contents == nil {
 		t.Fatal("No files added to snapshot.")
 	}
 	// Check contents of the snapshot, make sure contents is equivalent to snapshotFiles
@@ -93,11 +93,11 @@ func TestSnapshotChangePermissions(t *testing.T) {
 		t.Fatalf("Error changing permissions on %s: %v", batPath, err)
 	}
 	// Take another snapshot
-	contents, filesAdded, err := snapshotter.TakeSnapshot()
+	contents, err := snapshotter.TakeSnapshot()
 	if err != nil {
 		t.Fatalf("Error taking snapshot of fs: %s", err)
 	}
-	if !filesAdded {
+	if contents == nil {
 		t.Fatal("No files added to snapshot.")
 	}
 	// Check contents of the snapshot, make sure contents is equivalent to snapshotFiles
@@ -181,12 +181,12 @@ func TestEmptySnapshot(t *testing.T) {
 		t.Fatal(err)
 	}
 	// Take snapshot with no changes
-	_, filesAdded, err := snapshotter.TakeSnapshot()
+	contents, err := snapshotter.TakeSnapshot()
 	if err != nil {
 		t.Fatalf("Error taking snapshot of fs: %s", err)
 	}
 	// Since we took a snapshot with no changes, contents should be nil
-	if filesAdded {
+	if contents != nil {
 		t.Fatal("Files added even though no changes to file system were made.")
 	}
 }

pkg/util/tar_util.go

@@ -45,7 +45,7 @@ func AddToTar(p string, i os.FileInfo, w *tar.Writer) error {
 				inode := stat.Ino
 				if original, exists := hardlinks[inode]; exists && original != p {
 					hardlink = true
-					logrus.Infof("%s inode exists in hardlinks map, linking to %s", p, original)
+					logrus.Debugf("%s inode exists in hardlinks map, linking to %s", p, original)
 					linkDst = original
 				} else {
 					hardlinks[inode] = p