Fix go.mod, simplify boolean logic, add logging

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>

integration/client/go.mod

@@ -4,6 +4,7 @@ go 1.19
 
 require (
 	github.com/AdaLogics/go-fuzz-headers v0.0.0-20230106234847-43070de90fa1
+	github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62
 	github.com/Microsoft/hcsshim v0.10.0-rc.7
 	github.com/Microsoft/hcsshim/test v0.0.0-20210408205431-da33ecd607e1
 	github.com/containerd/cgroups/v3 v3.0.1
@@ -22,8 +23,6 @@ require (
 	golang.org/x/sys v0.6.0
 )
 
-require github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62
-
 require (
 	github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20221215162035-5330a85ea652 // indirect
 	github.com/cilium/ebpf v0.9.1 // indirect

mount/mount_windows.go

@@ -17,16 +17,17 @@
 package mount
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
-	"syscall"
 
 	"github.com/Microsoft/go-winio/pkg/bindfilter"
 	"github.com/Microsoft/hcsshim"
+	"github.com/containerd/containerd/log"
 	"golang.org/x/sys/windows"
 )
 
@@ -74,16 +75,21 @@ func (m *Mount) mount(target string) error {
 	}
 	defer func() {
 		if err != nil {
-			hcsshim.DeactivateLayer(di, layerID)
+			if layerErr := hcsshim.DeactivateLayer(di, layerID); layerErr != nil {
+				log.G(context.TODO()).WithError(layerErr).Error("failed to deactivate layer during mount failure cleanup")
+			}
 		}
 	}()
 
 	if err = hcsshim.PrepareLayer(di, layerID, parentLayerPaths); err != nil {
 		return fmt.Errorf("failed to prepare layer %s: %w", m.Source, err)
 	}
+
 	defer func() {
 		if err != nil {
-			hcsshim.UnprepareLayer(di, layerID)
+			if layerErr := hcsshim.UnprepareLayer(di, layerID); layerErr != nil {
+				log.G(context.TODO()).WithError(layerErr).Error("failed to unprepare layer during mount failure cleanup")
+			}
 		}
 	}()
 
@@ -97,7 +103,9 @@ func (m *Mount) mount(target string) error {
 	}
 	defer func() {
 		if err != nil {
-			bindfilter.RemoveFileBinding(target)
+			if bindErr := bindfilter.RemoveFileBinding(target); bindErr != nil {
+				log.G(context.TODO()).WithError(bindErr).Error("failed to remove binding during mount failure cleanup")
+			}
 		}
 	}()
 
@@ -144,7 +152,7 @@ func Unmount(mount string, flags int) error {
 	}
 
 	if err := bindfilter.RemoveFileBinding(mount); err != nil {
-		if errno, ok := errors.Unwrap(err).(syscall.Errno); ok && errno == windows.ERROR_INVALID_PARAMETER || errno == windows.ERROR_NOT_FOUND {
+		if errors.Is(err, windows.ERROR_INVALID_PARAMETER) || errors.Is(err, windows.ERROR_NOT_FOUND) {
 			// not a mount point
 			return nil
 		}

snapshots/windows/windows.go

@@ -362,7 +362,7 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 		// Create the new snapshot dir
 		snDir := s.getSnapshotDir(newSnapshot.ID)
 		if err = os.MkdirAll(snDir, 0700); err != nil {
-			return err
+			return fmt.Errorf("creating snapshot dir: %w", err)
 		}
 
 		if strings.Contains(key, snapshots.UnpackKeyPrefix) {
@@ -372,57 +372,59 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 			// snapshot if this isn't the snapshot that just houses the final sandbox.vhd
 			// that will be mounted as the containers scratch. Currently the key for a snapshot
 			// where a layer will be extracted to will have the string `extract-` in it.
-		} else if len(newSnapshot.ParentIDs) == 0 {
+			return nil
+		}
+
+		if len(newSnapshot.ParentIDs) == 0 {
 			// A parentless snapshot is just a bind-mount to a directory named
 			// "Files". When committed, there'll be some post-processing to fill in the rest
 			// of the metadata.
 			filesDir := filepath.Join(snDir, "Files")
 			if err := os.MkdirAll(filesDir, 0700); err != nil {
-				return err
+				return fmt.Errorf("creating Files dir: %w", err)
 			}
-		} else {
+			return nil
-			parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs)
+		}
 
-			var snapshotInfo snapshots.Info
+		parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs)
-			for _, o := range opts {
+		var snapshotInfo snapshots.Info
-				o(&snapshotInfo)
+		for _, o := range opts {
+			o(&snapshotInfo)
+		}
+
+		var sizeInBytes uint64
+		if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok {
+			log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel)
+
+			sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32)
+			if err != nil {
+				return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err)
 			}
+			sizeInBytes = sizeInGB * 1024 * 1024 * 1024
+		}
 
-			var sizeInBytes uint64
+		// Prefer the newer label in bytes over the deprecated Windows specific GB variant.
-			if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok {
+		if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok {
-				log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel)
+			sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64)
-
+			if err != nil {
-				sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32)
+				return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err)
-				if err != nil {
-					return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err)
-				}
-				sizeInBytes = sizeInGB * 1024 * 1024 * 1024
-			}
-
-			// Prefer the newer label in bytes over the deprecated Windows specific GB variant.
-			if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok {
-				sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64)
-				if err != nil {
-					return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err)
-				}
-			}
-
-			var makeUVMScratch bool
-			if _, ok := snapshotInfo.Labels[uvmScratchLabel]; ok {
-				makeUVMScratch = true
-			}
-
-			// This has to be run first to avoid clashing with the containers sandbox.vhdx.
-			if makeUVMScratch {
-				if err = s.createUVMScratchLayer(ctx, snDir, parentLayerPaths); err != nil {
-					return fmt.Errorf("failed to make UVM's scratch layer: %w", err)
-				}
-			}
-			if err = s.createScratchLayer(ctx, snDir, parentLayerPaths, sizeInBytes); err != nil {
-				return fmt.Errorf("failed to create scratch layer: %w", err)
 			}
 		}
 
+		var makeUVMScratch bool
+		if _, ok := snapshotInfo.Labels[uvmScratchLabel]; ok {
+			makeUVMScratch = true
+		}
+
+		// This has to be run first to avoid clashing with the containers sandbox.vhdx.
+		if makeUVMScratch {
+			if err = s.createUVMScratchLayer(ctx, snDir, parentLayerPaths); err != nil {
+				return fmt.Errorf("failed to make UVM's scratch layer: %w", err)
+			}
+		}
+		if err = s.createScratchLayer(ctx, snDir, parentLayerPaths, sizeInBytes); err != nil {
+			return fmt.Errorf("failed to create scratch layer: %w", err)
+		}
 		return nil
 	})
 	if err != nil {