From d4325f42fb9ae7c61c409f16954cba0375308d8b Mon Sep 17 00:00:00 2001 From: "Keerthan Reddy,Mala" Date: Fri, 29 May 2020 12:39:22 -0700 Subject: [PATCH] Check for sandboxes before deleting the pod from apiserver --- pkg/kubelet/BUILD | 11 +--- pkg/kubelet/container/runtime.go | 1 - pkg/kubelet/cri/remote/fake/fake_runtime.go | 2 +- pkg/kubelet/kubelet_pods.go | 10 +++ pkg/kubelet/kubelet_pods_test.go | 68 +++++++++++++++++++++ 5 files changed, 81 insertions(+), 11 deletions(-) diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD index 7efbf4a6927..ea8482725ed 100644 --- a/pkg/kubelet/BUILD +++ b/pkg/kubelet/BUILD @@ -254,20 +254,13 @@ go_test( "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//staging/src/k8s.io/component-base/version:go_default_library", "//vendor/github.com/golang/groupcache/lru:go_default_library", + "//staging/src/k8s.io/cri-api/pkg/apis/runtime/v1alpha2:go_default_library", "//vendor/github.com/google/cadvisor/info/v1:go_default_library", "//vendor/github.com/google/cadvisor/info/v2:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require:go_default_library", "//vendor/k8s.io/utils/mount:go_default_library", - ] + select({ - "@io_bazel_rules_go//go/platform:android": [ - "//staging/src/k8s.io/cri-api/pkg/apis/runtime/v1alpha2:go_default_library", - ], - "@io_bazel_rules_go//go/platform:linux": [ - "//staging/src/k8s.io/cri-api/pkg/apis/runtime/v1alpha2:go_default_library", - ], - "//conditions:default": [], - }), + ], ) filegroup( diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 30f815b0718..c280d5f57c7 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -299,7 +299,6 @@ type PodStatus struct { // Status of containers in the pod. ContainerStatuses []*Status // Status of the pod sandbox. - // Only for kuberuntime now, other runtime may keep it nil. SandboxStatuses []*runtimeapi.PodSandboxStatus } diff --git a/pkg/kubelet/cri/remote/fake/fake_runtime.go b/pkg/kubelet/cri/remote/fake/fake_runtime.go index e49f311aa18..339561f9b8e 100644 --- a/pkg/kubelet/cri/remote/fake/fake_runtime.go +++ b/pkg/kubelet/cri/remote/fake/fake_runtime.go @@ -112,7 +112,7 @@ func (f *RemoteRuntime) StopPodSandbox(ctx context.Context, req *kubeapi.StopPod // This call is idempotent, and must not return an error if the sandbox has // already been removed. func (f *RemoteRuntime) RemovePodSandbox(ctx context.Context, req *kubeapi.RemovePodSandboxRequest) (*kubeapi.RemovePodSandboxResponse, error) { - err := f.RuntimeService.StopPodSandbox(req.PodSandboxId) + err := f.RuntimeService.RemovePodSandbox(req.PodSandboxId) if err != nil { return nil, err } diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 386f82d1591..2d56d3df6f7 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -969,6 +969,16 @@ func (kl *Kubelet) PodResourcesAreReclaimed(pod *v1.Pod, status v1.PodStatus) bo klog.V(3).Infof("Pod %q is terminated, but some containers have not been cleaned up: %s", format.Pod(pod), statusStr) return false } + // pod's sandboxes should be deleted + if len(runtimeStatus.SandboxStatuses) > 0 { + var sandboxStr string + for _, sandbox := range runtimeStatus.SandboxStatuses { + sandboxStr += fmt.Sprintf("%+v ", *sandbox) + } + klog.V(3).Infof("Pod %q is terminated, but some pod sandboxes have not been cleaned up: %s", format.Pod(pod), sandboxStr) + return false + } + if kl.podVolumesExist(pod.UID) && !kl.keepTerminatedPodVolumes { // We shouldn't delete pods whose volumes have not been cleaned up if we are not keeping terminated pod volumes klog.V(3).Infof("Pod %q is terminated, but some volumes have not been cleaned up", format.Pod(pod)) diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 467c21f4440..f49301eae6f 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -42,6 +42,7 @@ import ( // api.Registry.GroupOrDie(v1.GroupName).GroupVersions[0].String() is changed // to "v1"? + runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" _ "k8s.io/kubernetes/pkg/apis/core/install" "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -2421,3 +2422,70 @@ func TestTruncatePodHostname(t *testing.T) { assert.Equal(t, test.output, output) } } + +func TestPodResourcesAreReclaimed(t *testing.T) { + + type args struct { + pod *v1.Pod + status v1.PodStatus + runtimeStatus kubecontainer.PodStatus + } + tests := []struct { + name string + args args + want bool + }{ + { + "pod with running containers", + args{ + pod: &v1.Pod{}, + status: v1.PodStatus{ + ContainerStatuses: []v1.ContainerStatus{ + runningState("containerA"), + runningState("containerB"), + }, + }, + }, + false, + }, + { + "pod with containers in runtime cache", + args{ + pod: &v1.Pod{}, + status: v1.PodStatus{}, + runtimeStatus: kubecontainer.PodStatus{ + ContainerStatuses: []*kubecontainer.ContainerStatus{ + {}, + }, + }, + }, + false, + }, + { + "pod with sandbox present", + args{ + pod: &v1.Pod{}, + status: v1.PodStatus{}, + runtimeStatus: kubecontainer.PodStatus{ + SandboxStatuses: []*runtimeapi.PodSandboxStatus{ + {}, + }, + }, + }, + false, + }, + } + + testKubelet := newTestKubelet(t, false) + defer testKubelet.Cleanup() + kl := testKubelet.kubelet + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testKubelet.fakeRuntime.PodStatus = tt.args.runtimeStatus + if got := kl.PodResourcesAreReclaimed(tt.args.pod, tt.args.status); got != tt.want { + t.Errorf("PodResourcesAreReclaimed() = %v, want %v", got, tt.want) + } + }) + } +}