Don't block snapshot garbage collection on Remove failures

ambarve · ambarve · commit ed7c6895bd3b · 2023-12-07T09:20:54.000-08:00
If a snapshot removal fails (during garbage collection), the entire garbage collection operation is cancelled. This is problematic because once cleanup of any snapshot fails no other snapshots will be cleaned and the disk usage will just keep increasing. Solution is to return snapshot removal errors wrapped as "ErrFailedPrecondition" errors. The garbage collectors continues cleanup if the error is of this type. Signed-off-by: Amit Barve <ambarve@microsoft.com> (cherry picked from commit ad96fde) Signed-off-by: Amit Barve <ambarve@microsoft.com>
diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go
@@ -280,7 +280,9 @@ func (s *snapshotter) Remove(ctx context.Context, key string) error {
 				log.G(ctx).WithError(err1).WithField("path", renamed).Error("Failed to rename after failed commit")
 			}
 		}
-		return err
+		// Return the error wrapped in ErrFailedPrecondition so that cleanup of other snapshots will
+		// still continue.
+		return errors.Join(errdefs.ErrFailedPrecondition, err)
 	}
 
 	if err = hcsshim.DestroyLayer(s.info, renamedID); err != nil {

Original file line number	Diff line number	Diff line change
`@@ -280,7 +280,9 @@ func (s *snapshotter) Remove(ctx context.Context, key string) error {`
`280`	`280`	`log.G(ctx).WithError(err1).WithField("path", renamed).Error("Failed to rename after failed commit")`
`281`	`281`	`}`
`282`	`282`	`}`
`283`		`- return err`
	`283`	`+ // Return the error wrapped in ErrFailedPrecondition so that cleanup of other snapshots will`
	`284`	`+ // still continue.`
	`285`	`+ return errors.Join(errdefs.ErrFailedPrecondition, err)`
`284`	`286`	`}`
`285`	`287`
`286`	`288`	`if err = hcsshim.DestroyLayer(s.info, renamedID); err != nil {`