Merge pull request #1084 from harche/passthrough_annotations

Add support for passing sandbox annotations to runtime

docs/config.md

@@ -138,6 +138,12 @@ The explanation and default value of each configuration item are as follows:
       # runtime_type is the runtime type to use in containerd e.g. io.containerd.runtime.v1.linux
       runtime_type = "io.containerd.runc.v1"
 
+      # pod_annotations is list of pod annotations passed to both pod sandbox as well as
+      # container OCI annotations. Pod_annotations also support golang supported
+      # regular expression - https://github.com/google/re2/wiki/Syntax.
+      # e.g. ["runc.com.github.containers.runc.*"]
+      pod_annotations = []
+
       # "plugins.cri.containerd.runtimes.runc.options" is options specific to
       # "io.containerd.runc.v1". Its corresponding options type is:
       #   https://github.com/containerd/containerd/blob/v1.2.0-rc.1/runtime/v2/runc/options/oci.pb.go#L39.

pkg/config/config.go

@@ -31,6 +31,9 @@ type Runtime struct {
 	// This only works for runtime type "io.containerd.runtime.v1.linux".
 	// DEPRECATED: use Options instead. Remove when shim v1 is deprecated.
 	Engine string `toml:"runtime_engine" json:"runtimeEngine"`
+	// PodAnnotations is list of pod annotations passed to both pod sandbox as well as
+	// container OCI annotations.
+	PodAnnotations []string `toml:"pod_annotations" json:"PodAnnotations"`
 	// Root is the directory used by containerd for runtime state.
 	// DEPRECATED: use Options instead. Remove when shim v1 is deprecated.
 	// This only works for runtime type "io.containerd.runtime.v1.linux".

pkg/server/container_create.go

@@ -165,7 +165,14 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	// Generate container runtime spec.
 	mounts := c.generateContainerMounts(sandboxID, config)
 
-	spec, err := c.generateContainerSpec(id, sandboxID, sandboxPid, config, sandboxConfig, &image.ImageSpec.Config, append(mounts, volumeMounts...))
+	ociRuntime, err := c.getSandboxRuntime(sandboxConfig, sandbox.Metadata.RuntimeHandler)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get sandbox runtime")
+	}
+	logrus.Debugf("Use OCI %+v for sandbox %q and container %q", ociRuntime, sandboxID, id)
+
+	spec, err := c.generateContainerSpec(id, sandboxID, sandboxPid, config, sandboxConfig,
+		&image.ImageSpec.Config, append(mounts, volumeMounts...), ociRuntime.PodAnnotations)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to generate container %q spec", id)
 	}
