The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8195
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) ===
From 3bd40fc94bc6d7496154a1203dc9c7155fb2b7ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@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 <stgra...@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?= <stgra...@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 <stgra...@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
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel