From 6668fa0d6fe81a2632a071b07bfa74fb05a1bf1e Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 7 Mar 2018 15:34:14 -0800 Subject: [PATCH 1/6] Update add to tar to correctly handle hard links --- pkg/util/tar_util.go | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index 30e036c38..048200be8 100644 --- a/pkg/util/tar_util.go +++ b/pkg/util/tar_util.go @@ -18,10 +18,14 @@ package util import ( "archive/tar" + "github.com/sirupsen/logrus" "io" "os" + "syscall" ) +var hardlinks = make(map[uint64]string) + // AddToTar adds the file i to tar w at path p func AddToTar(p string, i os.FileInfo, w *tar.Writer) error { linkDst := "" @@ -32,19 +36,46 @@ 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.Infof("%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 - w.WriteHeader(hdr) - if !i.Mode().IsRegular() { + + if hardlink { + hdr.Linkname = linkDst + hdr.Typeflag = tar.TypeLink + hdr.Size = 0 + } + if err := w.WriteHeader(hdr); err != nil { + return err + } + if !(i.Mode().IsRegular()) || hardlink { return nil } r, err := os.Open(p) if err != nil { return err } + defer r.Close() if _, err := io.Copy(w, r); err != nil { return err } From cefb4448b1fd87215f414cde9ac650dab4269542 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 7 Mar 2018 15:34:56 -0800 Subject: [PATCH 2/6] Integration tests for run cmd --- Makefile | 4 +- executor/cmd/root.go | 52 +++++++++++++++++ .../dockerfiles/Dockerfile_test_run | 5 ++ .../dockerfiles/config_test_run.json | 57 +++++++++++++++++++ integration_tests/integration_test_yaml.go | 7 +++ pkg/commands/commands.go | 36 ++++++++++++ pkg/commands/run.go | 55 ++++++++++++++++++ pkg/image/image.go | 19 +++++++ 8 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 integration_tests/dockerfiles/Dockerfile_test_run create mode 100644 integration_tests/dockerfiles/config_test_run.json create mode 100644 pkg/commands/commands.go create mode 100644 pkg/commands/run.go diff --git a/Makefile b/Makefile index 80e9fe5f2..9d1064eb8 100644 --- a/Makefile +++ b/Makefile @@ -29,13 +29,13 @@ REPOPATH ?= $(ORG)/$(PROJECT) GO_FILES := $(shell find . -type f -name '*.go' -not -path "./vendor/*") GO_LDFLAGS := '-extldflags "-static"' -GO_BUILD_TAGS := "containers_image_ostree_stub containers_image_openpgp exclude_graphdriver_devicemapper exclude_graphdriver_btrfs" +GO_BUILD_TAGS := "containers_image_ostree_stub containers_image_openpgp exclude_graphdriver_devicemapper exclude_graphdriver_btrfs exclude_graphdriver_overlay" EXECUTOR_PACKAGE = $(REPOPATH)/executor KBUILD_PACKAGE = $(REPOPATH)/kbuild out/executor: $(GO_FILES) - GOOS=$* GOARCH=$(GOARCH) CGO_ENABLED=1 go build -ldflags $(GO_LDFLAGS) -tags $(GO_BUILD_TAGS) -o $@ $(EXECUTOR_PACKAGE) + GOOS=$* GOARCH=$(GOARCH) CGO_ENABLED=0 go build -ldflags $(GO_LDFLAGS) -tags $(GO_BUILD_TAGS) -o $@ $(EXECUTOR_PACKAGE) out/kbuild: $(GO_FILES) diff --git a/executor/cmd/root.go b/executor/cmd/root.go index 0afb6dae3..f0c5be45b 100644 --- a/executor/cmd/root.go +++ b/executor/cmd/root.go @@ -17,11 +17,13 @@ limitations under the License. package cmd import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/commands" "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/constants" "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/dockerfile" "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" @@ -88,7 +90,57 @@ func execute() error { } // Execute commands here + if err := image.SetEnvVariables(); err != nil { + return err + } + // 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 { + 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() + 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 + } + } 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 + } + } + } + } // Push the image return image.PushImage(destination) } diff --git a/integration_tests/dockerfiles/Dockerfile_test_run b/integration_tests/dockerfiles/Dockerfile_test_run new file mode 100644 index 000000000..f928750c3 --- /dev/null +++ b/integration_tests/dockerfiles/Dockerfile_test_run @@ -0,0 +1,5 @@ +FROM gcr.io/google-appengine/debian9 +RUN echo "hey" > /etc/foo +RUN apt-get update && apt-get install -y \ + bzr \ + cvs \ diff --git a/integration_tests/dockerfiles/config_test_run.json b/integration_tests/dockerfiles/config_test_run.json new file mode 100644 index 000000000..53f0a3eb2 --- /dev/null +++ b/integration_tests/dockerfiles/config_test_run.json @@ -0,0 +1,57 @@ +[ + { + "Image1": "gcr.io/kbuild-test/docker-test-run:latest", + "Image2": "gcr.io/kbuild-test/kbuild-test-run:latest", + "DiffType": "File", + "Diff": { + "Adds": [ + { + "Name": "/bin/bzcat", + "Size": 35448 + }, + { + "Name": "/bin/bzip2", + "Size": 35448 + } + ], + "Dels": null, + "Mods": [ + { + "Name": "/var/log/dpkg.log", + "Size1": 57425, + "Size2": 57425 + }, + { + "Name": "/var/log/apt/term.log", + "Size1": 24400, + "Size2": 24400 + }, + { + "Name": "/var/cache/ldconfig/aux-cache", + "Size1": 8057, + "Size2": 8057 + }, + { + "Name": "/var/log/apt/history.log", + "Size1": 5089, + "Size2": 5089 + }, + { + "Name": "/var/log/alternatives.log", + "Size1": 2579, + "Size2": 2579 + }, + { + "Name": "/usr/lib/python2.7/dist-packages/keyrings/__init__.pyc", + "Size1": 140, + "Size2": 140 + }, + { + "Name": "/usr/lib/python2.7/dist-packages/lazr/__init__.pyc", + "Size1": 136, + "Size2": 136 + } + ] + } + } +] \ No newline at end of file diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index c11b901ec..f422c9989 100644 --- a/integration_tests/integration_test_yaml.go +++ b/integration_tests/integration_test_yaml.go @@ -35,6 +35,13 @@ var tests = []struct { context: "integration_tests/dockerfiles/", repo: "extract-filesystem", }, + { + description: "test run", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run", + configPath: "/workspace/integration_tests/dockerfiles/config_test_run.json", + context: "integration_tests/dockerfiles/", + repo: "test-run", + }, } type step struct { diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go new file mode 100644 index 000000000..e8cfa8ebb --- /dev/null +++ b/pkg/commands/commands.go @@ -0,0 +1,36 @@ +/* +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. +*/ + +package commands + +import ( + "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/sirupsen/logrus" +) + +type DockerCommand interface { + ExecuteCommand() error + // The config file has an "author" field, should return information about the command + Author() 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 { + switch c := cmd.(type) { + case *instructions.RunCommand: + return &RunCommand{cmd: c} + } + logrus.Errorf("%s is not a supported command.", cmd.Name()) + return nil +} diff --git a/pkg/commands/run.go b/pkg/commands/run.go new file mode 100644 index 000000000..9b85387b4 --- /dev/null +++ b/pkg/commands/run.go @@ -0,0 +1,55 @@ +/* +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. +*/ + +package commands + +import ( + "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/sirupsen/logrus" + "os" + "os/exec" + "strings" +) + +type RunCommand struct { + cmd *instructions.RunCommand +} + +func (r *RunCommand) ExecuteCommand() error { + var newCommand []string + if r.cmd.PrependShell { + // This is the default shell on Linux + shell := []string{"/bin/sh", "-c"} + newCommand = append(shell, strings.Join(r.cmd.CmdLine, " ")) + } else { + newCommand = r.cmd.CmdLine + } + + logrus.Infof("cmd: %s", newCommand[0]) + logrus.Infof("args: %s", newCommand[1:]) + + cmd := exec.Command(newCommand[0], newCommand[1:]...) + cmd.Stdout = os.Stdout + return cmd.Run() +} + +// FilesToSnapshot returns nil for this command because we don't know which files +// have changed, so we snapshot the entire system. +func (r *RunCommand) FilesToSnapshot() []string { + return nil +} + +// 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, " ") +} diff --git a/pkg/image/image.go b/pkg/image/image.go index b76ad1269..a55c122f8 100644 --- a/pkg/image/image.go +++ b/pkg/image/image.go @@ -23,6 +23,7 @@ import ( "github.com/containers/image/signature" "github.com/containers/image/transports/alltransports" "github.com/sirupsen/logrus" + "os" ) // sourceImage is the image that will be modified by the executor @@ -45,6 +46,7 @@ func InitializeSourceImage(srcImg string) error { // AppendLayer appends a layer onto the base image func AppendLayer(contents []byte, author string) error { + logrus.Info("Appending layer to base image") return sourceImage.AppendLayer(contents, author) } @@ -66,6 +68,23 @@ func PushImage(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() + for key, val := range envVars { + if err := os.Setenv(key, val); err != nil { + return err + } + logrus.Debugf("Setting environment variable %s=%s", key, val) + } + return nil +} + func getPolicyContext() (*signature.PolicyContext, error) { policyContext, err := signature.NewPolicyContext(&signature.Policy{ Default: signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()}, From cd9467dcf412fc68f16b5d34b652f4eb4dd0bd29 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 7 Mar 2018 15:35:44 -0800 Subject: [PATCH 3/6] Updated vendor for new version of container-diff --- Gopkg.lock | 12 ++++++------ .../container-diff/pkg/image/mutable_source.go | 8 ++++---- vendor/github.com/containers/storage/store.go | 5 +++-- vendor/golang.org/x/sys/unix/syscall_linux_arm64.go | 7 +++++-- .../golang.org/x/sys/unix/syscall_linux_mips64x.go | 7 +++++-- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 1fd8e4e4c..fb33774a2 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -15,7 +15,7 @@ "pkg/image", "pkg/util" ] - revision = "ce3228b4afba2b2fb9b42c416c8f59a14ee7c1dd" + revision = "6a521891eafa833a08adf664edb6e67b18220ea7" source = "github.com/GoogleCloudPlatform/container-diff" [[projects]] @@ -119,7 +119,7 @@ "pkg/system", "pkg/truncindex" ] - revision = "66a38bb219e9776482c18e8c02f438e5112916f1" + revision = "1e5ce40cdb84ab66e26186435b1273e04b879fef" [[projects]] branch = "master" @@ -370,7 +370,7 @@ "nfs", "xfs" ] - revision = "d274e363d5759d1c916232217be421f1cc89c5fe" + revision = "1c7ff3de94ae006f58cba483a4c9c6d7c61e1d98" [[projects]] name = "github.com/sirupsen/logrus" @@ -419,7 +419,7 @@ "openpgp/s2k", "ssh/terminal" ] - revision = "91a49db82a88618983a78a06c1cbd4e00ab749ab" + revision = "85f98707c97e11569271e4d9b3d397e079c4f4d0" [[projects]] branch = "master" @@ -433,7 +433,7 @@ "lex/httplex", "proxy" ] - revision = "22ae77b79946ea320088417e4d50825671d82d57" + revision = "07e8617a6db2368fa55d4616f371ee1b1403c817" [[projects]] branch = "master" @@ -442,7 +442,7 @@ "unix", "windows" ] - revision = "f6cff0780e542efa0c8e864dc8fa522808f6a598" + revision = "dd2ff4accc098aceecb86b36eaa7829b2a17b1c9" [[projects]] name = "golang.org/x/text" diff --git a/vendor/github.com/GoogleCloudPlatform/container-diff/pkg/image/mutable_source.go b/vendor/github.com/GoogleCloudPlatform/container-diff/pkg/image/mutable_source.go index fcd4521da..2c7e2795a 100644 --- a/vendor/github.com/GoogleCloudPlatform/container-diff/pkg/image/mutable_source.go +++ b/vendor/github.com/GoogleCloudPlatform/container-diff/pkg/image/mutable_source.go @@ -142,7 +142,7 @@ func (m *MutableSource) AppendLayer(content []byte, author string) error { // Also add it to the config. diffID := digest.FromBytes(content) m.cfg.RootFS.DiffIDs = append(m.cfg.RootFS.DiffIDs, diffID) - m.appendConfigHistory(author, false) + m.AppendConfigHistory(author, false) return nil } @@ -184,7 +184,7 @@ func (m *MutableSource) SetEnv(envMap map[string]string, author string) { envArray = append(envArray, entry) } m.cfg.Schema2V1Image.Config.Env = envArray - m.appendConfigHistory(author, true) + m.AppendConfigHistory(author, true) } func (m *MutableSource) Config() *manifest.Schema2Config { @@ -193,10 +193,10 @@ func (m *MutableSource) Config() *manifest.Schema2Config { func (m *MutableSource) SetConfig(config *manifest.Schema2Config, author string, emptyLayer bool) { m.cfg.Schema2V1Image.Config = config - m.appendConfigHistory(author, emptyLayer) + m.AppendConfigHistory(author, emptyLayer) } -func (m *MutableSource) appendConfigHistory(author string, emptyLayer bool) { +func (m *MutableSource) AppendConfigHistory(author string, emptyLayer bool) { history := manifest.Schema2History{ Created: time.Now(), Author: author, diff --git a/vendor/github.com/containers/storage/store.go b/vendor/github.com/containers/storage/store.go index 02dd7a1b3..a31a08b2a 100644 --- a/vendor/github.com/containers/storage/store.go +++ b/vendor/github.com/containers/storage/store.go @@ -2276,14 +2276,15 @@ func (s *store) Shutdown(force bool) ([]string, error) { return mounted, err } + s.graphLock.Lock() + defer s.graphLock.Unlock() + rlstore.Lock() defer rlstore.Unlock() if modified, err := rlstore.Modified(); modified || err != nil { rlstore.Load() } - s.graphLock.Lock() - defer s.graphLock.Unlock() layers, err := rlstore.Layers() if err != nil { return mounted, err diff --git a/vendor/golang.org/x/sys/unix/syscall_linux_arm64.go b/vendor/golang.org/x/sys/unix/syscall_linux_arm64.go index 9a8e6e411..c464783d8 100644 --- a/vendor/golang.org/x/sys/unix/syscall_linux_arm64.go +++ b/vendor/golang.org/x/sys/unix/syscall_linux_arm64.go @@ -23,8 +23,11 @@ package unix //sys Seek(fd int, offset int64, whence int) (off int64, err error) = SYS_LSEEK func Select(nfd int, r *FdSet, w *FdSet, e *FdSet, timeout *Timeval) (n int, err error) { - ts := Timespec{Sec: timeout.Sec, Nsec: timeout.Usec * 1000} - return Pselect(nfd, r, w, e, &ts, nil) + var ts *Timespec + if timeout != nil { + ts = &Timespec{Sec: timeout.Sec, Nsec: timeout.Usec * 1000} + } + return Pselect(nfd, r, w, e, ts, nil) } //sys sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) diff --git a/vendor/golang.org/x/sys/unix/syscall_linux_mips64x.go b/vendor/golang.org/x/sys/unix/syscall_linux_mips64x.go index 46aa4ff9c..15a69cbdd 100644 --- a/vendor/golang.org/x/sys/unix/syscall_linux_mips64x.go +++ b/vendor/golang.org/x/sys/unix/syscall_linux_mips64x.go @@ -26,8 +26,11 @@ package unix //sys Seek(fd int, offset int64, whence int) (off int64, err error) = SYS_LSEEK func Select(nfd int, r *FdSet, w *FdSet, e *FdSet, timeout *Timeval) (n int, err error) { - ts := Timespec{Sec: timeout.Sec, Nsec: timeout.Usec * 1000} - return Pselect(nfd, r, w, e, &ts, nil) + var ts *Timespec + if timeout != nil { + ts = &Timespec{Sec: timeout.Sec, Nsec: timeout.Usec * 1000} + } + return Pselect(nfd, r, w, e, ts, nil) } //sys sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) From 75e7e47b760ee44f8a49f545a7eaa35c28613c9d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 8 Mar 2018 11:21:33 -0800 Subject: [PATCH 4/6] Added integration test, minor changes to files --- executor/cmd/root.go | 45 +++++++------------ .../dockerfiles/Dockerfile_test_run | 14 ++++++ .../dockerfiles/Dockerfile_test_run_2 | 19 ++++++++ .../dockerfiles/config_test_run.json | 2 +- .../dockerfiles/config_test_run_2.json | 12 +++++ integration_tests/integration_test_yaml.go | 7 +++ pkg/commands/run.go | 11 ++++- pkg/constants/constants.go | 2 + pkg/image/image.go | 9 +--- pkg/snapshot/snapshot.go | 14 +++--- pkg/snapshot/snapshot_test.go | 12 ++--- pkg/util/tar_util.go | 2 +- 12 files changed, 99 insertions(+), 50 deletions(-) create mode 100644 integration_tests/dockerfiles/Dockerfile_test_run_2 create mode 100644 integration_tests/dockerfiles/config_test_run_2.json 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 From 55314cef9c66ee9d595ef915d6206e9844a61b0a Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 9 Mar 2018 11:00:25 -0800 Subject: [PATCH 5/6] Update Gopkg.lock --- Gopkg.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gopkg.lock b/Gopkg.lock index fb33774a2..8f3631a98 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -486,6 +486,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "d806d861c987e81225e9da4da86d9417d01fd42d84dbe34e21daf076651e17b5" + inputs-digest = "fd21de0404336debb893db778210835a27a3612fe9b9e5e412dcdc80d288a986" solver-name = "gps-cdcl" solver-version = 1 From 27b09f28be07a9c94a4f8a9f01b2da16bfee75fe Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 9 Mar 2018 11:17:59 -0800 Subject: [PATCH 6/6] 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 +}