Skip to content

guest: use OCIBundlePath as sandbox root source of truth #2653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shreyanshjain7174 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

guest: use OCIBundlePath as sandbox root source of truth #2653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions internal/guest/runtime/hcsv2/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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
Expand Down Expand Up @@ -77,6 +76,10 @@ type Container struct {
// of this container is located. Usually, this is either `/run/gcs/c/<containerID>` or
// `/run/gcs/c/<UVMID>/container_<containerID>` 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) {
Expand Down Expand Up @@ -229,25 +232,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 != "" {
Copy link
Copy Markdown
Contributor

@rawahars rawahars Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We should likely keep the comments while removing the virtual pod semantics.
Example here would be // remove user mounts in sandbox container

// 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")
}
}
Expand Down
34 changes: 14 additions & 20 deletions internal/guest/runtime/hcsv2/sandbox_container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}()

Expand All @@ -62,19 +57,20 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
}
}

sandboxHostnamePath := getSandboxHostnamePath(id, virtualSandboxID)
Copy link
Copy Markdown
Contributor

@rawahars rawahars Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep these utility methods as well.
For example this can be-

func getSandboxHostnamePath(sandboxRoot string) string {
	return filepath.Join(sandboxRoot, "hostname")
}

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))
Expand All @@ -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")
}
Expand Down Expand Up @@ -125,10 +121,8 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (

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

Expand Down
Loading
Loading