The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6399
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
- Renames `GetPoolByInstanceName` to `GetPoolByInstance` - now accepts an Instance type directly.
- This allows it to inspect the Instance's Type and check it is supported by the underlying storage pool driver.
- If not it returns `drivers.ErrNotImplemented` if the instance type is not supported by the storage pool's driver.
- This allows us to cleanly fail operations where the storage driver hasn't implemented VM support yet.
- It also allows a nice side effect of allowing new storage drivers to be merged and used when they only support custom volumes, allowing for new drivers to be implemented in smaller testable chunks.
From 70b0bc0ff0c1a6a978c5dc04b5de5d353fe00032 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 Nov 2019 08:54:10 +0000
Subject: [PATCH 1/4] lxd/storage/drivers/driver/dir: Comment grammar
consistency
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_dir.go | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lxd/storage/drivers/driver_dir.go
b/lxd/storage/drivers/driver_dir.go
index 0062e75798..d95bc00f1a 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -304,13 +304,13 @@ func (d *dir) CreateVolumeFromMigration(vol Volume, conn
io.ReadWriteCloser, vol
return err
}
- // Create the snapshot itself
+ // Create the snapshot itself.
err = d.CreateVolumeSnapshot(vol.volType, vol.name,
snapName, op)
if err != nil {
return err
}
- // Setup the revert
+ // Setup the revert.
snapPath := GetVolumeMountPath(d.name, vol.volType,
GetSnapshotVolumeName(vol.name, snapName))
revertPaths = append(revertPaths, snapPath)
}
@@ -389,20 +389,20 @@ func (d *dir) CreateVolumeFromCopy(vol Volume, srcVol
Volume, copySnapshots bool
for _, srcSnapshot := range srcSnapshots {
_, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(srcSnapshot.name)
- // Mount the source snapshot
+ // Mount the source snapshot.
err = srcSnapshot.MountTask(func(srcMountPath
string, op *operations.Operation) error {
- // Copy the snapshot
+ // Copy the snapshot.
_, err = rsync.LocalCopy(srcMountPath,
mountPath, bwlimit, true)
return err
}, op)
- // Create the snapshot itself
+ // Create the snapshot itself.
err = d.CreateVolumeSnapshot(vol.volType,
vol.name, snapName, op)
if err != nil {
return err
}
- // Setup the revert
+ // Setup the revert.
snapPath := GetVolumeMountPath(d.name,
vol.volType, GetSnapshotVolumeName(vol.name, snapName))
revertPaths = append(revertPaths, snapPath)
}
From 0bbcae61d5e0722e15078a199e3cf7360db29f3a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 Nov 2019 09:08:58 +0000
Subject: [PATCH 2/4] lxd/storage/load: Renames GetPoolByInstanceName to
GetPoolByInstance
- Adds additional check to GetPoolByInstance that checks that the Instance is
of a type that is compatible with the underlying storage pool driver.
- If not it returns storageDrivers.ErrNotImplemented (for consistency with
storageDrivers.ErrUnknownDriver where the pool's driver itself is not
implemented).
- This allows the caller to decide to fallback to the old storage layer if
either the pool's driver is not known or the driver doesn't implement support
for the instance's type.
- This allows for operations to cleanly fail when we introduce VM support
rather than have unexpected failures.
- It also allows for new storage drivers to be partially implemented, such that
they only implement custom volume functionality, allowing quicker iteration and
testing of new drivers.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/load.go | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/lxd/storage/load.go b/lxd/storage/load.go
index d2f1e24072..5249f9a32e 100644
--- a/lxd/storage/load.go
+++ b/lxd/storage/load.go
@@ -60,6 +60,7 @@ func volIDFuncMake(state *state.State, poolID int64)
func(volType drivers.Volume
}
// CreatePool creates a new storage pool on disk and returns a Pool interface.
+// If the pool's driver is not recognised then drivers.ErrUnknownDriver is
returned.
func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePool, op
*operations.Operation) (Pool, error) {
// Sanity checks.
if dbPool == nil {
@@ -106,6 +107,7 @@ func CreatePool(state *state.State, poolID int64, dbPool
*api.StoragePool, op *o
}
// GetPoolByName retrieves the pool from the database by its name and returns
a Pool interface.
+// If the pool's driver is not recognised then drivers.ErrUnknownDriver is
returned.
func GetPoolByName(state *state.State, name string) (Pool, error) {
// Handle mock requests.
if MockBackend {
@@ -146,12 +148,32 @@ func GetPoolByName(state *state.State, name string)
(Pool, error) {
return &pool, nil
}
-// GetPoolByInstanceName retrieves the pool from the database using the
instance's project and name.
-func GetPoolByInstanceName(s *state.State, projectName, instanceName string)
(Pool, error) {
- poolName, err := s.Cluster.ContainerPool(projectName, instanceName)
+// GetPoolByInstance retrieves the pool from the database using the instance's
pool.
+// If the pool's driver is not recognised then drivers.ErrUnknownDriver is
returned. If the pool's
+// driver does not support the instance's type then drivers.ErrNotImplemented
is returned.
+func GetPoolByInstance(s *state.State, inst Instance) (Pool, error) {
+ poolName, err := s.Cluster.ContainerPool(inst.Project(), inst.Name())
if err != nil {
return nil, err
}
- return GetPoolByName(s, poolName)
+ pool, err := GetPoolByName(s, poolName)
+ if err != nil {
+ return nil, err
+ }
+
+ volType, err := InstanceTypeToVolumeType(inst.Type())
+ if err != nil {
+ return nil, err
+ }
+
+ for _, supportedType := range pool.Driver().Info().VolumeTypes {
+ if supportedType == volType {
+ return pool, nil
+ }
+ }
+
+ // Return drivers not implemented error for consistency with predefined
errors returned by
+ // GetPoolByName (which can return drivers.ErrUnknownDriver).
+ return nil, drivers.ErrNotImplemented
}
From 90639b9c3c875e0b26246a2dd71ab10e09041f3f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 Nov 2019 09:12:34 +0000
Subject: [PATCH 3/4] lxd/container: Updates use of
storagePools.GetPoolByInstance and fallback for container types
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/container.go | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/lxd/container.go b/lxd/container.go
index 8f52cc1a8f..b841fc7e17 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -406,7 +406,7 @@ func containerCreateEmptySnapshot(s *state.State, args
db.InstanceArgs) (contain
func containerCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op
*operations.Operation) (container, error) {
s := d.State()
- // Get the image properties
+ // Get the image properties.
_, img, err := s.Cluster.ImageGet(args.Project, hash, false, false)
if err != nil {
return nil, errors.Wrapf(err, "Fetch image %s from database",
hash)
@@ -428,8 +428,7 @@ func containerCreateFromImage(d *Daemon, args
db.InstanceArgs, hash string, op *
return nil, errors.Wrapf(err, "Locate image %s in the cluster",
hash)
}
if nodeAddress != "" {
- // The image is available from another node, let's try to
- // import it.
+ // The image is available from another node, let's try to
import it.
logger.Debugf("Transferring image %s from node %s", hash,
nodeAddress)
client, err := cluster.Connect(nodeAddress,
d.endpoints.NetworkCert(), false)
if err != nil {
@@ -449,14 +448,14 @@ func containerCreateFromImage(d *Daemon, args
db.InstanceArgs, hash string, op *
}
}
- // Set the "image.*" keys
+ // Set the "image.*" keys.
if img.Properties != nil {
for k, v := range img.Properties {
args.Config[fmt.Sprintf("image.%s", k)] = v
}
}
- // Set the BaseImage field (regardless of previous value)
+ // Set the BaseImage field (regardless of previous value).
args.BaseImage = hash
// Create the container
@@ -472,8 +471,8 @@ func containerCreateFromImage(d *Daemon, args
db.InstanceArgs, hash string, op *
}
// Check if we can load new storage layer for pool driver type.
- pool, err := storagePools.GetPoolByInstanceName(d.State(), c.Project(),
c.Name())
- if err != storageDrivers.ErrUnknownDriver {
+ pool, err := storagePools.GetPoolByInstance(d.State(), c)
+ if err != storageDrivers.ErrUnknownDriver && err !=
storageDrivers.ErrNotImplemented {
if err != nil {
return nil, errors.Wrap(err, "Load instance storage
pool")
}
@@ -482,7 +481,7 @@ func containerCreateFromImage(d *Daemon, args
db.InstanceArgs, hash string, op *
if err != nil {
return nil, errors.Wrap(err, "Create instance from
image")
}
- } else {
+ } else if c.Type() == instancetype.Container {
metadata := make(map[string]interface{})
var tracker *ioprogress.ProgressTracker
if op != nil {
@@ -493,15 +492,17 @@ func containerCreateFromImage(d *Daemon, args
db.InstanceArgs, hash string, op *
}}
}
- // Now create the storage from an image
+ // Now create the storage from an image.
err = c.Storage().ContainerCreateFromImage(c, hash, tracker)
if err != nil {
c.Delete()
return nil, errors.Wrap(err, "Create container from
image")
}
+ } else {
+ return nil, fmt.Errorf("Instance type not supported")
}
- // Apply any post-storage configuration
+ // Apply any post-storage configuration.
err = containerConfigureInternal(c)
if err != nil {
c.Delete()
From 435d598bc9206be776fef4601b047d25a9c25258 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 Nov 2019 09:12:51 +0000
Subject: [PATCH 4/4] lxd/storage/drivers/errors: Removes unused error
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/errors.go | 3 ---
1 file changed, 3 deletions(-)
diff --git a/lxd/storage/drivers/errors.go b/lxd/storage/drivers/errors.go
index 7cca047d15..56d1bdab3c 100644
--- a/lxd/storage/drivers/errors.go
+++ b/lxd/storage/drivers/errors.go
@@ -4,9 +4,6 @@ import (
"fmt"
)
-// ErrNilValue is the "Nil value provided" error
-var ErrNilValue = fmt.Errorf("Nil value provided")
-
// ErrNotImplemented is the "Not implemented" error
var ErrNotImplemented = fmt.Errorf("Not implemented")
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
