Merge pull request #3427 from mheon/openstack_backports

Openstack backports

3 files changed, 155 insertions, 87 deletions

diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go
index b154d8bda..fdae08bf3 100644
--- a/libpod/boltdb_state.go
+++ b/libpod/boltdb_state.go
@@ -71,42 +71,50 @@ func NewBoltState(path string, runtime *Runtime) (State, error) {
 	// As such, just a db.Close() is fine here.
 	defer db.Close()
 
-	// Perform initial database setup
-	err = db.Update(func(tx *bolt.Tx) error {
-		if _, err := tx.CreateBucketIfNotExists(idRegistryBkt); err != nil {
-			return errors.Wrapf(err, "error creating id-registry bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(nameRegistryBkt); err != nil {
-			return errors.Wrapf(err, "error creating name-registry bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(nsRegistryBkt); err != nil {
-			return errors.Wrapf(err, "error creating ns-registry bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(ctrBkt); err != nil {
-			return errors.Wrapf(err, "error creating containers bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(allCtrsBkt); err != nil {
-			return errors.Wrapf(err, "error creating all containers bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(podBkt); err != nil {
-			return errors.Wrapf(err, "error creating pods bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(allPodsBkt); err != nil {
-			return errors.Wrapf(err, "error creating all pods bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(volBkt); err != nil {
-			return errors.Wrapf(err, "error creating volume bucket")
-		}
-		if _, err := tx.CreateBucketIfNotExists(allVolsBkt); err != nil {
-			return errors.Wrapf(err, "error creating all volumes bucket")
+	createBuckets := [][]byte{
+		idRegistryBkt,
+		nameRegistryBkt,
+		nsRegistryBkt,
+		ctrBkt,
+		allCtrsBkt,
+		podBkt,
+		allPodsBkt,
+		volBkt,
+		allVolsBkt,
+		runtimeConfigBkt,
+	}
+
+	// Does the DB need an update?
+	needsUpdate := false
+	err = db.View(func(tx *bolt.Tx) error {
+		for _, bkt := range createBuckets {
+			if test := tx.Bucket(bkt); test == nil {
+				needsUpdate = true
+				break
+			}
 		}
-		if _, err := tx.CreateBucketIfNotExists(runtimeConfigBkt); err != nil {
-			return errors.Wrapf(err, "error creating runtime-config bucket")
+		return nil
+	})
+	if err != nil {
+		return nil, errors.Wrapf(err, "error checking DB schema")
+	}
+
+	if !needsUpdate {
+		state.valid = true
+		return state, nil
+	}
+
+	// Ensure schema is properly created in DB
+	err = db.Update(func(tx *bolt.Tx) error {
+		for _, bkt := range createBuckets {
+			if _, err := tx.CreateBucketIfNotExists(bkt); err != nil {
+				return errors.Wrapf(err, "error creating bucket %s", string(bkt))
+			}
 		}
 		return nil
 	})
 	if err != nil {
-		return nil, errors.Wrapf(err, "error creating initial database layout")
+		return nil, errors.Wrapf(err, "error creating buckets for DB")
 	}
 
 	state.valid = true
diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go
index 06f8dcb24..1c746c724 100644
--- a/libpod/boltdb_state_internal.go
+++ b/libpod/boltdb_state_internal.go
@@ -71,89 +71,149 @@ var (
 	osKey          = []byte(osName)
 )
 
+// This represents a field in the runtime configuration that will be validated
+// against the DB to ensure no configuration mismatches occur.
+type dbConfigValidation struct {
+	name         string // Only used for error messages
+	runtimeValue string
+	key          []byte
+	defaultValue string
+}
+
 // Check if the configuration of the database is compatible with the
 // configuration of the runtime opening it
 // If there is no runtime configuration loaded, load our own
 func checkRuntimeConfig(db *bolt.DB, rt *Runtime) error {
-	err := db.Update(func(tx *bolt.Tx) error {
+	// We need to validate the following things
+	checks := []dbConfigValidation{
+		{
+			"OS",
+			runtime.GOOS,
+			osKey,
+			runtime.GOOS,
+		},
+		{
+			"libpod root directory (staticdir)",
+			rt.config.StaticDir,
+			staticDirKey,
+			"",
+		},
+		{
+			"libpod temporary files directory (tmpdir)",
+			rt.config.TmpDir,
+			tmpDirKey,
+			"",
+		},
+		{
+			"storage temporary directory (runroot)",
+			rt.config.StorageConfig.RunRoot,
+			runRootKey,
+			storage.DefaultStoreOptions.RunRoot,
+		},
+		{
+			"storage graph root directory (graphroot)",
+			rt.config.StorageConfig.GraphRoot,
+			graphRootKey,
+			storage.DefaultStoreOptions.GraphRoot,
+		},
+		{
+			"storage graph driver",
+			rt.config.StorageConfig.GraphDriverName,
+			graphDriverKey,
+			storage.DefaultStoreOptions.GraphDriverName,
+		},
+	}
+
+	// These fields were missing and will have to be recreated.
+	missingFields := []dbConfigValidation{}
+
+	// Let's try and validate read-only first
+	err := db.View(func(tx *bolt.Tx) error {
 		configBkt, err := getRuntimeConfigBucket(tx)
 		if err != nil {
 			return err
 		}
 
-		if err := validateDBAgainstConfig(configBkt, "OS", runtime.GOOS, osKey, runtime.GOOS); err != nil {
-			return err
+		for _, check := range checks {
+			exists, err := readOnlyValidateConfig(configBkt, check)
+			if err != nil {
+				return err
+			}
+			if !exists {
+				missingFields = append(missingFields, check)
+			}
 		}
 
-		if err := validateDBAgainstConfig(configBkt, "libpod root directory (staticdir)",
-			rt.config.StaticDir, staticDirKey, ""); err != nil {
-			return err
-		}
+		return nil
+	})
+	if err != nil {
+		return err
+	}
 
-		if err := validateDBAgainstConfig(configBkt, "libpod temporary files directory (tmpdir)",
-			rt.config.TmpDir, tmpDirKey, ""); err != nil {
-			return err
-		}
+	if len(missingFields) == 0 {
+		return nil
+	}
 
-		if err := validateDBAgainstConfig(configBkt, "storage temporary directory (runroot)",
-			rt.config.StorageConfig.RunRoot, runRootKey,
-			storage.DefaultStoreOptions.RunRoot); err != nil {
+	// Populate missing fields
+	return db.Update(func(tx *bolt.Tx) error {
+		configBkt, err := getRuntimeConfigBucket(tx)
+		if err != nil {
 			return err
 		}
 
-		if err := validateDBAgainstConfig(configBkt, "storage graph root directory (graphroot)",
-			rt.config.StorageConfig.GraphRoot, graphRootKey,
-			storage.DefaultStoreOptions.GraphRoot); err != nil {
-			return err
+		for _, missing := range missingFields {
+			dbValue := []byte(missing.runtimeValue)
+			if missing.runtimeValue == "" && missing.defaultValue != "" {
+				dbValue = []byte(missing.defaultValue)
+			}
+
+			if err := configBkt.Put(missing.key, dbValue); err != nil {
+				return errors.Wrapf(err, "error updating %s in DB runtime config", missing.name)
+			}
 		}
 
-		return validateDBAgainstConfig(configBkt, "storage graph driver",
-			rt.config.StorageConfig.GraphDriverName,
-			graphDriverKey,
-			storage.DefaultStoreOptions.GraphDriverName)
+		return nil
 	})
-
-	return err
 }
 
-// Validate a configuration entry in the DB against current runtime config
-// If the given configuration key does not exist it will be created
-// If the given runtimeValue or value retrieved from the database are the empty
-// string and defaultValue is not, defaultValue will be checked instead. This
-// ensures that we will not fail on configuration changes in configured c/storage.
-func validateDBAgainstConfig(bucket *bolt.Bucket, fieldName, runtimeValue string, keyName []byte, defaultValue string) error {
-	keyBytes := bucket.Get(keyName)
+// Attempt a read-only validation of a configuration entry in the DB against an
+// element of the current runtime configuration.
+// If the configuration key in question does not exist, (false, nil) will be
+// returned.
+// If the configuration key does exist, and matches the runtime configuration
+// successfully, (true, nil) is returned.
+// An error is only returned when validation fails.
+// if the given runtimeValue or value retrieved from the database are empty,
+// and defaultValue is not, defaultValue will be checked instead. This ensures
+// that we will not fail on configuration changes in c/storage (where we may
+// pass the empty string to use defaults).
+func readOnlyValidateConfig(bucket *bolt.Bucket, toCheck dbConfigValidation) (bool, error) {
+	keyBytes := bucket.Get(toCheck.key)
 	if keyBytes == nil {
-		dbValue := []byte(runtimeValue)
-		if runtimeValue == "" && defaultValue != "" {
-			dbValue = []byte(defaultValue)
-		}
+		// False return indicates missing key
+		return false, nil
+	}
 
-		if err := bucket.Put(keyName, dbValue); err != nil {
-			return errors.Wrapf(err, "error updating %s in DB runtime config", fieldName)
-		}
-	} else {
-		if runtimeValue != string(keyBytes) {
-			// If runtimeValue is the empty string, check against
-			// the default
-			if runtimeValue == "" && defaultValue != "" &&
-				string(keyBytes) == defaultValue {
-				return nil
-			}
+	dbValue := string(keyBytes)
 
-			// If DB value is the empty string, check that the
-			// runtime value is the default
-			if string(keyBytes) == "" && defaultValue != "" &&
-				runtimeValue == defaultValue {
-				return nil
-			}
+	if toCheck.runtimeValue != dbValue {
+		// If the runtime value is the empty string and default is not,
+		// check against default.
+		if toCheck.runtimeValue == "" && toCheck.defaultValue != "" && dbValue == toCheck.defaultValue {
+			return true, nil
+		}
 
-			return errors.Wrapf(ErrDBBadConfig, "database %s %s does not match our %s %s",
-				fieldName, string(keyBytes), fieldName, runtimeValue)
+		// If the DB value is the empty string, check that the runtime
+		// value is the default.
+		if dbValue == "" && toCheck.defaultValue != "" && toCheck.runtimeValue == toCheck.defaultValue {
+			return true, nil
 		}
+
+		return true, errors.Wrapf(ErrDBBadConfig, "database %s %q does not match our %s %q",
+			toCheck.name, dbValue, toCheck.name, toCheck.runtimeValue)
 	}
 
-	return nil
+	return true, nil
 }
 
 // Open a connection to the database.
diff --git a/libpod/container_api.go b/libpod/container_api.go
index 4eaf737b0..832801aeb 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -285,7 +285,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 
 	// TODO can probably relax this once we track exec sessions
 	if conState != ContainerStateRunning {
-		return errors.Errorf("cannot exec into container that is not running")
+		return errors.Wrapf(ErrCtrStateInvalid, "cannot exec into container that is not running")
 	}
 	if privileged || c.config.Privileged {
 		capList = caps.GetAllCapabilities()