@@ -310,7 +317,8 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 }
 
 func (c *criService) generateContainerSpec(id string, sandboxID string, sandboxPid uint32, config *runtime.ContainerConfig,
-	sandboxConfig *runtime.PodSandboxConfig, imageConfig *imagespec.ImageConfig, extraMounts []*runtime.Mount) (*runtimespec.Spec, error) {
+	sandboxConfig *runtime.PodSandboxConfig, imageConfig *imagespec.ImageConfig, extraMounts []*runtime.Mount,
+	runtimePodAnnotations []string) (*runtimespec.Spec, error) {
 	// Creates a spec Generator with the default spec.
 	spec, err := defaultRuntimeSpec(id)
 	if err != nil {
@@ -444,6 +452,11 @@ func (c *criService) generateContainerSpec(id string, sandboxID string, sandboxP
 		g.AddProcessAdditionalGid(uint32(group))
 	}
 
+	for pKey, pValue := range getPassthroughAnnotations(sandboxConfig.Annotations,
+		runtimePodAnnotations) {
+		g.AddAnnotation(pKey, pValue)
+	}
+
 	g.AddAnnotation(annotations.ContainerType, annotations.ContainerTypeContainer)
 	g.AddAnnotation(annotations.SandboxID, sandboxID)
 

pkg/server/container_create_test.go

@@ -36,6 +36,7 @@ import (
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 
 	"github.com/containerd/cri/pkg/annotations"
+	criconfig "github.com/containerd/cri/pkg/config"
 	ostesting "github.com/containerd/cri/pkg/os/testing"
 	"github.com/containerd/cri/pkg/util"
 )
@@ -116,6 +117,7 @@ func getCreateContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandbox
 			Namespace: "test-sandbox-ns",
 			Attempt:   2,
 		},
+		Annotations: map[string]string{"c": "d"},
 		Linux: &runtime.LinuxPodSandboxConfig{
 			CgroupParent: "/test/cgroup/parent",
 		},
@@ -193,7 +195,7 @@ func TestGeneralContainerSpec(t *testing.T) {
 	config, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData()
 	c := newTestCRIService()
 	testSandboxID := "sandbox-id"
-	spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+	spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 	require.NoError(t, err)
 	specCheck(t, testID, testSandboxID, testPid, spec)
 }
@@ -248,7 +250,7 @@ func TestContainerCapabilities(t *testing.T) {
 	} {
 		t.Logf("TestCase %q", desc)
 		config.Linux.SecurityContext.Capabilities = test.capability
-		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 		require.NoError(t, err)
 		specCheck(t, testID, testSandboxID, testPid, spec)
 		for _, include := range test.includes {
@@ -275,7 +277,7 @@ func TestContainerSpecTty(t *testing.T) {
 	c := newTestCRIService()
 	for _, tty := range []bool{true, false} {
 		config.Tty = tty
-		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 		require.NoError(t, err)
 		specCheck(t, testID, testSandboxID, testPid, spec)
 		assert.Equal(t, tty, spec.Process.Terminal)
@@ -287,6 +289,92 @@ func TestContainerSpecTty(t *testing.T) {
 	}
 }
 
+func TestPodAnnotationPassthroughContainerSpec(t *testing.T) {
+	testID := "test-id"
+	testSandboxID := "sandbox-id"
+	testPid := uint32(1234)
+
+	for desc, test := range map[string]struct {
+		configCriService func(*criService)
+		configChange     func(*runtime.PodSandboxConfig)
+		specCheck        func(*testing.T, *runtimespec.Spec)
+	}{
+		// Scenario 1 - containerd config allows "c" and podSpec defines "c" to be passed to OCI
+		"passthroughAnnotations scenario 1": {
+			configCriService: func(c *criService) {
+				c.config.Runtimes = make(map[string]criconfig.Runtime)
+				c.config.Runtimes["runc"] = criconfig.Runtime{
+					PodAnnotations: []string{"c"},
+				}
+			},
+			specCheck: func(t *testing.T, spec *runtimespec.Spec) {
+				assert.Equal(t, spec.Annotations["c"], "d")
+			},
+		},
+		// Scenario 2 - containerd config allows only "c" but podSpec defines "c" and "d"
+		// Only annotation "c" will be injected in OCI annotations and "d" should be dropped.
+		"passthroughAnnotations scenario 2": {
+			configChange: func(c *runtime.PodSandboxConfig) {
+				c.Annotations["d"] = "e"
+			},
+			configCriService: func(c *criService) {
+				c.config.Runtimes = make(map[string]criconfig.Runtime)
+				c.config.Runtimes["runc"] = criconfig.Runtime{
+					PodAnnotations: []string{"c"},
+				}
+			},
+			specCheck: func(t *testing.T, spec *runtimespec.Spec) {
+				assert.Equal(t, spec.Annotations["c"], "d")
+				_, ok := spec.Annotations["d"]
+				assert.Equal(t, ok, false)
+			},
+		},
+		// Scenario 3 - Let's test some wildcard support
+		// podSpec has following annotations
+		"passthroughAnnotations scenario 3": {
+			configChange: func(c *runtime.PodSandboxConfig) {
+				c.Annotations["t.f"] = "j"
+				c.Annotations["z.g"] = "o"
+				c.Annotations["y.ca"] = "b"
+			},
+			configCriService: func(c *criService) {
+				c.config.Runtimes = make(map[string]criconfig.Runtime)
+				c.config.Runtimes["runc"] = criconfig.Runtime{
+					PodAnnotations: []string{"t*", "z.*", "y.c*"},
+				}
+			},
+			specCheck: func(t *testing.T, spec *runtimespec.Spec) {
+				assert.Equal(t, spec.Annotations["t.f"], "j")
+				assert.Equal(t, spec.Annotations["z.g"], "o")
+				assert.Equal(t, spec.Annotations["y.ca"], "b")
+			},
+		},
+	} {
+
+		t.Logf("TestCase %q", desc)
+		c := newTestCRIService()
+		config, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData()
+		if test.configChange != nil {
+			test.configChange(sandboxConfig)
+		}
+
+		if test.configCriService != nil {
+			test.configCriService(c)
+		}
+
+		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid,
+			config, sandboxConfig, imageConfig, nil, c.config.Runtimes["runc"].PodAnnotations)
+
+		assert.NoError(t, err)
+		assert.NotNil(t, spec)
+		specCheck(t, testID, testSandboxID, testPid, spec)
+		if test.specCheck != nil {
+			test.specCheck(t, spec)
+		}
+	}
+
+}
+
 func TestContainerSpecReadonlyRootfs(t *testing.T) {
 	testID := "test-id"
 	testSandboxID := "sandbox-id"
@@ -295,7 +383,7 @@ func TestContainerSpecReadonlyRootfs(t *testing.T) {
 	c := newTestCRIService()
 	for _, readonly := range []bool{true, false} {
 		config.Linux.SecurityContext.ReadonlyRootfs = readonly
-		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 		require.NoError(t, err)
 		specCheck(t, testID, testSandboxID, testPid, spec)
 		assert.Equal(t, readonly, spec.Root.Readonly)
@@ -332,7 +420,7 @@ func TestContainerSpecWithExtraMounts(t *testing.T) {
 			Readonly:      false,
 		},
 	}
-	spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, extraMounts)
+	spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, extraMounts, []string{})
 	require.NoError(t, err)
 	specCheck(t, testID, testSandboxID, testPid, spec)
 	var mounts, sysMounts, devMounts []runtimespec.Mount
@@ -398,7 +486,7 @@ func TestContainerAndSandboxPrivileged(t *testing.T) {
 		sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{
 			Privileged: test.sandboxPrivileged,
 		}
-		_, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+		_, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 		if test.expectError {
 			assert.Error(t, err)
 		} else {
@@ -867,7 +955,7 @@ func TestPidNamespace(t *testing.T) {
 	} {
 		t.Logf("TestCase %q", desc)
 		config.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{Pid: test.pidNS}
-		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 		require.NoError(t, err)
 		assert.Contains(t, spec.Linux.Namespaces, test.expected)
 	}
@@ -1065,7 +1153,7 @@ func TestMaskedAndReadonlyPaths(t *testing.T) {
 		sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{
 			Privileged: test.privileged,
 		}
-		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 		require.NoError(t, err)
 		if !test.privileged { // specCheck presumes an unprivileged container
 			specCheck(t, testID, testSandboxID, testPid, spec)
@@ -1110,7 +1198,7 @@ func TestHostname(t *testing.T) {
 		sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{
 			NamespaceOptions: &runtime.NamespaceOption{Network: test.networkNs},
 		}
-		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil)
+		spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, nil, []string{})
 		require.NoError(t, err)
 		specCheck(t, testID, testSandboxID, testPid, spec)
 		assert.Contains(t, spec.Process.Env, test.expectedEnv)
@@ -1121,7 +1209,7 @@ func TestDisableCgroup(t *testing.T) {
 	config, sandboxConfig, imageConfig, _ := getCreateContainerTestData()
 	c := newTestCRIService()
 	c.config.DisableCgroup = true
-	spec, err := c.generateContainerSpec("test-id", "sandbox-id", 1234, config, sandboxConfig, imageConfig, nil)
+	spec, err := c.generateContainerSpec("test-id", "sandbox-id", 1234, config, sandboxConfig, imageConfig, nil, []string{})
 	require.NoError(t, err)
 
 	t.Log("resource limit should not be set")

pkg/server/helpers.go

@@ -626,3 +626,20 @@ func getTaskStatus(ctx context.Context, task containerd.Task) (containerd.Status
 	}
 	return status, nil
 }
+
+// getPassthroughAnnotations filters requested pod annotations by comparing
+// against permitted annotations for the given runtime.
+func getPassthroughAnnotations(podAnnotations map[string]string,
+	runtimePodAnnotations []string) (passthroughAnnotations map[string]string) {
+	passthroughAnnotations = make(map[string]string)
+
+	for podAnnotationKey, podAnnotationValue := range podAnnotations {
+		for _, r := range runtimePodAnnotations {
+			match, _ := regexp.MatchString(r, podAnnotationKey)
+			if match {
+				passthroughAnnotations[podAnnotationKey] = podAnnotationValue
+			}
+		}
+	}
+	return passthroughAnnotations
+}

pkg/server/helpers_test.go

@@ -406,3 +406,40 @@ func TestCustomGenerator(t *testing.T) {
 		}
 	}
 }
+
+func TestPassThroughAnnotationsFilter(t *testing.T) {
+	for desc, test := range map[string]struct {
+		podAnnotations         map[string]string
+		runtimePodAnnotations  []string
+		passthroughAnnotations map[string]string
+	}{
+		// Scenario 1 - containerd config allows "c" and podSpec defines "c" to be passed to OCI
+		"scenario1": {
+			podAnnotations:         map[string]string{"c": "d"},
+			runtimePodAnnotations:  []string{"c"},
+			passthroughAnnotations: map[string]string{"c": "d"},
+		},
+		// Scenario 2 - containerd config allows only "c" but podSpec defines "c" and "d"
+		// Only annotation "c" will be injected in OCI annotations and "d" should be dropped.
+		"scenario2": {
+			podAnnotations:         map[string]string{"c": "d", "d": "e"},
+			runtimePodAnnotations:  []string{"c"},
+			passthroughAnnotations: map[string]string{"c": "d"},
+		},
+		// Scenario 3 - Let's test some wildcard support
+		// podSpec has following annotations
+		"scenario3": {
+			podAnnotations: map[string]string{"t.f": "j",
+				"z.g":  "o",
+				"y.ca": "b"},
+			runtimePodAnnotations: []string{"t*", "z.*", "y.c*"},
+			passthroughAnnotations: map[string]string{"t.f": "j",
+				"z.g":  "o",
+				"y.ca": "b"},
+		},
+	} {
+		t.Logf("TestCase %q", desc)
+		passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations)
+		assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations)
+	}
+}

pkg/server/sandbox_run.go

@@ -147,7 +147,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
 	}
 
 	// Create sandbox container.
-	spec, err := c.generateSandboxContainerSpec(id, config, &image.ImageSpec.Config, sandbox.NetNSPath)
+	spec, err := c.generateSandboxContainerSpec(id, config, &image.ImageSpec.Config, sandbox.NetNSPath, ociRuntime.PodAnnotations)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to generate sandbox container spec")
 	}
