From f6084f7eababdbea012a705ce309358a31168fea Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Wed, 17 Oct 2018 17:52:17 +0200 Subject: [PATCH] Remove container type from kubelet runtime labels We've changed the Ephemeral Containers API, and container type will no longer be required. Since this is the only feature using it, remove it. This reverts commit ba6f31a6c60e9bead6947c01e3cddfa6148523c5. --- pkg/kubelet/container/runtime.go | 7 -- .../kuberuntime/kuberuntime_container.go | 8 +- .../kuberuntime_container_linux_test.go | 9 +-- .../kuberuntime/kuberuntime_container_test.go | 2 +- .../kuberuntime/kuberuntime_manager.go | 4 +- .../kuberuntime/kuberuntime_manager_test.go | 3 +- pkg/kubelet/kuberuntime/labels.go | 13 +-- pkg/kubelet/kuberuntime/labels_test.go | 80 ++----------------- pkg/kubelet/types/labels.go | 1 - 9 files changed, 20 insertions(+), 107 deletions(-) diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index aacbf9e36b..53e503fa3c 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -254,13 +254,6 @@ const ( ContainerStateUnknown ContainerState = "unknown" ) -type ContainerType string - -const ( - ContainerTypeInit ContainerType = "INIT" - ContainerTypeRegular ContainerType = "REGULAR" -) - // Container provides the runtime information for a container, such as ID, hash, // state of the container. type Container struct { diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index d0624ee048..8959a01c3f 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -90,7 +90,7 @@ func (m *kubeGenericRuntimeManager) recordContainerEvent(pod *v1.Pod, container // * create the container // * start the container // * run the post start lifecycle hooks (if applicable) -func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandboxConfig *runtimeapi.PodSandboxConfig, container *v1.Container, pod *v1.Pod, podStatus *kubecontainer.PodStatus, pullSecrets []v1.Secret, podIP string, containerType kubecontainer.ContainerType) (string, error) { +func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandboxConfig *runtimeapi.PodSandboxConfig, container *v1.Container, pod *v1.Pod, podStatus *kubecontainer.PodStatus, pullSecrets []v1.Secret, podIP string) (string, error) { // Step 1: pull the image. imageRef, msg, err := m.imagePuller.EnsureImageExists(pod, container, pullSecrets) if err != nil { @@ -112,7 +112,7 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb restartCount = containerStatus.RestartCount + 1 } - containerConfig, cleanupAction, err := m.generateContainerConfig(container, pod, restartCount, podIP, imageRef, containerType) + containerConfig, cleanupAction, err := m.generateContainerConfig(container, pod, restartCount, podIP, imageRef) if cleanupAction != nil { defer cleanupAction() } @@ -188,7 +188,7 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb } // generateContainerConfig generates container config for kubelet runtime v1. -func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Container, pod *v1.Pod, restartCount int, podIP, imageRef string, containerType kubecontainer.ContainerType) (*runtimeapi.ContainerConfig, func(), error) { +func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Container, pod *v1.Pod, restartCount int, podIP, imageRef string) (*runtimeapi.ContainerConfig, func(), error) { opts, cleanupAction, err := m.runtimeHelper.GenerateRunContainerOptions(pod, container, podIP) if err != nil { return nil, nil, err @@ -221,7 +221,7 @@ func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Contai Command: command, Args: args, WorkingDir: container.WorkingDir, - Labels: newContainerLabels(container, pod, containerType), + Labels: newContainerLabels(container, pod), Annotations: newContainerAnnotations(container, pod, restartCount, opts), Devices: makeDevices(opts), Mounts: m.makeMounts(opts, container), diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go index ae1f66149c..b1739b6f22 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go @@ -25,7 +25,6 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" - kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerIndex int) *runtimeapi.ContainerConfig { @@ -46,7 +45,7 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde Command: container.Command, Args: []string(nil), WorkingDir: container.WorkingDir, - Labels: newContainerLabels(container, pod, kubecontainer.ContainerTypeRegular), + Labels: newContainerLabels(container, pod), Annotations: newContainerAnnotations(container, pod, restartCount, opts), Devices: makeDevices(opts), Mounts: m.makeMounts(opts, container), @@ -90,7 +89,7 @@ func TestGenerateContainerConfig(t *testing.T) { } expectedConfig := makeExpectedConfig(m, pod, 0) - containerConfig, _, err := m.generateContainerConfig(&pod.Spec.Containers[0], pod, 0, "", pod.Spec.Containers[0].Image, kubecontainer.ContainerTypeRegular) + containerConfig, _, err := m.generateContainerConfig(&pod.Spec.Containers[0], pod, 0, "", pod.Spec.Containers[0].Image) assert.NoError(t, err) assert.Equal(t, expectedConfig, containerConfig, "generate container config for kubelet runtime v1.") assert.Equal(t, runAsUser, containerConfig.GetLinux().GetSecurityContext().GetRunAsUser().GetValue(), "RunAsUser should be set") @@ -121,7 +120,7 @@ func TestGenerateContainerConfig(t *testing.T) { }, } - _, _, err = m.generateContainerConfig(&podWithContainerSecurityContext.Spec.Containers[0], podWithContainerSecurityContext, 0, "", podWithContainerSecurityContext.Spec.Containers[0].Image, kubecontainer.ContainerTypeRegular) + _, _, err = m.generateContainerConfig(&podWithContainerSecurityContext.Spec.Containers[0], podWithContainerSecurityContext, 0, "", podWithContainerSecurityContext.Spec.Containers[0].Image) assert.Error(t, err) imageID, _ := imageService.PullImage(&runtimeapi.ImageSpec{Image: "busybox"}, nil) @@ -133,6 +132,6 @@ func TestGenerateContainerConfig(t *testing.T) { podWithContainerSecurityContext.Spec.Containers[0].SecurityContext.RunAsUser = nil podWithContainerSecurityContext.Spec.Containers[0].SecurityContext.RunAsNonRoot = &runAsNonRootTrue - _, _, err = m.generateContainerConfig(&podWithContainerSecurityContext.Spec.Containers[0], podWithContainerSecurityContext, 0, "", podWithContainerSecurityContext.Spec.Containers[0].Image, kubecontainer.ContainerTypeRegular) + _, _, err = m.generateContainerConfig(&podWithContainerSecurityContext.Spec.Containers[0], podWithContainerSecurityContext, 0, "", podWithContainerSecurityContext.Spec.Containers[0].Image) assert.Error(t, err, "RunAsNonRoot should fail for non-numeric username") } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go index 01df93d24f..fd0c92e096 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go @@ -323,7 +323,7 @@ func TestLifeCycleHook(t *testing.T) { } // Now try to create a container, which should in turn invoke PostStart Hook - _, err := m.startContainer(fakeSandBox.Id, fakeSandBoxConfig, testContainer, testPod, fakePodStatus, nil, "", kubecontainer.ContainerTypeRegular) + _, err := m.startContainer(fakeSandBox.Id, fakeSandBoxConfig, testContainer, testPod, fakePodStatus, nil, "") if err != nil { t.Errorf("startContainer erro =%v", err) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index 3e6313bf46..c60b398efd 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -726,7 +726,7 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, _ v1.PodStatus, podStat } klog.V(4).Infof("Creating init container %+v in pod %v", container, format.Pod(pod)) - if msg, err := m.startContainer(podSandboxID, podSandboxConfig, container, pod, podStatus, pullSecrets, podIP, kubecontainer.ContainerTypeInit); err != nil { + if msg, err := m.startContainer(podSandboxID, podSandboxConfig, container, pod, podStatus, pullSecrets, podIP); err != nil { startContainerResult.Fail(err, msg) utilruntime.HandleError(fmt.Errorf("init container start failed: %v: %s", err, msg)) return @@ -750,7 +750,7 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, _ v1.PodStatus, podStat } klog.V(4).Infof("Creating container %+v in pod %v", container, format.Pod(pod)) - if msg, err := m.startContainer(podSandboxID, podSandboxConfig, container, pod, podStatus, pullSecrets, podIP, kubecontainer.ContainerTypeRegular); err != nil { + if msg, err := m.startContainer(podSandboxID, podSandboxConfig, container, pod, podStatus, pullSecrets, podIP); err != nil { startContainerResult.Fail(err, msg) // known errors that are logged in other places are logged at higher levels here to avoid // repetitive log spam diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index fc02734d0e..e71c7d4a1a 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -70,7 +70,6 @@ type sandboxTemplate struct { type containerTemplate struct { pod *v1.Pod container *v1.Container - containerType kubecontainer.ContainerType sandboxAttempt uint32 attempt int createdAt int64 @@ -143,7 +142,7 @@ func makeFakeContainer(t *testing.T, m *kubeGenericRuntimeManager, template cont sandboxConfig, err := m.generatePodSandboxConfig(template.pod, template.sandboxAttempt) assert.NoError(t, err, "generatePodSandboxConfig for container template %+v", template) - containerConfig, _, err := m.generateContainerConfig(template.container, template.pod, template.attempt, "", template.container.Image, template.containerType) + containerConfig, _, err := m.generateContainerConfig(template.container, template.pod, template.attempt, "", template.container.Image) assert.NoError(t, err, "generateContainerConfig for container template %+v", template) podSandboxID := apitest.BuildSandboxName(sandboxConfig.Metadata) diff --git a/pkg/kubelet/kuberuntime/labels.go b/pkg/kubelet/kuberuntime/labels.go index 5f36e0b5d9..e3a67eb1aa 100644 --- a/pkg/kubelet/kuberuntime/labels.go +++ b/pkg/kubelet/kuberuntime/labels.go @@ -22,9 +22,7 @@ import ( "k8s.io/api/core/v1" kubetypes "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog" - "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/kubelet/util/format" @@ -58,7 +56,6 @@ type annotatedPodSandboxInfo struct { type labeledContainerInfo struct { ContainerName string - ContainerType kubecontainer.ContainerType PodName string PodNamespace string PodUID kubetypes.UID @@ -97,15 +94,12 @@ func newPodAnnotations(pod *v1.Pod) map[string]string { } // newContainerLabels creates container labels from v1.Container and v1.Pod. -func newContainerLabels(container *v1.Container, pod *v1.Pod, containerType kubecontainer.ContainerType) map[string]string { +func newContainerLabels(container *v1.Container, pod *v1.Pod) map[string]string { labels := map[string]string{} labels[types.KubernetesPodNameLabel] = pod.Name labels[types.KubernetesPodNamespaceLabel] = pod.Namespace labels[types.KubernetesPodUIDLabel] = string(pod.UID) labels[types.KubernetesContainerNameLabel] = container.Name - if utilfeature.DefaultFeatureGate.Enabled(features.DebugContainers) { - labels[types.KubernetesContainerTypeLabel] = string(containerType) - } return labels } @@ -181,16 +175,11 @@ func getPodSandboxInfoFromAnnotations(annotations map[string]string) *annotatedP // getContainerInfoFromLabels gets labeledContainerInfo from labels. func getContainerInfoFromLabels(labels map[string]string) *labeledContainerInfo { - var containerType kubecontainer.ContainerType - if utilfeature.DefaultFeatureGate.Enabled(features.DebugContainers) { - containerType = kubecontainer.ContainerType(getStringValueFromLabel(labels, types.KubernetesContainerTypeLabel)) - } return &labeledContainerInfo{ PodName: getStringValueFromLabel(labels, types.KubernetesPodNameLabel), PodNamespace: getStringValueFromLabel(labels, types.KubernetesPodNamespaceLabel), PodUID: kubetypes.UID(getStringValueFromLabel(labels, types.KubernetesPodUIDLabel)), ContainerName: getStringValueFromLabel(labels, types.KubernetesContainerNameLabel), - ContainerType: containerType, } } diff --git a/pkg/kubelet/kuberuntime/labels_test.go b/pkg/kubelet/kuberuntime/labels_test.go index ea7f586903..26b95d3311 100644 --- a/pkg/kubelet/kuberuntime/labels_test.go +++ b/pkg/kubelet/kuberuntime/labels_test.go @@ -23,9 +23,6 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - utilfeature "k8s.io/apiserver/pkg/util/feature" - utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" - "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) @@ -68,90 +65,27 @@ func TestContainerLabels(t *testing.T) { } var tests = []struct { - description string - featuresCreated bool // Features enabled when container is created - featuresStatus bool // Features enabled when container status is read - typeLabel kubecontainer.ContainerType - expected *labeledContainerInfo + description string + expected *labeledContainerInfo }{ - { - "Debug containers disabled", - false, - false, - "ignored", - &labeledContainerInfo{ - PodName: pod.Name, - PodNamespace: pod.Namespace, - PodUID: pod.UID, - ContainerName: container.Name, - ContainerType: "", - }, - }, { "Regular containers", - true, - true, - kubecontainer.ContainerTypeRegular, &labeledContainerInfo{ PodName: pod.Name, PodNamespace: pod.Namespace, PodUID: pod.UID, ContainerName: container.Name, - ContainerType: kubecontainer.ContainerTypeRegular, - }, - }, - { - "Init containers", - true, - true, - kubecontainer.ContainerTypeInit, - &labeledContainerInfo{ - PodName: pod.Name, - PodNamespace: pod.Namespace, - PodUID: pod.UID, - ContainerName: container.Name, - ContainerType: kubecontainer.ContainerTypeInit, - }, - }, - { - "Created without type label", - false, - true, - "ignored", - &labeledContainerInfo{ - PodName: pod.Name, - PodNamespace: pod.Namespace, - PodUID: pod.UID, - ContainerName: container.Name, - ContainerType: "", - }, - }, - { - "Created with type label, subsequently disabled", - true, - false, - kubecontainer.ContainerTypeRegular, - &labeledContainerInfo{ - PodName: pod.Name, - PodNamespace: pod.Namespace, - PodUID: pod.UID, - ContainerName: container.Name, - ContainerType: "", }, }, } // Test whether we can get right information from label for _, test := range tests { - func() { - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DebugContainers, test.featuresCreated)() - labels := newContainerLabels(container, pod, test.typeLabel) - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DebugContainers, test.featuresStatus)() - containerInfo := getContainerInfoFromLabels(labels) - if !reflect.DeepEqual(containerInfo, test.expected) { - t.Errorf("%v: expected %v, got %v", test.description, test.expected, containerInfo) - } - }() + labels := newContainerLabels(container, pod) + containerInfo := getContainerInfoFromLabels(labels) + if !reflect.DeepEqual(containerInfo, test.expected) { + t.Errorf("%v: expected %v, got %v", test.description, test.expected, containerInfo) + } } } diff --git a/pkg/kubelet/types/labels.go b/pkg/kubelet/types/labels.go index 67c84f6d61..c4dad6302e 100644 --- a/pkg/kubelet/types/labels.go +++ b/pkg/kubelet/types/labels.go @@ -21,7 +21,6 @@ const ( KubernetesPodNamespaceLabel = "io.kubernetes.pod.namespace" KubernetesPodUIDLabel = "io.kubernetes.pod.uid" KubernetesContainerNameLabel = "io.kubernetes.container.name" - KubernetesContainerTypeLabel = "io.kubernetes.container.type" ) func GetContainerName(labels map[string]string) string {