From 52a44fbf6d9d402ff4713d66f97e4d99dabcb80a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 16 Feb 2022 22:28:55 +0100 Subject: [PATCH] Log all kinds of failures to delete a layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, if the attempts to recover from a failure themselves fail, we don't record that at all. That makes diagnosing the situation, or hypothetically detecting that the cleanup could never work, much harder. So, log the errors. Alternatively, we could include those failures as extra text in the returned error; that's less likely to be lost (e.g. a Go caller would have the extra text available, without setting up extra infrastructure to capture logs), but possibly harder to follow. Signed-off-by: Miloslav Trmač --- drivers/overlay/overlay.go | 4 +++- layers.go | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index e5355590b..d8810e81c 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -920,7 +920,9 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, disable defer func() { // Clean up on failure if retErr != nil { - os.RemoveAll(dir) + if err2 := os.RemoveAll(dir); err2 != nil { + logrus.Errorf("While recovering from a failure creating a layer, error deleting %#v: %v", dir, err2) + } } }() diff --git a/layers.go b/layers.go index 00863a1c8..822177fd2 100644 --- a/layers.go +++ b/layers.go @@ -768,7 +768,9 @@ func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLab if err = r.driver.UpdateLayerIDMap(id, oldMappings, idMappings, mountLabel); err != nil { // We don't have a record of this layer, but at least // try to clean it up underneath us. - r.driver.Remove(id) + if err2 := r.driver.Remove(id); err2 != nil { + logrus.Errorf("While recovering from a failure creating in UpdateLayerIDMap, error deleting layer %#v: %v", id, err2) + } return nil, -1, err } } @@ -799,15 +801,18 @@ func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLab if err != nil { // We don't have a record of this layer, but at least // try to clean it up underneath us. - r.driver.Remove(id) + if err2 := r.driver.Remove(id); err2 != nil { + logrus.Errorf("While recovering from a failure saving incomplete layer metadata, error deleting layer %#v: %v", id, err2) + } return nil, -1, err } size, err = r.applyDiffWithOptions(layer.ID, moreOptions, diff) if err != nil { - if r.Delete(layer.ID) != nil { + if err2 := r.Delete(layer.ID); err2 != nil { // Either a driver error or an error saving. // We now have a layer that's been marked for // deletion but which we failed to remove. + logrus.Errorf("While recovering from a failure applying layer diff, error deleting layer %#v: %v", layer.ID, err2) } return nil, -1, err } @@ -817,7 +822,9 @@ func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLab if err != nil { // We don't have a record of this layer, but at least // try to clean it up underneath us. - r.driver.Remove(id) + if err2 := r.driver.Remove(id); err2 != nil { + logrus.Errorf("While recovering from a failure saving finished layer metadata, error deleting layer %#v: %v", id, err2) + } return nil, -1, err } layer = copyLayer(layer)