Merge pull request microsoft#2612 from rawahars/refactor-into-vmutils

Refactor GCS logging and guest management for reusability

Author: rawahars

cmd/containerd-shim-runhcs-v1/task_hcs.go

@@ -37,6 +37,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/jobcontainers"
 	"github.com/Microsoft/hcsshim/internal/layers"
 	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/internal/logfields"
 	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/Microsoft/hcsshim/internal/oc"
 	"github.com/Microsoft/hcsshim/internal/oci"
@@ -768,7 +769,9 @@ func (ht *hcsTask) ExecInHost(ctx context.Context, req *shimdiag.ExecProcessRequ
 	if ht.host == nil {
 		return cmd.ExecInShimHost(ctx, cmdReq)
 	}
-	return cmd.ExecInUvm(ctx, ht.host, cmdReq)
+
+	ctx, _ = log.SetEntry(ctx, logrus.Fields{logfields.UVMID: ht.host.ID()})
+	return ht.host.ExecInUVM(ctx, cmdReq)
 }
 
 func (ht *hcsTask) DumpGuestStacks(ctx context.Context) string {

cmd/containerd-shim-runhcs-v1/task_wcow_podsandbox.go

@@ -11,16 +11,19 @@ import (
 	"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats"
 	"github.com/Microsoft/hcsshim/internal/cmd"
 	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/internal/logfields"
 	"github.com/Microsoft/hcsshim/internal/oc"
 	"github.com/Microsoft/hcsshim/internal/shimdiag"
 	"github.com/Microsoft/hcsshim/internal/uvm"
+
 	eventstypes "github.com/containerd/containerd/api/events"
 	task "github.com/containerd/containerd/api/runtime/task/v2"
 	"github.com/containerd/containerd/v2/core/runtime"
 	"github.com/containerd/errdefs"
 	typeurl "github.com/containerd/typeurl/v2"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"go.opencensus.io/trace"
 )
 
@@ -253,7 +256,9 @@ func (wpst *wcowPodSandboxTask) ExecInHost(ctx context.Context, req *shimdiag.Ex
 	if wpst.host == nil {
 		return 0, errTaskNotIsolated
 	}
-	return cmd.ExecInUvm(ctx, wpst.host, cmdReq)
+
+	ctx, _ = log.SetEntry(ctx, logrus.Fields{logfields.UVMID: wpst.host.ID()})
+	return wpst.host.ExecInUVM(ctx, cmdReq)
 }
 
 func (wpst *wcowPodSandboxTask) DumpGuestStacks(ctx context.Context) string {

internal/cmd/diag.go

@@ -7,13 +7,12 @@ import (
 	"errors"
 	"os/exec"
 
+	"github.com/Microsoft/hcsshim/internal/gcs"
 	"github.com/Microsoft/hcsshim/internal/log"
-	"github.com/Microsoft/hcsshim/internal/logfields"
-	"github.com/Microsoft/hcsshim/internal/uvm"
 )
 
 // ExecInUvm is a helper function used to execute commands specified in `req` inside the given UVM.
-func ExecInUvm(ctx context.Context, vm *uvm.UtilityVM, req *CmdProcessRequest) (int, error) {
+func ExecInUvm(ctx context.Context, guestVM *gcs.GuestConnection, req *CmdProcessRequest) (int, error) {
 	if len(req.Args) == 0 {
 		return 0, errors.New("missing command")
 	}
@@ -22,18 +21,18 @@ func ExecInUvm(ctx context.Context, vm *uvm.UtilityVM, req *CmdProcessRequest) (
 		return 0, err
 	}
 	defer np.Close(ctx)
-	cmd := CommandContext(ctx, vm, req.Args[0], req.Args[1:]...)
+	cmd := CommandContext(ctx, guestVM, req.Args[0], req.Args[1:]...)
 	if req.Workdir != "" {
 		cmd.Spec.Cwd = req.Workdir
 	}
-	if vm.OS() == "windows" {
+	if guestVM.OS() == "windows" {
 		cmd.Spec.User.Username = `NT AUTHORITY\SYSTEM`
 	}
 	cmd.Spec.Terminal = req.Terminal
 	cmd.Stdin = np.Stdin()
 	cmd.Stdout = np.Stdout()
 	cmd.Stderr = np.Stderr()
-	cmd.Log = log.G(ctx).WithField(logfields.UVMID, vm.ID())
+	cmd.Log = log.G(ctx)
 	err = cmd.Run()
 	return cmd.ExitState.ExitCode(), err
 }

internal/devices/assigned_devices.go

@@ -75,7 +75,7 @@ func getChildrenDeviceLocationPaths(ctx context.Context, vm *uvm.UtilityVM, vmBu
 		Args:   args,
 		Stdout: p,
 	}
-	exitCode, err := cmd.ExecInUvm(ctx, vm, cmdReq)
+	exitCode, err := vm.ExecInUVM(ctx, cmdReq)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to find devices with exit code %d", exitCode)
 	}

internal/devices/drivers.go

@@ -17,10 +17,10 @@ import (
 	"github.com/Microsoft/hcsshim/internal/uvm/scsi"
 )
 
-// InstallDriver mounts a share from the host into the UVM, installs any kernel drivers in the share,
+// InstallDrivers mounts a share from the host into the UVM, installs any kernel drivers in the share,
 // and configures the environment for library files and/or binaries in the share.
 //
-// InstallDriver mounts a specified kernel driver, then installs it in the UVM.
+// InstallDrivers mounts a specified kernel driver, then installs it in the UVM.
 //
 // `share` is a directory path on the host that contains files for standard driver installation.
 // For windows this means files for pnp installation (.inf, .cat, .sys, .cert files).
@@ -112,7 +112,7 @@ func execGCSInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir stri
 	// - There's an error copying IO. No need to wait for stderr logs.
 	//
 	// Since we cannot distinguish between the cases above, we should always wait to read the stderr output.
-	exitCode, execErr := cmd.ExecInUvm(ctx, vm, req)
+	exitCode, execErr := vm.ExecInUVM(ctx, req)
 
 	// wait to finish parsing stdout results
 	select {

internal/devices/pnp.go

@@ -51,7 +51,7 @@ func execPnPInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir stri
 	cmdReq := &cmd.CmdProcessRequest{
 		Args: args,
 	}
-	exitCode, err := cmd.ExecInUvm(ctx, vm, cmdReq)
+	exitCode, err := vm.ExecInUVM(ctx, cmdReq)
 	if err != nil && exitCode != winapi.ERROR_NO_MORE_ITEMS {
 		return errors.Wrapf(err, "failed to install driver %s in uvm with exit code %d", driverDir, exitCode)
 	} else if exitCode == winapi.ERROR_NO_MORE_ITEMS {

internal/hcs/system.go

@@ -37,6 +37,7 @@ type System struct {
 	exitError      error
 	os, typ, owner string
 	startTime      time.Time
+	stopTime       time.Time
 }
 
 var _ cow.Container = &System{}
@@ -292,6 +293,7 @@ func (computeSystem *System) waitBackground() {
 	}
 	computeSystem.closedWaitOnce.Do(func() {
 		computeSystem.waitError = err
+		computeSystem.stopTime = time.Now()
 		close(computeSystem.waitBlock)
 	})
 	oc.SetSpanStatus(span, err)
@@ -871,3 +873,11 @@ func (computeSystem *System) Modify(ctx context.Context, config interface{}) err
 
 	return nil
 }
+
+func (computeSystem *System) StoppedTime() time.Time {
+	return computeSystem.stopTime
+}
+
+func (computeSystem *System) StartedTime() time.Time {
+	return computeSystem.startTime
+}

internal/hcsoci/create.go

@@ -22,6 +22,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/hvsocket"
 	"github.com/Microsoft/hcsshim/internal/layers"
 	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/internal/logfields"
 	"github.com/Microsoft/hcsshim/internal/oci"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
 	"github.com/Microsoft/hcsshim/internal/resources"
@@ -204,6 +205,9 @@ func CreateContainer(ctx context.Context, createOptions *CreateOptions) (_ cow.C
 	}()
 
 	if coi.HostingSystem != nil {
+		// Set the UVM ID in ctx so that it gets logged in all subsequent calls.
+		ctx, _ = log.SetEntry(ctx, logrus.Fields{logfields.UVMID: coi.HostingSystem.ID()})
+
 		if coi.Spec.Linux != nil {
 			r.SetContainerRootInUVM(fmt.Sprintf(lcowRootInUVM, coi.ID))
 		} else {

internal/hcsoci/resources_wcow.go

@@ -217,7 +217,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R
 				Args:   []string{"cmd", "/c", "mkdir", sandboxPath, "&", "dir", sandboxPath},
 				Stderr: stderr,
 			}
-			exitCode, err := cmd.ExecInUvm(ctx, coi.HostingSystem, req)
+			exitCode, err := coi.HostingSystem.ExecInUVM(ctx, req)
 			if err != nil {
 				return errors.Wrapf(err, "failed to create sandbox mount directory in utility VM with exit code %d %q", exitCode, b.String())
 			}

internal/oci/uvm.go

@@ -10,6 +10,7 @@ import (
 	"strconv"
 
 	runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils"
 	"github.com/Microsoft/hcsshim/pkg/annotations"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
@@ -156,7 +157,7 @@ func handleAnnotationBootFilesPath(ctx context.Context, a map[string]string, lop
 func handleAnnotationKernelDirectBoot(ctx context.Context, a map[string]string, lopts *uvm.OptionsLCOW) {
 	lopts.KernelDirect = ParseAnnotationsBool(ctx, a, annotations.KernelDirectBoot, lopts.KernelDirect)
 	if !lopts.KernelDirect {
-		lopts.KernelFile = uvm.KernelFile
+		lopts.KernelFile = vmutils.KernelFile
 	}
 }
 
@@ -166,9 +167,9 @@ func handleAnnotationPreferredRootFSType(ctx context.Context, a map[string]strin
 	lopts.PreferredRootFSType = parseAnnotationsPreferredRootFSType(ctx, a, annotations.PreferredRootFSType, lopts.PreferredRootFSType)
 	switch lopts.PreferredRootFSType {
 	case uvm.PreferredRootFSTypeInitRd:
-		lopts.RootFSFile = uvm.InitrdFile
+		lopts.RootFSFile = vmutils.InitrdFile
 	case uvm.PreferredRootFSTypeVHD:
-		lopts.RootFSFile = uvm.VhdFile
+		lopts.RootFSFile = vmutils.VhdFile
 	}
 }
 
@@ -206,7 +207,7 @@ func handleLCOWSecurityPolicy(ctx context.Context, a map[string]string, lopts *u
 		// VPMem not supported by the enlightened kernel for SNP so set count to zero.
 		lopts.VPMemDeviceCount = 0
 		// set the default GuestState filename.
-		lopts.GuestStateFilePath = uvm.GuestStateFile
+		lopts.GuestStateFilePath = vmutils.DefaultGuestStateFile
 		lopts.KernelBootOptions = ""
 		lopts.AllowOvercommit = false
 		lopts.SecurityPolicyEnabled = true
@@ -219,7 +220,7 @@ func handleLCOWSecurityPolicy(ctx context.Context, a map[string]string, lopts *u
 		// The default behavior is to use kernel.vmgs and a rootfs-verity.vhd file with Merkle tree appended to ext4 filesystem.
 		lopts.PreferredRootFSType = uvm.PreferredRootFSTypeNA
 		lopts.RootFSFile = ""
-		lopts.DmVerityRootFsVhd = uvm.DefaultDmVerityRootfsVhd
+		lopts.DmVerityRootFsVhd = vmutils.DefaultDmVerityRootfsVhd
 		lopts.DmVerityMode = true
 	}