Merge pull request #44150 from lukaszo/claim-time

Automatic merge from submit-queue

Skip pods marked for deletion

Fixes #44144

pkg/controller/BUILD

@@ -61,7 +61,10 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["controller_utils_test.go"],
+    srcs = [
+        "controller_ref_manager_test.go",
+        "controller_utils_test.go",
+    ],
     library = ":go_default_library",
     tags = ["automanaged"],
     deps = [
@@ -73,9 +76,13 @@ go_test(
         "//pkg/securitycontext:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
+        "//vendor/k8s.io/client-go/pkg/apis/extensions:go_default_library",
         "//vendor/k8s.io/client-go/rest:go_default_library",
         "//vendor/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/k8s.io/client-go/tools/record:go_default_library",

pkg/controller/controller_ref_manager.go

@@ -113,6 +113,10 @@ func (m *baseControllerRefManager) claimObject(obj metav1.Object, match func(met
 		// Ignore if we're being deleted or selector doesn't match.
 		return false, nil
 	}
+	if obj.GetDeletionTimestamp() != nil {
+		// Ignore if the object is being deleted
+		return false, nil
+	}
 	// Selector matches. Try to adopt.
 	if err := adopt(obj); err != nil {
 		// If the pod no longer exists, ignore the error.

pkg/controller/controller_ref_manager_test.go

@@ -0,0 +1,196 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	"reflect"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/api/v1"
+)
+
+var (
+	productionLabel         = map[string]string{"type": "production"}
+	testLabel               = map[string]string{"type": "testing"}
+	productionLabelSelector = labels.Set{"type": "production"}.AsSelector()
+	testLabelSelector       = labels.Set{"type": "testing"}.AsSelector()
+	controllerUID           = "123"
+)
+
+func newControllerRef(controller metav1.Object) *metav1.OwnerReference {
+	var controllerKind = extensions.SchemeGroupVersion.WithKind("Fake")
+	blockOwnerDeletion := true
+	isController := true
+	return &metav1.OwnerReference{
+		APIVersion:         controllerKind.GroupVersion().String(),
+		Kind:               controllerKind.Kind,
+		Name:               "Fake",
+		UID:                controller.GetUID(),
+		BlockOwnerDeletion: &blockOwnerDeletion,
+		Controller:         &isController,
+	}
+}
+
+func newPod(podName string, label map[string]string, owner metav1.Object) *v1.Pod {
+	pod := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      podName,
+			Labels:    label,
+			Namespace: metav1.NamespaceDefault,
+		},
+		Spec: v1.PodSpec{
+			Containers: []v1.Container{
+				{
+					Image: "foo/bar",
+				},
+			},
+		},
+	}
+	if owner != nil {
+		pod.OwnerReferences = []metav1.OwnerReference{*newControllerRef(owner)}
+	}
+	return pod
+}
+
+func TestClaimPods(t *testing.T) {
+	controllerKind := schema.GroupVersionKind{}
+	type test struct {
+		name        string
+		manager     *PodControllerRefManager
+		pods        []*v1.Pod
+		filters     []func(*v1.Pod) bool
+		claimed     []*v1.Pod
+		released    []*v1.Pod
+		expectError bool
+	}
+	var tests = []test{
+		{
+			name: "Claim pods with correct label",
+			manager: NewPodControllerRefManager(&FakePodControl{},
+				&v1.ReplicationController{},
+				productionLabelSelector,
+				controllerKind,
+				func() error { return nil }),
+			pods:    []*v1.Pod{newPod("pod1", productionLabel, nil), newPod("pod2", testLabel, nil)},
+			claimed: []*v1.Pod{newPod("pod1", productionLabel, nil)},
+		},
+		func() test {
+			controller := v1.ReplicationController{}
+			controller.UID = types.UID(controllerUID)
+			now := metav1.Now()
+			controller.DeletionTimestamp = &now
+			return test{
+				name: "Controller marked for deletion can not claim pods",
+				manager: NewPodControllerRefManager(&FakePodControl{},
+					&controller,
+					productionLabelSelector,
+					controllerKind,
+					func() error { return nil }),
+				pods:    []*v1.Pod{newPod("pod1", productionLabel, nil), newPod("pod2", productionLabel, nil)},
+				claimed: nil,
+			}
+		}(),
+		func() test {
+			controller := v1.ReplicationController{}
+			controller.UID = types.UID(controllerUID)
+			now := metav1.Now()
+			controller.DeletionTimestamp = &now
+			return test{
+				name: "Controller marked for deletion can not claim new pods",
+				manager: NewPodControllerRefManager(&FakePodControl{},
+					&controller,
+					productionLabelSelector,
+					controllerKind,
+					func() error { return nil }),
+				pods:    []*v1.Pod{newPod("pod1", productionLabel, &controller), newPod("pod2", productionLabel, nil)},
+				claimed: []*v1.Pod{newPod("pod1", productionLabel, &controller)},
+			}
+		}(),
+		func() test {
+			controller := v1.ReplicationController{}
+			controller2 := v1.ReplicationController{}
+			controller.UID = types.UID(controllerUID)
+			controller2.UID = types.UID("AAAAA")
+			return test{
+				name: "Controller can not claim pods owned by another controller",
+				manager: NewPodControllerRefManager(&FakePodControl{},
+					&controller,
+					productionLabelSelector,
+					controllerKind,
+					func() error { return nil }),
+				pods:    []*v1.Pod{newPod("pod1", productionLabel, &controller), newPod("pod2", productionLabel, &controller2)},
+				claimed: []*v1.Pod{newPod("pod1", productionLabel, &controller)},
+			}
+		}(),
+		func() test {
+			controller := v1.ReplicationController{}
+			controller.UID = types.UID(controllerUID)
+			return test{
+				name: "Controller releases claimed pods when selector doesn't match",
+				manager: NewPodControllerRefManager(&FakePodControl{},
+					&controller,
+					productionLabelSelector,
+					controllerKind,
+					func() error { return nil }),
+				pods:    []*v1.Pod{newPod("pod1", productionLabel, &controller), newPod("pod2", testLabel, &controller)},
+				claimed: []*v1.Pod{newPod("pod1", productionLabel, &controller)},
+			}
+		}(),
+		func() test {
+			controller := v1.ReplicationController{}
+			controller.UID = types.UID(controllerUID)
+			podToDelete1 := newPod("pod1", productionLabel, &controller)
+			podToDelete2 := newPod("pod2", productionLabel, nil)
+			now := metav1.Now()
+			podToDelete1.DeletionTimestamp = &now
+			podToDelete2.DeletionTimestamp = &now
+
+			return test{
+				name: "Controller does not claim orphaned pods marked for deletion",
+				manager: NewPodControllerRefManager(&FakePodControl{},
+					&controller,
+					productionLabelSelector,
+					controllerKind,
+					func() error { return nil }),
+				pods:    []*v1.Pod{podToDelete1, podToDelete2},
+				claimed: []*v1.Pod{podToDelete1},
+			}
+		}(),
+	}
+	for _, test := range tests {
+		claimed, err := test.manager.ClaimPods(test.pods)
+		if test.expectError && err == nil {
+			t.Errorf("Test case `%s`, expected error but got nil", test.name)
+		} else if !reflect.DeepEqual(test.claimed, claimed) {
+			t.Errorf("Test case `%s`, claimed wrong pods. Expected %v, got %v", test.name, podToStringSlice(test.claimed), podToStringSlice(claimed))
+		}
+
+	}
+}
+
+func podToStringSlice(pods []*v1.Pod) []string {
+	var names []string
+	for _, pod := range pods {
+		names = append(names, pod.Name)
+	}
+	return names
+}