[lxc-devel] [lxd/master] Ceph speed up and instance delete improvements

stgraber on Github lxc-bot at linuxcontainers.org
Thu Nov 26 22:25:33 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/20201126/6ae36cda/attachment.bin>
-------------- next part --------------
From 3bd40fc94bc6d7496154a1203dc9c7155fb2b7ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 26 Nov 2020 16:57:27 -0500
Subject: [PATCH 1/2] lxd/storage/ceph: Support background image delete
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This uses a background manager task to perform image deletion rather
than relying on the sometimes slow synchronous mechanism.

Requires CEPH 14.x at least.

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index bdd41448ff..42867dbd8e 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -100,7 +100,22 @@ func (d *ceph) rbdCreateVolume(vol Volume, size string) error {
 //   to be sure that this call actually deleted an RBD storage volume it needs
 //   to check for the existence of the pool first.
 func (d *ceph) rbdDeleteVolume(vol Volume) error {
+	// Try using a background task first.
 	_, err := shared.RunCommand(
+		"ceph",
+		"--name", fmt.Sprintf("client.%s", d.config["ceph.user.name"]),
+		"--cluster", d.config["ceph.cluster_name"],
+		"rbd",
+		"task",
+		"add",
+		"remove",
+		fmt.Sprintf("%s/%s", d.config["ceph.osd.pool_name"], d.getRBDVolumeName(vol, "", false, false)))
+	if err == nil {
+		return nil
+	}
+
+	// Fallback to the slow way.
+	_, err = shared.RunCommand(
 		"rbd",
 		"--id", d.config["ceph.user.name"],
 		"--cluster", d.config["ceph.cluster_name"],

From 12daa54fcada6acff2c855d36c66be701612504b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 26 Nov 2020 17:06:51 -0500
Subject: [PATCH 2/2] lxd/isntance: Bypass delete protection for internal calls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This effectively restricts the accidental deletion protection to user
interactions, allowing for LXD to still perform internal deletions,
primarily when reverting a failed creation/migration.

Closes #8141

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/instance.go                     | 12 ++++++------
 lxd/instance/drivers/driver_lxc.go  |  8 ++++----
 lxd/instance/drivers/driver_qemu.go |  8 ++++----
 lxd/instance/instance_interface.go  |  2 +-
 lxd/instance/instance_utils.go      |  3 ++-
 lxd/instance_delete.go              |  2 +-
 lxd/instance_snapshot.go            |  2 +-
 lxd/instances_post.go               |  6 +++---
 lxd/migrate_instance.go             |  2 +-
 9 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/lxd/instance.go b/lxd/instance.go
index 515f7bc414..6a1d392a39 100644
--- a/lxd/instance.go
+++ b/lxd/instance.go
@@ -48,7 +48,7 @@ func instanceCreateAsEmpty(d *Daemon, args db.InstanceArgs) (instance.Instance,
 			return
 		}
 
-		inst.Delete()
+		inst.Delete(true)
 	}()
 
 	pool, err := storagePools.GetPoolByInstance(d.State(), inst)
@@ -151,7 +151,7 @@ func instanceCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *o
 
 	revert := revert.New()
 	defer revert.Fail()
-	revert.Add(func() { inst.Delete() })
+	revert.Add(func() { inst.Delete(true) })
 
 	err = s.Cluster.UpdateImageLastUseDate(hash, time.Now().UTC())
 	if err != nil {
@@ -186,7 +186,7 @@ func instanceCreateAsCopy(s *state.State, args db.InstanceArgs, sourceInst insta
 			return
 		}
 
-		revertInst.Delete()
+		revertInst.Delete(true)
 	}()
 
 	if refresh {
@@ -233,7 +233,7 @@ func instanceCreateAsCopy(s *state.State, args db.InstanceArgs, sourceInst insta
 
 			// Delete extra snapshots first.
 			for _, snap := range deleteSnapshots {
-				err := snap.Delete()
+				err := snap.Delete(true)
 				if err != nil {
 					return nil, err
 				}
@@ -384,7 +384,7 @@ func instanceCreateAsSnapshot(s *state.State, args db.InstanceArgs, sourceInstan
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed creating instance snapshot record %q", args.Name)
 	}
-	revert.Add(func() { inst.Delete() })
+	revert.Add(func() { inst.Delete(true) })
 
 	pool, err := storagePools.GetPoolByInstance(s, inst)
 	if err != nil {
@@ -871,7 +871,7 @@ func pruneExpiredContainerSnapshotsTask(d *Daemon) (task.Func, task.Schedule) {
 func pruneExpiredContainerSnapshots(ctx context.Context, d *Daemon, snapshots []instance.Instance) error {
 	// Find snapshots to delete
 	for _, snapshot := range snapshots {
-		err := snapshot.Delete()
+		err := snapshot.Delete(true)
 		if err != nil {
 			return errors.Wrapf(err, "Failed to delete expired instance snapshot '%s' in project '%s'", snapshot.Name(), snapshot.Project())
 		}
diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go
index 8ca678e9bf..45d2844562 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -169,7 +169,7 @@ func lxcCreate(s *state.State, args db.InstanceArgs) (instance.Instance, error)
 
 	// Use d.Delete() in revert on error as this function doesn't just create DB records, it can also cause
 	// other modifications to the host when devices are added.
-	revert.Add(func() { d.Delete() })
+	revert.Add(func() { d.Delete(true) })
 
 	// Cleanup the zero values
 	if d.expiryDate.IsZero() {
@@ -2856,7 +2856,7 @@ func (d *lxc) onStop(args map[string]string) error {
 
 		// Destroy ephemeral containers
 		if d.ephemeral {
-			err = d.Delete()
+			err = d.Delete(true)
 			if err != nil {
 				op.Done(errors.Wrap(err, "Failed deleting ephemeral container"))
 				return
@@ -3364,7 +3364,7 @@ func (d *lxc) cleanup() {
 }
 
 // Delete deletes the instance.
-func (d *lxc) Delete() error {
+func (d *lxc) Delete(force bool) error {
 	ctxMap := log.Ctx{
 		"project":   d.project,
 		"name":      d.name,
@@ -3374,7 +3374,7 @@ func (d *lxc) Delete() error {
 
 	logger.Info("Deleting container", ctxMap)
 
-	if shared.IsTrue(d.expandedConfig["security.protection.delete"]) && !d.IsSnapshot() {
+	if !force && shared.IsTrue(d.expandedConfig["security.protection.delete"]) && !d.IsSnapshot() {
 		err := fmt.Errorf("Container is protected")
 		logger.Warn("Failed to delete container", log.Ctx{"name": d.Name(), "err": err})
 		return err
diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 54bad6669f..6233b50983 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -180,7 +180,7 @@ func qemuCreate(s *state.State, args db.InstanceArgs) (instance.Instance, error)
 
 	// Use d.Delete() in revert on error as this function doesn't just create DB records, it can also cause
 	// other modifications to the host when devices are added.
-	revert.Add(func() { d.Delete() })
+	revert.Add(func() { d.Delete(true) })
 
 	// Get the architecture name.
 	archName, err := osarch.ArchitectureName(d.architecture)
@@ -541,7 +541,7 @@ func (d *qemu) onStop(target string) error {
 			fmt.Sprintf("/1.0/virtual-machines/%s", d.name), nil)
 	} else if d.ephemeral {
 		// Destroy ephemeral virtual machines
-		err = d.Delete()
+		err = d.Delete(true)
 		if err != nil {
 			op.Done(err)
 			return err
@@ -3511,7 +3511,7 @@ func (d *qemu) init() error {
 }
 
 // Delete the instance.
-func (d *qemu) Delete() error {
+func (d *qemu) Delete(force bool) error {
 	ctxMap := log.Ctx{
 		"project":   d.project,
 		"name":      d.name,
@@ -3522,7 +3522,7 @@ func (d *qemu) Delete() error {
 	logger.Info("Deleting instance", ctxMap)
 
 	// Check if instance is delete protected.
-	if shared.IsTrue(d.expandedConfig["security.protection.delete"]) && !d.IsSnapshot() {
+	if !force && shared.IsTrue(d.expandedConfig["security.protection.delete"]) && !d.IsSnapshot() {
 		return fmt.Errorf("Instance is protected")
 	}
 
diff --git a/lxd/instance/instance_interface.go b/lxd/instance/instance_interface.go
index c096230bf3..6a38e9594b 100644
--- a/lxd/instance/instance_interface.go
+++ b/lxd/instance/instance_interface.go
@@ -69,7 +69,7 @@ type Instance interface {
 	Rename(newName string) error
 	Update(newConfig db.InstanceArgs, userRequested bool) error
 
-	Delete() error
+	Delete(force bool) error
 	Export(w io.Writer, properties map[string]string) (api.ImageMetadata, error)
 
 	// Used for security.
diff --git a/lxd/instance/instance_utils.go b/lxd/instance/instance_utils.go
index 3ea6b4d6ae..4509781389 100644
--- a/lxd/instance/instance_utils.go
+++ b/lxd/instance/instance_utils.go
@@ -633,7 +633,8 @@ func DeleteSnapshots(s *state.State, projectName, instanceName string) error {
 			continue
 		}
 
-		if err := snapInst.Delete(); err != nil {
+		err = snapInst.Delete(true)
+		if err != nil {
 			logger.Error("DeleteSnapshots: Failed to delete the snapshot", log.Ctx{"project": projectName, "instance": instanceName, "snapshot": snapName, "err": err})
 		}
 	}
diff --git a/lxd/instance_delete.go b/lxd/instance_delete.go
index 963b31f20f..ffa9a718d9 100644
--- a/lxd/instance_delete.go
+++ b/lxd/instance_delete.go
@@ -38,7 +38,7 @@ func containerDelete(d *Daemon, r *http.Request) response.Response {
 	}
 
 	rmct := func(op *operations.Operation) error {
-		return c.Delete()
+		return c.Delete(false)
 	}
 
 	resources := map[string][]string{}
diff --git a/lxd/instance_snapshot.go b/lxd/instance_snapshot.go
index e54c90f741..15379d67aa 100644
--- a/lxd/instance_snapshot.go
+++ b/lxd/instance_snapshot.go
@@ -433,7 +433,7 @@ func snapshotPost(d *Daemon, r *http.Request, sc instance.Instance, containerNam
 
 func snapshotDelete(s *state.State, sc instance.Instance, name string) response.Response {
 	remove := func(op *operations.Operation) error {
-		return sc.Delete()
+		return sc.Delete(false)
 	}
 
 	resources := map[string][]string{}
diff --git a/lxd/instances_post.go b/lxd/instances_post.go
index 369f44230b..3ae8a97f43 100644
--- a/lxd/instances_post.go
+++ b/lxd/instances_post.go
@@ -263,7 +263,7 @@ func createFromMigration(d *Daemon, project string, req *api.InstancesPost) resp
 	revert := true
 	defer func() {
 		if revert && !req.Source.Refresh && inst != nil {
-			inst.Delete()
+			inst.Delete(true)
 		}
 	}()
 
@@ -330,7 +330,7 @@ func createFromMigration(d *Daemon, project string, req *api.InstancesPost) resp
 		opRevert := true
 		defer func() {
 			if opRevert && !req.Source.Refresh && inst != nil {
-				inst.Delete()
+				inst.Delete(true)
 			}
 		}()
 
@@ -693,7 +693,7 @@ func createFromBackup(d *Daemon, projectName string, data io.Reader, pool string
 		}
 
 		// Clean up created instance if the post hook fails below.
-		runRevert.Add(func() { inst.Delete() })
+		runRevert.Add(func() { inst.Delete(true) })
 
 		// Run the storage post hook to perform any final actions now that the instance has been created
 		// in the database (this normally includes unmounting volumes that were mounted).
diff --git a/lxd/migrate_instance.go b/lxd/migrate_instance.go
index e55969a02a..1b922e9760 100644
--- a/lxd/migrate_instance.go
+++ b/lxd/migrate_instance.go
@@ -946,7 +946,7 @@ func (c *migrationSink) Do(state *state.State, migrateOp *operations.Operation)
 
 		// Delete the extra local ones.
 		for _, snap := range deleteSnapshots {
-			err := snap.Delete()
+			err := snap.Delete(true)
 			if err != nil {
 				controller(err)
 				return err


More information about the lxc-devel mailing list