Skip to content

Commit 9de87ec

Browse files
author
Shreyansh Sancheti
committed
guest: use OCIBundlePath as sandbox root source of truth
Replace heuristic sandbox path derivation (hard-coded /run/gcs/c prefix + ID) with host-provided OCIBundlePath as the canonical sandbox root directory. This change prepares the guest-side GCS for Shim v2 and multi-pod UVM support, where the host may use a different path layout than the legacy /run/gcs/c/<id>. Key changes: - Add sandboxRoots mapping on Host to store resolved sandbox root per sandbox ID - Sandbox containers: register OCIBundlePath as sandbox root - Virtual pods: derive sandbox root from OCIBundlePath parent + /virtual-pods/<id> - Workload containers: resolve sandbox root from Host mapping (fallback to legacy) - Standalone containers: use OCIBundlePath directly as root - Container.Delete: use stored sandboxRoot for cleanup paths - Remove duplicate setup functions (setupVirtualPod* merged into unified setup*) The refactor produces identical paths when the old shim sends OCIBundlePath in the legacy format, ensuring zero behavior change for existing deployments. Security: virtualPodID is validated against path traversal before use. Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>
1 parent 5a0252a commit 9de87ec

File tree

6 files changed

+531
-206
lines changed

6 files changed

+531
-206
lines changed

internal/guest/runtime/hcsv2/container.go

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"fmt"
99
"os"
10+
"path/filepath"
1011
"sync"
1112
"sync/atomic"
1213
"syscall"
@@ -30,7 +31,6 @@ import (
3031
"github.com/Microsoft/hcsshim/internal/oc"
3132
"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
3233
"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
33-
"github.com/Microsoft/hcsshim/pkg/annotations"
3434
)
3535

3636
// containerStatus has been introduced to enable parallel container creation
@@ -77,6 +77,11 @@ type Container struct {
7777
// of this container is located. Usually, this is either `/run/gcs/c/<containerID>` or
7878
// `/run/gcs/c/<UVMID>/container_<containerID>` if scratch is shared with UVM scratch.
7979
scratchDirPath string
80+
81+
// sandboxRoot is the resolved sandbox root directory for this container,
82+
// populated from the Host's sandbox root mapping. Used during cleanup
83+
// to unmount sandbox-specific paths without re-deriving from the ID.
84+
sandboxRoot string
8085
}
8186

