Merge pull request kubernetes#89667 from kmala/kubelet

Check for sandboxes before deleting the pod from apiserver

Author: k8s-ci-robot

pkg/kubelet/BUILD

@@ -250,20 +250,13 @@ go_test(
         "//staging/src/k8s.io/client-go/util/testing:go_default_library",
         "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
         "//staging/src/k8s.io/component-base/version: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(

pkg/kubelet/container/runtime.go

@@ -285,7 +285,6 @@ type PodStatus struct {
 	// Status of containers in the pod.
 	ContainerStatuses []*ContainerStatus
 	// Status of the pod sandbox.
-	// Only for kuberuntime now, other runtime may keep it nil.
 	SandboxStatuses []*runtimeapi.PodSandboxStatus
 }
 

pkg/kubelet/kubelet_pods.go

@@ -940,6 +940,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))

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"
@@ -2385,3 +2386,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)
+			}
+		})
+	}
+}

pkg/kubelet/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
 	}