Backport: Resolve deadlock if Docker is not fully initialized when it…

… loads Trident

CHANGELOG.md

@@ -2,7 +2,14 @@
 
 [Releases](https://github.com/NetApp/trident/releases)
 
-## Changes since v18.01.0
+## Changes since v18.04.0
+
+**Fixes:**
+- **Docker:** Resolved issue where containers might not restart after restarting Docker (Issue [#160](https://github.com/NetApp/trident/issues/160)).
+
+**Enhancements:**
+
+## v18.04.0
 
 **Fixes:**
 - Clone operations are more resilient to busy storage controllers.

frontend/docker/plugin.go

@@ -11,14 +11,21 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"time"
 
+	"github.com/cenkalti/backoff"
 	"github.com/docker/go-plugins-helpers/volume"
 	log "github.com/sirupsen/logrus"
 
+	"github.com/netapp/trident/config"
 	"github.com/netapp/trident/core"
 	"github.com/netapp/trident/storage"
 )
 
+const (
+	startupTimeout = 50 * time.Second
+)
+
 type Plugin struct {
 	orchestrator core.Orchestrator
 	driverName   string
@@ -64,18 +71,20 @@ func registerDockerVolumePlugin(root string) error {
 	}
 	// If root (volumeDir) isn't a directory, error
 	if dir != nil && !dir.IsDir() {
-		return fmt.Errorf("Volume directory '%v' exists and it's not a directory", root)
+		return fmt.Errorf("volume directory '%v' exists and it's not a directory", root)
 	}
 
 	return nil
 }
 
-func getDockerVersion() (*Version, error) {
+func (p *Plugin) initDockerVersion() {
+
+	time.Sleep(5 * time.Second)
 
 	// Get Docker version
 	out, err := exec.Command("docker", "version", "--format", "'{{json .}}'").CombinedOutput()
 	if err != nil {
-		return nil, err
+		log.Errorf("could not get Docker version: %v", err)
 	}
 	versionJSON := string(out)
 	versionJSON = strings.TrimSpace(versionJSON)
@@ -85,7 +94,7 @@ func getDockerVersion() (*Version, error) {
 	var version Version
 	err = json.Unmarshal([]byte(versionJSON), &version)
 	if err != nil {
-		return nil, err
+		log.Errorf("could not parse Docker version: %v", err)
 	}
 
 	log.WithFields(log.Fields{
@@ -99,11 +108,15 @@ func getDockerVersion() (*Version, error) {
 		"clientOS":         version.Server.Os,
 	}).Debug("Docker version info.")
 
-	return &version, nil
+	p.version = &version
+	config.OrchestratorTelemetry.PlatformVersion = version.Server.Version
 }
 
 func (p *Plugin) Activate() error {
+
 	handler := volume.NewHandler(p)
+
+	// Start serving requests on a different thread
 	go func() {
 		var err error
 		if p.driverPort != "" {
@@ -125,6 +138,10 @@ func (p *Plugin) Activate() error {
 			log.Fatalf("Failed to activate Docker frontend: %v", err)
 		}
 	}()
+
+	// Read the Docker version on a different thread so we don't deadlock if Docker is also initializing
+	go p.initDockerVersion()
+
 	return nil
 }
 
@@ -139,16 +156,8 @@ func (p *Plugin) GetName() string {
 
 func (p *Plugin) Version() string {
 
-	// Get the Docker version on demand
 	if p.version == nil {
-
-		version, err := getDockerVersion()
-		if err != nil {
-			log.Errorf("Failed to get the Docker version: %v", err)
-			return "unknown"
-		}
-
-		p.version = version
+		return "unknown"
 	}
 
 	return p.version.Server.Version
@@ -189,7 +198,7 @@ func (p *Plugin) List() (*volume.ListResponse, error) {
 		"method": "List",
 	}).Debug("Docker frontend method is invoked.")
 
-	err := p.orchestrator.ReloadVolumes()
+	err := p.reloadVolumes()
 	if err != nil {
 		return &volume.ListResponse{}, p.dockerError(err)
 	}
@@ -218,7 +227,7 @@ func (p *Plugin) Get(request *volume.GetRequest) (*volume.GetResponse, error) {
 
 	// Get is called at the start of every 'docker volume' workflow except List & Unmount,
 	// so refresh the volume list here.
-	err := p.orchestrator.ReloadVolumes()
+	err := p.reloadVolumes()
 	if err != nil {
 		return &volume.GetResponse{}, p.dockerError(err)
 	}
@@ -383,3 +392,36 @@ func (p *Plugin) dockerError(err error) error {
 		return err
 	}
 }
+
+// reloadVolumes instructs Trident core to refresh its cached volume info from its
+// backend storage controller(s).  If Trident isn't ready, it will retry for nearly
+// the Docker timeout of 60 seconds.  Otherwise, it returns immediately with any
+// other error or nil if the operation succeeded.
+func (p *Plugin) reloadVolumes() error {
+
+	reloadVolumesFunc := func() error {
+
+		err := p.orchestrator.ReloadVolumes()
+		if err == nil {
+			return nil
+		} else if core.IsNotReadyError(err) {
+			return err
+		} else {
+			return backoff.Permanent(err)
+		}
+	}
+	reloadNotify := func(err error, duration time.Duration) {
+		log.WithFields(log.Fields{
+			"increment": duration,
+			"message":   err.Error(),
+		}).Debugf("Docker frontend waiting to reload volumes.")
+	}
+	reloadBackoff := backoff.NewExponentialBackOff()
+	reloadBackoff.InitialInterval = 1 * time.Second
+	reloadBackoff.RandomizationFactor = 0.0
+	reloadBackoff.Multiplier = 1.0
+	reloadBackoff.MaxInterval = 1 * time.Second
+	reloadBackoff.MaxElapsedTime = startupTimeout
+
+	return backoff.RetryNotify(reloadVolumesFunc, reloadBackoff, reloadNotify)
+}

storage_drivers/eseries/api/eseries.go

@@ -93,19 +93,20 @@ func NewAPIClient(config ClientConfig) *Client {
 	}
 	c.config.CompiledPoolNameSearchPattern = compiledRegex
 
-	volumeTags = []VolumeTag{
-		{"IF", c.config.Protocol},
-		{"version", c.config.Telemetry["version"]},
-		{"platform", c.config.Telemetry["platform"]},
-		{"platformVersion", c.config.Telemetry["platformVersion"]},
-		{"plugin", c.config.Telemetry["plugin"]},
-		{"storagePrefix", c.config.Telemetry["storagePrefix"]},
-	}
-
 	return c
 }
 
-var volumeTags []VolumeTag
+func (d Client) makeVolumeTags() []VolumeTag {
+
+	return []VolumeTag{
+		{"IF", d.config.Protocol},
+		{"version", d.config.Telemetry["version"]},
+		{"platform", tridentconfig.OrchestratorTelemetry.Platform},
+		{"platformVersion", tridentconfig.OrchestratorTelemetry.PlatformVersion},
+		{"plugin", d.config.Telemetry["plugin"]},
+		{"storagePrefix", d.config.Telemetry["storagePrefix"]},
+	}
+}
 
 // InvokeAPI makes a REST call to the Web Services Proxy. The body must be a marshaled JSON byte array (or nil).
 // The method is the HTTP verb (i.e. GET, POST, ...).  The resource path is appended to the base URL to identify
@@ -619,7 +620,7 @@ func (d Client) CreateVolume(
 	}
 
 	// Copy static volume metadata and add fstype
-	tags := append([]VolumeTag(nil), volumeTags...)
+	tags := d.makeVolumeTags()
 	tags = append(tags, VolumeTag{"fstype", fstype})
 
 	// Set up the volume create request

storage_drivers/eseries/eseries_iscsi.go

@@ -20,7 +20,7 @@ import (
 	"github.com/pborman/uuid"
 	log "github.com/sirupsen/logrus"
 
-	trident "github.com/netapp/trident/config"
+	tridentconfig "github.com/netapp/trident/config"
 	"github.com/netapp/trident/storage"
 	sa "github.com/netapp/trident/storage_attribute"
 	drivers "github.com/netapp/trident/storage_drivers"
@@ -57,7 +57,7 @@ func (d *SANStorageDriver) Protocol() string {
 
 // Initialize from the provided config
 func (d *SANStorageDriver) Initialize(
-	context trident.DriverContext, configJSON string, commonConfig *drivers.CommonStorageDriverConfig,
+	context tridentconfig.DriverContext, configJSON string, commonConfig *drivers.CommonStorageDriverConfig,
 ) error {
 
 	// Trace logging hasn't been set up yet, so always do it here
@@ -102,9 +102,7 @@ func (d *SANStorageDriver) Initialize(
 	}
 
 	telemetry := make(map[string]string)
-	telemetry["version"] = trident.OrchestratorVersion.ShortString()
-	telemetry["platform"] = trident.OrchestratorTelemetry.Platform
-	telemetry["platformVersion"] = trident.OrchestratorTelemetry.PlatformVersion
+	telemetry["version"] = tridentconfig.OrchestratorVersion.ShortString()
 	telemetry["plugin"] = d.Name()
 	telemetry["storagePrefix"] = *d.Config.StoragePrefix
 
@@ -145,7 +143,7 @@ func (d *SANStorageDriver) Initialize(
 		}).Info("Controller serial numbers.")
 	}
 
-	if context == trident.ContextDocker {
+	if context == tridentconfig.ContextDocker {
 		// Make sure this host is logged into the E-series iSCSI target
 		err = utils.EnsureISCSISession(d.Config.HostDataIP)
 		if err != nil {
@@ -346,7 +344,7 @@ func (d *SANStorageDriver) Destroy(name string) error {
 		return fmt.Errorf("could not find volume %s: %v", name, err)
 	}
 
-	if d.Config.DriverContext == trident.ContextDocker {
+	if d.Config.DriverContext == tridentconfig.ContextDocker {
 
 		// Get target info
 		iSCSINodeName, _, err = d.getISCSITargetInfo()
@@ -777,7 +775,7 @@ func (d *SANStorageDriver) CreatePrepare(volConfig *storage.VolumeConfig) bool {
 
 func (d *SANStorageDriver) GetInternalVolumeName(name string) string {
 
-	if trident.UsingPassthroughStore {
+	if tridentconfig.UsingPassthroughStore {
 		// With a passthrough store, the name mapping must remain reversible
 		return *d.Config.StoragePrefix + name
 	} else {
@@ -870,7 +868,7 @@ func (d *SANStorageDriver) CreateFollowup(volConfig *storage.VolumeConfig) error
 		defer log.WithFields(fields).Debug("<<<< CreateFollowup")
 	}
 
-	if d.Config.DriverContext == trident.ContextDocker {
+	if d.Config.DriverContext == tridentconfig.ContextDocker {
 		log.Debug("No follow-up create actions for Docker.")
 		return nil
 	}
@@ -922,8 +920,8 @@ func (d *SANStorageDriver) CreateFollowup(volConfig *storage.VolumeConfig) error
 	return nil
 }
 
-func (d *SANStorageDriver) GetProtocol() trident.Protocol {
-	return trident.Block
+func (d *SANStorageDriver) GetProtocol() tridentconfig.Protocol {
+	return tridentconfig.Block
 }
 
 func (d *SANStorageDriver) StoreConfig(b *storage.PersistentStorageBackendConfig) {
@@ -1068,17 +1066,17 @@ func (d *SANStorageDriver) getVolumeExternal(
 	name := internalName[len(*d.Config.StoragePrefix):]
 
 	volumeConfig := &storage.VolumeConfig{
-		Version:         trident.OrchestratorAPIVersion,
+		Version:         tridentconfig.OrchestratorAPIVersion,
 		Name:            name,
 		InternalName:    internalName,
 		Size:            volumeAttrs.VolumeSize,
-		Protocol:        trident.Block,
+		Protocol:        tridentconfig.Block,
 		SnapshotPolicy:  "",
 		ExportPolicy:    "",
 		SnapshotDir:     "false",
 		UnixPermissions: "",
 		StorageClass:    "",
-		AccessMode:      trident.ReadWriteOnce,
+		AccessMode:      tridentconfig.ReadWriteOnce,
 		AccessInfo:      storage.VolumeAccessInfo{},
 		BlockSize:       "",
 		FileSystem:      "",

storage_drivers/ontap/ontap_common.go

@@ -18,7 +18,7 @@ import (
 	"github.com/cenkalti/backoff"
 	log "github.com/sirupsen/logrus"
 
-	trident "github.com/netapp/trident/config"
+	tridentconfig "github.com/netapp/trident/config"
 	"github.com/netapp/trident/storage"
 	sa "github.com/netapp/trident/storage_attribute"
 	drivers "github.com/netapp/trident/storage_drivers"
@@ -34,7 +34,7 @@ const (
 )
 
 type Telemetry struct {
-	trident.Telemetry
+	tridentconfig.Telemetry
 	Plugin        string        `json:"plugin"`
 	SVM           string        `json:"svm"`
 	StoragePrefix string        `json:"storagePrefix"`
@@ -52,7 +52,7 @@ type StorageDriver interface {
 
 // InitializeOntapConfig parses the ONTAP config, mixing in the specified common config.
 func InitializeOntapConfig(
-	context trident.DriverContext, configJSON string, commonConfig *drivers.CommonStorageDriverConfig,
+	context tridentconfig.DriverContext, configJSON string, commonConfig *drivers.CommonStorageDriverConfig,
 ) (*drivers.OntapStorageDriverConfig, error) {
 
 	if commonConfig.DebugTraceFlags["method"] {
@@ -77,7 +77,6 @@ func InitializeOntapConfig(
 
 func NewOntapTelemetry(d StorageDriver) *Telemetry {
 	t := &Telemetry{
-		Telemetry:     trident.OrchestratorTelemetry,
 		Plugin:        d.Name(),
 		SVM:           d.GetConfig().SVM,
 		StoragePrefix: *d.GetConfig().StoragePrefix,
@@ -463,7 +462,7 @@ func EMSHeartbeat(driver StorageDriver) {
 
 	emsResponse, err := driver.GetAPI().EmsAutosupportLog(
 		strconv.Itoa(drivers.ConfigVersion), false, "heartbeat", hostname,
-		string(message), 1, trident.OrchestratorName, 5)
+		string(message), 1, tridentconfig.OrchestratorName, 5)
 
 	if err = api.GetError(emsResponse, err); err != nil {
 		log.WithFields(log.Fields{
@@ -1078,7 +1077,7 @@ func getVolumeOptsCommon(
 
 func getInternalVolumeNameCommon(commonConfig *drivers.CommonStorageDriverConfig, name string) string {
 
-	if trident.UsingPassthroughStore {
+	if tridentconfig.UsingPassthroughStore {
 		// With a passthrough store, the name mapping must remain reversible
 		return *commonConfig.StoragePrefix + name
 	} else {