From 27b09f28be07a9c94a4f8a9f01b2da16bfee75fe Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 9 Mar 2018 11:17:59 -0800 Subject: [PATCH] Add config as point to ExecuteCommand, fix snapshots --- executor/cmd/root.go | 29 ++++++++++--------------- pkg/commands/commands.go | 20 ++++++++++------- pkg/commands/run.go | 15 ++++++------- pkg/snapshot/snapshot.go | 5 ++++- pkg/snapshot/snapshot_test.go | 8 +++---- pkg/util/tar_util.go | 41 ++++++++++++++++++++--------------- 6 files changed, 61 insertions(+), 57 deletions(-) diff --git a/executor/cmd/root.go b/executor/cmd/root.go index d1d6fb561..a34a5f3cd 100644 --- a/executor/cmd/root.go +++ b/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) - if dockerCommand == nil { - return errors.Errorf("Invalid or unsupported docker command: %v", cmd) - } - if err := dockerCommand.ExecuteCommand(); err != nil { + dockerCommand, err := commands.GetCommand(cmd) + if err != nil { return err } - // Now, we get the files to snapshot from this command - // 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 { - contents, err = snapshotter.TakeSnapshot() - } else { - contents, err = snapshotter.TakeSnapshotOfFiles(snapshotFiles) + if err := dockerCommand.ExecuteCommand(imageConfig); err != nil { + return err } + // 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 } } diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index e2477c381..eb2cab5bc 100644 --- a/pkg/commands/commands.go +++ b/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 - // The config file has an "author" field, should return information about the command - Author() string + // ExecuteCommand is responsible for: + // 1. Making required changes to the filesystem (ex. copying files for ADD/COPY or setting ENV variables) + // 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 + return nil, errors.Errorf("%s is not a supported command", cmd.Name()) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 6097720ed..b08cf8800 100644 --- a/pkg/commands/run.go +++ b/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 { - author := []string{constants.Author} +func (r *RunCommand) CreatedBy() string { 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(author, " ") + " " + cmdLine + return strings.Join(append(shell, cmdLine), " ") } - author = append(author, cmdLine) - return strings.Join(author, " ") + return cmdLine } diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index fd46b3310..c75bf4296 100644 --- a/pkg/snapshot/snapshot.go +++ b/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) diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index ceae4cb39..ef77e6f81 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/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) } diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index 5e3245595..417f16f04 100644 --- a/pkg/util/tar_util.go +++ b/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 +}