diff --git a/cmd/gcs/main.go b/cmd/gcs/main.go index e45fa55d03..68ab28d499 100644 --- a/cmd/gcs/main.go +++ b/cmd/gcs/main.go @@ -27,7 +27,6 @@ import ( "github.com/Microsoft/hcsshim/internal/guest/runtime/hcsv2" "github.com/Microsoft/hcsshim/internal/guest/runtime/runc" "github.com/Microsoft/hcsshim/internal/guest/transport" - "github.com/Microsoft/hcsshim/internal/guestpath" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/internal/version" @@ -297,8 +296,6 @@ func main() { log.SetScrubbing(*scrubLogs) - baseLogPath := guestpath.LCOWRootPrefixInUVM - logrus.WithFields(logrus.Fields{ "branch": version.Branch, "commit": version.Commit, @@ -384,7 +381,7 @@ func main() { } tport := &transport.VsockTransport{} - rtime, err := runc.NewRuntime(baseLogPath) + rtime, err := runc.NewRuntime() if err != nil { logrus.WithError(err).Fatal("failed to initialize new runc runtime") } diff --git a/internal/guest/runtime/hcsv2/container.go b/internal/guest/runtime/hcsv2/container.go index 6c4be709ca..0ce930dd65 100644 --- a/internal/guest/runtime/hcsv2/container.go +++ b/internal/guest/runtime/hcsv2/container.go @@ -29,7 +29,6 @@ import ( "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" "github.com/Microsoft/hcsshim/internal/protocol/guestresource" - "github.com/Microsoft/hcsshim/pkg/annotations" ) // containerStatus has been introduced to enable parallel container creation @@ -76,6 +75,10 @@ type Container struct { // of this container is located. Usually, this is either `/run/gcs/c/` or // `/run/gcs/c//container_` if scratch is shared with UVM scratch. scratchDirPath string + + // sandboxRoot is the root directory of the pod within the guest. + // Used during cleanup to unmount sandbox-specific paths. + sandboxRoot string } func (c *Container) Start(ctx context.Context, conSettings stdio.ConnectionSettings) (_ int, err error) { @@ -228,25 +231,19 @@ func (c *Container) Kill(ctx context.Context, signal syscall.Signal) error { func (c *Container) Delete(ctx context.Context) error { entity := log.G(ctx).WithField(logfields.ContainerID, c.id) entity.Info("opengcs::Container::Delete") - if c.isSandbox { - // Check if this is a virtual pod - virtualSandboxID := "" - if c.spec != nil && c.spec.Annotations != nil { - virtualSandboxID = c.spec.Annotations[annotations.VirtualPodID] - } - - // remove user mounts in sandbox container - use virtual pod aware paths - if err := storage.UnmountAllInPath(ctx, specGuest.VirtualPodAwareSandboxMountsDir(c.id, virtualSandboxID), true); err != nil { + if c.isSandbox && c.sandboxRoot != "" { + // remove user mounts in sandbox container + if err := storage.UnmountAllInPath(ctx, specGuest.SandboxMountsDirFromRoot(c.sandboxRoot), true); err != nil { entity.WithError(err).Error("failed to unmount sandbox mounts") } - // remove user mounts in tmpfs sandbox container - use virtual pod aware paths - if err := storage.UnmountAllInPath(ctx, specGuest.VirtualPodAwareSandboxTmpfsMountsDir(c.id, virtualSandboxID), true); err != nil { + // remove tmpfs mounts in sandbox container + if err := storage.UnmountAllInPath(ctx, specGuest.SandboxTmpfsMountsDirFromRoot(c.sandboxRoot), true); err != nil { entity.WithError(err).Error("failed to unmount tmpfs sandbox mounts") } - // remove hugepages mounts in sandbox container - use virtual pod aware paths - if err := storage.UnmountAllInPath(ctx, specGuest.VirtualPodAwareHugePagesMountsDir(c.id, virtualSandboxID), true); err != nil { + // remove hugepages mounts in sandbox container + if err := storage.UnmountAllInPath(ctx, specGuest.SandboxHugePagesMountsDirFromRoot(c.sandboxRoot), true); err != nil { entity.WithError(err).Error("failed to unmount hugepages mounts") } } diff --git a/internal/guest/runtime/hcsv2/sandbox_container.go b/internal/guest/runtime/hcsv2/sandbox_container.go index 471a28bfe1..12590f4dc0 100644 --- a/internal/guest/runtime/hcsv2/sandbox_container.go +++ b/internal/guest/runtime/hcsv2/sandbox_container.go @@ -20,35 +20,30 @@ import ( "github.com/Microsoft/hcsshim/pkg/annotations" ) -func getSandboxHostnamePath(id, virtualSandboxID string) string { - return filepath.Join(specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID), "hostname") +func getSandboxHostnamePath(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "hostname") } -func getSandboxHostsPath(id, virtualSandboxID string) string { - return filepath.Join(specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID), "hosts") +func getSandboxHostsPath(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "hosts") } -func getSandboxResolvPath(id, virtualSandboxID string) string { - return filepath.Join(specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID), "resolv.conf") +func getSandboxResolvPath(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "resolv.conf") } -func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (err error) { +func setupSandboxContainerSpec(ctx context.Context, id, sandboxRoot string, spec *oci.Spec) (err error) { ctx, span := oc.StartSpan(ctx, "hcsv2::setupSandboxContainerSpec") defer span.End() defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", id)) - // Check if this is a virtual pod to use appropriate root directory - virtualSandboxID := spec.Annotations[annotations.VirtualPodID] - - // Generate the sandbox root dir - virtual pod aware - rootDir := specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID) - if err := os.MkdirAll(rootDir, 0755); err != nil { - return errors.Wrapf(err, "failed to create sandbox root directory %q", rootDir) + if err := os.MkdirAll(sandboxRoot, 0755); err != nil { + return errors.Wrapf(err, "failed to create sandbox root directory %q", sandboxRoot) } defer func() { if err != nil { - _ = os.RemoveAll(rootDir) + _ = os.RemoveAll(sandboxRoot) } }() @@ -62,19 +57,20 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) ( } } - sandboxHostnamePath := getSandboxHostnamePath(id, virtualSandboxID) + sandboxHostnamePath := getSandboxHostnamePath(sandboxRoot) if err := os.WriteFile(sandboxHostnamePath, []byte(hostname+"\n"), 0644); err != nil { return errors.Wrapf(err, "failed to write hostname to %q", sandboxHostnamePath) } // Write the hosts sandboxHostsContent := network.GenerateEtcHostsContent(ctx, hostname) - sandboxHostsPath := getSandboxHostsPath(id, virtualSandboxID) + sandboxHostsPath := getSandboxHostsPath(sandboxRoot) if err := os.WriteFile(sandboxHostsPath, []byte(sandboxHostsContent), 0644); err != nil { return errors.Wrapf(err, "failed to write sandbox hosts to %q", sandboxHostsPath) } // Check if this is a virtual pod sandbox container by comparing container ID with virtual pod ID + virtualSandboxID := spec.Annotations[annotations.VirtualPodID] isVirtualPodSandbox := virtualSandboxID != "" && id == virtualSandboxID if strings.EqualFold(spec.Annotations[annotations.SkipPodNetworking], "true") || isVirtualPodSandbox { ns := GetOrAddNetworkNamespace(specGuest.GetNetworkNamespaceID(spec)) @@ -97,7 +93,7 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) ( if err != nil { return errors.Wrap(err, "failed to generate sandbox resolv.conf content") } - sandboxResolvPath := getSandboxResolvPath(id, virtualSandboxID) + sandboxResolvPath := getSandboxResolvPath(sandboxRoot) if err := os.WriteFile(sandboxResolvPath, []byte(resolvContent), 0644); err != nil { return errors.Wrap(err, "failed to write sandbox resolv.conf") } diff --git a/internal/guest/runtime/hcsv2/sandbox_root_test.go b/internal/guest/runtime/hcsv2/sandbox_root_test.go new file mode 100644 index 0000000000..0c22321d74 --- /dev/null +++ b/internal/guest/runtime/hcsv2/sandbox_root_test.go @@ -0,0 +1,420 @@ +//go:build linux +// +build linux + +package hcsv2 + +import ( + "path/filepath" + + "testing" + + specGuest "github.com/Microsoft/hcsshim/internal/guest/spec" +) + +func TestRegisterAndResolveSandboxRoot(t *testing.T) { + h := &Host{ + sandboxRoots: make(map[string]string), + } + + if _, err := h.registerSandboxRoot("sandbox-1", "/run/gcs/c/sandbox-1", ""); err != nil { + t.Fatal(err) + } + got := h.resolveSandboxRoot("sandbox-1") + if got != "/run/gcs/c/sandbox-1" { + t.Fatalf("expected /run/gcs/c/sandbox-1, got %s", got) + } +} + +func TestResolveSandboxRootFallback(t *testing.T) { + h := &Host{ + sandboxRoots: make(map[string]string), + } + + // No registration — should fall back to legacy derivation + got := h.resolveSandboxRoot("sandbox-missing") + want := specGuest.SandboxRootDir("sandbox-missing") + if got != want { + t.Fatalf("expected fallback %s, got %s", want, got) + } +} + +func TestUnregisterSandboxRoot(t *testing.T) { + h := &Host{ + sandboxRoots: make(map[string]string), + } + + if _, err := h.registerSandboxRoot("sandbox-1", "/some/path", ""); err != nil { + t.Fatal(err) + } + h.unregisterSandboxRoot("sandbox-1") + + // After unregister, should fall back to legacy + got := h.resolveSandboxRoot("sandbox-1") + want := specGuest.SandboxRootDir("sandbox-1") + if got != want { + t.Fatalf("expected fallback %s after unregister, got %s", want, got) + } +} + +func TestSandboxRootFromOCIBundlePath(t *testing.T) { + h := &Host{sandboxRoots: make(map[string]string)} + ociBundlePath := "/run/gcs/c/my-sandbox-id" + + got, err := h.registerSandboxRoot("my-sandbox-id", ociBundlePath, "") + if err != nil { + t.Fatal(err) + } + if got != ociBundlePath { + t.Fatalf("expected sandbox root %q, got %q", ociBundlePath, got) + } +} + +func TestVirtualPodRootFromOCIBundlePath(t *testing.T) { + tests := []struct { + name string + ociBundlePath string + virtualPodID string + want string + }{ + { + name: "legacy shim path", + ociBundlePath: "/run/gcs/c/container-abc", + virtualPodID: "vpod-123", + want: "/run/gcs/c/virtual-pods/vpod-123", + }, + { + name: "new shim path", + ociBundlePath: "/new/layout/sandboxes/container-abc", + virtualPodID: "vpod-456", + want: "/new/layout/sandboxes/virtual-pods/vpod-456", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filepath.Join(filepath.Dir(tt.ociBundlePath), "virtual-pods", tt.virtualPodID) + if got != tt.want { + t.Fatalf("got %s, want %s", got, tt.want) + } + }) + } +} + +func TestVirtualPodRootMatchesLegacy(t *testing.T) { + // When OCIBundlePath uses the legacy prefix, the derived virtual pod root + // must match what VirtualPodRootDir() would have produced. + ociBundlePath := "/run/gcs/c/container-id" + virtualPodID := "vpod-abc" + + derived := filepath.Join(filepath.Dir(ociBundlePath), "virtual-pods", virtualPodID) + legacy := specGuest.VirtualPodRootDir(virtualPodID) + + if derived != legacy { + t.Fatalf("derived %q != legacy %q — backwards compatibility broken", derived, legacy) + } +} + +func TestSubdirectoryPaths(t *testing.T) { + sandboxRoot := "/run/gcs/c/sandbox-xyz" + + // Verify FromRoot helpers match legacy spec.go functions + if specGuest.SandboxMountsDirFromRoot(sandboxRoot) != specGuest.SandboxMountsDir("sandbox-xyz") { + t.Fatal("SandboxMountsDirFromRoot doesn't match legacy SandboxMountsDir") + } + if specGuest.SandboxTmpfsMountsDirFromRoot(sandboxRoot) != specGuest.SandboxTmpfsMountsDir("sandbox-xyz") { + t.Fatal("SandboxTmpfsMountsDirFromRoot doesn't match legacy SandboxTmpfsMountsDir") + } + if specGuest.SandboxHugePagesMountsDirFromRoot(sandboxRoot) != specGuest.HugePagesMountsDir("sandbox-xyz") { + t.Fatal("SandboxHugePagesMountsDirFromRoot doesn't match legacy HugePagesMountsDir") + } + if specGuest.SandboxLogsDirFromRoot(sandboxRoot) != specGuest.SandboxLogsDir("sandbox-xyz", "") { + t.Fatal("SandboxLogsDirFromRoot doesn't match legacy SandboxLogsDir") + } +} + +// TestOldVsNewPathParity exhaustively compares every path the old code +// would have produced (via spec.go functions with hard-coded prefix + ID) +// against the new code (resolved sandboxRoot + inline filepath.Join). +// If any pair diverges, backwards compatibility is broken. +func TestOldVsNewPathParity(t *testing.T) { + type pathCase struct { + name string + oldPath string // what the old code produced + newPath string // what the new code produces + } + + // Simulate old shim: OCIBundlePath = /run/gcs/c/ + sandboxID := "abc-123-sandbox" + ociBundlePath := "/run/gcs/c/" + sandboxID + sandboxRoot := ociBundlePath // new code: sandboxRoot = OCIBundlePath + + regularCases := []pathCase{ + { + name: "sandbox root", + oldPath: specGuest.SandboxRootDir(sandboxID), + newPath: sandboxRoot, + }, + { + name: "sandboxMounts dir", + oldPath: specGuest.SandboxMountsDir(sandboxID), + newPath: filepath.Join(sandboxRoot, "sandboxMounts"), + }, + { + name: "sandboxTmpfsMounts dir", + oldPath: specGuest.SandboxTmpfsMountsDir(sandboxID), + newPath: filepath.Join(sandboxRoot, "sandboxTmpfsMounts"), + }, + { + name: "hugepages dir", + oldPath: specGuest.HugePagesMountsDir(sandboxID), + newPath: filepath.Join(sandboxRoot, "hugepages"), + }, + { + name: "logs dir", + oldPath: specGuest.SandboxLogsDir(sandboxID, ""), + newPath: filepath.Join(sandboxRoot, "logs"), + }, + { + name: "log file path", + oldPath: specGuest.SandboxLogPath(sandboxID, "", "container.log"), + newPath: filepath.Join(sandboxRoot, "logs", "container.log"), + }, + { + name: "hostname file", + oldPath: filepath.Join(specGuest.SandboxRootDir(sandboxID), "hostname"), + newPath: filepath.Join(sandboxRoot, "hostname"), + }, + { + name: "hosts file", + oldPath: filepath.Join(specGuest.SandboxRootDir(sandboxID), "hosts"), + newPath: filepath.Join(sandboxRoot, "hosts"), + }, + { + name: "resolv.conf file", + oldPath: filepath.Join(specGuest.SandboxRootDir(sandboxID), "resolv.conf"), + newPath: filepath.Join(sandboxRoot, "resolv.conf"), + }, + } + + t.Run("regular_sandbox", func(t *testing.T) { + for _, tc := range regularCases { + if tc.oldPath != tc.newPath { + t.Errorf("%s: old=%q new=%q", tc.name, tc.oldPath, tc.newPath) + } + } + }) + + // Virtual pod: old code used VirtualPodRootDir(vpodID), + // new code uses filepath.Join(filepath.Dir(ociBundlePath), "virtual-pods", vpodID) + vpodID := "vpod-456" + vpodOCIBundlePath := "/run/gcs/c/" + sandboxID // container's own bundle + vpodSandboxRoot := filepath.Join(filepath.Dir(vpodOCIBundlePath), "virtual-pods", vpodID) + + vpodCases := []pathCase{ + { + name: "virtual pod root", + oldPath: specGuest.VirtualPodRootDir(vpodID), + newPath: vpodSandboxRoot, + }, + { + name: "virtual pod sandboxMounts", + oldPath: specGuest.VirtualPodMountsDir(vpodID), + newPath: filepath.Join(vpodSandboxRoot, "sandboxMounts"), + }, + { + name: "virtual pod tmpfs mounts", + oldPath: specGuest.VirtualPodTmpfsMountsDir(vpodID), + newPath: filepath.Join(vpodSandboxRoot, "sandboxTmpfsMounts"), + }, + { + name: "virtual pod hugepages", + oldPath: specGuest.VirtualPodHugePagesMountsDir(vpodID), + newPath: filepath.Join(vpodSandboxRoot, "hugepages"), + }, + { + name: "virtual pod logs", + oldPath: specGuest.SandboxLogsDir(sandboxID, vpodID), + newPath: filepath.Join(vpodSandboxRoot, "logs"), + }, + { + name: "virtual pod hostname", + oldPath: filepath.Join(specGuest.VirtualPodRootDir(vpodID), "hostname"), + newPath: filepath.Join(vpodSandboxRoot, "hostname"), + }, + { + name: "virtual pod hosts", + oldPath: filepath.Join(specGuest.VirtualPodRootDir(vpodID), "hosts"), + newPath: filepath.Join(vpodSandboxRoot, "hosts"), + }, + { + name: "virtual pod resolv.conf", + oldPath: filepath.Join(specGuest.VirtualPodRootDir(vpodID), "resolv.conf"), + newPath: filepath.Join(vpodSandboxRoot, "resolv.conf"), + }, + } + + t.Run("virtual_pod", func(t *testing.T) { + for _, tc := range vpodCases { + if tc.oldPath != tc.newPath { + t.Errorf("%s: old=%q new=%q", tc.name, tc.oldPath, tc.newPath) + } + } + }) + + // Workload container: sandbox root comes from mapping, not OCIBundlePath + t.Run("workload_uses_sandbox_root", func(t *testing.T) { + h := &Host{sandboxRoots: make(map[string]string)} + if _, err := h.registerSandboxRoot(sandboxID, ociBundlePath, ""); err != nil { + t.Fatal(err) + } + + workloadSandboxRoot := h.resolveSandboxRoot(sandboxID) + if workloadSandboxRoot != ociBundlePath { + t.Fatalf("workload got sandboxRoot=%q, want %q", workloadSandboxRoot, ociBundlePath) + } + // Networking mount: hostname from sandbox root, not workload's own bundle + workloadBundle := "/run/gcs/c/workload-container-999" + hostsOld := filepath.Join(specGuest.SandboxRootDir(sandboxID), "hosts") + hostsNew := filepath.Join(workloadSandboxRoot, "hosts") + if hostsOld != hostsNew { + t.Errorf("workload hosts mount: old=%q new=%q", hostsOld, hostsNew) + } + // Verify it's NOT derived from workload's own bundle + if hostsNew == filepath.Join(workloadBundle, "hosts") { + t.Error("workload hosts incorrectly derived from its own bundle path") + } + }) + + // Standalone: sandboxRoot = OCIBundlePath directly + t.Run("standalone", func(t *testing.T) { + standaloneBundle := "/run/gcs/c/standalone-789" + standaloneRoot := standaloneBundle // new code: sandboxRoot = OCIBundlePath + oldRoot := specGuest.SandboxRootDir("standalone-789") + + if standaloneRoot != oldRoot { + t.Errorf("standalone root: old=%q new=%q", oldRoot, standaloneRoot) + } + }) +} + +// TestMultiPodIsolation simulates two virtual pods sharing a UVM and verifies +// that each gets its own isolated sandbox root, mounts, and networking paths. +func TestMultiPodIsolation(t *testing.T) { + h := &Host{sandboxRoots: make(map[string]string)} + + // Simulate two virtual pod sandboxes created in the same UVM. + // Each has its own OCIBundlePath and virtual pod ID. + pod1BundlePath := "/run/gcs/c/sandbox-pod1" + pod1VPodID := "vpod-aaa" + pod1Root, err := h.registerSandboxRoot("sandbox-pod1", pod1BundlePath, pod1VPodID) + if err != nil { + t.Fatal(err) + } + + pod2BundlePath := "/run/gcs/c/sandbox-pod2" + pod2VPodID := "vpod-bbb" + pod2Root, err := h.registerSandboxRoot("sandbox-pod2", pod2BundlePath, pod2VPodID) + if err != nil { + t.Fatal(err) + } + + // Verify roots are distinct + if pod1Root == pod2Root { + t.Fatalf("pod roots must be different: both are %q", pod1Root) + } + + // Verify each resolves correctly + if got := h.resolveSandboxRoot("sandbox-pod1"); got != pod1Root { + t.Errorf("pod1: got %q, want %q", got, pod1Root) + } + if got := h.resolveSandboxRoot("sandbox-pod2"); got != pod2Root { + t.Errorf("pod2: got %q, want %q", got, pod2Root) + } + + // Verify subdirectories are isolated + pod1Hosts := filepath.Join(pod1Root, "hosts") + pod2Hosts := filepath.Join(pod2Root, "hosts") + if pod1Hosts == pod2Hosts { + t.Error("hosts files should be in different directories for different pods") + } + + pod1Mounts := filepath.Join(pod1Root, "sandboxMounts") + pod2Mounts := filepath.Join(pod2Root, "sandboxMounts") + if pod1Mounts == pod2Mounts { + t.Error("sandboxMounts should be in different directories for different pods") + } + + // A workload referencing pod1 should get pod1's root, not pod2's + workloadRoot := h.resolveSandboxRoot("sandbox-pod1") + if workloadRoot != pod1Root { + t.Errorf("workload resolved to %q, want pod1 root %q", workloadRoot, pod1Root) + } + workloadHosts := filepath.Join(workloadRoot, "hosts") + if workloadHosts != pod1Hosts { + t.Errorf("workload hosts %q should match pod1 hosts %q", workloadHosts, pod1Hosts) + } + + // Unregister pod1, pod2 still works + h.unregisterSandboxRoot("sandbox-pod1") + if got := h.resolveSandboxRoot("sandbox-pod2"); got != pod2Root { + t.Errorf("pod2 after pod1 removal: got %q, want %q", got, pod2Root) + } + + // pod1 now falls back to legacy + fallback := h.resolveSandboxRoot("sandbox-pod1") + legacy := specGuest.SandboxRootDir("sandbox-pod1") + if fallback != legacy { + t.Errorf("pod1 fallback: got %q, want legacy %q", fallback, legacy) + } +} + +// TestV2ShimPathLayout verifies that when the V2 shim sends an OCIBundlePath +// with a different prefix (e.g., /run/gcs/pods/...), the sandbox root and all +// subdirectories follow that path instead of the legacy /run/gcs/c/ prefix. +func TestV2ShimPathLayout(t *testing.T) { + h := &Host{sandboxRoots: make(map[string]string)} + + // V2 shim sends a path under /run/gcs/pods/ instead of /run/gcs/c/ + v2BundlePath := "/run/gcs/pods/sandbox-abc/sandbox-abc" + sandboxRoot, err := h.registerSandboxRoot("sandbox-abc", v2BundlePath, "") + if err != nil { + t.Fatal(err) + } + + // Sandbox root should be the V2 path, NOT /run/gcs/c/sandbox-abc + if sandboxRoot != v2BundlePath { + t.Fatalf("sandbox root = %q, want %q", sandboxRoot, v2BundlePath) + } + if sandboxRoot == specGuest.SandboxRootDir("sandbox-abc") { + t.Fatal("sandbox root should NOT match legacy SandboxRootDir for V2 paths") + } + + // Subdirectories should be under the V2 path + checks := map[string]string{ + "sandboxMounts": filepath.Join(v2BundlePath, "sandboxMounts"), + "sandboxTmpfsMounts": filepath.Join(v2BundlePath, "sandboxTmpfsMounts"), + "hugepages": filepath.Join(v2BundlePath, "hugepages"), + "logs": filepath.Join(v2BundlePath, "logs"), + "hostname": filepath.Join(v2BundlePath, "hostname"), + "hosts": filepath.Join(v2BundlePath, "hosts"), + "resolv.conf": filepath.Join(v2BundlePath, "resolv.conf"), + } + for name, want := range checks { + got := filepath.Join(sandboxRoot, name) + if got != want { + t.Errorf("%s: got %q, want %q", name, got, want) + } + } + + // Workload container should resolve to the same V2 sandbox root + workloadRoot := h.resolveSandboxRoot("sandbox-abc") + if workloadRoot != v2BundlePath { + t.Errorf("workload sandbox root = %q, want %q", workloadRoot, v2BundlePath) + } + + // Workload networking mounts should come from V2 path + workloadHosts := filepath.Join(workloadRoot, "hosts") + if workloadHosts != filepath.Join(v2BundlePath, "hosts") { + t.Errorf("workload hosts = %q, want from V2 path", workloadHosts) + } +} diff --git a/internal/guest/runtime/hcsv2/standalone_container.go b/internal/guest/runtime/hcsv2/standalone_container.go index 1a23d276fc..768adda344 100644 --- a/internal/guest/runtime/hcsv2/standalone_container.go +++ b/internal/guest/runtime/hcsv2/standalone_container.go @@ -14,42 +14,28 @@ import ( "github.com/Microsoft/hcsshim/internal/guest/network" specGuest "github.com/Microsoft/hcsshim/internal/guest/spec" - "github.com/Microsoft/hcsshim/internal/guestpath" "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/pkg/annotations" ) -func getStandaloneRootDir(id, virtualSandboxID string) string { - if virtualSandboxID != "" { - // Standalone container in virtual pod gets its own subdir - return filepath.Join(guestpath.LCOWRootPrefixInUVM, "virtual-pods", virtualSandboxID, id) - } - return filepath.Join(guestpath.LCOWRootPrefixInUVM, id) -} - -func getStandaloneHostnamePath(id, virtualSandboxID string) string { - return filepath.Join(getStandaloneRootDir(id, virtualSandboxID), "hostname") +func getStandaloneHostnamePath(rootDir string) string { + return filepath.Join(rootDir, "hostname") } -func getStandaloneHostsPath(id, virtualSandboxID string) string { - return filepath.Join(getStandaloneRootDir(id, virtualSandboxID), "hosts") +func getStandaloneHostsPath(rootDir string) string { + return filepath.Join(rootDir, "hosts") } -func getStandaloneResolvPath(id, virtualSandboxID string) string { - return filepath.Join(getStandaloneRootDir(id, virtualSandboxID), "resolv.conf") +func getStandaloneResolvPath(rootDir string) string { + return filepath.Join(rootDir, "resolv.conf") } -func setupStandaloneContainerSpec(ctx context.Context, id string, spec *oci.Spec) (err error) { +func setupStandaloneContainerSpec(ctx context.Context, id, rootDir string, spec *oci.Spec) (err error) { ctx, span := oc.StartSpan(ctx, "hcsv2::setupStandaloneContainerSpec") defer span.End() defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", id)) - // Check if this is a virtual pod (unlikely for standalone) - virtualSandboxID := spec.Annotations[annotations.VirtualPodID] - - // Generate the standalone root dir - virtual pod aware - rootDir := getStandaloneRootDir(id, virtualSandboxID) if err := os.MkdirAll(rootDir, 0755); err != nil { return errors.Wrapf(err, "failed to create container root directory %q", rootDir) } @@ -70,15 +56,15 @@ func setupStandaloneContainerSpec(ctx context.Context, id string, spec *oci.Spec // Write the hostname if !specGuest.MountPresent("/etc/hostname", spec.Mounts) { - standaloneHostnamePath := getStandaloneHostnamePath(id, virtualSandboxID) - if err := os.WriteFile(standaloneHostnamePath, []byte(hostname+"\n"), 0644); err != nil { - return errors.Wrapf(err, "failed to write hostname to %q", standaloneHostnamePath) + hostnamePath := getStandaloneHostnamePath(rootDir) + if err := os.WriteFile(hostnamePath, []byte(hostname+"\n"), 0644); err != nil { + return errors.Wrapf(err, "failed to write hostname to %q", hostnamePath) } mt := oci.Mount{ Destination: "/etc/hostname", Type: "bind", - Source: getStandaloneHostnamePath(id, virtualSandboxID), + Source: hostnamePath, Options: []string{"bind"}, } if specGuest.IsRootReadonly(spec) { @@ -89,16 +75,16 @@ func setupStandaloneContainerSpec(ctx context.Context, id string, spec *oci.Spec // Write the hosts if !specGuest.MountPresent("/etc/hosts", spec.Mounts) { - standaloneHostsContent := network.GenerateEtcHostsContent(ctx, hostname) - standaloneHostsPath := getStandaloneHostsPath(id, virtualSandboxID) - if err := os.WriteFile(standaloneHostsPath, []byte(standaloneHostsContent), 0644); err != nil { - return errors.Wrapf(err, "failed to write standalone hosts to %q", standaloneHostsPath) + hostsContent := network.GenerateEtcHostsContent(ctx, hostname) + hostsPath := getStandaloneHostsPath(rootDir) + if err := os.WriteFile(hostsPath, []byte(hostsContent), 0644); err != nil { + return errors.Wrapf(err, "failed to write standalone hosts to %q", hostsPath) } mt := oci.Mount{ Destination: "/etc/hosts", Type: "bind", - Source: getStandaloneHostsPath(id, virtualSandboxID), + Source: hostsPath, Options: []string{"bind"}, } if specGuest.IsRootReadonly(spec) { @@ -115,15 +101,15 @@ func setupStandaloneContainerSpec(ctx context.Context, id string, spec *oci.Spec if err != nil { return errors.Wrap(err, "failed to generate standalone resolv.conf content") } - standaloneResolvPath := getStandaloneResolvPath(id, virtualSandboxID) - if err := os.WriteFile(standaloneResolvPath, []byte(resolvContent), 0644); err != nil { + resolvPath := getStandaloneResolvPath(rootDir) + if err := os.WriteFile(resolvPath, []byte(resolvContent), 0644); err != nil { return errors.Wrap(err, "failed to write standalone resolv.conf") } mt := oci.Mount{ Destination: "/etc/resolv.conf", Type: "bind", - Source: getStandaloneResolvPath(id, virtualSandboxID), + Source: resolvPath, Options: []string{"bind"}, } if specGuest.IsRootReadonly(spec) { @@ -132,13 +118,11 @@ func setupStandaloneContainerSpec(ctx context.Context, id string, spec *oci.Spec spec.Mounts = append(spec.Mounts, mt) } - // Set cgroup path - check if this is part of a virtual pod (unlikely for standalone) + // Set cgroup path + virtualSandboxID := spec.Annotations[annotations.VirtualPodID] if virtualSandboxID != "" { - // Standalone container in virtual pod goes under /containers/virtual-pods/{virtualSandboxID}/{containerID} - // Each virtualSandboxID creates its own pod-level cgroup for all containers in that virtual pod spec.Linux.CgroupsPath = "/containers/virtual-pods/" + virtualSandboxID + "/" + id } else { - // Traditional standalone container goes under /containers spec.Linux.CgroupsPath = "/containers/" + id } diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index 0efde16561..bbec0b7564 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -99,6 +99,13 @@ type Host struct { containerToVirtualPod map[string]string // containerID -> virtualSandboxID virtualPodsCgroupManager cgroup.Manager // Parent cgroup for all virtual pods + // sandboxRoots maps sandboxID to the resolved sandbox root directory. + // Populated via registerSandboxRoot during sandbox creation using + // the host-provided OCIBundlePath as source of truth. + // Lock ordering: containersMutex -> sandboxRootsMutex (never reverse). + sandboxRootsMutex sync.RWMutex + sandboxRoots map[string]string + rtime runtime.Runtime vsock transport.Transport devNullTransport transport.Transport @@ -123,6 +130,7 @@ func NewHost(rtime runtime.Runtime, vsock transport.Transport, initialEnforcer s externalProcesses: make(map[int]*externalProcess), virtualPods: make(map[string]*VirtualPod), containerToVirtualPod: make(map[string]string), + sandboxRoots: make(map[string]string), rtime: rtime, vsock: vsock, devNullTransport: &transport.DevNullTransport{}, @@ -131,6 +139,60 @@ func NewHost(rtime runtime.Runtime, vsock transport.Transport, initialEnforcer s } } +// registerSandboxRoot stores the resolved sandbox root directory for a given sandbox ID. +// For virtual pods, it derives the shared root from OCIBundlePath's parent directory. +func (h *Host) registerSandboxRoot(sandboxID, ociBundlePath, virtualPodID string) (string, error) { + var sandboxRoot string + + if virtualPodID != "" { + // Validate virtualPodID to prevent path traversal. + cleanID := filepath.Clean(virtualPodID) + if filepath.IsAbs(cleanID) || strings.Contains(cleanID, "..") { + return "", errors.Errorf("invalid virtual pod ID %q: path traversal attempt", virtualPodID) + } + sandboxRoot = filepath.Join(filepath.Dir(ociBundlePath), "virtual-pods", cleanID) + } else { + sandboxRoot = ociBundlePath + } + + h.sandboxRootsMutex.Lock() + defer h.sandboxRootsMutex.Unlock() + h.sandboxRoots[sandboxID] = sandboxRoot + + logrus.WithFields(logrus.Fields{ + "sandboxID": sandboxID, + "sandboxRoot": sandboxRoot, + }).Debug("registered sandbox root") + + return sandboxRoot, nil +} + +// resolveSandboxRoot returns the resolved sandbox root for the given sandbox ID. +// Falls back to legacy path derivation if no mapping exists. +func (h *Host) resolveSandboxRoot(sandboxID string) string { + h.sandboxRootsMutex.RLock() + root, ok := h.sandboxRoots[sandboxID] + h.sandboxRootsMutex.RUnlock() + if ok { + return root + } + // Fallback to legacy derivation for backwards compatibility. + // TODO: remove fallback after shim v1 sunset + fallback := specGuest.SandboxRootDir(sandboxID) + logrus.WithFields(logrus.Fields{ + "sandboxID": sandboxID, + "fallback": fallback, + }).Warn("sandbox root not found in mapping, falling back to legacy path derivation") + return fallback +} + +// unregisterSandboxRoot removes the sandbox root mapping for a given sandbox ID. +func (h *Host) unregisterSandboxRoot(sandboxID string) { + h.sandboxRootsMutex.Lock() + defer h.sandboxRootsMutex.Unlock() + delete(h.sandboxRoots, sandboxID) +} + func (h *Host) SecurityPolicyEnforcer() securitypolicy.SecurityPolicyEnforcer { return h.securityOptions.PolicyEnforcer } @@ -171,6 +233,11 @@ func (h *Host) RemoveContainer(id string) { } delete(h.containers, id) + + // Clean up the sandbox root mapping for sandbox containers. + if c.isSandbox { + h.unregisterSandboxRoot(id) + } } func (h *Host) GetCreatedContainer(id string) (*Container, error) { @@ -199,10 +266,11 @@ func (h *Host) AddContainer(id string, c *Container) error { return nil } -func setupSandboxMountsPath(id string) (err error) { - mountPath := specGuest.SandboxMountsDir(id) +// setupSandboxMountsPath creates the sandboxMounts directory from a resolved root. +func setupSandboxMountsPath(sandboxRoot string) (err error) { + mountPath := specGuest.SandboxMountsDirFromRoot(sandboxRoot) if err := os.MkdirAll(mountPath, 0755); err != nil { - return errors.Wrapf(err, "failed to create sandboxMounts dir in sandbox %v", id) + return errors.Wrapf(err, "failed to create sandboxMounts dir at %v", mountPath) } defer func() { if err != nil { @@ -213,10 +281,11 @@ func setupSandboxMountsPath(id string) (err error) { return storage.MountRShared(mountPath) } -func setupSandboxTmpfsMountsPath(id string) (err error) { - tmpfsDir := specGuest.SandboxTmpfsMountsDir(id) +// setupSandboxTmpfsMountsPath creates the sandbox tmpfs mounts directory from a resolved root. +func setupSandboxTmpfsMountsPath(sandboxRoot string) (err error) { + tmpfsDir := specGuest.SandboxTmpfsMountsDirFromRoot(sandboxRoot) if err := os.MkdirAll(tmpfsDir, 0755); err != nil { - return errors.Wrapf(err, "failed to create sandbox tmpfs mounts dir in sandbox %v", id) + return errors.Wrapf(err, "failed to create sandbox tmpfs mounts dir at %v", tmpfsDir) } defer func() { @@ -237,26 +306,21 @@ func setupSandboxTmpfsMountsPath(id string) (err error) { return storage.MountRShared(tmpfsDir) } -func setupSandboxHugePageMountsPath(id string) error { - mountPath := specGuest.HugePagesMountsDir(id) +// setupSandboxHugePageMountsPath creates the hugepages mounts directory from a resolved root. +func setupSandboxHugePageMountsPath(sandboxRoot string) error { + mountPath := specGuest.SandboxHugePagesMountsDirFromRoot(sandboxRoot) if err := os.MkdirAll(mountPath, 0755); err != nil { - return errors.Wrapf(err, "failed to create hugepage Mounts dir in sandbox %v", id) + return errors.Wrapf(err, "failed to create hugepage mounts dir at %v", mountPath) } 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) +// setupSandboxLogDir creates the directory to house all redirected stdio logs from a resolved root. +func setupSandboxLogDir(sandboxRoot string) error { + mountPath := specGuest.SandboxLogsDirFromRoot(sandboxRoot) 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 errors.Wrapf(err, "failed to create sandbox logs dir at %v", mountPath) } return nil } @@ -406,7 +470,14 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM // Capture namespaceID if any because setupSandboxContainerSpec clears the Windows section. namespaceID = specGuest.GetNetworkNamespaceID(settings.OCISpecification) - err = setupSandboxContainerSpec(ctx, id, settings.OCISpecification) + // Resolve the sandbox root from OCIBundlePath. + sandboxRoot, err := h.registerSandboxRoot(id, settings.OCIBundlePath, virtualPodID) + if err != nil { + return nil, err + } + c.sandboxRoot = sandboxRoot + + err = setupSandboxContainerSpec(ctx, id, sandboxRoot, settings.OCISpecification) if err != nil { return nil, err } @@ -416,30 +487,16 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM } }() - if isVirtualPod { - // For virtual pods, create virtual pod specific paths - if err = setupVirtualPodMountsPath(virtualPodID); err != nil { - return nil, err - } - if err = setupVirtualPodTmpfsMountsPath(virtualPodID); err != nil { - return nil, err - } - if err = setupVirtualPodHugePageMountsPath(virtualPodID); err != nil { - return nil, err - } - } else { - // Traditional sandbox setup - if err = setupSandboxMountsPath(id); err != nil { - return nil, err - } - if err = setupSandboxTmpfsMountsPath(id); err != nil { - return nil, err - } - if err = setupSandboxHugePageMountsPath(id); err != nil { - return nil, err - } + if err = setupSandboxMountsPath(sandboxRoot); err != nil { + return nil, err + } + if err = setupSandboxTmpfsMountsPath(sandboxRoot); err != nil { + return nil, err } - if err = setupSandboxLogDir(id, virtualPodID); err != nil { + if err = setupSandboxHugePageMountsPath(sandboxRoot); err != nil { + return nil, err + } + if err = setupSandboxLogDir(sandboxRoot); err != nil { return nil, err } @@ -457,7 +514,9 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM if !ok || sid == "" { return nil, errors.Errorf("unsupported 'io.kubernetes.cri.sandbox-id': '%s'", sid) } - if err = setupWorkloadContainerSpec(ctx, sid, id, settings.OCISpecification, settings.OCIBundlePath); err != nil { + sandboxRoot := h.resolveSandboxRoot(sid) + c.sandboxRoot = sandboxRoot + if err = setupWorkloadContainerSpec(ctx, sid, id, sandboxRoot, settings.OCISpecification, settings.OCIBundlePath); err != nil { return nil, err } @@ -484,7 +543,9 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM } else { // Capture namespaceID if any because setupStandaloneContainerSpec clears the Windows section. namespaceID = specGuest.GetNetworkNamespaceID(settings.OCISpecification) - if err := setupStandaloneContainerSpec(ctx, id, settings.OCISpecification); err != nil { + // Standalone uses OCIBundlePath directly as its root. + c.sandboxRoot = settings.OCIBundlePath + if err := setupStandaloneContainerSpec(ctx, id, settings.OCIBundlePath, settings.OCISpecification); err != nil { return nil, err } defer func() { @@ -504,7 +565,7 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM settings.OCISpecification.Mounts = append(settings.OCISpecification.Mounts, specs.Mount{ Destination: logDirMount, Type: "bind", - Source: specGuest.SandboxLogsDir(sandboxID, virtualPodID), + Source: specGuest.SandboxLogsDirFromRoot(c.sandboxRoot), Options: []string{"bind"}, }) } @@ -559,9 +620,10 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM 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) + logsDir := specGuest.SandboxLogsDirFromRoot(c.sandboxRoot) + c.logPath = filepath.Join(logsDir, logPath) // verify the logpath is still under the correct directory - if !strings.HasPrefix(c.logPath, specGuest.SandboxLogsDir(sandboxID, virtualPodID)) { + if !strings.HasPrefix(c.logPath, logsDir+"/") { return nil, errors.Errorf("log path %v is not within sandbox's log dir", c.logPath) } @@ -598,7 +660,7 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM return nil, err } - con, err := h.rtime.CreateContainer(id, settings.OCIBundlePath, nil) + con, err := h.rtime.CreateContainer(sandboxID, id, settings.OCIBundlePath, nil) if err != nil { return nil, errors.Wrapf(err, "failed to create container") } @@ -1618,50 +1680,3 @@ func (h *Host) cleanupVirtualPod(ctx context.Context, virtualSandboxID string) { delete(h.virtualPods, virtualSandboxID) entry.Info("Virtual pod cleaned up") } - -// setupVirtualPodMountsPath creates mount directories for virtual pods -func setupVirtualPodMountsPath(virtualSandboxID string) (err error) { - // Create virtual pod specific mount path using the new path generation functions - mountPath := specGuest.VirtualPodMountsDir(virtualSandboxID) - if err := os.MkdirAll(mountPath, 0755); err != nil { - return errors.Wrapf(err, "failed to create virtual pod mounts dir in sandbox %v", virtualSandboxID) - } - defer func() { - if err != nil { - _ = os.RemoveAll(mountPath) - } - }() - - return storage.MountRShared(mountPath) -} - -func setupVirtualPodTmpfsMountsPath(virtualSandboxID string) (err error) { - tmpfsDir := specGuest.VirtualPodTmpfsMountsDir(virtualSandboxID) - if err := os.MkdirAll(tmpfsDir, 0755); err != nil { - return errors.Wrapf(err, "failed to create virtual pod tmpfs mounts dir in sandbox %v", virtualSandboxID) - } - - defer func() { - if err != nil { - _ = os.RemoveAll(tmpfsDir) - } - }() - - // mount a tmpfs at the tmpfsDir - // this ensures that the tmpfsDir is a mount point and not just a directory - // we don't care if it is already mounted, so ignore EBUSY - if err := unix.Mount("tmpfs", tmpfsDir, "tmpfs", 0, ""); err != nil && !errors.Is(err, unix.EBUSY) { - return errors.Wrapf(err, "failed to mount tmpfs at %s", tmpfsDir) - } - - return storage.MountRShared(tmpfsDir) -} - -func setupVirtualPodHugePageMountsPath(virtualSandboxID string) error { - mountPath := specGuest.VirtualPodHugePagesMountsDir(virtualSandboxID) - if err := os.MkdirAll(mountPath, 0755); err != nil { - return errors.Wrapf(err, "failed to create virtual pod hugepage mounts dir %v", virtualSandboxID) - } - - return storage.MountRShared(mountPath) -} diff --git a/internal/guest/runtime/hcsv2/workload_container.go b/internal/guest/runtime/hcsv2/workload_container.go index 683c076e2c..d118a4d35c 100644 --- a/internal/guest/runtime/hcsv2/workload_container.go +++ b/internal/guest/runtime/hcsv2/workload_container.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "os" - "path/filepath" "strings" "github.com/opencontainers/runc/libcontainer/devices" @@ -32,9 +31,9 @@ func mkdirAllModePerm(target string) error { return os.MkdirAll(target, os.ModePerm) } -func updateSandboxMounts(sbid string, spec *oci.Spec) error { - // Check if this is a virtual pod - virtualSandboxID := spec.Annotations[annotations.VirtualPodID] +func updateSandboxMounts(sandboxRoot string, spec *oci.Spec) error { + mountsDir := specGuest.SandboxMountsDirFromRoot(sandboxRoot) + tmpfsMountsDir := specGuest.SandboxTmpfsMountsDirFromRoot(sandboxRoot) for i, m := range spec.Mounts { if !strings.HasPrefix(m.Source, guestpath.SandboxMountPrefix) && @@ -43,25 +42,16 @@ func updateSandboxMounts(sbid string, spec *oci.Spec) error { } var sandboxSource string - // if using `sandbox-tmp://` prefix, we mount a tmpfs in sandboxTmpfsMountsDir if strings.HasPrefix(m.Source, guestpath.SandboxTmpfsMountPrefix) { - // Use virtual pod aware mount source - sandboxSource = specGuest.VirtualPodAwareSandboxTmpfsMountSource(sbid, virtualSandboxID, m.Source) - expectedMountsDir := specGuest.VirtualPodAwareSandboxTmpfsMountsDir(sbid, virtualSandboxID) + sandboxSource = specGuest.SandboxTmpfsMountSourceFromRoot(sandboxRoot, m.Source) - // filepath.Join cleans the resulting path before returning, so it would resolve the relative path if one was given. - // Hence, we need to ensure that the resolved path is still under the correct directory - if !strings.HasPrefix(sandboxSource, expectedMountsDir) { + if !strings.HasPrefix(sandboxSource, tmpfsMountsDir) { return errors.Errorf("mount path %v for mount %v is not within sandbox's tmpfs mounts dir", sandboxSource, m.Source) } } else { - // Use virtual pod aware mount source - sandboxSource = specGuest.VirtualPodAwareSandboxMountSource(sbid, virtualSandboxID, m.Source) - expectedMountsDir := specGuest.VirtualPodAwareSandboxMountsDir(sbid, virtualSandboxID) + sandboxSource = specGuest.SandboxMountSourceFromRoot(sandboxRoot, m.Source) - // filepath.Join cleans the resulting path before returning, so it would resolve the relative path if one was given. - // Hence, we need to ensure that the resolved path is still under the correct directory - if !strings.HasPrefix(sandboxSource, expectedMountsDir) { + if !strings.HasPrefix(sandboxSource, mountsDir) { return errors.Errorf("mount path %v for mount %v is not within sandbox's mounts dir", sandboxSource, m.Source) } } @@ -78,24 +68,21 @@ func updateSandboxMounts(sbid string, spec *oci.Spec) error { return nil } -func updateHugePageMounts(sbid string, spec *oci.Spec) error { - // Check if this is a virtual pod - virtualSandboxID := spec.Annotations[annotations.VirtualPodID] +func updateHugePageMounts(sandboxRoot string, spec *oci.Spec) error { + hugepagesDir := specGuest.SandboxHugePagesMountsDirFromRoot(sandboxRoot) for i, m := range spec.Mounts { if !strings.HasPrefix(m.Source, guestpath.HugePagesMountPrefix) { continue } - // Use virtual pod aware hugepages directory - mountsDir := specGuest.VirtualPodAwareHugePagesMountsDir(sbid, virtualSandboxID) subPath := strings.TrimPrefix(m.Source, guestpath.HugePagesMountPrefix) pageSize := strings.Split(subPath, string(os.PathSeparator))[0] - hugePageMountSource := filepath.Join(mountsDir, subPath) + hugePageMountSource := specGuest.HugePagesMountSourceFromRoot(sandboxRoot, m.Source) // filepath.Join cleans the resulting path before returning so it would resolve the relative path if one was given. // Hence, we need to ensure that the resolved path is still under the correct directory - if !strings.HasPrefix(hugePageMountSource, mountsDir) { + if !strings.HasPrefix(hugePageMountSource, hugepagesDir) { return errors.Errorf("mount path %v for mount %v is not within hugepages's mounts dir", hugePageMountSource, m.Source) } @@ -178,7 +165,7 @@ func specHasGPUDevice(spec *oci.Spec) bool { return false } -func setupWorkloadContainerSpec(ctx context.Context, sbid, id string, spec *oci.Spec, ociBundlePath string) (err error) { +func setupWorkloadContainerSpec(ctx context.Context, sbid, id, sandboxRoot string, spec *oci.Spec, ociBundlePath string) (err error) { ctx, span := oc.StartSpan(ctx, "hcsv2::setupWorkloadContainerSpec") defer span.End() defer func() { oc.SetSpanStatus(span, err) }() @@ -192,11 +179,11 @@ func setupWorkloadContainerSpec(ctx context.Context, sbid, id string, spec *oci. } // update any sandbox mounts with the sandboxMounts directory path and create files - if err = updateSandboxMounts(sbid, spec); err != nil { + if err = updateSandboxMounts(sandboxRoot, spec); err != nil { return errors.Wrapf(err, "failed to update sandbox mounts for container %v in sandbox %v", id, sbid) } - if err = updateHugePageMounts(sbid, spec); err != nil { + if err = updateHugePageMounts(sandboxRoot, spec); err != nil { return errors.Wrapf(err, "failed to update hugepages mounts for container %v in sandbox %v", id, sbid) } @@ -210,7 +197,7 @@ func setupWorkloadContainerSpec(ctx context.Context, sbid, id string, spec *oci. // Add default mounts for container networking (e.g. /etc/hostname, /etc/hosts), // if spec didn't override them explicitly. - networkingMounts := specGuest.GenerateWorkloadContainerNetworkMounts(sbid, spec) + networkingMounts := specGuest.GenerateWorkloadContainerNetworkMountsFromRoot(sandboxRoot, spec) spec.Mounts = append(spec.Mounts, networkingMounts...) // TODO: JTERRY75 /dev/shm is not properly setup for LCOW I believe. CRI diff --git a/internal/guest/runtime/runc/container.go b/internal/guest/runtime/runc/container.go index c18dd1cba0..ce8412a966 100644 --- a/internal/guest/runtime/runc/container.go +++ b/internal/guest/runtime/runc/container.go @@ -23,9 +23,11 @@ import ( ) type container struct { - r *runcRuntime - id string - init *process + r *runcRuntime + id string + sandboxID string + bundlePath string + init *process // ownsPidNamespace indicates whether the container's init process is also // the init process for its pid namespace. ownsPidNamespace bool @@ -37,6 +39,10 @@ func (c *container) ID() string { return c.id } +func (c *container) SandboxID() string { + return c.sandboxID +} + func (c *container) Pid() int { return c.init.Pid() } @@ -52,7 +58,7 @@ func (c *container) PipeRelay() *stdio.PipeRelay { // Start unblocks the container's init process created by the call to // CreateContainer. func (c *container) Start() error { - logPath := c.r.getLogPath(c.id) + logPath := getLogPath(c.bundlePath) args := []string{"start", c.id} cmd := runcCommandLog(logPath, args...) out, err := cmd.CombinedOutput() @@ -145,7 +151,7 @@ func (c *container) Pause() error { // Resume unsuspends processes running in the container. func (c *container) Resume() error { - logPath := c.r.getLogPath(c.id) + logPath := getLogPath(c.bundlePath) args := []string{"resume", c.id} cmd := runcCommandLog(logPath, args...) out, err := cmd.CombinedOutput() @@ -376,11 +382,11 @@ func (c *container) startProcess( if err := setSubreaper(1); err != nil { return nil, errors.Wrapf(err, "failed to set process as subreaper for process in container %s", c.id) } - if err := c.r.makeLogDir(c.id); err != nil { + if err := makeLogDir(c.bundlePath); err != nil { return nil, err } - logPath := c.r.getLogPath(c.id) + logPath := getLogPath(c.bundlePath) args = append(args, "--pid-file", filepath.Join(tempProcessDir, "pid")) var sockListener *net.UnixListener diff --git a/internal/guest/runtime/runc/runc.go b/internal/guest/runtime/runc/runc.go index cd11cefdda..f981712973 100644 --- a/internal/guest/runtime/runc/runc.go +++ b/internal/guest/runtime/runc/runc.go @@ -31,8 +31,8 @@ func setSubreaper(i int) error { } // NewRuntime instantiates a new runcRuntime struct. -func NewRuntime(logBasePath string) (runtime.Runtime, error) { - rtime := &runcRuntime{runcLogBasePath: logBasePath} +func NewRuntime() (runtime.Runtime, error) { + rtime := &runcRuntime{} if err := rtime.initialize(); err != nil { return nil, err } @@ -42,26 +42,21 @@ func NewRuntime(logBasePath string) (runtime.Runtime, error) { // runcRuntime is an implementation of the Runtime interface which uses runC as // the container runtime. type runcRuntime struct { - runcLogBasePath string } var _ runtime.Runtime = &runcRuntime{} // initialize sets up any state necessary for the runcRuntime to function. func (r *runcRuntime) initialize() error { - paths := [2]string{containerFilesDir, r.runcLogBasePath} - for _, p := range paths { - _, err := os.Stat(p) - if err != nil { - if !os.IsNotExist(err) { - return err - } - if err := os.MkdirAll(p, 0700); err != nil { - return errors.Wrapf(err, "failed making runC container files directory %s", p) - } + _, err := os.Stat(containerFilesDir) + if err != nil { + if !os.IsNotExist(err) { + return err + } + if err := os.MkdirAll(containerFilesDir, 0700); err != nil { + return errors.Wrapf(err, "failed making runC container files directory %s", containerFilesDir) } } - return nil } @@ -69,8 +64,8 @@ func (r *runcRuntime) initialize() error { // bundlePath. // bundlePath should be a path to an OCI bundle containing a config.json file // and a rootfs for the container. -func (r *runcRuntime) CreateContainer(id string, bundlePath string, stdioSet *stdio.ConnectionSet) (c runtime.Container, err error) { - c, err = r.runCreateCommand(id, bundlePath, stdioSet) +func (r *runcRuntime) CreateContainer(sandboxID, id string, bundlePath string, stdioSet *stdio.ConnectionSet) (c runtime.Container, err error) { + c, err = r.runCreateCommand(sandboxID, id, bundlePath, stdioSet) if err != nil { return nil, err } @@ -154,8 +149,8 @@ func (r *runcRuntime) waitOnProcess(pid int) (int, error) { } // runCreateCommand sets up the arguments for calling runc create. -func (r *runcRuntime) runCreateCommand(id string, bundlePath string, stdioSet *stdio.ConnectionSet) (runtime.Container, error) { - c := &container{r: r, id: id} +func (r *runcRuntime) runCreateCommand(sandboxID, id string, bundlePath string, stdioSet *stdio.ConnectionSet) (runtime.Container, error) { + c := &container{r: r, sandboxID: sandboxID, id: id, bundlePath: bundlePath} if err := r.makeContainerDir(id); err != nil { return nil, err } diff --git a/internal/guest/runtime/runc/runc_test.go b/internal/guest/runtime/runc/runc_test.go new file mode 100644 index 0000000000..fecd8df6db --- /dev/null +++ b/internal/guest/runtime/runc/runc_test.go @@ -0,0 +1,42 @@ +//go:build linux +// +build linux + +package runc + +import ( + "path/filepath" + "testing" +) + +func TestGetLogPath(t *testing.T) { + got := getLogPath("/run/gcs/c/sandbox-1") + want := "/run/gcs/c/sandbox-1/runc.log" + if got != want { + t.Errorf("getLogPath = %q, want %q", got, want) + } +} + +func TestGetContainerDir(t *testing.T) { + r := &runcRuntime{} + got := r.getContainerDir("test-id") + want := filepath.Join(containerFilesDir, "test-id") + if got != want { + t.Errorf("getContainerDir = %q, want %q", got, want) + } +} + +func TestContainerSandboxID(t *testing.T) { + c := &container{sandboxID: "my-sandbox"} + if c.SandboxID() != "my-sandbox" { + t.Errorf("SandboxID() = %q, want %q", c.SandboxID(), "my-sandbox") + } +} + +func TestContainerBundlePath(t *testing.T) { + c := &container{bundlePath: "/run/gcs/c/sb/ctr"} + got := getLogPath(c.bundlePath) + want := "/run/gcs/c/sb/ctr/runc.log" + if got != want { + t.Errorf("log path from bundlePath = %q, want %q", got, want) + } +} diff --git a/internal/guest/runtime/runc/utils.go b/internal/guest/runtime/runc/utils.go index 12fc07ee54..dc6fb1f3a9 100644 --- a/internal/guest/runtime/runc/utils.go +++ b/internal/guest/runtime/runc/utils.go @@ -71,31 +71,17 @@ func (r *runcRuntime) makeContainerDir(id string) error { return nil } -// getLogDir gets the path to the runc logs directory. -func (r *runcRuntime) getLogDir(id string) string { - return filepath.Join(r.runcLogBasePath, id) -} - -// makeLogDir creates the runc logs directory if it doesnt exist. -func (r *runcRuntime) makeLogDir(id string) error { - dir := r.getLogDir(id) - if err := os.MkdirAll(dir, os.ModeDir); err != nil { - return errors.Wrapf(err, "failed making runc log directory for container %s", id) +// makeLogDir ensures the runc log directory exists. +func makeLogDir(bundlePath string) error { + if err := os.MkdirAll(bundlePath, os.ModeDir); err != nil { + return errors.Wrapf(err, "failed making runc log directory at %s", bundlePath) } return nil } -// getLogPath returns the path to the log file used by the runC wrapper for a particular container -func (r *runcRuntime) getLogPath(id string) string { - return filepath.Join(r.getLogDir(id), "runc.log") -} - -// getLogPath returns the path to the log file used by the runC wrapper. -// -//nolint:unused -func (r *runcRuntime) getGlobalLogPath() string { - // runcLogBasePath should be created by r.initialize - return filepath.Join(r.runcLogBasePath, "global-runc.log") +// getLogPath returns the path to the runc.log file for a container. +func getLogPath(bundlePath string) string { + return filepath.Join(bundlePath, "runc.log") } // processExists returns true if the given process exists in /proc, false if diff --git a/internal/guest/runtime/runtime.go b/internal/guest/runtime/runtime.go index db24459c27..5fa61816b2 100644 --- a/internal/guest/runtime/runtime.go +++ b/internal/guest/runtime/runtime.go @@ -63,6 +63,7 @@ type Process interface { type Container interface { Process ID() string + SandboxID() string Exists() (bool, error) Start() error ExecProcess(process *oci.Process, stdioSet *stdio.ConnectionSet) (p Process, err error) @@ -79,6 +80,6 @@ type Container interface { // Runtime is the interface defining commands over an OCI container runtime, // such as runC. type Runtime interface { - CreateContainer(id string, bundlePath string, stdioSet *stdio.ConnectionSet) (c Container, err error) + CreateContainer(sandboxID, id string, bundlePath string, stdioSet *stdio.ConnectionSet) (c Container, err error) ListContainerStates() ([]ContainerState, error) } diff --git a/internal/guest/spec/spec.go b/internal/guest/spec/spec.go index 25755458ca..b4b9bebf0f 100644 --- a/internal/guest/spec/spec.go +++ b/internal/guest/spec/spec.go @@ -230,6 +230,71 @@ func SandboxLogPath(sandboxID, virtualSandboxID, path string) string { return filepath.Join(SandboxLogsDir(sandboxID, virtualSandboxID), path) } +// Root-based path helpers. These accept a pre-resolved sandbox root directory +// and are used by the guest-side sandbox root mapping for Shim v2 / multi-pod support. + +// SandboxMountsDirFromRoot returns the sandbox mounts directory for a given root. +func SandboxMountsDirFromRoot(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "sandboxMounts") +} + +// SandboxTmpfsMountsDirFromRoot returns the sandbox tmpfs mounts directory for a given root. +func SandboxTmpfsMountsDirFromRoot(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "sandboxTmpfsMounts") +} + +// SandboxHugePagesMountsDirFromRoot returns the hugepages directory for a given root. +func SandboxHugePagesMountsDirFromRoot(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "hugepages") +} + +// SandboxLogsDirFromRoot returns the logs directory for a given root. +func SandboxLogsDirFromRoot(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "logs") +} + +// SandboxMountSourceFromRoot resolves a sandbox:// mount source from a given root. +func SandboxMountSourceFromRoot(sandboxRoot, mountPath string) string { + subPath := strings.TrimPrefix(mountPath, guestpath.SandboxMountPrefix) + return filepath.Join(SandboxMountsDirFromRoot(sandboxRoot), subPath) +} + +// SandboxTmpfsMountSourceFromRoot resolves a sandbox-tmp:// mount source from a given root. +func SandboxTmpfsMountSourceFromRoot(sandboxRoot, mountPath string) string { + subPath := strings.TrimPrefix(mountPath, guestpath.SandboxTmpfsMountPrefix) + return filepath.Join(SandboxTmpfsMountsDirFromRoot(sandboxRoot), subPath) +} + +// HugePagesMountSourceFromRoot resolves a hugepages:// mount source from a given root. +func HugePagesMountSourceFromRoot(sandboxRoot, mountPath string) string { + subPath := strings.TrimPrefix(mountPath, guestpath.HugePagesMountPrefix) + return filepath.Join(SandboxHugePagesMountsDirFromRoot(sandboxRoot), subPath) +} + +// GenerateWorkloadContainerNetworkMountsFromRoot generates networking mounts +// using a pre-resolved sandbox root directory. +func GenerateWorkloadContainerNetworkMountsFromRoot(sandboxRoot string, spec *oci.Spec) []oci.Mount { + var nMounts []oci.Mount + for _, mountPath := range networkingMountPaths() { + if MountPresent(mountPath, spec.Mounts) { + continue + } + options := []string{"bind"} + if spec.Root != nil && spec.Root.Readonly { + options = append(options, "ro") + } + trimmedMountPath := strings.TrimPrefix(mountPath, "/etc/") + mt := oci.Mount{ + Destination: mountPath, + Type: "bind", + Source: filepath.Join(sandboxRoot, trimmedMountPath), + Options: options, + } + nMounts = append(nMounts, mt) + } + return nMounts +} + // GetNetworkNamespaceID returns the `ToLower` of // `spec.Windows.Network.NetworkNamespace` or `""`. func GetNetworkNamespaceID(spec *oci.Spec) string { diff --git a/test/gcs/main_test.go b/test/gcs/main_test.go index 3497a6f786..0875102005 100644 --- a/test/gcs/main_test.go +++ b/test/gcs/main_test.go @@ -199,7 +199,7 @@ func getRuntime(_ context.Context, tb testing.TB) runtime.Runtime { } func getRuntimeErr() (runtime.Runtime, error) { - rt, err := runc.NewRuntime(guestpath.LCOWRootPrefixInUVM) + rt, err := runc.NewRuntime() if err != nil { return rt, fmt.Errorf("failed to initialize runc runtime: %w", err) }