Skip to content

Commit

Permalink
Fix handling of volume directive
Browse files Browse the repository at this point in the history
  • Loading branch information
peter-evans committed Sep 25, 2018
1 parent 57ede49 commit 5cac31e
Show file tree
Hide file tree
Showing 22 changed files with 151 additions and 42 deletions.
2 changes: 1 addition & 1 deletion integration/dockerfiles/Dockerfile_test_volume
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0
RUN mkdir /foo
RUN echo "hello" > /foo/hey
VOLUME /foo/bar /tmp
VOLUME /foo/bar /tmp /qux/quux
ENV VOL /baz/bat
VOLUME ["${VOL}"]
RUN echo "hello again" > /tmp/hey
9 changes: 9 additions & 0 deletions integration/dockerfiles/Dockerfile_test_volume_2
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0
VOLUME /foo1
RUN mkdir /bar1
VOLUME /foo2
VOLUME /foo3
RUN echo "bar2"
VOLUME /foo4
RUN mkdir /bar3
VOLUME /foo5
3 changes: 0 additions & 3 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,6 @@ func TestLayers(t *testing.T) {
offset := map[string]int{
"Dockerfile_test_add": 10,
"Dockerfile_test_scratch": 3,
// the Docker built image combined some of the dirs defined by separate VOLUME commands into one layer
// which is why this offset exists
"Dockerfile_test_volume": 1,
}
for dockerfile, built := range imageBuilder.FilesBuilt {
t.Run("test_layer_"+dockerfile, func(t *testing.T) {
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ func (a *AddCommand) String() string {
return a.cmd.String()
}

// Name returns the name of the command
func (a *AddCommand) Name() string {
return a.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (a *AddCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/arg.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ func (r *ArgCommand) String() string {
return r.cmd.String()
}

// Name returns the name of the command
func (r *ArgCommand) Name() string {
return r.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (r *ArgCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ func (c *CmdCommand) String() string {
return c.cmd.String()
}

// Name returns the name of the command
func (c *CmdCommand) Name() string {
return c.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (c *CmdCommand) CacheCommand() bool {
return false
Expand Down
2 changes: 2 additions & 0 deletions pkg/commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ type DockerCommand interface {
ExecuteCommand(*v1.Config, *dockerfile.BuildArgs) error
// Returns a string representation of the command
String() string
// Returns the name of the command
Name() string
// A list of files to snapshot, empty for metadata commands or nil if we don't know
FilesToSnapshot() []string
// Return true if this command should be true
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ func (c *CopyCommand) String() string {
return c.cmd.String()
}

// Name returns the name of the command
func (c *CopyCommand) Name() string {
return c.cmd.Name()
}

// CacheCommand returns true since this command should be cached
func (c *CopyCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ func (e *EntrypointCommand) String() string {
return e.cmd.String()
}

// Name returns the name of the command
func (e *EntrypointCommand) Name() string {
return e.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (e *EntrypointCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ func (e *EnvCommand) String() string {
return e.cmd.String()
}

// Name returns the name of the command
func (e *EnvCommand) Name() string {
return e.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (e *EnvCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/expose.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ func (r *ExposeCommand) String() string {
return r.cmd.String()
}

// Name returns the name of the command
func (r *ExposeCommand) Name() string {
return r.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (r *ExposeCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ func (h *HealthCheckCommand) String() string {
return h.cmd.String()
}

// Name returns the name of the command
func (h *HealthCheckCommand) Name() string {
return h.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (h *HealthCheckCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ func (r *LabelCommand) String() string {
return r.cmd.String()
}

// Name returns the name of the command
func (r *LabelCommand) Name() string {
return r.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (r *LabelCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/onbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ func (o *OnBuildCommand) String() string {
return o.cmd.String()
}

// Name returns the name of the command
func (o *OnBuildCommand) Name() string {
return o.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (o *OnBuildCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ func (r *RunCommand) String() string {
return r.cmd.String()
}

// Name returns the name of the command
func (r *RunCommand) Name() string {
return r.cmd.Name()
}

// CacheCommand returns true since this command should be cached
func (r *RunCommand) CacheCommand() bool {
return true
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func (s *ShellCommand) String() string {
return s.cmd.String()
}

// Name returns the name of the command
func (s *ShellCommand) Name() string {
return s.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (s *ShellCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/stopsignal.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ func (s *StopSignalCommand) String() string {
return s.cmd.String()
}

// Name returns the name of the command
func (s *StopSignalCommand) Name() string {
return s.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (s *StopSignalCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ func (r *UserCommand) String() string {
return r.cmd.String()
}

// Name returns the name of the command
func (r *UserCommand) Name() string {
return r.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (r *UserCommand) CacheCommand() bool {
return false
Expand Down
9 changes: 7 additions & 2 deletions pkg/commands/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.
for _, volume := range resolvedVolumes {
var x struct{}
existingVolumes[volume] = x
err := util.AddPathToVolumeWhitelist(volume)
err := util.AddVolumePathToWhitelist(volume)
if err != nil {
return err
}

// Only create and snapshot the dir if it didn't exist already
if _, err := os.Stat(volume); os.IsNotExist(err) {
logrus.Infof("Creating directory %s", volume)
v.snapshotFiles = []string{volume}
v.snapshotFiles = append(v.snapshotFiles, volume)
if err := os.MkdirAll(volume, 0755); err != nil {
return fmt.Errorf("Could not create directory for volume %s: %s", volume, err)
}
Expand All @@ -75,6 +75,11 @@ func (v *VolumeCommand) String() string {
return v.cmd.String()
}

// Name returns the name of the command
func (v *VolumeCommand) Name() string {
return v.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (v *VolumeCommand) CacheCommand() bool {
return false
Expand Down
5 changes: 5 additions & 0 deletions pkg/commands/workdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ func (w *WorkdirCommand) String() string {
return w.cmd.String()
}

// Name returns the name of the command
func (w *WorkdirCommand) Name() string {
return w.cmd.Name()
}

// CacheCommand returns false since this command shouldn't be cached
func (w *WorkdirCommand) CacheCommand() bool {
return false
Expand Down
11 changes: 10 additions & 1 deletion pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error {
if err := s.snapshotter.Init(); err != nil {
return err
}
var volumes []string
args := dockerfile.NewBuildArgs(opts.BuildArgs)
for index, cmd := range s.stage.Commands {
finalCmd := index == len(s.stage.Commands)-1
Expand Down Expand Up @@ -167,6 +168,10 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error {
return err
}
files := command.FilesToSnapshot()
if command.Name() == "volume" {
volumes = append(volumes, files...)
continue
}
var contents []byte

// If this is an intermediate stage, we only snapshot for the last command and we
Expand All @@ -188,17 +193,21 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error {
// the files that were changed, we'll snapshot those explicitly, otherwise we'll
// check if anything in the filesystem changed.
if files != nil {
if len(files) > 0 {
files = append(files, volumes...)
volumes = []string{}
}
contents, err = s.snapshotter.TakeSnapshot(files)
} else {
contents, err = s.snapshotter.TakeSnapshotFS()
volumes = []string{}
}
}
}
if err != nil {
return fmt.Errorf("Error taking snapshot of files for command %s: %s", command, err)
}

util.MoveVolumeWhitelistToWhitelist()
if contents == nil {
logrus.Info("No files were changed, appending empty layer to config. No layer added to image.")
continue
Expand Down
Loading

0 comments on commit 5cac31e

Please sign in to comment.