Add LCOW logpath within uVM

A common scenario for pods is to write container logs (`stdout` and
`stderr`) to a file on the host (specified by the `LogPath` and
`LogDirectory` CRI fields), and then mount that directory to another
(e.g., a monitoring or observability container) in the pod for
consumption.
For LCOW, this means logs must traverse from the uVM to the host, and
then back into the uVM via (plan9) share, which both:
 - adds unnecessary overhead; and
 - exposes data to the host.

Address this by adding annotations to mimic `LogPath` and `LogDirectory`
functionality, but within the uVM.
Since the uVM rootfs is not directly backed by a VHD (i.e., directories
are either RO or `tmpfs` backed), create a dedicated directory in the
sandbox scratch for logs, and allow that path to be mounted within a
container.

Note: containerd expects to be the end-all sink for container
logs, use `io.MultiWriter` to keep default behavior and write logs to
the specified path while also exposing them to the host.
Therefore, annotations use `TeeLog` (instead of just `Log`) to make
behavior explicit.

Add `LCOWTeeLogPath` annotation to tee container `stdin` and `stderr`
to a specified file within the uVM, relative to a dedicated log
directory in the sandbox scratch.

Add `LCOWTeeLogDirMount` annotation to mount the above log directory to
the specified path within the container.

Add a dedicated `trapsort.MultiWriter` type that writes to both an
underlying `transport.Connection` and the provided `io.Writer`.

Move `logConnection` from `stdio` to `transport`, to keep the
different `Connection` implementations together.

Add functional tests.

Streamline `test/internal/util` by making `CleanName` operation on the
test name directly (`testing.TB.Name()`), since that is the majority of
the usage.
Add `CleanString` to handle the original functionality.

PR: move log file creation/management to (*Container).Start

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>

Author: helsaawy

internal/guest/runtime/hcsv2/container.go