8287
func (c *Container) Start(ctx context.Context, conSettings stdio.ConnectionSettings) (_ int, err error) {
@@ -229,25 +234,14 @@ func (c *Container) Kill(ctx context.Context, signal syscall.Signal) error {
229234
func (c *Container) Delete(ctx context.Context) error {
230235
entity := log.G(ctx).WithField(logfields.ContainerID, c.id)
231236
entity.Info("opengcs::Container::Delete")
232-
if c.isSandbox {
233-
// Check if this is a virtual pod
234-
virtualSandboxID := ""
235-
if c.spec != nil && c.spec.Annotations != nil {
236-
virtualSandboxID = c.spec.Annotations[annotations.VirtualPodID]
237-
}
238-
239-
// remove user mounts in sandbox container - use virtual pod aware paths
240-
if err := storage.UnmountAllInPath(ctx, specGuest.VirtualPodAwareSandboxMountsDir(c.id, virtualSandboxID), true); err != nil {
237+
if c.isSandbox && c.sandboxRoot != "" {
238+
if err := storage.UnmountAllInPath(ctx, filepath.Join(c.sandboxRoot, "sandboxMounts"), true); err != nil {
241239
entity.WithError(err).Error("failed to unmount sandbox mounts")
242240
}
243-
244-
// remove user mounts in tmpfs sandbox container - use virtual pod aware paths
245-
if err := storage.UnmountAllInPath(ctx, specGuest.VirtualPodAwareSandboxTmpfsMountsDir(c.id, virtualSandboxID), true); err != nil {
241+
if err := storage.UnmountAllInPath(ctx, filepath.Join(c.sandboxRoot, "sandboxTmpfsMounts"), true); err != nil {
246242
entity.WithError(err).Error("failed to unmount tmpfs sandbox mounts")
247243
}
248-
249-
// remove hugepages mounts in sandbox container - use virtual pod aware paths
250-
if err := storage.UnmountAllInPath(ctx, specGuest.VirtualPodAwareHugePagesMountsDir(c.id, virtualSandboxID), true); err != nil {
244+
if err := storage.UnmountAllInPath(ctx, filepath.Join(c.sandboxRoot, "hugepages"), true); err != nil {
251245
entity.WithError(err).Error("failed to unmount hugepages mounts")
252246
}
253247
}

internal/guest/runtime/hcsv2/sandbox_container.go

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,35 +20,18 @@ import (
2020
"github.com/Microsoft/hcsshim/pkg/annotations"
2121
)
2222

23-
func getSandboxHostnamePath(id, virtualSandboxID string) string {
24-
return filepath.Join(specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID), "hostname")
25-
}
26-
27-
func getSandboxHostsPath(id, virtualSandboxID string) string {
28-
return filepath.Join(specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID), "hosts")
29-
}
30-
31-
func getSandboxResolvPath(id, virtualSandboxID string) string {
32-
return filepath.Join(specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID), "resolv.conf")
33-
}
34-
35-
func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (err error) {
23+
func setupSandboxContainerSpec(ctx context.Context, id, sandboxRoot string, spec *oci.Spec) (err error) {
3624
ctx, span := oc.StartSpan(ctx, "hcsv2::setupSandboxContainerSpec")
3725
defer span.End()
3826
defer func() { oc.SetSpanStatus(span, err) }()
3927
span.AddAttributes(trace.StringAttribute("cid", id))
4028

41-
// Check if this is a virtual pod to use appropriate root directory
42-
virtualSandboxID := spec.Annotations[annotations.VirtualPodID]
43-
44-
// Generate the sandbox root dir - virtual pod aware
45-
rootDir := specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID)
46-
if err := os.MkdirAll(rootDir, 0755); err != nil {
47-
return errors.Wrapf(err, "failed to create sandbox root directory %q", rootDir)
29+
if err := os.MkdirAll(sandboxRoot, 0755); err != nil {
30+
return errors.Wrapf(err, "failed to create sandbox root directory %q", sandboxRoot)
4831
}
4932
defer func() {
5033
if err != nil {
51-
_ = os.RemoveAll(rootDir)
34+
_ = os.RemoveAll(sandboxRoot)
5235
}
5336
}()
5437

@@ -62,19 +45,20 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
6245
}
6346
}
6447

65-
sandboxHostnamePath := getSandboxHostnamePath(id, virtualSandboxID)
48+
sandboxHostnamePath := filepath.Join(sandboxRoot, "hostname")
6649
if err := os.WriteFile(sandboxHostnamePath, []byte(hostname+"\n"), 0644); err != nil {
6750
return errors.Wrapf(err, "failed to write hostname to %q", sandboxHostnamePath)
6851
}
6952

7053
// Write the hosts
7154
sandboxHostsContent := network.GenerateEtcHostsContent(ctx, hostname)
72-
sandboxHostsPath := getSandboxHostsPath(id, virtualSandboxID)
55+
sandboxHostsPath := filepath.Join(sandboxRoot, "hosts")
7356
if err := os.WriteFile(sandboxHostsPath, []byte(sandboxHostsContent), 0644); err != nil {
7457
return errors.Wrapf(err, "failed to write sandbox hosts to %q", sandboxHostsPath)
7558
}
7659

7760
// Check if this is a virtual pod sandbox container by comparing container ID with virtual pod ID
61+
virtualSandboxID := spec.Annotations[annotations.VirtualPodID]
7862
isVirtualPodSandbox := virtualSandboxID != "" && id == virtualSandboxID
7963
if strings.EqualFold(spec.Annotations[annotations.SkipPodNetworking], "true") || isVirtualPodSandbox {
8064
ns := GetOrAddNetworkNamespace(specGuest.GetNetworkNamespaceID(spec))
@@ -97,7 +81,7 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
9781
if err != nil {
9882
return errors.Wrap(err, "failed to generate sandbox resolv.conf content")
9983
}
100-
sandboxResolvPath := getSandboxResolvPath(id, virtualSandboxID)
84+
sandboxResolvPath := filepath.Join(sandboxRoot, "resolv.conf")
10185
if err := os.WriteFile(sandboxResolvPath, []byte(resolvContent), 0644); err != nil {
10286
return errors.Wrap(err, "failed to write sandbox resolv.conf")
10387
}
@@ -125,10 +109,8 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
125109

126110
// Set cgroup path - check if this is a virtual pod
127111
if virtualSandboxID != "" {
128-
// Virtual pod sandbox gets its own cgroup under /containers/virtual-pods using the virtual pod ID
129112
spec.Linux.CgroupsPath = "/containers/virtual-pods/" + virtualSandboxID
130113
} else {
131-
// Traditional sandbox goes under /containers
132114
spec.Linux.CgroupsPath = "/containers/" + id
133115
}
134116

0 commit comments

Comments
 (0)