diff --git a/executor/cmd/root.go b/executor/cmd/root.go index b850756f9..d1d6fb561 100644 --- a/executor/cmd/root.go +++ b/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, 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 - } + contents, err = snapshotter.TakeSnapshot() } else { - logrus.Infof("Taking snapshot of files %v...", snapshotFiles) - contents, err := snapshotter.TakeSnapshotOfFiles(snapshotFiles) - if err != nil { - return err - } - if contents == nil { - 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 - } + contents, err = snapshotter.TakeSnapshotOfFiles(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) + continue + } + // Append the layer to the image + if err := sourceImage.AppendLayer(contents, dockerCommand.Author()); err != nil { + return err } } } diff --git a/integration_tests/dockerfiles/Dockerfile_test_run b/integration_tests/dockerfiles/Dockerfile_test_run index f928750c3..cd225fc03 100644 --- a/integration_tests/dockerfiles/Dockerfile_test_run +++ b/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 \ diff --git a/integration_tests/dockerfiles/Dockerfile_test_run_2 b/integration_tests/dockerfiles/Dockerfile_test_run_2 new file mode 100644 index 000000000..b77ea9ea0 --- /dev/null +++ b/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" diff --git a/integration_tests/dockerfiles/config_test_run.json b/integration_tests/dockerfiles/config_test_run.json index 5bde728c3..0544a3d22 100644 --- a/integration_tests/dockerfiles/config_test_run.json +++ b/integration_tests/dockerfiles/config_test_run.json @@ -45,4 +45,4 @@ ] } } -] +] \ No newline at end of file diff --git a/integration_tests/dockerfiles/config_test_run_2.json b/integration_tests/dockerfiles/config_test_run_2.json new file mode 100644 index 000000000..0301973df --- /dev/null +++ b/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 + } + } +] \ No newline at end of file diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index 2598a9487..d13be52e9 100644 --- a/integration_tests/integration_test_yaml.go +++ b/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 { diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 16c138e0c..6097720ed 100644 --- a/pkg/commands/run.go +++ b/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, " ") } diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 8a619ed1e..5845194bd 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -27,4 +27,6 @@ const ( WorkspaceDir = "/workspace" WhitelistPath = "/proc/self/mountinfo" + + Author = "kbuild" ) diff --git a/pkg/image/image.go b/pkg/image/image.go index 7f389e9a6..f458ab1f1 100644 --- a/pkg/image/image.go +++ b/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 { - envVars := sourceImage.Env() +func SetEnvVariables(ms *img.MutableSource) error { + envVars := ms.Env() for key, val := range envVars { if err := os.Setenv(key, val); err != nil { return err diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 05cdd8a58..fd46b3310 100644 --- a/pkg/snapshot/snapshot.go +++ b/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.") diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index 5e51d88bc..ceae4cb39 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/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.") } } diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index 048200be8..5e3245595 100644 --- a/pkg/util/tar_util.go +++ b/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