Merge pull request containerd#6304 from lippertmarkus/fix-ctr-cni-windows

dmcgowan · web-flow · commit bf60e2d1749f · 2022-01-03T10:47:44.000-08:00
fix(ctr): enable networking for Windows containers
diff --git a/cmd/ctr/commands/run/run.go b/cmd/ctr/commands/run/run.go
@@ -123,6 +123,10 @@ var Command = cli.Command{
 			Name:  "platform",
 			Usage: "run image for specific platform",
 		},
+		cli.BoolFlag{
+			Name:  "cni",
+			Usage: "enable cni networking for the container",
+		},
 	}, append(platformRunFlags,
 		append(append(commands.SnapshotterFlags, []cli.Flag{commands.SnapshotterLabels}...),
 			commands.ContainerFlags...)...)...),
@@ -209,7 +213,12 @@ var Command = cli.Command{
 			}
 		}
 		if enableCNI {
-			if _, err := network.Setup(ctx, fullID(ctx, container), fmt.Sprintf("/proc/%d/ns/net", task.Pid())); err != nil {
+			netNsPath, err := getNetNSPath(ctx, task)
+			if err != nil {
+				return err
+			}
+
+			if _, err := network.Setup(ctx, fullID(ctx, container), netNsPath); err != nil {
 				return err
 			}
 		}
diff --git a/cmd/ctr/commands/run/run_unix.go b/cmd/ctr/commands/run/run_unix.go
@@ -79,10 +79,6 @@ var platformRunFlags = []cli.Flag{
 		Usage: "set the cpu shares",
 		Value: 1024,
 	},
-	cli.BoolFlag{
-		Name:  "cni",
-		Usage: "enable cni networking for the container",
-	},
 }
 
 // NewContainer creates a new container
@@ -449,3 +445,7 @@ func validNamespace(ns string) bool {
 		return false
 	}
 }
+
+func getNetNSPath(_ gocontext.Context, task containerd.Task) (string, error) {
+	return fmt.Sprintf("/proc/%d/ns/net", task.Pid()), nil
+}
diff --git a/cmd/ctr/commands/run/run_windows.go b/cmd/ctr/commands/run/run_windows.go
@@ -24,6 +24,7 @@ import (
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cmd/ctr/commands"
 	"github.com/containerd/containerd/oci"
+	"github.com/containerd/containerd/pkg/netns"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -116,6 +117,13 @@ func NewContainer(ctx gocontext.Context, client *containerd.Client, context *cli
 		if context.Bool("net-host") {
 			return nil, errors.New("Cannot use host mode networking with Windows containers")
 		}
+		if context.Bool("cni") {
+			ns, err := netns.NewNetNS("")
+			if err != nil {
+				return nil, err
+			}
+			opts = append(opts, oci.WithWindowsNetworkNamespace(ns.GetPath()))
+		}
 		if context.Bool("isolated") {
 			opts = append(opts, oci.WithWindowsHyperV)
 		}
@@ -149,3 +157,14 @@ func NewContainer(ctx gocontext.Context, client *containerd.Client, context *cli
 func getNewTaskOpts(_ *cli.Context) []containerd.NewTaskOpts {
 	return nil
 }
+
+func getNetNSPath(ctx gocontext.Context, t containerd.Task) (string, error) {
+	s, err := t.Spec(ctx)
+	if err != nil {
+		return "", err
+	}
+	if s.Windows == nil || s.Windows.Network == nil {
+		return "", nil
+	}
+	return s.Windows.Network.NetworkNamespace, nil
+}
diff --git a/oci/spec_opts_windows.go b/oci/spec_opts_windows.go
@@ -75,3 +75,17 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 func DeviceFromPath(path string) (*specs.LinuxDevice, error) {
 	return nil, errors.New("device from path not supported on Windows")
 }
+
+// WithWindowsNetworkNamespace sets the network namespace for a Windows container.
+func WithWindowsNetworkNamespace(ns string) SpecOpts {
+	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
+		if s.Windows == nil {
+			s.Windows = &specs.Windows{}
+		}
+		if s.Windows.Network == nil {
+			s.Windows.Network = &specs.WindowsNetwork{}
+		}
+		s.Windows.Network.NetworkNamespace = ns
+		return nil
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -123,6 +123,10 @@ var Command = cli.Command{`
`123`	`123`	`Name: "platform",`
`124`	`124`	`Usage: "run image for specific platform",`
`125`	`125`	`},`
	`126`	`+ cli.BoolFlag{`
	`127`	`+ Name: "cni",`
	`128`	`+ Usage: "enable cni networking for the container",`
	`129`	`+ },`
`126`	`130`	`}, append(platformRunFlags,`
`127`	`131`	`append(append(commands.SnapshotterFlags, []cli.Flag{commands.SnapshotterLabels}...),`
`128`	`132`	`commands.ContainerFlags...)...)...),`
`@@ -209,7 +213,12 @@ var Command = cli.Command{`
`209`	`213`	`}`
`210`	`214`	`}`
`211`	`215`	`if enableCNI {`
`212`		`- if _, err := network.Setup(ctx, fullID(ctx, container), fmt.Sprintf("/proc/%d/ns/net", task.Pid())); err != nil {`
	`216`	`+ netNsPath, err := getNetNSPath(ctx, task)`
	`217`	`+ if err != nil {`
	`218`	`+ return err`
	`219`	`+ }`
	`220`	`+`
	`221`	`+ if _, err := network.Setup(ctx, fullID(ctx, container), netNsPath); err != nil {`
`213`	`222`	`return err`
`214`	`223`	`}`
`215`	`224`	`}`
Original file line number	Diff line number	Diff line change
`@@ -79,10 +79,6 @@ var platformRunFlags = []cli.Flag{`
`79`	`79`	`Usage: "set the cpu shares",`
`80`	`80`	`Value: 1024,`
`81`	`81`	`},`
`82`		`- cli.BoolFlag{`
`83`		`- Name: "cni",`
`84`		`- Usage: "enable cni networking for the container",`
`85`		`- },`
`86`	`82`	`}`
`87`	`83`
`88`	`84`	`// NewContainer creates a new container`
`@@ -449,3 +445,7 @@ func validNamespace(ns string) bool {`
`449`	`445`	`return false`
`450`	`446`	`}`
`451`	`447`	`}`
	`448`	`+`
	`449`	`+func getNetNSPath(_ gocontext.Context, task containerd.Task) (string, error) {`
	`450`	`+ return fmt.Sprintf("/proc/%d/ns/net", task.Pid()), nil`
	`451`	`+}`