From bf29a6073f30dcb3e1534fb0cae4cae8591ce00e Mon Sep 17 00:00:00 2001 From: ravisantoshgudimetla Date: Thu, 4 Jan 2018 23:56:23 +0530 Subject: [PATCH] Owner ref switch Signed-off-by: ravisantoshgudimetla --- pkg/descheduler/pod/pods.go | 30 +++++++------------ pkg/descheduler/pod/pods_test.go | 18 ++++++----- pkg/descheduler/strategies/duplicates.go | 9 ++++-- pkg/descheduler/strategies/duplicates_test.go | 10 +++---- .../strategies/lownodeutilization_test.go | 16 +++++----- .../strategies/pod_antiaffinity_test.go | 6 ++-- test/test_utils.go | 30 +++++++++---------- 7 files changed, 57 insertions(+), 62 deletions(-) diff --git a/pkg/descheduler/pod/pods.go b/pkg/descheduler/pod/pods.go index 813804e38..521a598e1 100644 --- a/pkg/descheduler/pod/pods.go +++ b/pkg/descheduler/pod/pods.go @@ -19,7 +19,6 @@ package pod import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/api/v1/helper/qos" @@ -54,11 +53,8 @@ func IsLatencySensitivePod(pod *v1.Pod) bool { // IsEvictable checks if a pod is evictable or not. func IsEvictable(pod *v1.Pod) bool { - sr, err := CreatorRef(pod) - if err != nil { - sr = nil - } - if IsMirrorPod(pod) || IsPodWithLocalStorage(pod) || sr == nil || IsDaemonsetPod(sr) || IsCriticalPod(pod) || IsLatencySensitivePod(pod) { + ownerRefList := OwnerRef(pod) + if IsMirrorPod(pod) || IsPodWithLocalStorage(pod) || len(ownerRefList) == 0 || IsDaemonsetPod(ownerRefList) || IsCriticalPod(pod) { return false } return true @@ -116,9 +112,11 @@ func IsGuaranteedPod(pod *v1.Pod) bool { return qos.GetPodQOS(pod) == v1.PodQOSGuaranteed } -func IsDaemonsetPod(sr *v1.SerializedReference) bool { - if sr != nil { - return sr.Reference.Kind == "DaemonSet" +func IsDaemonsetPod(ownerRefList []metav1.OwnerReference) bool { + for _, ownerRef := range ownerRefList { + if ownerRef.Kind == "DaemonSet" { + return true + } } return false } @@ -139,15 +137,7 @@ func IsPodWithLocalStorage(pod *v1.Pod) bool { return false } -// CreatorRef returns the kind of the creator reference of the pod. -func CreatorRef(pod *v1.Pod) (*v1.SerializedReference, error) { - creatorRef, found := pod.ObjectMeta.Annotations[v1.CreatedByAnnotation] - if !found { - return nil, nil - } - var sr v1.SerializedReference - if err := runtime.DecodeInto(api.Codecs.UniversalDecoder(), []byte(creatorRef), &sr); err != nil { - return nil, err - } - return &sr, nil +// OwnerRef returns the ownerRefList for the pod. +func OwnerRef(pod *v1.Pod) []metav1.OwnerReference { + return pod.ObjectMeta.GetOwnerReferences() } diff --git a/pkg/descheduler/pod/pods_test.go b/pkg/descheduler/pod/pods_test.go index 8f678b80b..fe31c949e 100644 --- a/pkg/descheduler/pod/pods_test.go +++ b/pkg/descheduler/pod/pods_test.go @@ -35,14 +35,16 @@ func TestPodTypes(t *testing.T) { p5 := test.BuildTestPod("p5", 400, 0, n1.Name) p6 := test.BuildTestPod("p6", 400, 0, n1.Name) p6.Spec.Containers[0].Resources.Requests[v1.ResourceNvidiaGPU] = *resource.NewMilliQuantity(3, resource.DecimalSI) - p6.Annotations = test.GetNormalPodAnnotation() - p1.Annotations = test.GetReplicaSetAnnotation() + p6.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() + + p1.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() // The following 4 pods won't get evicted. // A daemonset. - p2.Annotations = test.GetDaemonSetAnnotation() + //p2.Annotations = test.GetDaemonSetAnnotation() + p2.ObjectMeta.OwnerReferences = test.GetDaemonSetOwnerRefList() // A pod with local storage. - p3.Annotations = test.GetNormalPodAnnotation() + p3.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() p3.Spec.Volumes = []v1.Volume{ { Name: "sample", @@ -67,12 +69,12 @@ func TestPodTypes(t *testing.T) { if !IsPodWithLocalStorage(p3) { t.Errorf("Expected p3 to be a pod with local storage.") } - sr, _ := CreatorRef(p2) - if !IsDaemonsetPod(sr) { + ownerRefList := OwnerRef(p2) + if !IsDaemonsetPod(ownerRefList) { t.Errorf("Expected p2 to be a daemonset pod.") } - sr, _ = CreatorRef(p1) - if IsDaemonsetPod(sr) || IsPodWithLocalStorage(p1) || IsCriticalPod(p1) || IsMirrorPod(p1) { + ownerRefList = OwnerRef(p1) + if IsDaemonsetPod(ownerRefList) || IsPodWithLocalStorage(p1) || IsCriticalPod(p1) || IsMirrorPod(p1) { t.Errorf("Expected p1 to be a normal pod.") } if !IsLatencySensitivePod(p6) { diff --git a/pkg/descheduler/strategies/duplicates.go b/pkg/descheduler/strategies/duplicates.go index 3320afc34..6911f2e71 100644 --- a/pkg/descheduler/strategies/duplicates.go +++ b/pkg/descheduler/strategies/duplicates.go @@ -83,9 +83,12 @@ func FindDuplicatePods(pods []*v1.Pod) DuplicatePodsMap { for _, pod := range pods { // Ignoring the error here as in the ListDuplicatePodsOnNode function we call ListEvictablePodsOnNode // which checks for error. - sr, _ := podutil.CreatorRef(pod) - s := strings.Join([]string{sr.Reference.Kind, sr.Reference.Namespace, sr.Reference.Name}, "/") - dpm[s] = append(dpm[s], pod) + ownerRefList := podutil.OwnerRef(pod) + for _, ownerRef := range ownerRefList { + // ownerRef doesn't need namespace since the assumption is owner needs to be in the same namespace. + s := strings.Join([]string{ownerRef.Kind, ownerRef.Name}, "/") + dpm[s] = append(dpm[s], pod) + } } return dpm } diff --git a/pkg/descheduler/strategies/duplicates_test.go b/pkg/descheduler/strategies/duplicates_test.go index 89d6833a7..81df9a6c1 100644 --- a/pkg/descheduler/strategies/duplicates_test.go +++ b/pkg/descheduler/strategies/duplicates_test.go @@ -39,15 +39,15 @@ func TestFindDuplicatePods(t *testing.T) { p7 := test.BuildTestPod("p7", 100, 0, node.Name) // All the following pods expect for one will be evicted. - p1.Annotations = test.GetReplicaSetAnnotation() - p2.Annotations = test.GetReplicaSetAnnotation() - p3.Annotations = test.GetReplicaSetAnnotation() + p1.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() + p2.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() + p3.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() // The following 4 pods won't get evicted. // A daemonset. - p4.Annotations = test.GetDaemonSetAnnotation() + p4.ObjectMeta.OwnerReferences = test.GetDaemonSetOwnerRefList() // A pod with local storage. - p5.Annotations = test.GetNormalPodAnnotation() + p5.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() p5.Spec.Volumes = []v1.Volume{ { Name: "sample", diff --git a/pkg/descheduler/strategies/lownodeutilization_test.go b/pkg/descheduler/strategies/lownodeutilization_test.go index 849177197..8cc41d0c6 100644 --- a/pkg/descheduler/strategies/lownodeutilization_test.go +++ b/pkg/descheduler/strategies/lownodeutilization_test.go @@ -55,16 +55,16 @@ func TestLowNodeUtilization(t *testing.T) { p7 := test.BuildTestPod("p7", 400, 0, n1.Name) p8 := test.BuildTestPod("p8", 400, 0, n1.Name) - p1.Annotations = test.GetReplicaSetAnnotation() - p2.Annotations = test.GetReplicaSetAnnotation() - p3.Annotations = test.GetReplicaSetAnnotation() - p4.Annotations = test.GetReplicaSetAnnotation() - p5.Annotations = test.GetReplicaSetAnnotation() + p1.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() + p2.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() + p3.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() + p4.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() + p5.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() // The following 4 pods won't get evicted. // A daemonset. - p6.Annotations = test.GetDaemonSetAnnotation() + p6.ObjectMeta.OwnerReferences = test.GetDaemonSetOwnerRefList() // A pod with local storage. - p7.Annotations = test.GetNormalPodAnnotation() + p7.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() p7.Spec.Volumes = []v1.Volume{ { Name: "sample", @@ -81,7 +81,7 @@ func TestLowNodeUtilization(t *testing.T) { p8.Namespace = "kube-system" p8.Annotations = test.GetCriticalPodAnnotation() p9 := test.BuildTestPod("p9", 400, 0, n1.Name) - p9.Annotations = test.GetReplicaSetAnnotation() + p9.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() fakeClient := &fake.Clientset{} fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) { list := action.(core.ListAction) diff --git a/pkg/descheduler/strategies/pod_antiaffinity_test.go b/pkg/descheduler/strategies/pod_antiaffinity_test.go index a59011a77..cb0375522 100644 --- a/pkg/descheduler/strategies/pod_antiaffinity_test.go +++ b/pkg/descheduler/strategies/pod_antiaffinity_test.go @@ -33,9 +33,9 @@ func TestPodAntiAffinity(t *testing.T) { p2 := test.BuildTestPod("p2", 100, 0, node.Name) p3 := test.BuildTestPod("p3", 100, 0, node.Name) p3.Labels = map[string]string{"foo": "bar"} - p1.Annotations = test.GetNormalPodAnnotation() - p2.Annotations = test.GetNormalPodAnnotation() - p3.Annotations = test.GetNormalPodAnnotation() + p1.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() + p2.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() + p3.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() p1.Spec.Affinity = &v1.Affinity{ PodAntiAffinity: &v1.PodAntiAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{ diff --git a/test/test_utils.go b/test/test_utils.go index 7ce8bdead..5036d634b 100644 --- a/test/test_utils.go +++ b/test/test_utils.go @@ -62,25 +62,25 @@ func GetMirrorPodAnnotation() map[string]string { } } -// GetNormalPodAnnotation returns the annotation needed for a pod. -func GetNormalPodAnnotation() map[string]string { - return map[string]string{ - "kubernetes.io/created-by": "{\"kind\":\"SerializedReference\",\"apiVersion\":\"v1\",\"reference\":{\"kind\":\"Pod\"}}", - } +// GetNormalPodOwnerRefList returns the ownerRef needed for a pod. +func GetNormalPodOwnerRefList() []metav1.OwnerReference { + ownerRefList := make([]metav1.OwnerReference, 0) + ownerRefList = append(ownerRefList, metav1.OwnerReference{Kind: "Pod", APIVersion: "v1"}) + return ownerRefList } -// GetReplicaSetAnnotation returns the annotation needed for replicaset pod. -func GetReplicaSetAnnotation() map[string]string { - return map[string]string{ - "kubernetes.io/created-by": "{\"kind\":\"SerializedReference\",\"apiVersion\":\"v1\",\"reference\":{\"kind\":\"ReplicaSet\"}}", - } +// GetReplicaSetOwnerRefList returns the ownerRef needed for replicaset pod. +func GetReplicaSetOwnerRefList() []metav1.OwnerReference { + ownerRefList := make([]metav1.OwnerReference, 0) + ownerRefList = append(ownerRefList, metav1.OwnerReference{Kind: "ReplicaSet", APIVersion: "v1"}) + return ownerRefList } -// GetDaemonSetAnnotation returns the annotation needed for daemonset pod. -func GetDaemonSetAnnotation() map[string]string { - return map[string]string{ - "kubernetes.io/created-by": "{\"kind\":\"SerializedReference\",\"apiVersion\":\"v1\",\"reference\":{\"kind\":\"DaemonSet\"}}", - } +// GetDaemonSetOwnerRefList returns the ownerRef needed for daemonset pod. +func GetDaemonSetOwnerRefList() []metav1.OwnerReference { + ownerRefList := make([]metav1.OwnerReference, 0) + ownerRefList = append(ownerRefList, metav1.OwnerReference{Kind: "DaemonSet", APIVersion: "v1"}) + return ownerRefList } // GetCriticalPodAnnotation returns the annotation needed for critical pod.