[lxc-devel] [lxd/master] Tp storage forwarded response if volume is remote exclusive volumes
tomponline on Github
lxc-bot at linuxcontainers.org
Thu Oct 22 11:45:07 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201022/eec524ae/attachment-0001.bin>
-------------- next part --------------
From d520357207a0bb535f78212b25372aaf6478c9c5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 09:15:04 +0100
Subject: [PATCH 01/14] lxd/cluster/connect: Renames project arg to projectName
in ConnectIfInstanceIsRemote
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/cluster/connect.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/cluster/connect.go b/lxd/cluster/connect.go
index 7ed1dd0773..a180d796f8 100644
--- a/lxd/cluster/connect.go
+++ b/lxd/cluster/connect.go
@@ -97,11 +97,11 @@ func Connect(address string, cert *shared.CertInfo, notify bool) (lxd.InstanceSe
// running the container with the given name. If it's not the local node will
// connect to it and return the connected client, otherwise it will just return
// nil.
-func ConnectIfInstanceIsRemote(cluster *db.Cluster, project, name string, cert *shared.CertInfo, instanceType instancetype.Type) (lxd.InstanceServer, error) {
+func ConnectIfInstanceIsRemote(cluster *db.Cluster, projectName string, name string, cert *shared.CertInfo, instanceType instancetype.Type) (lxd.InstanceServer, error) {
var address string // Node address
err := cluster.Transaction(func(tx *db.ClusterTx) error {
var err error
- address, err = tx.GetNodeAddressOfInstance(project, name, instanceType)
+ address, err = tx.GetNodeAddressOfInstance(projectName, name, instanceType)
return err
})
if err != nil {
From a5a1f424b9cdba09d91fd86532cdde51d896b253 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 09:15:21 +0100
Subject: [PATCH 02/14] lxd/cluster/connect: Adds projectName arg to
ConnectIfVolumeIsRemote
Rather than being hardcoded to "default" project.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/cluster/connect.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/cluster/connect.go b/lxd/cluster/connect.go
index a180d796f8..1d93cef6bf 100644
--- a/lxd/cluster/connect.go
+++ b/lxd/cluster/connect.go
@@ -121,11 +121,11 @@ func ConnectIfInstanceIsRemote(cluster *db.Cluster, projectName string, name str
//
// If there is more than one node with a matching volume name, an error is
// returned.
-func ConnectIfVolumeIsRemote(cluster *db.Cluster, poolID int64, volumeName string, volumeType int, cert *shared.CertInfo) (lxd.InstanceServer, error) {
+func ConnectIfVolumeIsRemote(cluster *db.Cluster, poolID int64, projectName string, volumeName string, volumeType int, cert *shared.CertInfo) (lxd.InstanceServer, error) {
var addresses []string // Node addresses
err := cluster.Transaction(func(tx *db.ClusterTx) error {
var err error
- addresses, err = tx.GetStorageVolumeNodeAddresses(poolID, "default", volumeName, volumeType)
+ addresses, err = tx.GetStorageVolumeNodeAddresses(poolID, projectName, volumeName, volumeType)
return err
})
if err != nil {
From 845d371084edd548435fb26bc8cfa293b8b06705 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 09:15:54 +0100
Subject: [PATCH 03/14] lxd/response: Adds projectName argument to
forwardedResponseIfVolumeIsRemote
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/response.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/response.go b/lxd/response.go
index 01c3c9c605..77557643cb 100644
--- a/lxd/response.go
+++ b/lxd/response.go
@@ -59,13 +59,13 @@ func forwardedResponseIfInstanceIsRemote(d *Daemon, r *http.Request, project, na
//
// This is used when no targetNode is specified, and saves users some typing
// when the volume name/type is unique to a node.
-func forwardedResponseIfVolumeIsRemote(d *Daemon, r *http.Request, poolID int64, volumeName string, volumeType int) response.Response {
+func forwardedResponseIfVolumeIsRemote(d *Daemon, r *http.Request, poolID int64, projectName string, volumeName string, volumeType int) response.Response {
if queryParam(r, "target") != "" {
return nil
}
cert := d.endpoints.NetworkCert()
- client, err := cluster.ConnectIfVolumeIsRemote(d.cluster, poolID, volumeName, volumeType, cert)
+ client, err := cluster.ConnectIfVolumeIsRemote(d.cluster, poolID, projectName, volumeName, volumeType, cert)
if err != nil && err != db.ErrNoSuchObject {
return response.SmartError(err)
}
From ca3badc5dc32930ab754615b4eae6763b2f6149b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 09:16:19 +0100
Subject: [PATCH 04/14] lxd/storage/volumes: forwardedResponseIfVolumeIsRemote
projectName argument usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_volumes.go | 10 +++++-----
lxd/storage_volumes_backup.go | 12 ++++++------
lxd/storage_volumes_snapshot.go | 10 +++++-----
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index a7c0f6c4ca..e823b181cd 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -634,7 +634,7 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
return response.BadRequest(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -857,7 +857,7 @@ func storagePoolVolumeTypeGet(d *Daemon, r *http.Request, volumeTypeName string)
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -936,7 +936,7 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request, volumeTypeName string)
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, pool.ID(), volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, pool.ID(), projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -1078,7 +1078,7 @@ func storagePoolVolumeTypePatch(d *Daemon, r *http.Request, volumeTypeName strin
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, pool.ID(), volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, pool.ID(), projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -1176,7 +1176,7 @@ func storagePoolVolumeTypeDelete(d *Daemon, r *http.Request, volumeTypeName stri
return response.SmartError(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
diff --git a/lxd/storage_volumes_backup.go b/lxd/storage_volumes_backup.go
index 2979481bbe..2b85e6f6b8 100644
--- a/lxd/storage_volumes_backup.go
+++ b/lxd/storage_volumes_backup.go
@@ -72,7 +72,7 @@ func storagePoolVolumeTypeCustomBackupsGet(d *Daemon, r *http.Request) response.
}
// Handle requests targeted to a volume on a different node
- resp := forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp := forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -145,7 +145,7 @@ func storagePoolVolumeTypeCustomBackupsPost(d *Daemon, r *http.Request) response
return response.SmartError(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -287,7 +287,7 @@ func storagePoolVolumeTypeCustomBackupGet(d *Daemon, r *http.Request) response.R
return response.SmartError(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -338,7 +338,7 @@ func storagePoolVolumeTypeCustomBackupPost(d *Daemon, r *http.Request) response.
return response.SmartError(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -420,7 +420,7 @@ func storagePoolVolumeTypeCustomBackupDelete(d *Daemon, r *http.Request) respons
return response.SmartError(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -489,7 +489,7 @@ func storagePoolVolumeTypeCustomBackupExportGet(d *Daemon, r *http.Request) resp
return response.SmartError(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index 9878c0d822..d766a8f7db 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -114,7 +114,7 @@ func storagePoolVolumeSnapshotsTypePost(d *Daemon, r *http.Request) response.Res
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -302,7 +302,7 @@ func storagePoolVolumeSnapshotTypePost(d *Daemon, r *http.Request) response.Resp
}
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
@@ -368,7 +368,7 @@ func storagePoolVolumeSnapshotTypeGet(d *Daemon, r *http.Request) response.Respo
}
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
@@ -437,7 +437,7 @@ func storagePoolVolumeSnapshotTypePut(d *Daemon, r *http.Request) response.Respo
}
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
@@ -533,7 +533,7 @@ func storagePoolVolumeSnapshotTypeDelete(d *Daemon, r *http.Request) response.Re
}
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
From d1b92fe24513fef9698c2c25d8a1459d2b658e88 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 09:29:29 +0100
Subject: [PATCH 05/14] lxd/db/storage/volumes: Corrects mispelled argument
name in GetStorageVolumeNodeAddresses
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/storage_volumes.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/db/storage_volumes.go b/lxd/db/storage_volumes.go
index d6680de8a3..b4ef307423 100644
--- a/lxd/db/storage_volumes.go
+++ b/lxd/db/storage_volumes.go
@@ -628,7 +628,7 @@ type StorageVolumeArgs struct {
// The volume name can be either a regular name or a volume snapshot name.
//
// The empty string is used in place of the address of the current node.
-func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, project, name string, typ int) ([]string, error) {
+func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, project, name string, volumeType int) ([]string, error) {
nodes := []struct {
id int64
address string
@@ -656,7 +656,7 @@ SELECT nodes.id, nodes.address
return nil, err
}
defer stmt.Close()
- err = query.SelectObjects(stmt, dest, poolID, project, name, typ)
+ err = query.SelectObjects(stmt, dest, poolID, project, name, volumeType)
if err != nil {
return nil, err
}
From ac8a07df911434a6482abe72c6a360c1fa188ff4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:37:46 +0100
Subject: [PATCH 06/14] lxd/db/errors: Adds ErrNoNode var used to indicate no
node has been found for a resource
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/errors.go | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lxd/db/errors.go b/lxd/db/errors.go
index c1f046a7dc..1208a36343 100644
--- a/lxd/db/errors.go
+++ b/lxd/db/errors.go
@@ -16,4 +16,7 @@ var (
// isn't found so we don't abuse sql.ErrNoRows any more than we
// already do.
ErrNoSuchObject = fmt.Errorf("No such object")
+
+ // ErrNoNode is used to indicate no node has been found for a resource.
+ ErrNoNode = fmt.Errorf("No node found")
)
From aa23b4f8eb30b3558e8b073c563dc69f3f773ca8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:39:12 +0100
Subject: [PATCH 07/14] lxd/db/storage/volumes: Modifies
GetStorageVolumeNodeAddresses to detect volumes that are not bound to a
single node
Returns special error value in that case.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/storage_volumes.go | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/lxd/db/storage_volumes.go b/lxd/db/storage_volumes.go
index b4ef307423..40cf6d33d6 100644
--- a/lxd/db/storage_volumes.go
+++ b/lxd/db/storage_volumes.go
@@ -628,7 +628,7 @@ type StorageVolumeArgs struct {
// The volume name can be either a regular name or a volume snapshot name.
//
// The empty string is used in place of the address of the current node.
-func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, project, name string, volumeType int) ([]string, error) {
+func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, projectName string, volumeName string, volumeType int) ([]string, error) {
nodes := []struct {
id int64
address string
@@ -642,27 +642,32 @@ func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, project, name st
}
sql := `
-SELECT nodes.id, nodes.address
- FROM nodes
- JOIN storage_volumes_all ON storage_volumes_all.node_id=nodes.id
- JOIN projects ON projects.id = storage_volumes_all.project_id
- WHERE storage_volumes_all.storage_pool_id=?
- AND projects.name=?
- AND storage_volumes_all.name=?
- AND storage_volumes_all.type=?
+ SELECT coalesce(nodes.id,0) AS nodeID, coalesce(nodes.address,"") AS nodeAddress
+ FROM storage_volumes_all
+ JOIN projects ON projects.id = storage_volumes_all.project_id
+ LEFT JOIN nodes ON storage_volumes_all.node_id=nodes.id
+ WHERE storage_volumes_all.storage_pool_id=?
+ AND projects.name=?
+ AND storage_volumes_all.name=?
+ AND storage_volumes_all.type=?
`
stmt, err := c.tx.Prepare(sql)
if err != nil {
return nil, err
}
defer stmt.Close()
- err = query.SelectObjects(stmt, dest, poolID, project, name, volumeType)
+ err = query.SelectObjects(stmt, dest, poolID, projectName, volumeName, volumeType)
if err != nil {
return nil, err
}
addresses := []string{}
for _, node := range nodes {
+ // Volume is defined without a node.
+ if node.id == 0 && node.address == "" {
+ return nil, ErrNoNode
+ }
+
address := node.address
if node.id == c.nodeID {
address = ""
From 4cc9d74e880558f43f2bb882344cee2050955501 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:40:16 +0100
Subject: [PATCH 08/14] lxd/db/storage/volumes: Renames
StorageVolumeIsAvailable to StorageVolumeAttachedExclusive
And modifies return value of function to return the remote instance that has the volume attached exclusively.
This will be used to redirect API requests for the volume to the instance's node.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/storage_volumes.go | 116 +++++++++++++++++---------------------
1 file changed, 52 insertions(+), 64 deletions(-)
diff --git a/lxd/db/storage_volumes.go b/lxd/db/storage_volumes.go
index 40cf6d33d6..d30dc21fab 100644
--- a/lxd/db/storage_volumes.go
+++ b/lxd/db/storage_volumes.go
@@ -812,70 +812,6 @@ SELECT storage_volumes_snapshots.name FROM storage_volumes_snapshots
return max
}
-// StorageVolumeIsAvailable checks that if a custom volume available for being attached.
-//
-// Always return true for non-Ceph volumes.
-//
-// For Ceph volumes, return true if the volume is either not attached to any
-// other container, or attached to containers on this node.
-func (c *Cluster) StorageVolumeIsAvailable(pool, volume string) (bool, error) {
- isAvailable := false
-
- err := c.Transaction(func(tx *ClusterTx) error {
- id, err := tx.GetStoragePoolID(pool)
- if err != nil {
- return errors.Wrapf(err, "Fetch storage pool ID for %q", pool)
- }
-
- driver, err := tx.GetStoragePoolDriver(id)
- if err != nil {
- return errors.Wrapf(err, "Fetch storage pool driver for %q", pool)
- }
-
- if driver != "ceph" {
- isAvailable = true
- return nil
- }
-
- node, err := tx.GetLocalNodeName()
- if err != nil {
- return errors.Wrapf(err, "Fetch node name")
- }
-
- containers, err := tx.instanceListExpanded()
- if err != nil {
- return errors.Wrapf(err, "Fetch instances")
- }
-
- for _, container := range containers {
- for _, device := range container.Devices {
- if device["type"] != "disk" {
- continue
- }
- if device["pool"] != pool {
- continue
- }
- if device["source"] != volume {
- continue
- }
- if container.Node != node {
- // This ceph volume is already attached
- // to a container on a different node.
- return nil
- }
- }
- }
- isAvailable = true
-
- return nil
- })
- if err != nil {
- return false, err
- }
-
- return isAvailable, nil
-}
-
// Updates the description of a storage volume.
func storageVolumeDescriptionUpdate(tx *sql.Tx, volumeID int64, description string, isSnapshot bool) error {
var table string
@@ -1057,3 +993,55 @@ WHERE storage_volumes.type = ? AND projects.name = ?
return volumes, nil
}
+
+// StorageVolumeAttachedExclusive checks whether a custom volume available for being attached.
+// Returns the remote instance that has the volume exclusively attached. If not returns nil if available.
+func (c *ClusterTx) StorageVolumeAttachedExclusive(poolName string, volumeName string) (*Instance, error) {
+ id, err := c.GetStoragePoolID(poolName)
+ if err != nil {
+ return nil, errors.Wrapf(err, "Fetch storage pool ID for %q", poolName)
+ }
+
+ driver, err := c.GetStoragePoolDriver(id)
+ if err != nil {
+ return nil, errors.Wrapf(err, "Fetch storage pool driver for %q", poolName)
+ }
+
+ // Always return nil for non-Ceph volumes.
+ if driver != "ceph" {
+ return nil, nil
+ }
+
+ // For Ceph volumes, return nil if the volume is either not attached to any other instance,
+ // or attached to instance(s) on this node.
+ node, err := c.GetLocalNodeName()
+ if err != nil {
+ return nil, errors.Wrapf(err, "Fetch node name")
+ }
+
+ instances, err := c.instanceListExpanded()
+ if err != nil {
+ return nil, errors.Wrapf(err, "Fetch instances")
+ }
+
+ for _, inst := range instances {
+ for _, device := range inst.Devices {
+ if device["type"] != "disk" {
+ continue
+ }
+ if device["pool"] != poolName {
+ continue
+ }
+ if device["source"] != volumeName {
+ continue
+ }
+
+ if inst.Node != node {
+ // Volume is exclusively attached to an instance on a different node.
+ return &inst, nil
+ }
+ }
+ }
+
+ return nil, nil
+}
From 108b941dab25306f77adf04c43d28bc3884ede74 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:41:58 +0100
Subject: [PATCH 09/14] lxd/cluster/connect: Modifies ConnectIfVolumeIsRemote
to take poolName rather than poolID
This is so it can use StorageVolumeAttachedExclusive when a custom volume is not bound to a specific node.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/cluster/connect.go | 74 ++++++++++++++++++++++++++++++------------
1 file changed, 53 insertions(+), 21 deletions(-)
diff --git a/lxd/cluster/connect.go b/lxd/cluster/connect.go
index 1d93cef6bf..38d1d994a7 100644
--- a/lxd/cluster/connect.go
+++ b/lxd/cluster/connect.go
@@ -121,40 +121,72 @@ func ConnectIfInstanceIsRemote(cluster *db.Cluster, projectName string, name str
//
// If there is more than one node with a matching volume name, an error is
// returned.
-func ConnectIfVolumeIsRemote(cluster *db.Cluster, poolID int64, projectName string, volumeName string, volumeType int, cert *shared.CertInfo) (lxd.InstanceServer, error) {
- var addresses []string // Node addresses
+func ConnectIfVolumeIsRemote(cluster *db.Cluster, poolName string, projectName string, volumeName string, volumeType int, cert *shared.CertInfo) (lxd.InstanceServer, error) {
+ var address string
+
err := cluster.Transaction(func(tx *db.ClusterTx) error {
- var err error
- addresses, err = tx.GetStorageVolumeNodeAddresses(poolID, projectName, volumeName, volumeType)
- return err
- })
- if err != nil {
- return nil, err
- }
+ poolID, err := tx.GetStoragePoolID(poolName)
+ if err != nil {
+ return err
+ }
- if len(addresses) > 1 {
- var driver string
- err := cluster.Transaction(func(tx *db.ClusterTx) error {
- var err error
- driver, err = tx.GetStoragePoolDriver(poolID)
+ addresses, err := tx.GetStorageVolumeNodeAddresses(poolID, projectName, volumeName, volumeType)
+ if err != nil && err != db.ErrNoNode {
return err
- })
- if err != nil {
- return nil, err
}
- if driver == "ceph" || driver == "cephfs" {
- return nil, nil
+ // If volume uses a remote storage driver and so has no explicit node, then we need to check
+ // whether it is exclusively attached to remote instance, and if so then we need to forward the
+ // request to the node where it is currently used. This avoids using the volume locally when that
+ // may cause issues when in use on another node.
+ if err == db.ErrNoNode {
+ remoteInstance, err := tx.StorageVolumeAttachedExclusive(poolName, volumeName)
+ if err != nil {
+ return fmt.Errorf("Check if volume is available: %v", err)
+ }
+
+ if remoteInstance != nil {
+ node, err := tx.GetNodeByName(remoteInstance.Node)
+ if err != nil {
+ return fmt.Errorf("Check node of remote instance: %v", err)
+ }
+
+ addresses = []string{node.Address} // Replace address list with instance's node.
+ } else {
+ // Volume isn't exclusively attached to an instance and has no fixed node.
+ addresses = []string{""} // Use local node.
+ }
+ }
+
+ addressCount := len(addresses)
+ if addressCount > 1 {
+ driver, err := tx.GetStoragePoolDriver(poolID)
+ if err != nil {
+ return err
+ }
+
+ if driver == "ceph" || driver == "cephfs" {
+ return nil
+ }
+
+ return fmt.Errorf("More than one node has a volume named %q", volumeName)
+ } else if addressCount < 1 {
+ return fmt.Errorf("Volume %q has empty node list", volumeName)
}
- return nil, fmt.Errorf("more than one node has a volume named %s", volumeName)
+ address = addresses[0]
+ return nil
+ })
+ if err != nil {
+ return nil, err
}
- address := addresses[0]
+ // Use local node.
if address == "" {
return nil, nil
}
+ // Connect to remote node.
return Connect(address, cert, false)
}
From 542520e533a3e9f9d0c69d2eeaa828436e05db5d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:43:03 +0100
Subject: [PATCH 10/14] lxd/device/disk: tx.StorageVolumeAttachedExclusive
updated usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/disk.go | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 4f728007e7..c76183a088 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -174,12 +174,19 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
// device (check for instancetype.Any), and we have least one expanded device (this is so we only
// do this expensive check after devices have been expanded).
if instConf.Type() != instancetype.Any && len(instConf.ExpandedDevices()) > 0 && d.config["source"] != "" && d.config["path"] != "/" {
- isAvailable, err := d.state.Cluster.StorageVolumeIsAvailable(d.config["pool"], d.config["source"])
+ err := d.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+ remoteInstance, err := tx.StorageVolumeAttachedExclusive(d.config["pool"], d.config["source"])
+ if err != nil {
+ return errors.Wrapf(err, "Failed checking if volume is exclusively attached")
+ }
+ if remoteInstance != nil {
+ return fmt.Errorf("Storage volume %q is already attached to an instance on a different node", d.config["source"])
+ }
+
+ return nil
+ })
if err != nil {
- return fmt.Errorf("Check if volume is available: %v", err)
- }
- if !isAvailable {
- return fmt.Errorf("Storage volume %q is already attached to an instance on a different node", d.config["source"])
+ return err
}
}
From b9fd0b0df5c999a95d502d8b519b571dcf1c9666 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:43:26 +0100
Subject: [PATCH 11/14] lxd/response: Updates forwardedResponseIfVolumeIsRemote
to accept poolName rather than poolID
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/response.go | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lxd/response.go b/lxd/response.go
index 77557643cb..e61b9a7461 100644
--- a/lxd/response.go
+++ b/lxd/response.go
@@ -4,7 +4,6 @@ import (
"net/http"
"github.com/lxc/lxd/lxd/cluster"
- "github.com/lxc/lxd/lxd/db"
"github.com/lxc/lxd/lxd/instance/instancetype"
"github.com/lxc/lxd/lxd/response"
)
@@ -59,18 +58,20 @@ func forwardedResponseIfInstanceIsRemote(d *Daemon, r *http.Request, project, na
//
// This is used when no targetNode is specified, and saves users some typing
// when the volume name/type is unique to a node.
-func forwardedResponseIfVolumeIsRemote(d *Daemon, r *http.Request, poolID int64, projectName string, volumeName string, volumeType int) response.Response {
+func forwardedResponseIfVolumeIsRemote(d *Daemon, r *http.Request, poolName string, projectName string, volumeName string, volumeType int) response.Response {
if queryParam(r, "target") != "" {
return nil
}
cert := d.endpoints.NetworkCert()
- client, err := cluster.ConnectIfVolumeIsRemote(d.cluster, poolID, projectName, volumeName, volumeType, cert)
- if err != nil && err != db.ErrNoSuchObject {
+ client, err := cluster.ConnectIfVolumeIsRemote(d.cluster, poolName, projectName, volumeName, volumeType, cert)
+ if err != nil {
return response.SmartError(err)
}
+
if client == nil {
return nil
}
+
return response.ForwardedResponse(client, r)
}
From 386410d99b768a12719db8582a2d8f36258f146d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:43:56 +0100
Subject: [PATCH 12/14] lxd/storage/volumes: forwardedResponseIfVolumeIsRemote
usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_volumes.go | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index e823b181cd..28d4c8ef7d 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -634,7 +634,7 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
return response.BadRequest(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -857,7 +857,7 @@ func storagePoolVolumeTypeGet(d *Daemon, r *http.Request, volumeTypeName string)
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -936,7 +936,7 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request, volumeTypeName string)
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, pool.ID(), projectName, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, pool.Name(), projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -1078,7 +1078,7 @@ func storagePoolVolumeTypePatch(d *Daemon, r *http.Request, volumeTypeName strin
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, pool.ID(), projectName, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, pool.Name(), projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -1171,12 +1171,7 @@ func storagePoolVolumeTypeDelete(d *Daemon, r *http.Request, volumeTypeName stri
return resp
}
- poolID, _, err := d.cluster.GetStoragePool(poolName)
- if err != nil {
- return response.SmartError(err)
- }
-
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
From 5a51ea0e0ffe4aec7125210c7696e8572c70e6b2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:44:16 +0100
Subject: [PATCH 13/14] lxd/storage/volumes/backup:
forwardedResponseIfVolumeIsRemote usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_volumes_backup.go | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/lxd/storage_volumes_backup.go b/lxd/storage_volumes_backup.go
index 2b85e6f6b8..b39d87ad8f 100644
--- a/lxd/storage_volumes_backup.go
+++ b/lxd/storage_volumes_backup.go
@@ -72,7 +72,7 @@ func storagePoolVolumeTypeCustomBackupsGet(d *Daemon, r *http.Request) response.
}
// Handle requests targeted to a volume on a different node
- resp := forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp := forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -145,7 +145,7 @@ func storagePoolVolumeTypeCustomBackupsPost(d *Daemon, r *http.Request) response
return response.SmartError(err)
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -282,12 +282,7 @@ func storagePoolVolumeTypeCustomBackupGet(d *Daemon, r *http.Request) response.R
return resp
}
- poolID, _, err := d.cluster.GetStoragePool(poolName)
- if err != nil {
- return response.SmartError(err)
- }
-
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -333,12 +328,7 @@ func storagePoolVolumeTypeCustomBackupPost(d *Daemon, r *http.Request) response.
return resp
}
- poolID, _, err := d.cluster.GetStoragePool(poolName)
- if err != nil {
- return response.SmartError(err)
- }
-
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -415,12 +405,7 @@ func storagePoolVolumeTypeCustomBackupDelete(d *Daemon, r *http.Request) respons
return resp
}
- poolID, _, err := d.cluster.GetStoragePool(poolName)
- if err != nil {
- return response.SmartError(err)
- }
-
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
@@ -484,12 +469,7 @@ func storagePoolVolumeTypeCustomBackupExportGet(d *Daemon, r *http.Request) resp
return resp
}
- poolID, _, err := d.cluster.GetStoragePool(poolName)
- if err != nil {
- return response.SmartError(err)
- }
-
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, db.StoragePoolVolumeTypeCustom)
if resp != nil {
return resp
}
From 68f1906199abc68cc47f1f0f73d4c31434bd7ad3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Oct 2020 12:44:38 +0100
Subject: [PATCH 14/14] lxd/storage/volumes/snapshot:
forwardedResponseIfVolumeIsRemote
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_volumes_snapshot.go | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index d766a8f7db..323f813659 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -114,7 +114,7 @@ func storagePoolVolumeSnapshotsTypePost(d *Daemon, r *http.Request) response.Res
return resp
}
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, volumeName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, volumeType)
if resp != nil {
return resp
}
@@ -296,13 +296,8 @@ func storagePoolVolumeSnapshotTypePost(d *Daemon, r *http.Request) response.Resp
return resp
}
- poolID, _, err := d.cluster.GetStoragePool(poolName)
- if err != nil {
- return response.SmartError(err)
- }
-
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
@@ -368,7 +363,7 @@ func storagePoolVolumeSnapshotTypeGet(d *Daemon, r *http.Request) response.Respo
}
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
@@ -437,7 +432,7 @@ func storagePoolVolumeSnapshotTypePut(d *Daemon, r *http.Request) response.Respo
}
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
@@ -527,13 +522,8 @@ func storagePoolVolumeSnapshotTypeDelete(d *Daemon, r *http.Request) response.Re
return resp
}
- poolID, _, err := d.cluster.GetStoragePool(poolName)
- if err != nil {
- return response.SmartError(err)
- }
-
fullSnapshotName := fmt.Sprintf("%s/%s", volumeName, snapshotName)
- resp = forwardedResponseIfVolumeIsRemote(d, r, poolID, projectName, fullSnapshotName, volumeType)
+ resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, fullSnapshotName, volumeType)
if resp != nil {
return resp
}
More information about the lxc-devel
mailing list