@@ -46,8 +46,11 @@ const (
 )
 
 type Container struct {
-	id    string
-	vsock transport.Transport
+	id string
+
+	vsock   transport.Transport
+	logPath string   // path to [logFile].
+	logFile *os.File // file to redirect container's stdio to.
 
 	spec          *oci.Spec
 	ociBundlePath string
@@ -76,12 +79,42 @@ type Container struct {
 	scratchDirPath string
 }
 
-func (c *Container) Start(ctx context.Context, conSettings stdio.ConnectionSettings) (int, error) {
-	log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::Start")
-	stdioSet, err := stdio.Connect(c.vsock, conSettings)
+func (c *Container) Start(ctx context.Context, conSettings stdio.ConnectionSettings) (_ int, err error) {
+	entity := log.G(ctx).WithField(logfields.ContainerID, c.id)
+	entity.Info("opengcs::Container::Start")
+
+	// only use the logfile for the init process, since we don't want to tee stdio of execs
+	t := c.vsock
+	if c.logPath != "" {
+		// don't use [os.Create] since that truncates an existing file, which is not desired
+		if c.logFile, err = os.OpenFile(c.logPath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666); err != nil {
+			return -1, fmt.Errorf("failed to open log file: %s: %w", c.logPath, err)
+		}
+		go func() {
+			// initProcess is already created in [(*Host).CreateContainer], and is, therefore, "waitable"
+			// wait in `writersWg`, which is closed after process is cleaned up (including io Relays)
+			//
+			// Note: [PipeRelay] and [TtyRelay] are not safe to call multiple times, so it is safer to wait
+			// on the parent (init) process.
+			c.initProcess.writersWg.Wait()
+
+			if lfErr := c.logFile.Close(); lfErr != nil {
+				entity.WithFields(logrus.Fields{
+					logrus.ErrorKey: lfErr,
+					logfields.Path:  c.logFile.Name(),
+				}).Warn("failed to close log file")
+			}
+			c.logFile = nil
+		}()
+
+		t = transport.NewMultiWriter(c.vsock, c.logFile)
+	}
+
+	stdioSet, err := stdio.Connect(t, conSettings)
 	if err != nil {
 		return -1, err
 	}
+
 	if c.initProcess.spec.Terminal {
 		ttyr := c.container.Tty()
 		ttyr.ReplaceConnectionSet(stdioSet)

internal/guest/runtime/hcsv2/uvm.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/Microsoft/cosesign1go/pkg/cosesign1"
 	didx509resolver "github.com/Microsoft/didx509go/pkg/did-x509-resolver"
-	"github.com/Microsoft/hcsshim/internal/bridgeutils/gcserr"
 	cgroups "github.com/containerd/cgroups/v3/cgroup1"
 	cgroup1stats "github.com/containerd/cgroups/v3/cgroup1/stats"
 	"github.com/mattn/go-shellwords"
@@ -31,6 +30,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 
+	"github.com/Microsoft/hcsshim/internal/bridgeutils/gcserr"
 	"github.com/Microsoft/hcsshim/internal/debug"
 	"github.com/Microsoft/hcsshim/internal/guest/policy"
 	"github.com/Microsoft/hcsshim/internal/guest/prot"
@@ -45,6 +45,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/guest/storage/scsi"
 	"github.com/Microsoft/hcsshim/internal/guest/transport"
 	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/internal/logfields"
 	"github.com/Microsoft/hcsshim/internal/oci"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
@@ -355,6 +356,24 @@ func setupSandboxHugePageMountsPath(id string) error {
 	return storage.MountRShared(mountPath)
 }
 
+// setupSandboxLogDir creates the directory to house all redirected stdio logs from containers.
+//
+// Virtual pod aware.
+func setupSandboxLogDir(sandboxID, virtualSandboxID string) error {
+	mountPath := specGuest.SandboxLogsDir(sandboxID, virtualSandboxID)
+	if err := mkdirAllModePerm(mountPath); err != nil {
+		id := sandboxID
+		if virtualSandboxID != "" {
+			id = virtualSandboxID
+		}
+		return errors.Wrapf(err, "failed to create sandbox logs dir in sandbox %v", id)
+	}
+	return nil
+}
+
+// TODO: unify workload and standalone logic for non-sandbox features (e.g., block devices, huge pages, uVM mounts)
+// TODO(go1.24): use [os.Root] instead of `!strings.HasPrefix(<path>, <root>)`
+
 func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VMHostedContainerSettingsV2) (_ *Container, err error) {
 	criType, isCRI := settings.OCISpecification.Annotations[annotations.KubernetesContainerType]
 
@@ -493,6 +512,9 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 					return nil, err
 				}
 			}
+			if err = setupSandboxLogDir(id, virtualPodID); err != nil {
+				return nil, err
+			}
 
 			if err := policy.ExtendPolicyWithNetworkingMounts(id, h.securityPolicyEnforcer, settings.OCISpecification); err != nil {
 				return nil, err
@@ -544,6 +566,17 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 		}
 	}
 
+	// don't specialize tee logs (both files and mounts) just for workload containers
+	// add log directory mount before enforcing (mount) policy
+	if logDirMount := settings.OCISpecification.Annotations[annotations.LCOWTeeLogDirMount]; logDirMount != "" {
+		settings.OCISpecification.Mounts = append(settings.OCISpecification.Mounts, specs.Mount{
+			Destination: logDirMount,
+			Type:        "bind",
+			Source:      specGuest.SandboxLogsDir(sandboxID, virtualPodID),
+			Options:     []string{"bind"},
+		})
+	}
+
 	user, groups, umask, err := h.securityPolicyEnforcer.GetUserInfo(settings.OCISpecification.Process, settings.OCISpecification.Root.Path)
 	if err != nil {
 		return nil, err
@@ -580,6 +613,30 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 		c.vsock = h.devNullTransport
 	}
 
+	// delay creating the directory to house the container's stdio until after we've verified
+	// policy on log settings.
+	// TODO: is using allowStdio appropriate here, since longs aren't leaving the uVM?
+	if logPath := settings.OCISpecification.Annotations[annotations.LCOWTeeLogPath]; logPath != "" {
+		if !allowStdio {
+			return nil, errors.Errorf("teeing container stdio to log path %q denied due to policy not allowing stdio access", logPath)
+		}
+
+		c.logPath = specGuest.SandboxLogPath(sandboxID, virtualPodID, logPath)
+		// verify the logpath is still under the correct directory
+		if !strings.HasPrefix(c.logPath, specGuest.SandboxLogsDir(sandboxID, virtualPodID)) {
+			return nil, errors.Errorf("log path %v is not within sandbox's log dir", c.logPath)
+		}
+
+		dir := filepath.Dir(c.logPath)
+		log.G(ctx).WithFields(logrus.Fields{
+			logfields.Path:        dir,
+			logfields.ContainerID: id,
+		}).Debug("creating container log file parent directory in uVM")
+		if err := mkdirAllModePerm(dir); err != nil {
+			return nil, errors.Wrapf(err, "failed to create log file parent directory: %s", dir)
+		}
+	}
+
 	if envToKeep != nil {
 		settings.OCISpecification.Process.Env = []string(envToKeep)
 	}

internal/guest/spec/spec.go

@@ -80,7 +80,7 @@ func SandboxRootDir(sandboxID string) string {
 }
 
 // VirtualPodRootDir returns the virtual pod root directory inside UVM/host.
-// This is used when multiple pods share a UVM via virtualSandboxID
+// This is used when multiple pods share a UVM via virtualSandboxID.
 func VirtualPodRootDir(virtualSandboxID string) string {
 	// Ensure virtualSandboxID is a relative path to prevent directory traversal
 	sanitizedID := filepath.Clean(virtualSandboxID)
@@ -91,7 +91,7 @@ func VirtualPodRootDir(virtualSandboxID string) string {
 }
 
 // VirtualPodAwareSandboxRootDir returns the appropriate root directory based on whether
-// the sandbox is part of a virtual pod or traditional single-pod setup
+// the sandbox is part of a virtual pod or traditional single-pod setup.
 func VirtualPodAwareSandboxRootDir(sandboxID, virtualSandboxID string) string {
 	if virtualSandboxID != "" {
 		return VirtualPodRootDir(virtualSandboxID)
@@ -109,7 +109,7 @@ func VirtualPodMountsDir(virtualSandboxID string) string {
 	return filepath.Join(VirtualPodRootDir(virtualSandboxID), "sandboxMounts")
 }
 
-// VirtualPodAwareSandboxMountsDir returns the appropriate mounts directory
+// VirtualPodAwareSandboxMountsDir returns the appropriate mounts directory.
 func VirtualPodAwareSandboxMountsDir(sandboxID, virtualSandboxID string) string {
 	if virtualSandboxID != "" {
 		return VirtualPodMountsDir(virtualSandboxID)
@@ -127,7 +127,7 @@ func VirtualPodTmpfsMountsDir(virtualSandboxID string) string {
 	return filepath.Join(VirtualPodRootDir(virtualSandboxID), "sandboxTmpfsMounts")
 }
 
-// VirtualPodAwareSandboxTmpfsMountsDir returns the appropriate tmpfs mounts directory
+// VirtualPodAwareSandboxTmpfsMountsDir returns the appropriate tmpfs mounts directory.
 func VirtualPodAwareSandboxTmpfsMountsDir(sandboxID, virtualSandboxID string) string {
 	if virtualSandboxID != "" {
 		return VirtualPodTmpfsMountsDir(virtualSandboxID)
@@ -140,27 +140,27 @@ func HugePagesMountsDir(sandboxID string) string {
 	return filepath.Join(SandboxRootDir(sandboxID), "hugepages")
 }
 
-// VirtualPodHugePagesMountsDir returns virtual pod hugepages mounts directory
+// VirtualPodHugePagesMountsDir returns virtual pod hugepages mounts directory.
 func VirtualPodHugePagesMountsDir(virtualSandboxID string) string {
 	return filepath.Join(VirtualPodRootDir(virtualSandboxID), "hugepages")
 }
 
-// VirtualPodAwareHugePagesMountsDir returns the appropriate hugepages directory
+// VirtualPodAwareHugePagesMountsDir returns the appropriate hugepages directory.
 func VirtualPodAwareHugePagesMountsDir(sandboxID, virtualSandboxID string) string {
 	if virtualSandboxID != "" {
 		return VirtualPodHugePagesMountsDir(virtualSandboxID)
 	}
 	return HugePagesMountsDir(sandboxID)
 }
 
-// SandboxMountSource returns sandbox mount path inside UVM
+// SandboxMountSource returns sandbox mount path inside UVM.
 func SandboxMountSource(sandboxID, path string) string {
 	mountsDir := SandboxMountsDir(sandboxID)
 	subPath := strings.TrimPrefix(path, guestpath.SandboxMountPrefix)
 	return filepath.Join(mountsDir, subPath)
 }
 
-// VirtualPodAwareSandboxMountSource returns mount source path for virtual pod aware containers
+// VirtualPodAwareSandboxMountSource returns mount source path for virtual pod aware containers.
 func VirtualPodAwareSandboxMountSource(sandboxID, virtualSandboxID, path string) string {
 	if virtualSandboxID != "" {
 		mountsDir := VirtualPodMountsDir(virtualSandboxID)
@@ -170,14 +170,14 @@ func VirtualPodAwareSandboxMountSource(sandboxID, virtualSandboxID, path string)
 	return SandboxMountSource(sandboxID, path)
 }
 
-// SandboxTmpfsMountSource returns sandbox tmpfs mount path inside UVM
+// SandboxTmpfsMountSource returns sandbox tmpfs mount path inside UVM.
 func SandboxTmpfsMountSource(sandboxID, path string) string {
 	tmpfsMountDir := SandboxTmpfsMountsDir(sandboxID)
 	subPath := strings.TrimPrefix(path, guestpath.SandboxTmpfsMountPrefix)
 	return filepath.Join(tmpfsMountDir, subPath)
 }
 
-// VirtualPodAwareSandboxTmpfsMountSource returns tmpfs mount source path for virtual pod aware containers
+// VirtualPodAwareSandboxTmpfsMountSource returns tmpfs mount source path for virtual pod aware containers.
 func VirtualPodAwareSandboxTmpfsMountSource(sandboxID, virtualSandboxID, path string) string {
 	if virtualSandboxID != "" {
 		mountsDir := VirtualPodTmpfsMountsDir(virtualSandboxID)
@@ -187,14 +187,14 @@ func VirtualPodAwareSandboxTmpfsMountSource(sandboxID, virtualSandboxID, path st
 	return SandboxTmpfsMountSource(sandboxID, path)
 }
 
-// HugePagesMountSource returns hugepages mount path inside UVM
+// HugePagesMountSource returns hugepages mount path inside UVM.
 func HugePagesMountSource(sandboxID, path string) string {
 	mountsDir := HugePagesMountsDir(sandboxID)
 	subPath := strings.TrimPrefix(path, guestpath.HugePagesMountPrefix)
 	return filepath.Join(mountsDir, subPath)
 }
 
-// VirtualPodAwareHugePagesMountSource returns hugepages mount source for virtual pod aware containers
+// VirtualPodAwareHugePagesMountSource returns hugepages mount source for virtual pod aware containers.
 func VirtualPodAwareHugePagesMountSource(sandboxID, virtualSandboxID, path string) string {
 	if virtualSandboxID != "" {
 		mountsDir := VirtualPodHugePagesMountsDir(virtualSandboxID)
@@ -204,6 +204,20 @@ func VirtualPodAwareHugePagesMountSource(sandboxID, virtualSandboxID, path strin
 	return HugePagesMountSource(sandboxID, path)
 }
 
+// SandboxLogsDir returns the logs directory inside the UVM for forwarding container stdio to.
+//
+// Virtual pod aware.
+func SandboxLogsDir(sandboxID, virtualSandboxID string) string {
+	return filepath.Join(VirtualPodAwareSandboxRootDir(sandboxID, virtualSandboxID), "logs")
+}
+
+// SandboxLogPath returns the log path inside the UVM.
+//
+// Virtual pod aware.
+func SandboxLogPath(sandboxID, virtualSandboxID, path string) string {
+	return filepath.Join(SandboxLogsDir(sandboxID, virtualSandboxID), path)
+}
+
 // GetNetworkNamespaceID returns the `ToLower` of
 // `spec.Windows.Network.NetworkNamespace` or `""`.
 func GetNetworkNamespaceID(spec *oci.Spec) string {

internal/guest/stdio/connection.go

@@ -4,11 +4,9 @@
 package stdio
 
 import (
-	"os"
+	"github.com/pkg/errors"
 
 	"github.com/Microsoft/hcsshim/internal/guest/transport"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 // ConnectionSettings describe the stdin, stdout, stderr ports to connect the
@@ -19,49 +17,6 @@ type ConnectionSettings struct {
 	StdErr *uint32
 }
 
-type logConnection struct {
-	con  transport.Connection
-	port uint32
-}
-
-func (lc *logConnection) Read(b []byte) (int, error) {
-	return lc.con.Read(b)
-}
-
-func (lc *logConnection) Write(b []byte) (int, error) {
-	return lc.con.Write(b)
-}
-
-func (lc *logConnection) Close() error {
-	logrus.WithFields(logrus.Fields{
-		"port": lc.port,
-	}).Debug("opengcs::logConnection::Close - closing connection")
-
-	return lc.con.Close()
-}
-
-func (lc *logConnection) CloseRead() error {
-	logrus.WithFields(logrus.Fields{
-		"port": lc.port,
-	}).Debug("opengcs::logConnection::Close - closing read connection")
-
-	return lc.con.CloseRead()
-}
-
-func (lc *logConnection) CloseWrite() error {
-	logrus.WithFields(logrus.Fields{
-		"port": lc.port,
-	}).Debug("opengcs::logConnection::Close - closing write connection")
-
-	return lc.con.CloseWrite()
-}
-
-func (lc *logConnection) File() (*os.File, error) {
-	return lc.con.File()
-}
-
-var _ = (transport.Connection)(&logConnection{})
-
 // Connect returns new transport.Connection instances, one for each stdio pipe
 // to be used. If CreateStd*Pipe for a given pipe is false, the given Connection
 // is set to nil.
@@ -77,30 +32,21 @@ func Connect(tport transport.Transport, settings ConnectionSettings) (_ *Connect
 		if err != nil {
 			return nil, errors.Wrap(err, "failed creating stdin Connection")
 		}
-		connSet.In = &logConnection{
-			con:  c,
-			port: *settings.StdIn,
-		}
+		connSet.In = transport.NewLogConnection(c, *settings.StdIn)
 	}
 	if settings.StdOut != nil {
 		c, err := tport.Dial(*settings.StdOut)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed creating stdout Connection")
 		}
-		connSet.Out = &logConnection{
-			con:  c,
-			port: *settings.StdOut,
-		}
+		connSet.Out = transport.NewLogConnection(c, *settings.StdOut)
 	}
 	if settings.StdErr != nil {
 		c, err := tport.Dial(*settings.StdErr)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed creating stderr Connection")
 		}
-		connSet.Err = &logConnection{
-			con:  c,
-			port: *settings.StdErr,
-		}
+		connSet.Err = transport.NewLogConnection(c, *settings.StdErr)
 	}
 	return connSet, nil
 }