From ea212d5d49c64ee8b3ddd6ef075af47c7be7e05d Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Wed, 24 Jul 2019 16:24:26 +0000 Subject: [PATCH 1/5] Add support for ephemeral containers to the kubelet --- pkg/kubelet/container/helpers.go | 32 +-- pkg/kubelet/container/helpers_test.go | 70 +++++++ pkg/kubelet/container/ref.go | 13 ++ pkg/kubelet/kubelet_pods.go | 17 ++ .../kuberuntime/kuberuntime_manager.go | 81 +++++--- .../kuberuntime/kuberuntime_manager_test.go | 187 +++++++++++++++++- pkg/kubelet/server/server.go | 17 +- .../desired_state_of_world_populator.go | 18 +- 8 files changed, 355 insertions(+), 80 deletions(-) diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index d4eb114a42c..3919d9684dd 100644 --- a/pkg/kubelet/container/helpers.go +++ b/pkg/kubelet/container/helpers.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" + podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/kubelet/util/format" hashutil "k8s.io/kubernetes/pkg/util/hash" "k8s.io/kubernetes/third_party/forked/golang/expansion" @@ -278,29 +279,28 @@ func FormatPod(pod *Pod) string { // GetContainerSpec gets the container spec by containerName. func GetContainerSpec(pod *v1.Pod, containerName string) *v1.Container { - for i, c := range pod.Spec.Containers { + var containerSpec *v1.Container + podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool { if containerName == c.Name { - return &pod.Spec.Containers[i] + containerSpec = c + return false } - } - for i, c := range pod.Spec.InitContainers { - if containerName == c.Name { - return &pod.Spec.InitContainers[i] - } - } - return nil + return true + }) + return containerSpec } // HasPrivilegedContainer returns true if any of the containers in the pod are privileged. func HasPrivilegedContainer(pod *v1.Pod) bool { - for _, c := range append(pod.Spec.Containers, pod.Spec.InitContainers...) { - if c.SecurityContext != nil && - c.SecurityContext.Privileged != nil && - *c.SecurityContext.Privileged { - return true + var hasPrivileged bool + podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool { + if c.SecurityContext != nil && c.SecurityContext.Privileged != nil && *c.SecurityContext.Privileged { + hasPrivileged = true + return false } - } - return false + return true + }) + return hasPrivileged } // MakePortMappings creates internal port mapping from api port mapping. diff --git a/pkg/kubelet/container/helpers_test.go b/pkg/kubelet/container/helpers_test.go index 5a658241b3c..eff41d4e01d 100644 --- a/pkg/kubelet/container/helpers_test.go +++ b/pkg/kubelet/container/helpers_test.go @@ -20,10 +20,14 @@ import ( "reflect" "testing" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" ) func TestEnvVarsToMap(t *testing.T) { @@ -320,6 +324,72 @@ func TestExpandVolumeMountsWithSubpath(t *testing.T) { } +func TestGetContainerSpec(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, true)() + for _, tc := range []struct { + name string + havePod *v1.Pod + haveName string + wantContainer *v1.Container + }{ + { + name: "regular container", + havePod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + {Name: "plain-ole-container"}, + }, + InitContainers: []v1.Container{ + {Name: "init-container"}, + }, + }, + }, + haveName: "plain-ole-container", + wantContainer: &v1.Container{Name: "plain-ole-container"}, + }, + { + name: "init container", + havePod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + {Name: "plain-ole-container"}, + }, + InitContainers: []v1.Container{ + {Name: "init-container"}, + }, + }, + }, + haveName: "init-container", + wantContainer: &v1.Container{Name: "init-container"}, + }, + { + name: "ephemeral container", + havePod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + {Name: "plain-ole-container"}, + }, + InitContainers: []v1.Container{ + {Name: "init-container"}, + }, + EphemeralContainers: []v1.EphemeralContainer{ + {EphemeralContainerCommon: v1.EphemeralContainerCommon{ + Name: "debug-container", + }}, + }, + }, + }, + haveName: "debug-container", + wantContainer: &v1.Container{Name: "debug-container"}, + }, + } { + gotContainer := GetContainerSpec(tc.havePod, tc.haveName) + if diff := cmp.Diff(tc.wantContainer, gotContainer); diff != "" { + t.Errorf("GetContainerSpec for %q returned diff (-want +got):%v", tc.name, diff) + } + } +} + func TestShouldContainerBeRestarted(t *testing.T) { pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/kubelet/container/ref.go b/pkg/kubelet/container/ref.go index 99a1e003b3b..7e3f188bc88 100644 --- a/pkg/kubelet/container/ref.go +++ b/pkg/kubelet/container/ref.go @@ -20,8 +20,10 @@ import ( "fmt" "k8s.io/api/core/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" ref "k8s.io/client-go/tools/reference" "k8s.io/kubernetes/pkg/api/legacyscheme" + "k8s.io/kubernetes/pkg/features" ) var ImplicitContainerPrefix string = "implicitly required container " @@ -67,5 +69,16 @@ func fieldPath(pod *v1.Pod, container *v1.Container) (string, error) { return fmt.Sprintf("spec.initContainers{%s}", here.Name), nil } } + if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) { + for i := range pod.Spec.EphemeralContainers { + here := &pod.Spec.EphemeralContainers[i] + if here.Name == container.Name { + if here.Name == "" { + return fmt.Sprintf("spec.ephemeralContainers[%d]", i), nil + } + return fmt.Sprintf("spec.ephemeralContainers{%s}", here.Name), nil + } + } + } return "", fmt.Errorf("container %q not found in pod %s/%s", container.Name, pod.Namespace, pod.Name) } diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index e12f1e3eb76..a1a355379b2 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -1414,6 +1414,22 @@ func (kl *Kubelet) convertStatusToAPIStatus(pod *v1.Pod, podStatus *kubecontaine len(pod.Spec.InitContainers) > 0, true, ) + if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) { + var ecSpecs []v1.Container + for i := range pod.Spec.EphemeralContainers { + ecSpecs = append(ecSpecs, v1.Container(pod.Spec.EphemeralContainers[i].EphemeralContainerCommon)) + } + + // TODO(verb): By now we've iterated podStatus 3 times. We could refactor this to make a single + // pass through podStatus.ContainerStatuses + apiPodStatus.EphemeralContainerStatuses = kl.convertToAPIContainerStatuses( + pod, podStatus, + oldPodStatus.EphemeralContainerStatuses, + ecSpecs, + len(pod.Spec.InitContainers) > 0, + false, + ) + } // Preserves conditions not controlled by kubelet for _, c := range pod.Status.Conditions { @@ -1421,6 +1437,7 @@ func (kl *Kubelet) convertStatusToAPIStatus(pod *v1.Pod, podStatus *kubecontaine apiPodStatus.Conditions = append(apiPodStatus.Conditions, c) } } + return &apiPodStatus } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index 0a5e666da69..3cda1f3cb6c 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -30,6 +30,7 @@ import ( kubetypes "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" utilversion "k8s.io/apimachinery/pkg/util/version" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/record" ref "k8s.io/client-go/tools/reference" "k8s.io/client-go/util/flowcontrol" @@ -37,6 +38,7 @@ import ( runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/credentialprovider" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/cm" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/events" @@ -375,7 +377,7 @@ type containerToKillInfo struct { // podActions keeps information what to do for a pod. type podActions struct { - // Stop all running (regular and init) containers and the sandbox for the pod. + // Stop all running (regular, init and ephemeral) containers and the sandbox for the pod. KillPod bool // Whether need to create a new sandbox. If needed to kill pod and create // a new pod sandbox, all init containers need to be purged (i.e., removed). @@ -395,6 +397,9 @@ type podActions struct { // the key is the container ID of the container, while // the value contains necessary information to kill a container. ContainersToKill map[kubecontainer.ContainerID]containerToKillInfo + // EphemeralContainersToStart is a list of indexes for the ephemeral containers to start, + // where the index is the index of the specific container in pod.Spec.EphemeralContainers. + EphemeralContainersToStart []int } // podSandboxChanged checks whether the spec of the pod is changed and returns @@ -500,6 +505,18 @@ func (m *kubeGenericRuntimeManager) computePodActions(pod *v1.Pod, podStatus *ku return changes } + // Ephemeral containers may be started even if initialization is not yet complete. + if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) { + for i := range pod.Spec.EphemeralContainers { + c := (*v1.Container)(&pod.Spec.EphemeralContainers[i].EphemeralContainerCommon) + + // Ephemeral Containers are never restarted + if podStatus.FindContainerStatusByName(c.Name) == nil { + changes.EphemeralContainersToStart = append(changes.EphemeralContainersToStart, i) + } + } + } + // Check initialization progress. initLastStatus, next, done := findNextInitContainerToRun(pod, podStatus) if !done { @@ -610,6 +627,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(pod *v1.Pod, podStatus *ku // 4. Create sandbox if necessary. // 5. Create init containers. // 6. Create normal containers. +// 7. Create ephemeral containers. func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontainer.PodStatus, pullSecrets []v1.Secret, backOff *flowcontrol.Backoff) (result kubecontainer.PodSyncResult) { // Step 1: Compute sandbox and container changes. podContainerChanges := m.computePodActions(pod, podStatus) @@ -739,22 +757,40 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine return } - // Step 5: start the init container. - if container := podContainerChanges.NextInitContainerToStart; container != nil { - // Start the next init container. + // Helper containing boilerplate common to starting all types of containers. + // typeName is a label used to describe this type of container in log messages. + start := func(typeName string, container *v1.Container) error { startContainerResult := kubecontainer.NewSyncResult(kubecontainer.StartContainer, container.Name) result.AddSyncResult(startContainerResult) + isInBackOff, msg, err := m.doBackOff(pod, container, podStatus, backOff) if isInBackOff { startContainerResult.Fail(err, msg) - klog.V(4).Infof("Backing Off restarting init container %+v in pod %v", container, format.Pod(pod)) - return + klog.V(4).Infof("Backing Off restarting %v %+v in pod %v", typeName, container, format.Pod(pod)) + return err } - klog.V(4).Infof("Creating init container %+v in pod %v", container, format.Pod(pod)) + klog.V(4).Infof("Creating %v %+v in pod %v", typeName, container, format.Pod(pod)) 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)) + // known errors that are logged in other places are logged at higher levels here to avoid + // repetitive log spam + switch { + case err == images.ErrImagePullBackOff: + klog.V(3).Infof("%v start failed: %v: %s", typeName, err, msg) + default: + utilruntime.HandleError(fmt.Errorf("%v start failed: %v: %s", typeName, err, msg)) + } + return err + } + + return nil + } + + // Step 5: start the init container. + if container := podContainerChanges.NextInitContainerToStart; container != nil { + // Start the next init container. + if err := start("init container", container); err != nil { return } @@ -764,29 +800,14 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine // Step 6: start containers in podContainerChanges.ContainersToStart. for _, idx := range podContainerChanges.ContainersToStart { - container := &pod.Spec.Containers[idx] - startContainerResult := kubecontainer.NewSyncResult(kubecontainer.StartContainer, container.Name) - result.AddSyncResult(startContainerResult) + start("container", &pod.Spec.Containers[idx]) + } - isInBackOff, msg, err := m.doBackOff(pod, container, podStatus, backOff) - if isInBackOff { - startContainerResult.Fail(err, msg) - klog.V(4).Infof("Backing Off restarting container %+v in pod %v", container, format.Pod(pod)) - continue - } - - 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); 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 - switch { - case err == images.ErrImagePullBackOff: - klog.V(3).Infof("container start failed: %v: %s", err, msg) - default: - utilruntime.HandleError(fmt.Errorf("container start failed: %v: %s", err, msg)) - } - continue + // Step 7: start ephemeral containers + if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) { + for _, idx := range podContainerChanges.EphemeralContainersToStart { + c := (*v1.Container)(&pod.Spec.EphemeralContainers[idx].EphemeralContainerCommon) + start("ephemeral container", c) } } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index 8652350f69f..166eb794547 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -31,10 +31,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/util/flowcontrol" + featuregatetesting "k8s.io/component-base/featuregate/testing" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" apitest "k8s.io/cri-api/pkg/apis/testing" + podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/credentialprovider" + "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" ) @@ -96,12 +100,10 @@ func makeAndSetFakePod(t *testing.T, m *kubeGenericRuntimeManager, fakeRuntime * state: runtimeapi.ContainerState_CONTAINER_RUNNING, } } - for i := range pod.Spec.Containers { - containers = append(containers, makeFakeContainer(t, m, newTemplate(&pod.Spec.Containers[i]))) - } - for i := range pod.Spec.InitContainers { - containers = append(containers, makeFakeContainer(t, m, newTemplate(&pod.Spec.InitContainers[i]))) - } + podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool { + containers = append(containers, makeFakeContainer(t, m, newTemplate(c))) + return true + }) fakeRuntime.SetFakeSandboxes([]*apitest.FakePodSandbox{sandbox}) fakeRuntime.SetFakeContainers(containers) @@ -402,6 +404,9 @@ func TestGetPods(t *testing.T) { } func TestKillPod(t *testing.T) { + // Tests that KillPod also kills Ephemeral Containers + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, true)() + fakeRuntime, _, m, err := createTestRuntimeManager() assert.NoError(t, err) @@ -422,6 +427,14 @@ func TestKillPod(t *testing.T) { Image: "busybox", }, }, + EphemeralContainers: []v1.EphemeralContainer{ + { + EphemeralContainerCommon: v1.EphemeralContainerCommon{ + Name: "debug", + Image: "busybox", + }, + }, + }, }, } @@ -449,7 +462,7 @@ func TestKillPod(t *testing.T) { ID: pod.UID, Name: pod.Name, Namespace: pod.Namespace, - Containers: []*kubecontainer.Container{containers[0], containers[1]}, + Containers: []*kubecontainer.Container{containers[0], containers[1], containers[2]}, Sandboxes: []*kubecontainer.Container{ { ID: kubecontainer.ContainerID{ @@ -462,7 +475,7 @@ func TestKillPod(t *testing.T) { err = m.KillPod(pod, runningPod, nil) assert.NoError(t, err) - assert.Equal(t, 2, len(fakeRuntime.Containers)) + assert.Equal(t, 3, len(fakeRuntime.Containers)) assert.Equal(t, 1, len(fakeRuntime.Sandboxes)) for _, sandbox := range fakeRuntime.Sandboxes { assert.Equal(t, runtimeapi.PodSandboxState_SANDBOX_NOTREADY, sandbox.State) @@ -944,7 +957,7 @@ func TestComputePodActionsWithInitContainers(t *testing.T) { _, _, m, err := createTestRuntimeManager() require.NoError(t, err) - // Createing a pair reference pod and status for the test cases to refer + // Creating a pair reference pod and status for the test cases to refer // the specific fields. basePod, baseStatus := makeBasePodAndStatusWithInitContainers() noAction := podActions{ @@ -1139,3 +1152,159 @@ func makeBasePodAndStatusWithInitContainers() (*v1.Pod, *kubecontainer.PodStatus } return pod, status } + +func TestComputePodActionsWithInitAndEphemeralContainers(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, true)() + // Make sure existing test cases pass with feature enabled + TestComputePodActions(t) + TestComputePodActionsWithInitContainers(t) + + _, _, m, err := createTestRuntimeManager() + require.NoError(t, err) + + basePod, baseStatus := makeBasePodAndStatusWithInitAndEphemeralContainers() + noAction := podActions{ + SandboxID: baseStatus.SandboxStatuses[0].Id, + ContainersToStart: []int{}, + ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{}, + } + + for desc, test := range map[string]struct { + mutatePodFn func(*v1.Pod) + mutateStatusFn func(*kubecontainer.PodStatus) + actions podActions + }{ + "steady state; do nothing; ignore ephemeral container": { + actions: noAction, + }, + "No ephemeral containers running; start one": { + mutateStatusFn: func(status *kubecontainer.PodStatus) { + status.ContainerStatuses = status.ContainerStatuses[:4] + }, + actions: podActions{ + SandboxID: baseStatus.SandboxStatuses[0].Id, + ContainersToStart: []int{}, + ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{}, + EphemeralContainersToStart: []int{0}, + }, + }, + "Start second ephemeral container": { + mutatePodFn: func(pod *v1.Pod) { + pod.Spec.EphemeralContainers = append(pod.Spec.EphemeralContainers, v1.EphemeralContainer{ + EphemeralContainerCommon: v1.EphemeralContainerCommon{ + Name: "debug2", + Image: "busybox", + }, + }) + }, + actions: podActions{ + SandboxID: baseStatus.SandboxStatuses[0].Id, + ContainersToStart: []int{}, + ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{}, + EphemeralContainersToStart: []int{1}, + }, + }, + "Ephemeral container exited; do not restart": { + mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways }, + mutateStatusFn: func(status *kubecontainer.PodStatus) { + status.ContainerStatuses[4].State = kubecontainer.ContainerStateExited + }, + actions: podActions{ + SandboxID: baseStatus.SandboxStatuses[0].Id, + ContainersToStart: []int{}, + ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{}, + }, + }, + "initialization in progress; start ephemeral container": { + mutateStatusFn: func(status *kubecontainer.PodStatus) { + status.ContainerStatuses[3].State = kubecontainer.ContainerStateRunning + status.ContainerStatuses = status.ContainerStatuses[:4] + }, + actions: podActions{ + SandboxID: baseStatus.SandboxStatuses[0].Id, + ContainersToStart: []int{}, + ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{}, + EphemeralContainersToStart: []int{0}, + }, + }, + "Kill pod and do not restart ephemeral container if the pod sandbox is dead": { + mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways }, + mutateStatusFn: func(status *kubecontainer.PodStatus) { + status.SandboxStatuses[0].State = runtimeapi.PodSandboxState_SANDBOX_NOTREADY + }, + actions: podActions{ + KillPod: true, + CreateSandbox: true, + SandboxID: baseStatus.SandboxStatuses[0].Id, + Attempt: uint32(1), + NextInitContainerToStart: &basePod.Spec.InitContainers[0], + ContainersToStart: []int{}, + ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}), + }, + }, + "Kill pod if all containers exited except ephemeral container": { + mutatePodFn: func(pod *v1.Pod) { + pod.Spec.RestartPolicy = v1.RestartPolicyNever + }, + mutateStatusFn: func(status *kubecontainer.PodStatus) { + // all regular containers exited + for i := 0; i < 3; i++ { + status.ContainerStatuses[i].State = kubecontainer.ContainerStateExited + status.ContainerStatuses[i].ExitCode = 0 + } + }, + actions: podActions{ + SandboxID: baseStatus.SandboxStatuses[0].Id, + CreateSandbox: false, + KillPod: true, + ContainersToStart: []int{}, + ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{}, + }, + }, + "Ephemeral container is in unknown state; leave it alone": { + mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyNever }, + mutateStatusFn: func(status *kubecontainer.PodStatus) { + status.ContainerStatuses[4].State = kubecontainer.ContainerStateUnknown + }, + actions: noAction, + }, + } { + pod, status := makeBasePodAndStatusWithInitAndEphemeralContainers() + if test.mutatePodFn != nil { + test.mutatePodFn(pod) + } + if test.mutateStatusFn != nil { + test.mutateStatusFn(status) + } + actions := m.computePodActions(pod, status) + verifyActions(t, &test.actions, &actions, desc) + } +} + +func makeBasePodAndStatusWithInitAndEphemeralContainers() (*v1.Pod, *kubecontainer.PodStatus) { + pod, status := makeBasePodAndStatus() + pod.Spec.InitContainers = []v1.Container{ + { + Name: "init1", + Image: "bar-image", + }, + } + pod.Spec.EphemeralContainers = []v1.EphemeralContainer{ + { + EphemeralContainerCommon: v1.EphemeralContainerCommon{ + Name: "debug", + Image: "busybox", + }, + }, + } + status.ContainerStatuses = append(status.ContainerStatuses, &kubecontainer.ContainerStatus{ + ID: kubecontainer.ContainerID{ID: "initid1"}, + Name: "init1", State: kubecontainer.ContainerStateExited, + Hash: kubecontainer.HashContainer(&pod.Spec.InitContainers[0]), + }, &kubecontainer.ContainerStatus{ + ID: kubecontainer.ContainerID{ID: "debug1"}, + Name: "debug", State: kubecontainer.ContainerStateRunning, + Hash: kubecontainer.HashContainer((*v1.Container)(&pod.Spec.EphemeralContainers[0].EphemeralContainerCommon)), + }) + return pod, status +} diff --git a/pkg/kubelet/server/server.go b/pkg/kubelet/server/server.go index aca56886189..6ee3653309f 100644 --- a/pkg/kubelet/server/server.go +++ b/pkg/kubelet/server/server.go @@ -555,22 +555,7 @@ func (s *Server) getContainerLogs(request *restful.Request, response *restful.Re return } // Check if containerName is valid. - containerExists := false - for _, container := range pod.Spec.Containers { - if container.Name == containerName { - containerExists = true - break - } - } - if !containerExists { - for _, container := range pod.Spec.InitContainers { - if container.Name == containerName { - containerExists = true - break - } - } - } - if !containerExists { + if kubecontainer.GetContainerSpec(pod, containerName) == nil { response.WriteError(http.StatusNotFound, fmt.Errorf("container %q not found in pod %q", containerName, podID)) return } diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index f5aa5712fb4..63cb854f46a 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" + podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/config" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -396,17 +397,16 @@ func mountedReadOnlyByPod(podVolume v1.Volume, pod *v1.Pod) bool { if podVolume.PersistentVolumeClaim.ReadOnly { return true } - for _, container := range pod.Spec.InitContainers { - if !mountedReadOnlyByContainer(podVolume.Name, &container) { + + mountedReadOnly := true + podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool { + if !mountedReadOnlyByContainer(podVolume.Name, c) { + mountedReadOnly = false return false } - } - for _, container := range pod.Spec.Containers { - if !mountedReadOnlyByContainer(podVolume.Name, &container) { - return false - } - } - return true + return true + }) + return mountedReadOnly } func mountedReadOnlyByContainer(volumeName string, container *v1.Container) bool { From 7bce18b0cec2a9510e2fab946e659ceb42ffeca5 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 12 Apr 2019 17:00:17 +0200 Subject: [PATCH 2/5] Generated code for Ephemeral Containers in kubelet --- pkg/kubelet/container/BUILD | 7 +++++++ pkg/kubelet/kuberuntime/BUILD | 1 + pkg/kubelet/volumemanager/populator/BUILD | 1 + 3 files changed, 9 insertions(+) diff --git a/pkg/kubelet/container/BUILD b/pkg/kubelet/container/BUILD index 7fa05b91dfc..46e364d8272 100644 --- a/pkg/kubelet/container/BUILD +++ b/pkg/kubelet/container/BUILD @@ -19,6 +19,8 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/api/legacyscheme:go_default_library", + "//pkg/api/v1/pod:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubelet/util/format:go_default_library", "//pkg/util/hash:go_default_library", "//pkg/volume:go_default_library", @@ -29,6 +31,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_library", "//staging/src/k8s.io/client-go/tools/reference:go_default_library", "//staging/src/k8s.io/client-go/tools/remotecommand:go_default_library", @@ -51,10 +54,14 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/core/install:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubelet/container/testing:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], ) diff --git a/pkg/kubelet/kuberuntime/BUILD b/pkg/kubelet/kuberuntime/BUILD index 4e10569e8c7..444b45199b1 100644 --- a/pkg/kubelet/kuberuntime/BUILD +++ b/pkg/kubelet/kuberuntime/BUILD @@ -100,6 +100,7 @@ go_test( ], embed = [":go_default_library"], deps = [ + "//pkg/api/v1/pod:go_default_library", "//pkg/credentialprovider:go_default_library", "//pkg/features:go_default_library", "//pkg/kubelet/container:go_default_library", diff --git a/pkg/kubelet/volumemanager/populator/BUILD b/pkg/kubelet/volumemanager/populator/BUILD index 68a003ac6fe..74e549d6850 100644 --- a/pkg/kubelet/volumemanager/populator/BUILD +++ b/pkg/kubelet/volumemanager/populator/BUILD @@ -11,6 +11,7 @@ go_library( srcs = ["desired_state_of_world_populator.go"], importpath = "k8s.io/kubernetes/pkg/kubelet/volumemanager/populator", deps = [ + "//pkg/api/v1/pod:go_default_library", "//pkg/features:go_default_library", "//pkg/kubelet/config:go_default_library", "//pkg/kubelet/container:go_default_library", From 906286c743ec18cd428aa78a766fd46157fceb97 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 2 Aug 2019 19:33:49 +0000 Subject: [PATCH 3/5] Change order kubelet starts containers This starts ephemeral containers prior to init containers so that ephemeral containers will still be started when init containers fail to start. Also improves tests and comments with review suggestions. --- pkg/kubelet/container/helpers_test.go | 10 +++--- pkg/kubelet/kubelet_pods.go | 2 +- .../kuberuntime/kuberuntime_manager.go | 32 +++++++++++-------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/pkg/kubelet/container/helpers_test.go b/pkg/kubelet/container/helpers_test.go index eff41d4e01d..3acb1120cbd 100644 --- a/pkg/kubelet/container/helpers_test.go +++ b/pkg/kubelet/container/helpers_test.go @@ -383,10 +383,12 @@ func TestGetContainerSpec(t *testing.T) { wantContainer: &v1.Container{Name: "debug-container"}, }, } { - gotContainer := GetContainerSpec(tc.havePod, tc.haveName) - if diff := cmp.Diff(tc.wantContainer, gotContainer); diff != "" { - t.Errorf("GetContainerSpec for %q returned diff (-want +got):%v", tc.name, diff) - } + t.Run(tc.name, func(t *testing.T) { + gotContainer := GetContainerSpec(tc.havePod, tc.haveName) + if diff := cmp.Diff(tc.wantContainer, gotContainer); diff != "" { + t.Fatalf("GetContainerSpec for %q returned diff (-want +got):%v", tc.name, diff) + } + }) } } diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index a1a355379b2..d59828aaa37 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -1420,7 +1420,7 @@ func (kl *Kubelet) convertStatusToAPIStatus(pod *v1.Pod, podStatus *kubecontaine ecSpecs = append(ecSpecs, v1.Container(pod.Spec.EphemeralContainers[i].EphemeralContainerCommon)) } - // TODO(verb): By now we've iterated podStatus 3 times. We could refactor this to make a single + // #80875: By now we've iterated podStatus 3 times. We could refactor this to make a single // pass through podStatus.ContainerStatuses apiPodStatus.EphemeralContainerStatuses = kl.convertToAPIContainerStatuses( pod, podStatus, diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index 3cda1f3cb6c..af235957365 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -625,9 +625,9 @@ func (m *kubeGenericRuntimeManager) computePodActions(pod *v1.Pod, podStatus *ku // 2. Kill pod sandbox if necessary. // 3. Kill any containers that should not be running. // 4. Create sandbox if necessary. -// 5. Create init containers. -// 6. Create normal containers. -// 7. Create ephemeral containers. +// 5. Create ephemeral containers. +// 6. Create init containers. +// 7. Create normal containers. func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontainer.PodStatus, pullSecrets []v1.Secret, backOff *flowcontrol.Backoff) (result kubecontainer.PodSyncResult) { // Step 1: Compute sandbox and container changes. podContainerChanges := m.computePodActions(pod, podStatus) @@ -758,7 +758,8 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine } // Helper containing boilerplate common to starting all types of containers. - // typeName is a label used to describe this type of container in log messages. + // typeName is a label used to describe this type of container in log messages, + // currently: "container", "init container" or "ephemeral container" start := func(typeName string, container *v1.Container) error { startContainerResult := kubecontainer.NewSyncResult(kubecontainer.StartContainer, container.Name) result.AddSyncResult(startContainerResult) @@ -787,7 +788,18 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine return nil } - // Step 5: start the init container. + // Step 5: start ephemeral containers + // These are started "prior" to init containers to allow running ephemeral containers even when there + // are errors starting an init container. In practice init containers will start first since ephemeral + // containers cannot be specified on pod creation. + if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) { + for _, idx := range podContainerChanges.EphemeralContainersToStart { + c := (*v1.Container)(&pod.Spec.EphemeralContainers[idx].EphemeralContainerCommon) + start("ephemeral container", c) + } + } + + // Step 6: start the init container. if container := podContainerChanges.NextInitContainerToStart; container != nil { // Start the next init container. if err := start("init container", container); err != nil { @@ -798,19 +810,11 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine klog.V(4).Infof("Completed init container %q for pod %q", container.Name, format.Pod(pod)) } - // Step 6: start containers in podContainerChanges.ContainersToStart. + // Step 7: start containers in podContainerChanges.ContainersToStart. for _, idx := range podContainerChanges.ContainersToStart { start("container", &pod.Spec.Containers[idx]) } - // Step 7: start ephemeral containers - if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) { - for _, idx := range podContainerChanges.EphemeralContainersToStart { - c := (*v1.Container)(&pod.Spec.EphemeralContainers[idx].EphemeralContainerCommon) - start("ephemeral container", c) - } - } - return } From b5d592e7353d8163f1f99881ee2f2bc14d20d0b2 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Sat, 3 Aug 2019 14:13:46 +0000 Subject: [PATCH 4/5] Simplify VisitContainers pattern in volumemanager populator --- .../populator/desired_state_of_world_populator.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index 63cb854f46a..bf7ae114984 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -398,15 +398,12 @@ func mountedReadOnlyByPod(podVolume v1.Volume, pod *v1.Pod) bool { return true } - mountedReadOnly := true - podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool { + return podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool { if !mountedReadOnlyByContainer(podVolume.Name, c) { - mountedReadOnly = false return false } return true }) - return mountedReadOnly } func mountedReadOnlyByContainer(volumeName string, container *v1.Container) bool { From cb3976c02ba5040fe4194d39f34045038caff28e Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Tue, 20 Aug 2019 17:45:38 +0000 Subject: [PATCH 5/5] Fix returning logs from ephemeral containers --- pkg/kubelet/kubelet_pods.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index d59828aaa37..bab117f6178 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -1108,10 +1108,12 @@ func (kl *Kubelet) validateContainerLogStatus(podName string, podStatus *v1.PodS var cID string cStatus, found := podutil.GetContainerStatus(podStatus.ContainerStatuses, containerName) - // if not found, check the init containers if !found { cStatus, found = podutil.GetContainerStatus(podStatus.InitContainerStatuses, containerName) } + if !found && utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) { + cStatus, found = podutil.GetContainerStatus(podStatus.EphemeralContainerStatuses, containerName) + } if !found { return kubecontainer.ContainerID{}, fmt.Errorf("container %q in pod %q is not available", containerName, podName) }