Remove pause container creation for process isolated containers

This commit does the following: - Introduces new HostComputeNamespace.ReadyOnCreate field and set it for HNS versions that support pause container removal - Remove pause container creation while creating process isolated pods for HNS versions that support pause container creation - Add annotation to fall back to pause container creation Signed-off-by: Kirtana Ashok <[email protected]> (cherry picked from commit 492c98c8c862afbf0d61deb2a7359143c548a5cf) Signed-off-by: Kirtana Ashok <[email protected]>
kiashok · Jan 17, 2024 · e1b780a · e1b780a
1 parent b16edf6
commit e1b780a
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 23 deletions.
diff --git a/cmd/containerd-shim-runhcs-v1/pod.go b/cmd/containerd-shim-runhcs-v1/pod.go
@@ -10,6 +10,7 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/Microsoft/hcsshim/hcn"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/oci"
 	"github.com/Microsoft/hcsshim/internal/uvm"
@@ -149,7 +150,6 @@ func createPod(ctx context.Context, events publisher, req *task.CreateTaskReques
 			parent.Close()
 			return nil, err
 		}
-
 	} else if oci.IsJobContainer(s) {
 		// If we're making a job container fake a task (i.e reuse the wcowPodSandbox logic)
 		p.sandboxTask = newWcowPodSandboxTask(ctx, events, req.ID, req.Bundle, parent, "")
@@ -196,25 +196,21 @@ func createPod(ctx context.Context, events publisher, req *task.CreateTaskReques
 		}
 	}
 
-	// TODO: JTERRY75 - There is a bug in the compartment activation for Windows
-	// Process isolated that requires us to create the real pause container to
-	// hold the network compartment open. This is not required for Windows
-	// Hypervisor isolated. When we have a build that supports this for Windows
-	// Process isolated make sure to move back to this model.
-
 	// For WCOW we fake out the init task since we dont need it. We only
 	// need to provision the guest network namespace if this is hypervisor
 	// isolated. Process isolated WCOW gets the namespace endpoints
 	// automatically.
 	nsid := ""
