From effd82227c70a7694d68ee4433fd15d95c86022c Mon Sep 17 00:00:00 2001 From: Harshal Patil Date: Mon, 11 Mar 2019 13:39:20 +0530 Subject: [PATCH] Add support for passing sandbox annotations to runtime Signed-off-by: Harshal Patil --- docs/config.md | 6 ++ pkg/config/config.go | 3 + pkg/server/container_create.go | 17 ++++- pkg/server/container_create_test.go | 108 +++++++++++++++++++++++++--- pkg/server/helpers.go | 17 +++++ pkg/server/helpers_test.go | 37 ++++++++++ pkg/server/sandbox_run.go | 9 ++- pkg/server/sandbox_run_test.go | 62 +++++++++++++++- 8 files changed, 243 insertions(+), 16 deletions(-) diff --git a/docs/config.md b/docs/config.md index 2f386b2b4..85a416021 100644 --- a/docs/config.md +++ b/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. diff --git a/pkg/config/config.go b/pkg/config/config.go index 5f0631b0d..3aa096fab 100644 --- a/pkg/config/config.go +++ b/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". diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index b6c36569d..d245c247c 100644 --- a/pkg/server/container_create.go +++ b/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) diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go index 23fc38e9c..e79d84ab6 100644 --- a/pkg/server/container_create_test.go +++ b/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") diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index 06fb9f22b..451752d94 100644 --- a/pkg/server/helpers.go +++ b/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 +} diff --git a/pkg/server/helpers_test.go b/pkg/server/helpers_test.go index c0eb29839..0b73550c9 100644 --- a/pkg/server/helpers_test.go +++ b/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) + } +} diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index 05801bb27..4b33de498 100644 --- a/pkg/server/sandbox_run.go +++ b/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()) diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index ac9579def..5339755fc 100644 --- a/pkg/server/sandbox_run_test.go +++ b/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")