Skip to content

Commit

Permalink
This fixes a bug in the interaction between volumes in base images (G…
Browse files Browse the repository at this point in the history
…oogleContainerTools#555)

and our snapshot optimizations.

If a previous base image has a volume, the directory is added to the
list of files to snapshot. That directory may not actually exist in the image.
  • Loading branch information
dlorenc committed Feb 8, 2019
1 parent e14b660 commit 9047ccf
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
5 changes: 5 additions & 0 deletions integration/dockerfiles/Dockerfile_test_volume_4
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FROM rabbitmq@sha256:57b028a4bb9592ece3915e3e9cdbbaecb3eb82b753aaaf5250f8d25d81d318e2
# This base image has a volume declared at /var/lib/rabbitmq
# This is important because it should not exist in the child image.
COPY context/foo /usr/local/bin/
CMD ["script.sh"]
5 changes: 1 addition & 4 deletions pkg/commands/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.
for _, volume := range resolvedVolumes {
var x struct{}
existingVolumes[volume] = x
err := util.AddVolumePathToWhitelist(volume)
if err != nil {
return err
}
util.AddVolumePathToWhitelist(volume)

// Only create and snapshot the dir if it didn't exist already
if _, err := os.Stat(volume); os.IsNotExist(err) {
Expand Down
7 changes: 2 additions & 5 deletions pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,8 @@ func (s *stageBuilder) takeSnapshot(files []string) (string, error) {
if files == nil || s.opts.SingleSnapshot {
snapshot, err = s.snapshotter.TakeSnapshotFS()
} else {
// Volumes are very weird. They get created in their command, but snapshotted in the next one.
// Add them to the list of files to snapshot.
for v := range s.cf.Config.Volumes {
files = append(files, v)
}
// Volumes are very weird. They get snapshotted in the next command.
files = append(files, util.Volumes()...)
snapshot, err = s.snapshotter.TakeSnapshot(files)
}
timing.DefaultRun.Stop(t)
Expand Down
16 changes: 11 additions & 5 deletions pkg/util/fs_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ var initialWhitelist = []WhitelistEntry{

var whitelist = initialWhitelist

var volumes = []string{}

var excluded []string

// GetFSFromImage extracts the layers of img to root
Expand Down Expand Up @@ -308,6 +310,7 @@ func CheckWhitelist(path string) (bool, error) {
return true, nil
}
}

return false, nil
}

Expand All @@ -331,6 +334,7 @@ func checkWhitelistRoot(root string) bool {
// From: https://www.kernel.org/doc/Documentation/filesystems/proc.txt
func DetectFilesystemWhitelist(path string) error {
whitelist = initialWhitelist
volumes = []string{}
f, err := os.Open(path)
if err != nil {
return err
Expand Down Expand Up @@ -443,16 +447,14 @@ func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid
return dest.Chown(int(uid), int(gid))
}

// AddVolumePathToWhitelist adds the given path to the whitelist with
// PrefixMatchOnly set to true. Snapshotting will ignore paths prefixed
// with the volume, but the volume itself will not be ignored.
func AddVolumePathToWhitelist(path string) error {
// AddVolumePath adds the given path to the volume whitelist.
func AddVolumePathToWhitelist(path string) {
logrus.Infof("adding volume %s to whitelist", path)
whitelist = append(whitelist, WhitelistEntry{
Path: path,
PrefixMatchOnly: true,
})
return nil
volumes = append(volumes, path)
}

// DownloadFileToDest downloads the file at rawurl to the given dest for the ADD command
Expand Down Expand Up @@ -620,3 +622,7 @@ func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
}
return true
}

func Volumes() []string {
return volumes
}

0 comments on commit 9047ccf

Please sign in to comment.