bugfix: add attach volume when container start

Add attach volume when container start. Now volume attaching is at container create stage, but if host have been restarted, container won't be created, so volume can't be attached. Add Config() for volume driver, it makes volume driver can use the configure of volume in daemon. Now set `volume-meta-dir` and `volume-timeout` into driver config. Signed-off-by: Rudy Zhang <[email protected]>
AliyunContainerService · Jun 19, 2018 · 19c7bd8 · 19c7bd8
1 parent 453b628
commit 19c7bd8
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 11 deletions.
diff --git a/daemon/mgr/container.go b/daemon/mgr/container.go
@@ -450,18 +450,48 @@ func (mgr *ContainerManager) Start(ctx context.Context, id, detachKeys string) (
 }
 
 func (mgr *ContainerManager) start(ctx context.Context, c *Container, detachKeys string) error {
+	var err error
+
 	c.Lock()
 	if c.Config == nil || c.State == nil {
 		c.Unlock()
 		return errors.Wrapf(errtypes.ErrNotfound, "container %s", c.ID)
 	}
 	c.DetachKeys = detachKeys
 
+	// attach volume
+	attachedVolumes := map[string]struct{}{}
+	defer func() {
+		if err != nil {
+			for name := range attachedVolumes {
+				_, err = mgr.VolumeMgr.Detach(ctx, name, map[string]string{volumetypes.OptionRef: c.ID})
+				if err != nil {
+					logrus.Errorf("failed to detach volume(%s) when start container(%s) rollback",
+						name, c.ID)
+				}
+			}
+		}
+	}()
+
+	for _, mp := range c.Mounts {
+		if mp.Name == "" {
+			continue
+		}
+
+		_, err = mgr.VolumeMgr.Attach(ctx, mp.Name, map[string]string{volumetypes.OptionRef: c.ID})
+		if err != nil {
+			c.Unlock()
+			return errors.Wrapf(err, "failed to attach volume(%s)", mp.Name)
+		}
+		attachedVolumes[mp.Name] = struct{}{}
+	}
+
 	// initialise container network mode
 	networkMode := c.HostConfig.NetworkMode
 
 	if IsContainer(networkMode) {
-		origContainer, err := mgr.Get(ctx, strings.SplitN(networkMode, ":", 2)[1])
+		var origContainer *Container
+		origContainer, err = mgr.Get(ctx, strings.SplitN(networkMode, ":", 2)[1])
 		if err != nil {
 			c.Unlock()
 			return err
@@ -475,7 +505,8 @@ func (mgr *ContainerManager) start(ctx context.Context, c *Container, detachKeys
 	} else {
 		// initialise host network mode
 		if IsHost(networkMode) {
-			hostname, err := os.Hostname()
+			var hostname string
+			hostname, err = os.Hostname()
 			if err != nil {
 				c.Unlock()
 				return err
@@ -484,7 +515,7 @@ func (mgr *ContainerManager) start(ctx context.Context, c *Container, detachKeys
 		}
 
 		// build the network related path.
-		if err := mgr.buildNetworkRelatedPath(c); err != nil {
+		if err = mgr.buildNetworkRelatedPath(c); err != nil {
 			c.Unlock()
 			return err
 		}
@@ -495,7 +526,7 @@ func (mgr *ContainerManager) start(ctx context.Context, c *Container, detachKeys
 				endpoint := mgr.buildContainerEndpoint(c)
 				endpoint.Name = name
 				endpoint.EndpointConfig = endpointSetting
-				if _, err := mgr.NetworkMgr.EndpointCreate(ctx, endpoint); err != nil {
+				if _, err = mgr.NetworkMgr.EndpointCreate(ctx, endpoint); err != nil {
 					logrus.Errorf("failed to create endpoint: %v", err)
 					c.Unlock()
 					return err
@@ -505,7 +536,11 @@ func (mgr *ContainerManager) start(ctx context.Context, c *Container, detachKeys
 	}
 	c.Unlock()
 
-	return mgr.createContainerdContainer(ctx, c)
+	if err = mgr.createContainerdContainer(ctx, c); err != nil {
+		return errors.Wrapf(err, "failed to create container(%s) on containerd", c.ID)
+	}
+
+	return nil
 }
 
 // buildNetworkRelatedPath builds the network related path.

diff --git a/daemon/mgr/volume.go b/daemon/mgr/volume.go
@@ -164,7 +164,7 @@ func (vm *VolumeManager) Attach(ctx context.Context, name string, options map[st
 		ref := v.Option(types.OptionRef)
 		if ref == "" {
 			options[types.OptionRef] = cid
-		} else {
+		} else if !strings.Contains(ref, cid) {
 			options[types.OptionRef] = strings.Join([]string{ref, cid}, ",")
 		}
 	}
@@ -199,7 +199,6 @@ func (vm *VolumeManager) Detach(ctx context.Context, name string, options map[st
 			for i, id := range ids {
 				if id == cid {
 					ids = append(ids[:i], ids[i+1:]...)
-					break
 				}
 			}
 

diff --git a/storage/volume/core.go b/storage/volume/core.go
@@ -2,6 +2,7 @@ package volume
 
 import (
 	"fmt"
+	"path"
 	"reflect"
 	"strings"
 
@@ -24,6 +25,7 @@ type Core struct {
 func NewCore(cfg Config) (*Core, error) {
 	c := &Core{Config: cfg}
 
+	// initialize volume driver alias.
 	if cfg.DriverAlias != "" {
 		parts := strings.Split(cfg.DriverAlias, ";")
 		for _, p := range parts {
@@ -38,6 +40,7 @@ func NewCore(cfg Config) (*Core, error) {
 		}
 	}
 
+	// initialize volume metadata store.
 	volumeStore, err := metastore.NewStore(metastore.Config{
 		Driver:  "boltdb",
 		BaseDir: cfg.VolumeMetaPath,
@@ -52,9 +55,23 @@ func NewCore(cfg Config) (*Core, error) {
 		logrus.Errorf("failed to create volume meta store: %v", err)
 		return nil, err
 	}
-
 	c.store = volumeStore
 
+	// set configure into each driver
+	driverConfig := map[string]interface{}{
+		"volume-meta-dir": path.Dir(cfg.VolumeMetaPath),
+		"volume-timeout":  cfg.Timeout,
+	}
+	drivers, err := driver.GetAll()
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to get all volume driver")
+	}
+	for _, dv := range drivers {
+		if d, ok := dv.(driver.Conf); ok {
+			d.Config(driver.Contexts(), driverConfig)
+		}
+	}
+
 	return c, nil
 }
 

diff --git a/storage/volume/driver/driver_interface.go b/storage/volume/driver/driver_interface.go
@@ -28,6 +28,12 @@ type Opt interface {
 	Options() map[string]types.Option
 }
 
+// Conf represents pass volume config to volume driver.
+type Conf interface {
+	// Config is used to pass the daemon volume config into driver.
+	Config(Context, map[string]interface{}) error
+}
+
 // AttachDetach represents volume attach/detach interface.
 type AttachDetach interface {
 	// Attach a Volume to host, enable the volume.

diff --git a/storage/volume/modules/local/local.go b/storage/volume/modules/local/local.go
@@ -15,7 +15,7 @@ import (
 )
 
 var (
-	dataDir = "/var/lib/pouch/volume"
+	defaultDataPath = "/var/lib/pouch/volume"
 )
 
 func init() {
@@ -26,6 +26,7 @@ func init() {
 
 // Local represents local volume driver.
 type Local struct {
+	DataPath string
 }
 
 // Name returns local volume driver's name.
@@ -42,8 +43,13 @@ func (p *Local) StoreMode(ctx driver.Context) driver.VolumeStoreMode {
 func (p *Local) Create(ctx driver.Context, id types.VolumeID) (*types.Volume, error) {
 	ctx.Log.Debugf("Local create volume: %s", id.Name)
 
+	dataPath := defaultDataPath
+	if p.DataPath != "" {
+		dataPath = p.DataPath
+	}
+
 	var (
-		mountPath = path.Join(dataDir, id.Name)
+		mountPath = path.Join(dataPath, id.Name)
 		size      string
 	)
 
@@ -91,7 +97,10 @@ func (p *Local) Path(ctx driver.Context, v *types.Volume) (string, error) {
 
 	mountPath := v.Option("mount")
 	if mountPath == "" {
-		mountPath = dataDir
+		mountPath = defaultDataPath
+		if p.DataPath != "" {
+			mountPath = p.DataPath
+		}
 	}
 
 	return path.Join(mountPath, v.Name), nil
@@ -104,6 +113,13 @@ func (p *Local) Options() map[string]types.Option {
 	}
 }
 
+// Config is used to pass the daemon volume configure for local driver.
+func (p *Local) Config(ctx driver.Context, cfg map[string]interface{}) error {
+	p.DataPath = cfg["volume-meta-dir"].(string)
+
+	return nil
+}
+
 // Attach a local volume.
 func (p *Local) Attach(ctx driver.Context, v *types.Volume) error {
 	ctx.Log.Debugf("Local attach volume: %s", v.Name)