From 67b38ffe6ea3350f3cefd72caacd3f7ee9b1af42 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Fri, 8 Jul 2022 11:43:05 +0200 Subject: [PATCH] kubelet: propagate errors from namespacesForPod it is a preparatory change for the next commit. Signed-off-by: Giuseppe Scrivano --- .../kuberuntime_container_linux.go | 16 ++++++++++---- .../kuberuntime_container_linux_test.go | 22 +++++++++++++------ .../kuberuntime/kuberuntime_sandbox.go | 6 ++++- pkg/kubelet/kuberuntime/security_context.go | 10 ++++++--- pkg/kubelet/kuberuntime/util/util.go | 6 ++--- pkg/kubelet/kuberuntime/util/util_test.go | 4 +++- 6 files changed, 45 insertions(+), 19 deletions(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go index 25917803b1c..1e9a2377b51 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go @@ -45,15 +45,23 @@ func (m *kubeGenericRuntimeManager) applyPlatformSpecificContainerConfig(config libcontainercgroups.IsCgroup2UnifiedMode() { enforceMemoryQoS = true } - config.Linux = m.generateLinuxContainerConfig(container, pod, uid, username, nsTarget, enforceMemoryQoS) + cl, err := m.generateLinuxContainerConfig(container, pod, uid, username, nsTarget, enforceMemoryQoS) + if err != nil { + return err + } + config.Linux = cl return nil } // generateLinuxContainerConfig generates linux container config for kubelet runtime v1. -func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.Container, pod *v1.Pod, uid *int64, username string, nsTarget *kubecontainer.ContainerID, enforceMemoryQoS bool) *runtimeapi.LinuxContainerConfig { +func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.Container, pod *v1.Pod, uid *int64, username string, nsTarget *kubecontainer.ContainerID, enforceMemoryQoS bool) (*runtimeapi.LinuxContainerConfig, error) { + sc, err := m.determineEffectiveSecurityContext(pod, container, uid, username) + if err != nil { + return nil, err + } lc := &runtimeapi.LinuxContainerConfig{ Resources: &runtimeapi.LinuxContainerResources{}, - SecurityContext: m.determineEffectiveSecurityContext(pod, container, uid, username), + SecurityContext: sc, } if nsTarget != nil && lc.SecurityContext.NamespaceOptions.Pid == runtimeapi.NamespaceMode_CONTAINER { @@ -124,7 +132,7 @@ func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.C } } - return lc + return lc, nil } // calculateLinuxResources will create the linuxContainerResources type based on the provided CPU and memory resource requests, limits diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go index 307a40d0188..1a50d96143f 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go @@ -47,6 +47,8 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde restartCountUint32 := uint32(restartCount) envs := make([]*runtimeapi.KeyValue, len(opts.Envs)) + l, _ := m.generateLinuxContainerConfig(container, pod, new(int64), "", nil, enforceMemoryQoS) + expectedConfig := &runtimeapi.ContainerConfig{ Metadata: &runtimeapi.ContainerMetadata{ Name: container.Name, @@ -64,7 +66,7 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde Stdin: container.Stdin, StdinOnce: container.StdinOnce, Tty: container.TTY, - Linux: m.generateLinuxContainerConfig(container, pod, new(int64), "", nil, enforceMemoryQoS), + Linux: l, Envs: envs, } return expectedConfig @@ -215,7 +217,8 @@ func TestGenerateLinuxContainerConfigResources(t *testing.T) { }, } - linuxConfig := m.generateLinuxContainerConfig(&pod.Spec.Containers[0], pod, new(int64), "", nil, false) + linuxConfig, err := m.generateLinuxContainerConfig(&pod.Spec.Containers[0], pod, new(int64), "", nil, false) + assert.NoError(t, err) assert.Equal(t, test.expected.CpuPeriod, linuxConfig.GetResources().CpuPeriod, test.name) assert.Equal(t, test.expected.CpuQuota, linuxConfig.GetResources().CpuQuota, test.name) assert.Equal(t, test.expected.CpuShares, linuxConfig.GetResources().CpuShares, test.name) @@ -329,6 +332,8 @@ func TestGenerateContainerConfigWithMemoryQoSEnforced(t *testing.T) { memoryLow int64 memoryHigh int64 } + l1, _ := m.generateLinuxContainerConfig(&pod1.Spec.Containers[0], pod1, new(int64), "", nil, true) + l2, _ := m.generateLinuxContainerConfig(&pod2.Spec.Containers[0], pod2, new(int64), "", nil, true) tests := []struct { name string pod *v1.Pod @@ -338,7 +343,7 @@ func TestGenerateContainerConfigWithMemoryQoSEnforced(t *testing.T) { name: "Request128MBLimit256MB", pod: pod1, expected: &expectedResult{ - m.generateLinuxContainerConfig(&pod1.Spec.Containers[0], pod1, new(int64), "", nil, true), + l1, 128 * 1024 * 1024, int64(float64(256*1024*1024) * m.memoryThrottlingFactor), }, @@ -347,7 +352,7 @@ func TestGenerateContainerConfigWithMemoryQoSEnforced(t *testing.T) { name: "Request128MBWithoutLimit", pod: pod2, expected: &expectedResult{ - m.generateLinuxContainerConfig(&pod2.Spec.Containers[0], pod2, new(int64), "", nil, true), + l2, 128 * 1024 * 1024, int64(pod2MemoryHigh), }, @@ -355,7 +360,8 @@ func TestGenerateContainerConfigWithMemoryQoSEnforced(t *testing.T) { } for _, test := range tests { - linuxConfig := m.generateLinuxContainerConfig(&test.pod.Spec.Containers[0], test.pod, new(int64), "", nil, true) + linuxConfig, err := m.generateLinuxContainerConfig(&test.pod.Spec.Containers[0], test.pod, new(int64), "", nil, true) + assert.NoError(t, err) assert.Equal(t, test.expected.containerConfig, linuxConfig, test.name) assert.Equal(t, linuxConfig.GetResources().GetUnified()["memory.min"], strconv.FormatInt(test.expected.memoryLow, 10), test.name) assert.Equal(t, linuxConfig.GetResources().GetUnified()["memory.high"], strconv.FormatInt(test.expected.memoryHigh, 10), test.name) @@ -577,7 +583,8 @@ func TestGenerateLinuxContainerConfigNamespaces(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - got := m.generateLinuxContainerConfig(&tc.pod.Spec.Containers[0], tc.pod, nil, "", tc.target, false) + got, err := m.generateLinuxContainerConfig(&tc.pod.Spec.Containers[0], tc.pod, nil, "", tc.target, false) + assert.NoError(t, err) if diff := cmp.Diff(tc.want, got.SecurityContext.NamespaceOptions); diff != "" { t.Errorf("%v: diff (-want +got):\n%v", t.Name(), diff) } @@ -668,7 +675,8 @@ func TestGenerateLinuxContainerConfigSwap(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { m.memorySwapBehavior = tc.swapSetting - actual := m.generateLinuxContainerConfig(&tc.pod.Spec.Containers[0], tc.pod, nil, "", nil, false) + actual, err := m.generateLinuxContainerConfig(&tc.pod.Spec.Containers[0], tc.pod, nil, "", nil, false) + assert.NoError(t, err) assert.Equal(t, tc.expected, actual.Resources.MemorySwapLimitInBytes, "memory swap config for %s", tc.name) }) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go index 835dbbcd6fd..c37270d26d9 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go @@ -195,7 +195,11 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod) ( if sc.RunAsGroup != nil && runtime.GOOS != "windows" { lc.SecurityContext.RunAsGroup = &runtimeapi.Int64Value{Value: int64(*sc.RunAsGroup)} } - lc.SecurityContext.NamespaceOptions = runtimeutil.NamespacesForPod(pod) + namespaceOptions, err := runtimeutil.NamespacesForPod(pod, m.runtimeHelper) + if err != nil { + return nil, err + } + lc.SecurityContext.NamespaceOptions = namespaceOptions if sc.FSGroup != nil && runtime.GOOS != "windows" { lc.SecurityContext.SupplementalGroups = append(lc.SecurityContext.SupplementalGroups, int64(*sc.FSGroup)) diff --git a/pkg/kubelet/kuberuntime/security_context.go b/pkg/kubelet/kuberuntime/security_context.go index e315d66e73c..5e6f05b4e18 100644 --- a/pkg/kubelet/kuberuntime/security_context.go +++ b/pkg/kubelet/kuberuntime/security_context.go @@ -25,7 +25,7 @@ import ( ) // determineEffectiveSecurityContext gets container's security context from v1.Pod and v1.Container. -func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Pod, container *v1.Container, uid *int64, username string) *runtimeapi.LinuxContainerSecurityContext { +func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Pod, container *v1.Container, uid *int64, username string) (*runtimeapi.LinuxContainerSecurityContext, error) { effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container) synthesized := convertToRuntimeSecurityContext(effectiveSc) if synthesized == nil { @@ -53,7 +53,11 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po } // set namespace options and supplemental groups. - synthesized.NamespaceOptions = runtimeutil.NamespacesForPod(pod) + namespaceOptions, err := runtimeutil.NamespacesForPod(pod, m.runtimeHelper) + if err != nil { + return nil, err + } + synthesized.NamespaceOptions = namespaceOptions podSc := pod.Spec.SecurityContext if podSc != nil { if podSc.FSGroup != nil { @@ -75,7 +79,7 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po synthesized.MaskedPaths = securitycontext.ConvertToRuntimeMaskedPaths(effectiveSc.ProcMount) synthesized.ReadonlyPaths = securitycontext.ConvertToRuntimeReadonlyPaths(effectiveSc.ProcMount) - return synthesized + return synthesized, nil } // convertToRuntimeSecurityContext converts v1.SecurityContext to runtimeapi.SecurityContext. diff --git a/pkg/kubelet/kuberuntime/util/util.go b/pkg/kubelet/kuberuntime/util/util.go index 6ad66256a9a..5c84e7d82ce 100644 --- a/pkg/kubelet/kuberuntime/util/util.go +++ b/pkg/kubelet/kuberuntime/util/util.go @@ -97,12 +97,12 @@ func PidNamespaceForPod(pod *v1.Pod) runtimeapi.NamespaceMode { return runtimeapi.NamespaceMode_CONTAINER } -// NamespacesForPod returns the runtimeapi.NamespaceOption for a given pod. +// namespacesForPod returns the runtimeapi.NamespaceOption for a given pod. // An empty or nil pod can be used to get the namespace defaults for v1.Pod. -func NamespacesForPod(pod *v1.Pod) *runtimeapi.NamespaceOption { +func NamespacesForPod(pod *v1.Pod, runtimeHelper kubecontainer.RuntimeHelper) (*runtimeapi.NamespaceOption, error) { return &runtimeapi.NamespaceOption{ Ipc: IpcNamespaceForPod(pod), Network: NetworkNamespaceForPod(pod), Pid: PidNamespaceForPod(pod), - } + }, nil } diff --git a/pkg/kubelet/kuberuntime/util/util_test.go b/pkg/kubelet/kuberuntime/util/util_test.go index 7189742358f..7a96e23fa2e 100644 --- a/pkg/kubelet/kuberuntime/util/util_test.go +++ b/pkg/kubelet/kuberuntime/util/util_test.go @@ -24,6 +24,7 @@ import ( v1 "k8s.io/api/core/v1" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" + kubecontainertest "k8s.io/kubernetes/pkg/kubelet/container/testing" ) func TestPodSandboxChanged(t *testing.T) { @@ -222,7 +223,8 @@ func TestNamespacesForPod(t *testing.T) { }, } { t.Run(desc, func(t *testing.T) { - actual := NamespacesForPod(test.input) + actual, err := NamespacesForPod(test.input, &kubecontainertest.FakeRuntimeHelper{}) + require.NoError(t, err) require.Equal(t, test.expected, actual) }) }