From bbedf9e71e55ba4477dc57adb6b7a6efe95ebfd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Ole=C5=9B?= Date: Thu, 20 Apr 2017 15:58:55 +0200 Subject: [PATCH 1/2] Add unit tests for ClaimPods --- pkg/controller/BUILD | 9 +- pkg/controller/controller_ref_manager_test.go | 176 ++++++++++++++++++ 2 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 pkg/controller/controller_ref_manager_test.go diff --git a/pkg/controller/BUILD b/pkg/controller/BUILD index 3944683f721..55f57ed8fc7 100644 --- a/pkg/controller/BUILD +++ b/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", diff --git a/pkg/controller/controller_ref_manager_test.go b/pkg/controller/controller_ref_manager_test.go new file mode 100644 index 00000000000..80e176e3de7 --- /dev/null +++ b/pkg/controller/controller_ref_manager_test.go @@ -0,0 +1,176 @@ +/* +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)}, + } + }(), + } + 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 +} From b9611b95f4a233a49ed3f0927e5cb066b1229b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Ole=C5=9B?= Date: Thu, 6 Apr 2017 12:57:39 +0200 Subject: [PATCH 2/2] Skip pods and replica sets marked for deletion Fixes #44144 --- pkg/controller/controller_ref_manager.go | 4 ++++ pkg/controller/controller_ref_manager_test.go | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/pkg/controller/controller_ref_manager.go b/pkg/controller/controller_ref_manager.go index cffcc7795cf..507995c004d 100644 --- a/pkg/controller/controller_ref_manager.go +++ b/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. diff --git a/pkg/controller/controller_ref_manager_test.go b/pkg/controller/controller_ref_manager_test.go index 80e176e3de7..6a3045f83f2 100644 --- a/pkg/controller/controller_ref_manager_test.go +++ b/pkg/controller/controller_ref_manager_test.go @@ -155,6 +155,26 @@ func TestClaimPods(t *testing.T) { 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)