Add config as point to ExecuteCommand, fix snapshots

executor/cmd/root.go

@@ -23,7 +23,6 @@ import (
 	"github.com/GoogleCloudPlatform/k8s-container-builder/pkg/image"
 	"github.com/GoogleCloudPlatform/k8s-container-builder/pkg/snapshot"
 	"github.com/GoogleCloudPlatform/k8s-container-builder/pkg/util"
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"io/ioutil"
@@ -90,41 +89,35 @@ func execute() error {
 		return err
 	}
 
-	// Execute commands here
+	// Set environment variables within the image
 	if err := image.SetEnvVariables(sourceImage); err != nil {
 		return err
 	}
 
+	imageConfig := sourceImage.Config()
 	// Currently only supports single stage builds
 	for _, stage := range stages {
 		for _, cmd := range stage.Commands {
-			dockerCommand := commands.GetCommand(cmd)
+			dockerCommand, err := commands.GetCommand(cmd)
-			if dockerCommand == nil {
+			if err != nil {
-				return errors.Errorf("Invalid or unsupported docker command: %v", cmd)
-			}
-			if err := dockerCommand.ExecuteCommand(); err != nil {
 				return err
 			}
-			// Now, we get the files to snapshot from this command
+			if err := dockerCommand.ExecuteCommand(imageConfig); err != nil {
-			// If this is nil, snapshot the entire filesystem
+				return err
-			// Else take a snapshot of the specific files
-			snapshotFiles := dockerCommand.FilesToSnapshot()
-			var contents []byte
-			if snapshotFiles == nil {
-				contents, err = snapshotter.TakeSnapshot()
-			} else {
-				contents, err = snapshotter.TakeSnapshotOfFiles(snapshotFiles)
 			}
+			// Now, we get the files to snapshot from this command and take the snapshot
+			snapshotFiles := dockerCommand.FilesToSnapshot()
+			contents, err := snapshotter.TakeSnapshot(snapshotFiles)
 			if err != nil {
 				return err
 			}
 			if contents == nil {
 				logrus.Info("No files were changed, appending empty layer to config.")
-				sourceImage.AppendConfigHistory(dockerCommand.Author(), true)
+				sourceImage.AppendConfigHistory(constants.Author, true)
 				continue
 			}
 			// Append the layer to the image
-			if err := sourceImage.AppendLayer(contents, dockerCommand.Author()); err != nil {
+			if err := sourceImage.AppendLayer(contents, constants.Author); err != nil {
 				return err
 			}
 		}

pkg/commands/commands.go

@@ -17,23 +17,27 @@ limitations under the License.
 package commands
 
 import (
+	"github.com/containers/image/manifest"
 	"github.com/docker/docker/builder/dockerfile/instructions"
-	"github.com/sirupsen/logrus"
+	"github.com/pkg/errors"
 )
 
 type DockerCommand interface {
-	ExecuteCommand() error
+	// ExecuteCommand is responsible for:
-	// The config file has an "author" field, should return information about the command
+	// 	1. Making required changes to the filesystem (ex. copying files for ADD/COPY or setting ENV variables)
-	Author() string
+	//  2. Updating metadata fields in the config
+	// It should not change the config history.
+	ExecuteCommand(*manifest.Schema2Config) error
+	// The config history has a "created by" field, should return information about the command
+	CreatedBy() string
 	// A list of files to snapshot, empty for metadata commands or nil if we don't know
 	FilesToSnapshot() []string
 }
 
-func GetCommand(cmd instructions.Command) DockerCommand {
+func GetCommand(cmd instructions.Command) (DockerCommand, error) {
 	switch c := cmd.(type) {
 	case *instructions.RunCommand:
-		return &RunCommand{cmd: c}
+		return &RunCommand{cmd: c}, nil
 	}
-	logrus.Errorf("%s is not a supported command.", cmd.Name())
+	return nil, errors.Errorf("%s is not a supported command", cmd.Name())
-	return nil
 }

pkg/commands/run.go

@@ -17,7 +17,7 @@ limitations under the License.
 package commands
 
 import (
-	"github.com/GoogleCloudPlatform/k8s-container-builder/pkg/constants"
+	"github.com/containers/image/manifest"
 	"github.com/docker/docker/builder/dockerfile/instructions"
 	"github.com/sirupsen/logrus"
 	"os"
@@ -29,10 +29,11 @@ type RunCommand struct {
 	cmd *instructions.RunCommand
 }
 
-func (r *RunCommand) ExecuteCommand() error {
+func (r *RunCommand) ExecuteCommand(config *manifest.Schema2Config) error {
 	var newCommand []string
 	if r.cmd.PrependShell {
 		// This is the default shell on Linux
+		// TODO: Support shell command here
 		shell := []string{"/bin/sh", "-c"}
 		newCommand = append(shell, strings.Join(r.cmd.CmdLine, " "))
 	} else {
@@ -54,14 +55,12 @@ func (r *RunCommand) FilesToSnapshot() []string {
 }
 
 // Author returns some information about the command for the image config
-func (r *RunCommand) Author() string {
+func (r *RunCommand) CreatedBy() string {
-	author := []string{constants.Author}
 	cmdLine := strings.Join(r.cmd.CmdLine, " ")
 	if r.cmd.PrependShell {
+		// TODO: Support shell command here
 		shell := []string{"/bin/sh", "-c"}
-		author = append(author, shell...)
+		return strings.Join(append(shell, cmdLine), " ")
-		return strings.Join(author, " ") + " " + cmdLine
 	}
-	author = append(author, cmdLine)
+	return cmdLine
-	return strings.Join(author, " ")
 }

pkg/snapshot/snapshot.go

@@ -49,7 +49,10 @@ 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, error) {
+func (s *Snapshotter) TakeSnapshot(files []string) ([]byte, error) {
+	if files != nil {
+		return s.TakeSnapshotOfFiles(files)
+	}
 	logrus.Info("Taking snapshot of full filesystem...")
 	buf := bytes.NewBuffer([]byte{})
 	filesAdded, err := s.snapShotFS(buf)

pkg/snapshot/snapshot_test.go

@@ -45,7 +45,7 @@ func TestSnapshotFileChange(t *testing.T) {
 		t.Fatalf("Error setting up fs: %s", err)
 	}
 	// Take another snapshot
-	contents, err := snapshotter.TakeSnapshot()
+	contents, err := snapshotter.TakeSnapshot(nil)
 	if err != nil {
 		t.Fatalf("Error taking snapshot of fs: %s", err)
 	}
@@ -93,7 +93,7 @@ func TestSnapshotChangePermissions(t *testing.T) {
 		t.Fatalf("Error changing permissions on %s: %v", batPath, err)
 	}
 	// Take another snapshot
-	contents, err := snapshotter.TakeSnapshot()
+	contents, err := snapshotter.TakeSnapshot(nil)
 	if err != nil {
 		t.Fatalf("Error taking snapshot of fs: %s", err)
 	}
@@ -144,7 +144,7 @@ func TestSnapshotFiles(t *testing.T) {
 		filepath.Join(testDir, "foo"),
 		filepath.Join(testDir, "kbuild/file"),
 	}
-	contents, err := snapshotter.TakeSnapshotOfFiles(filesToSnapshot)
+	contents, err := snapshotter.TakeSnapshot(filesToSnapshot)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -181,7 +181,7 @@ func TestEmptySnapshot(t *testing.T) {
 		t.Fatal(err)
 	}
 	// Take snapshot with no changes
-	contents, err := snapshotter.TakeSnapshot()
+	contents, err := snapshotter.TakeSnapshot(nil)
 	if err != nil {
 		t.Fatalf("Error taking snapshot of fs: %s", err)
 	}

pkg/util/tar_util.go

@@ -36,30 +36,13 @@ func AddToTar(p string, i os.FileInfo, w *tar.Writer) error {
 			return err
 		}
 	}
-
-	hardlink := false
-	if sys := i.Sys(); sys != nil {
-		if stat, ok := sys.(*syscall.Stat_t); ok {
-			nlinks := stat.Nlink
-			if nlinks > 1 {
-				inode := stat.Ino
-				if original, exists := hardlinks[inode]; exists && original != p {
-					hardlink = true
-					logrus.Debugf("%s inode exists in hardlinks map, linking to %s", p, original)
-					linkDst = original
-				} else {
-					hardlinks[inode] = p
-				}
-			}
-		}
-	}
-
 	hdr, err := tar.FileInfoHeader(i, linkDst)
 	if err != nil {
 		return err
 	}
 	hdr.Name = p
 
+	hardlink, linkDst := checkHardlink(p, i)
 	if hardlink {
 		hdr.Linkname = linkDst
 		hdr.Typeflag = tar.TypeLink
@@ -81,3 +64,25 @@ func AddToTar(p string, i os.FileInfo, w *tar.Writer) error {
 	}
 	return nil
 }
+
+// Returns true if path is hardlink, and the link destination
+func checkHardlink(p string, i os.FileInfo) (bool, string) {
+	hardlink := false
+	linkDst := ""
+	if sys := i.Sys(); sys != nil {
+		if stat, ok := sys.(*syscall.Stat_t); ok {
+			nlinks := stat.Nlink
+			if nlinks > 1 {
+				inode := stat.Ino
+				if original, exists := hardlinks[inode]; exists && original != p {
+					hardlink = true
+					logrus.Debugf("%s inode exists in hardlinks map, linking to %s", p, original)
+					linkDst = original
+				} else {
+					hardlinks[inode] = p
+				}
+			}
+		}
+	}
+	return hardlink, linkDst
+}