Merge pull request #43599 from cpuguy83/rm_mountspec_backport

Remove mount spec backport

daemon/daemon.go

@@ -300,7 +300,6 @@ func (daemon *Daemon) restore() error {
 
 			log := logrus.WithField("container", c.ID)
 
-			daemon.backportMountSpec(c)
 			if err := daemon.checkpointAndSave(c); err != nil {
 				log.WithError(err).Error("error saving backported mountspec to disk")
 			}

daemon/volumes.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"os"
 	"path/filepath"
-	"reflect"
 	"strings"
 	"time"
 
@@ -256,128 +255,6 @@ func (daemon *Daemon) lazyInitializeVolume(containerID string, m *volumemounts.M
 	return nil
 }
 
-// backportMountSpec resolves mount specs (introduced in 1.13) from pre-1.13
-// mount configurations
-// The container lock should not be held when calling this function.
-// Changes are only made in-memory and may make changes to containers referenced
-// by `container.HostConfig.VolumesFrom`
-func (daemon *Daemon) backportMountSpec(container *container.Container) {
-	container.Lock()
-	defer container.Unlock()
-
-	maybeUpdate := make(map[string]bool)
-	for _, mp := range container.MountPoints {
-		if mp.Spec.Source != "" && mp.Type != "" {
-			continue
-		}
-		maybeUpdate[mp.Destination] = true
-	}
-	if len(maybeUpdate) == 0 {
-		return
-	}
-
-	mountSpecs := make(map[string]bool, len(container.HostConfig.Mounts))
-	for _, m := range container.HostConfig.Mounts {
-		mountSpecs[m.Target] = true
-	}
-
-	parser := volumemounts.NewParser()
-	binds := make(map[string]*volumemounts.MountPoint, len(container.HostConfig.Binds))
-	for _, rawSpec := range container.HostConfig.Binds {
-		mp, err := parser.ParseMountRaw(rawSpec, container.HostConfig.VolumeDriver)
-		if err != nil {
-			logrus.WithError(err).Error("Got unexpected error while re-parsing raw volume spec during spec backport")
-			continue
-		}
-		binds[mp.Destination] = mp
-	}
-
-	volumesFrom := make(map[string]volumemounts.MountPoint)
-	for _, fromSpec := range container.HostConfig.VolumesFrom {
-		from, _, err := parser.ParseVolumesFrom(fromSpec)
-		if err != nil {
-			logrus.WithError(err).WithField("id", container.ID).Error("Error reading volumes-from spec during mount spec backport")
-			continue
-		}
-		fromC, err := daemon.GetContainer(from)
-		if err != nil {
-			logrus.WithError(err).WithField("from-container", from).Error("Error looking up volumes-from container")
-			continue
-		}
-
-		// make sure from container's specs have been backported
-		daemon.backportMountSpec(fromC)
-
-		fromC.Lock()
-		for t, mp := range fromC.MountPoints {
-			volumesFrom[t] = *mp
-		}
-		fromC.Unlock()
-	}
-
-	needsUpdate := func(containerMount, other *volumemounts.MountPoint) bool {
-		if containerMount.Type != other.Type || !reflect.DeepEqual(containerMount.Spec, other.Spec) {
-			return true
-		}
-		return false
-	}
-
-	// main
-	for _, cm := range container.MountPoints {
-		if !maybeUpdate[cm.Destination] {
-			continue
-		}
-		// nothing to backport if from hostconfig.Mounts
-		if mountSpecs[cm.Destination] {
-			continue
-		}
-
-		if mp, exists := binds[cm.Destination]; exists {
-			if needsUpdate(cm, mp) {
-				cm.Spec = mp.Spec
-				cm.Type = mp.Type
-			}
-			continue
-		}
-
-		if cm.Name != "" {
-			if mp, exists := volumesFrom[cm.Destination]; exists {
-				if needsUpdate(cm, &mp) {
-					cm.Spec = mp.Spec
-					cm.Type = mp.Type
-				}
-				continue
-			}
-
-			if cm.Type != "" {
-				// probably specified via the hostconfig.Mounts
-				continue
-			}
-
-			// anon volume
-			cm.Type = mounttypes.TypeVolume
-			cm.Spec.Type = mounttypes.TypeVolume
-		} else {
-			if cm.Type != "" {
-				// already updated
-				continue
-			}
-
-			cm.Type = mounttypes.TypeBind
-			cm.Spec.Type = mounttypes.TypeBind
-			cm.Spec.Source = cm.Source
-			if cm.Propagation != "" {
-				cm.Spec.BindOptions = &mounttypes.BindOptions{
-					Propagation: cm.Propagation,
-				}
-			}
-		}
-
-		cm.Spec.Target = cm.Destination
-		cm.Spec.ReadOnly = !cm.RW
-	}
-}
-
 // VolumesService is used to perform volume operations
 func (daemon *Daemon) VolumesService() *service.VolumesService {
 	return daemon.volumes

daemon/volumes_unix_test.go

@@ -1,259 +0,0 @@
-//go:build !windows
-// +build !windows
-
-package daemon // import "github.com/docker/docker/daemon"
-
-import (
-	"encoding/json"
-	"fmt"
-	"reflect"
-	"testing"
-
-	containertypes "github.com/docker/docker/api/types/container"
-	mounttypes "github.com/docker/docker/api/types/mount"
-	"github.com/docker/docker/container"
-	volumemounts "github.com/docker/docker/volume/mounts"
-)
-
-func TestBackportMountSpec(t *testing.T) {
-	d := Daemon{containers: container.NewMemoryStore()}
-
-	c := &container.Container{
-		State: &container.State{},
-		MountPoints: map[string]*volumemounts.MountPoint{
-			"/apple":      {Destination: "/apple", Source: "/var/lib/docker/volumes/12345678", Name: "12345678", RW: true, CopyData: true}, // anonymous volume
-			"/banana":     {Destination: "/banana", Source: "/var/lib/docker/volumes/data", Name: "data", RW: true, CopyData: true},        // named volume
-			"/cherry":     {Destination: "/cherry", Source: "/var/lib/docker/volumes/data", Name: "data", CopyData: true},                  // RO named volume
-			"/dates":      {Destination: "/dates", Source: "/var/lib/docker/volumes/data", Name: "data"},                                   // named volume nocopy
-			"/elderberry": {Destination: "/elderberry", Source: "/var/lib/docker/volumes/data", Name: "data"},                              // masks anon vol
-			"/fig":        {Destination: "/fig", Source: "/data", RW: true},                                                                // RW bind
-			"/guava":      {Destination: "/guava", Source: "/data", RW: false, Propagation: "shared"},                                      // RO bind + propagation
-			"/kumquat":    {Destination: "/kumquat", Name: "data", RW: false, CopyData: true},                                              // volumes-from
-
-			// partially configured mountpoint due to #32613
-			// specifically, `mp.Spec.Source` is not set
-			"/honeydew": {
-				Type:        mounttypes.TypeVolume,
-				Destination: "/honeydew",
-				Name:        "data",
-				Source:      "/var/lib/docker/volumes/data",
-				Spec:        mounttypes.Mount{Type: mounttypes.TypeVolume, Target: "/honeydew", VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}},
-			},
-
-			// from hostconfig.Mounts
-			"/jambolan": {
-				Type:        mounttypes.TypeVolume,
-				Destination: "/jambolan",
-				Source:      "/var/lib/docker/volumes/data",
-				RW:          true,
-				Name:        "data",
-				Spec:        mounttypes.Mount{Type: mounttypes.TypeVolume, Target: "/jambolan", Source: "data"},
-			},
-		},
-		HostConfig: &containertypes.HostConfig{
-			Binds: []string{
-				"data:/banana",
-				"data:/cherry:ro",
-				"data:/dates:ro,nocopy",
-				"data:/elderberry:ro,nocopy",
-				"/data:/fig",
-				"/data:/guava:ro,shared",
-				"data:/honeydew:nocopy",
-			},
-			VolumesFrom: []string{"1:ro"},
-			Mounts: []mounttypes.Mount{
-				{Type: mounttypes.TypeVolume, Target: "/jambolan"},
-			},
-		},
-		Config: &containertypes.Config{Volumes: map[string]struct{}{
-			"/apple":      {},
-			"/elderberry": {},
-		}},
-	}
-
-	d.containers.Add("1", &container.Container{
-		State: &container.State{},
-		ID:    "1",
-		MountPoints: map[string]*volumemounts.MountPoint{
-			"/kumquat": {Destination: "/kumquat", Name: "data", RW: false, CopyData: true},
-		},
-		HostConfig: &containertypes.HostConfig{
-			Binds: []string{
-				"data:/kumquat:ro",
-			},
-		},
-	})
-
-	type expected struct {
-		mp      *volumemounts.MountPoint
-		comment string
-	}
-
-	pretty := func(mp *volumemounts.MountPoint) string {
-		b, err := json.MarshalIndent(mp, "\t", "    ")
-		if err != nil {
-			return fmt.Sprintf("%#v", mp)
-		}
-		return string(b)
-	}
-
-	mpc := *c.MountPoints["/jambolan"]
-
-	for _, x := range []expected{
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeVolume,
-				Destination: "/apple",
-				RW:          true,
-				Name:        "12345678",
-				Source:      "/var/lib/docker/volumes/12345678",
-				CopyData:    true,
-				Spec: mounttypes.Mount{
-					Type:   mounttypes.TypeVolume,
-					Source: "",
-					Target: "/apple",
-				},
-			},
-			comment: "anonymous volume",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeVolume,
-				Destination: "/banana",
-				RW:          true,
-				Name:        "data",
-				Source:      "/var/lib/docker/volumes/data",
-				CopyData:    true,
-				Spec: mounttypes.Mount{
-					Type:   mounttypes.TypeVolume,
-					Source: "data",
-					Target: "/banana",
-				},
-			},
-			comment: "named volume",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeVolume,
-				Destination: "/cherry",
-				Name:        "data",
-				Source:      "/var/lib/docker/volumes/data",
-				CopyData:    true,
-				Spec: mounttypes.Mount{
-					Type:     mounttypes.TypeVolume,
-					Source:   "data",
-					Target:   "/cherry",
-					ReadOnly: true,
-				},
-			},
-			comment: "read-only named volume",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeVolume,
-				Destination: "/dates",
-				Name:        "data",
-				Source:      "/var/lib/docker/volumes/data",
-				Spec: mounttypes.Mount{
-					Type:          mounttypes.TypeVolume,
-					Source:        "data",
-					Target:        "/dates",
-					ReadOnly:      true,
-					VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true},
-				},
-			},
-			comment: "named volume with nocopy",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeVolume,
-				Destination: "/elderberry",
-				Name:        "data",
-				Source:      "/var/lib/docker/volumes/data",
-				Spec: mounttypes.Mount{
-					Type:          mounttypes.TypeVolume,
-					Source:        "data",
-					Target:        "/elderberry",
-					ReadOnly:      true,
-					VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true},
-				},
-			},
-			comment: "masks an anonymous volume",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeBind,
-				Destination: "/fig",
-				Source:      "/data",
-				RW:          true,
-				Spec: mounttypes.Mount{
-					Type:   mounttypes.TypeBind,
-					Source: "/data",
-					Target: "/fig",
-				},
-			},
-			comment: "bind mount with read/write",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeBind,
-				Destination: "/guava",
-				Source:      "/data",
-				RW:          false,
-				Propagation: "shared",
-				Spec: mounttypes.Mount{
-					Type:        mounttypes.TypeBind,
-					Source:      "/data",
-					Target:      "/guava",
-					ReadOnly:    true,
-					BindOptions: &mounttypes.BindOptions{Propagation: "shared"},
-				},
-			},
-			comment: "bind mount with read/write + shared propagation",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeVolume,
-				Destination: "/honeydew",
-				Source:      "/var/lib/docker/volumes/data",
-				RW:          true,
-				Propagation: "shared",
-				Spec: mounttypes.Mount{
-					Type:          mounttypes.TypeVolume,
-					Source:        "data",
-					Target:        "/honeydew",
-					VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true},
-				},
-			},
-			comment: "partially configured named volume caused by #32613",
-		},
-		{
-			mp:      &mpc,
-			comment: "volume defined in mounts API",
-		},
-		{
-			mp: &volumemounts.MountPoint{
-				Type:        mounttypes.TypeVolume,
-				Destination: "/kumquat",
-				Source:      "/var/lib/docker/volumes/data",
-				RW:          false,
-				Name:        "data",
-				Spec: mounttypes.Mount{
-					Type:     mounttypes.TypeVolume,
-					Source:   "data",
-					Target:   "/kumquat",
-					ReadOnly: true,
-				},
-			},
-			comment: "partially configured named volume caused by #32613",
-		},
-	} {
-
-		mp := c.MountPoints[x.mp.Destination]
-		d.backportMountSpec(c)
-
-		if !reflect.DeepEqual(mp.Spec, x.mp.Spec) {
-			t.Fatalf("%s\nexpected:\n\t%s\n\ngot:\n\t%s", x.comment, pretty(x.mp), pretty(mp))
-		}
-	}
-}