From 268b1d63b9570e750bc13e011f475a17e6878041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20R=C3=B6hrich?= Date: Wed, 6 Dec 2023 14:59:59 +0100 Subject: [PATCH] fix: scope of err variable while migrating volume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the scope of the err variable while migrating a volume. The root of this problem is that in case a migration revert is attempted, `err` is re-defined in a defer'ed anonymous function. This shadows the definition of the variable `err` in the outer scope, which causes the original error during the migration to be lost. The simple fix is to define a different error and wrap the original error with a message containing the new error. This avoids redefining `err`, making it available with its original value in the defer'ed function. Signed-off-by: Moritz Röhrich --- controller/volume_controller.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/controller/volume_controller.go b/controller/volume_controller.go index 641826e2b1..28a890a2fd 100644 --- a/controller/volume_controller.go +++ b/controller/volume_controller.go @@ -3824,9 +3824,9 @@ func (c *VolumeController) processMigration(v *longhorn.Volume, es map[string]*l log.Warnf("The migration engine or all migration replicas crashed, will clean them up now") - currentEngine, err := c.getCurrentEngineAndCleanupOthers(v, es) - if err != nil { - err = errors.Wrap(err, "failed to get the current engine and clean up others during the migration revert") + currentEngine, err2 := c.getCurrentEngineAndCleanupOthers(v, es) + if err2 != nil { + err = errors.Wrapf(err, "failed to get the current engine and clean up others during the migration revert: %v", err2) return } @@ -3834,8 +3834,8 @@ func (c *VolumeController) processMigration(v *longhorn.Volume, es map[string]*l if r.Spec.EngineName == currentEngine.Name { continue } - if err := c.deleteReplica(r, rs); err != nil { - err = errors.Wrapf(err, "failed to delete the migration replica %v during the migration revert", r.Name) + if err2 := c.deleteReplica(r, rs); err2 != nil { + err = errors.Wrapf(err, "failed to delete the migration replica %v during the migration revert: %v", r.Name, err2) return } }