-	if isWCOW && parent != nil {
-		if s.Windows != nil && s.Windows.Network != nil {
-			nsid = s.Windows.Network.NetworkNamespace
-		}
+	if isWCOW && (parent != nil || (parent == nil && hcn.CanRemovePauseContainer(s.Annotations))) {
+		if parent != nil {
+			if s.Windows != nil && s.Windows.Network != nil {
+				nsid = s.Windows.Network.NetworkNamespace
+			}
 
-		if nsid != "" {
-			if err := parent.ConfigureNetworking(ctx, nsid); err != nil {
-				return nil, errors.Wrapf(err, "failed to setup networking for pod %q", req.ID)
+			if nsid != "" {
+				if err := parent.ConfigureNetworking(ctx, nsid); err != nil {
+					return nil, errors.Wrapf(err, "failed to setup networking for pod %q", req.ID)
+				}
 			}
 		}
 		p.sandboxTask = newWcowPodSandboxTask(ctx, events, req.ID, req.Bundle, parent, nsid)

diff --git a/cmd/ncproxy/hcn_networking_test.go b/cmd/ncproxy/hcn_networking_test.go
@@ -950,7 +950,7 @@ func TestAddEndpoint_NoError(t *testing.T) {
 	// the nominal AddEndpoint functionality, and when specifying attach to host
 	// the DefaultHost namespace is retrieved below.
 	namespace := hcn.NewNamespace(hcn.NamespaceTypeHost)
-	namespace, err = namespace.Create()
+	namespace, err = namespace.Create(nil)
 	if err != nil {
 		t.Fatalf("failed to create test namespace with %v", err)
 	}
@@ -961,7 +961,7 @@ func TestAddEndpoint_NoError(t *testing.T) {
 	hostDefaultNSID, err := getHostDefaultNamespace()
 	if err != nil {
 		ns := hcn.NewNamespace(hcn.NamespaceTypeHostDefault)
-		ns, err = ns.Create()
+		ns, err = ns.Create(nil)
 		if err != nil {
 			t.Fatalf("failed to create host-default test namespace: %v", err)
 		}
@@ -1062,7 +1062,7 @@ func TestAddEndpoint_Error_EmptyEndpointName(t *testing.T) {
 
 	// create test network namespace
 	namespace := hcn.NewNamespace(hcn.NamespaceTypeHostDefault)
-	namespace, err = namespace.Create()
+	namespace, err = namespace.Create(nil)
 	if err != nil {
 		t.Fatalf("failed to create test namespace with %v", err)
 	}
@@ -1096,7 +1096,7 @@ func TestAddEndpoint_Error_NoEndpoint(t *testing.T) {
 
 	// create test network namespace
 	namespace := hcn.NewNamespace(hcn.NamespaceTypeHostDefault)
-	namespace, err = namespace.Create()
+	namespace, err = namespace.Create(nil)
 	if err != nil {
 		t.Fatalf("failed to create test namespace with %v", err)
 	}

diff --git a/cmd/ncproxy/ncproxy_v0_service_test.go b/cmd/ncproxy/ncproxy_v0_service_test.go
@@ -687,7 +687,7 @@ func TestAddEndpoint_V0_NoError(t *testing.T) {
 	// the nominal AddEndpoint functionality, and when specifying attach to host
 	// the DefaultHost namespace is retrieved below.
 	namespace := hcn.NewNamespace(hcn.NamespaceTypeHost)
-	namespace, err = namespace.Create()
+	namespace, err = namespace.Create(nil)
 	if err != nil {
 		t.Fatalf("failed to create test namespace with %v", err)
 	}
@@ -749,7 +749,7 @@ func TestAddEndpoint_V0_Error_EmptyEndpointName(t *testing.T) {
 
 	// create test network namespace
 	namespace := hcn.NewNamespace(hcn.NamespaceTypeHostDefault)
-	namespace, err = namespace.Create()
+	namespace, err = namespace.Create(nil)
 	if err != nil {
 		t.Fatalf("failed to create test namespace with %v", err)
 	}
@@ -784,7 +784,7 @@ func TestAddEndpoint_V0_Error_NoEndpoint(t *testing.T) {
 
 	// create test network namespace
 	namespace := hcn.NewNamespace(hcn.NamespaceTypeHostDefault)
-	namespace, err = namespace.Create()
+	namespace, err = namespace.Create(nil)
 	if err != nil {
 		t.Fatalf("failed to create test namespace with %v", err)
 	}

diff --git a/hcn/hcnnamespace.go b/hcn/hcnnamespace.go
@@ -5,13 +5,16 @@ package hcn
 import (
 	"encoding/json"
 	"os"
+	"strings"
 	"syscall"
 
 	"github.com/Microsoft/go-winio/pkg/guid"
+	"github.com/Microsoft/hcsshim"
 	icni "github.com/Microsoft/hcsshim/internal/cni"
 	"github.com/Microsoft/hcsshim/internal/interop"
 	"github.com/Microsoft/hcsshim/internal/regstate"
 	"github.com/Microsoft/hcsshim/internal/runhcs"
+	"github.com/Microsoft/hcsshim/pkg/annotations"
 	"github.com/sirupsen/logrus"
 )
 
@@ -62,6 +65,7 @@ type HostComputeNamespace struct {
 	Type          NamespaceType       `json:",omitempty"` // Host, HostDefault, Guest, GuestDefault
 	Resources     []NamespaceResource `json:",omitempty"`
 	SchemaVersion SchemaVersion       `json:",omitempty"`
+	ReadyOnCreate bool                `json:",omitempty"`
 }
 
 // ModifyNamespaceSettingRequest is the structure used to send request to modify a namespace.
@@ -306,6 +310,31 @@ func GetNamespaceContainerIds(namespaceID string) ([]string, error) {
 	return containerIds, nil
 }
 
+func CanRemovePauseContainer(podAnnotations map[string]string) bool {
+	// If EnablePauseContainerCreation annotation is set to true, we fall back
+	// and create pause containers for process isolation.
+	if podAnnotations == nil {
+		return false
+	} else {
+		if isEnablePauseContainers, ok := podAnnotations[annotations.EnablePauseContainerCreation]; ok {
+			if strings.ToLower(isEnablePauseContainers) == "true" {
+				return false
+			}
+		}
+	}
+
+	// HNS versions >= 15.2 change how network compartments are
+	// initialized. They support removal of pause containers for
+	// process isolation.
+	hnsGlobals, err := hcsshim.GetHNSGlobals()
+	if err == nil {
+		return (hnsGlobals.Version.Major > 15) ||
+			(hnsGlobals.Version.Major == 15 && hnsGlobals.Version.Minor >= 2)
+	}
+
+	return false
+}
+
 // NewNamespace creates a new Namespace object
 func NewNamespace(nsType NamespaceType) *HostComputeNamespace {
 	return &HostComputeNamespace{
@@ -315,9 +344,15 @@ func NewNamespace(nsType NamespaceType) *HostComputeNamespace {
 }
 
 // Create Namespace.
-func (namespace *HostComputeNamespace) Create() (*HostComputeNamespace, error) {
+func (namespace *HostComputeNamespace) Create(podAnnotations map[string]string) (*HostComputeNamespace, error) {
 	logrus.Debugf("hcn::HostComputeNamespace::Create id=%s", namespace.Id)
 
+	// Set ReadyOnCreate flag to true only if pause containers
+	// can be removed.
+	if podAnnotations != nil && CanRemovePauseContainer(podAnnotations) {
+		namespace.ReadyOnCreate = true
+	}
+
 	jsonString, err := json.Marshal(namespace)
 	if err != nil {
 		return nil, err

diff --git a/internal/hcsoci/network.go b/internal/hcsoci/network.go
@@ -21,7 +21,8 @@ func createNetworkNamespace(ctx context.Context, coi *createOptionsInternal, r *
 		l.Debug(op + " - End")
 	}()
 
-	ns, err := hcn.NewNamespace("").Create()
+	annotations := coi.Spec.Annotations
+	ns, err := hcn.NewNamespace("").Create(annotations)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/annotations/annotations.go b/pkg/annotations/annotations.go
@@ -294,6 +294,10 @@ const (
 	// DumpDirectoryPath provides a path to the directory in which dumps for a UVM will be collected in
 	// case the UVM crashes.
 	DumpDirectoryPath = "io.microsoft.virtualmachine.dump-directory-path"
+
+	// EnablePauseContainerCreation provides a way to disable new HNS behavior
+	// that supports pause container removal and fallback to old networking behavior.
+	EnablePauseContainerCreation = "io.microsoft.enablepausecontainercreation"
 )
 
 // AnnotationExpansions maps annotations that will be expanded into an array of