@@ -340,7 +340,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
 }
 
 func (c *criService) generateSandboxContainerSpec(id string, config *runtime.PodSandboxConfig,
-	imageConfig *imagespec.ImageConfig, nsPath string) (*runtimespec.Spec, error) {
+	imageConfig *imagespec.ImageConfig, nsPath string, runtimePodAnnotations []string) (*runtimespec.Spec, error) {
 	// Creates a spec Generator with the default spec.
 	// TODO(random-liu): [P1] Compare the default settings with docker and containerd default.
 	spec, err := defaultRuntimeSpec(id)
@@ -452,6 +452,11 @@ func (c *criService) generateSandboxContainerSpec(id string, config *runtime.Pod
 	}
 	g.SetProcessOOMScoreAdj(adj)
 
+	for pKey, pValue := range getPassthroughAnnotations(config.Annotations,
+		runtimePodAnnotations) {
+		g.AddAnnotation(pKey, pValue)
+	}
+
 	g.AddAnnotation(annotations.ContainerType, annotations.ContainerTypeSandbox)
 	g.AddAnnotation(annotations.SandboxID, id)
 	g.AddAnnotation(annotations.SandboxLogDir, config.GetLogDirectory())

pkg/server/sandbox_run_test.go

@@ -86,6 +86,7 @@ func TestGenerateSandboxContainerSpec(t *testing.T) {
 	testID := "test-id"
 	nsPath := "test-cni"
 	for desc, test := range map[string]struct {
+		configCriService  func(*criService)
 		configChange      func(*runtime.PodSandboxConfig)
 		imageConfigChange func(*imagespec.ImageConfig)
 		specCheck         func(*testing.T, *runtimespec.Spec)
@@ -157,6 +158,56 @@ func TestGenerateSandboxContainerSpec(t *testing.T) {
 				assert.Contains(t, spec.Process.User.AdditionalGids, uint32(2222))
 			},
 		},
+		// Scenario 1 - containerd config allows "c" and podSpec defines "c" to be passed to OCI
+		"passthroughAnnotations scenario 1": {
+			configCriService: func(c *criService) {
+				c.config.Runtimes = make(map[string]criconfig.Runtime)
+				c.config.Runtimes["runc"] = criconfig.Runtime{
+					PodAnnotations: []string{"c"},
+				}
+			}, //assert.Equal(t, passthroughAnnotations["c"], "d")
+			specCheck: func(t *testing.T, spec *runtimespec.Spec) {
+				assert.Equal(t, spec.Annotations["c"], "d")
+			},
+		},
+		// Scenario 2 - containerd config allows only "c" but podSpec defines "c" and "d"
+		// Only annotation "c" will be injected in OCI annotations and "d" should be dropped.
+		"passthroughAnnotations scenario 2": {
+			configChange: func(c *runtime.PodSandboxConfig) {
+				c.Annotations["d"] = "e"
+			},
+			configCriService: func(c *criService) {
+				c.config.Runtimes = make(map[string]criconfig.Runtime)
+				c.config.Runtimes["runc"] = criconfig.Runtime{
+					PodAnnotations: []string{"c"},
+				}
+			},
+			specCheck: func(t *testing.T, spec *runtimespec.Spec) {
+				assert.Equal(t, spec.Annotations["c"], "d")
+				_, ok := spec.Annotations["d"]
+				assert.Equal(t, ok, false)
+			},
+		},
+		// Scenario 3 - Let's test some wildcard support
+		// podSpec has following annotations
+		"passthroughAnnotations scenario 3": {
+			configChange: func(c *runtime.PodSandboxConfig) {
+				c.Annotations["t.f"] = "j"
+				c.Annotations["z.g"] = "o"
+				c.Annotations["y.ca"] = "b"
+			},
+			configCriService: func(c *criService) {
+				c.config.Runtimes = make(map[string]criconfig.Runtime)
+				c.config.Runtimes["runc"] = criconfig.Runtime{
+					PodAnnotations: []string{"t*", "z.*", "y.c*"},
+				}
+			},
+			specCheck: func(t *testing.T, spec *runtimespec.Spec) {
+				assert.Equal(t, spec.Annotations["t.f"], "j")
+				assert.Equal(t, spec.Annotations["z.g"], "o")
+				assert.Equal(t, spec.Annotations["y.ca"], "b")
+			},
+		},
 	} {
 		t.Logf("TestCase %q", desc)
 		c := newTestCRIService()
@@ -164,10 +215,16 @@ func TestGenerateSandboxContainerSpec(t *testing.T) {
 		if test.configChange != nil {
 			test.configChange(config)
 		}
+
+		if test.configCriService != nil {
+			test.configCriService(c)
+		}
+
 		if test.imageConfigChange != nil {
 			test.imageConfigChange(imageConfig)
 		}
-		spec, err := c.generateSandboxContainerSpec(testID, config, imageConfig, nsPath)
+		spec, err := c.generateSandboxContainerSpec(testID, config, imageConfig, nsPath,
+			c.config.Runtimes["runc"].PodAnnotations)
 		if test.expectErr {
 			assert.Error(t, err)
 			assert.Nil(t, spec)
@@ -355,6 +412,7 @@ options timeout:1
 		c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) {
 			return realhostname, nil
 		}
+
 		cfg := &runtime.PodSandboxConfig{
 			Hostname:  test.hostname,
 			DnsConfig: test.dnsConfig,
@@ -773,7 +831,7 @@ func TestSandboxDisableCgroup(t *testing.T) {
 	config, imageConfig, _ := getRunPodSandboxTestData()
 	c := newTestCRIService()
 	c.config.DisableCgroup = true
-	spec, err := c.generateSandboxContainerSpec("test-id", config, imageConfig, "test-cni")
+	spec, err := c.generateSandboxContainerSpec("test-id", config, imageConfig, "test-cni", []string{})
 	require.NoError(t, err)
 
 	t.Log("resource limit should not be set")