From 8de89d9f74a82fb342be52675887f133bbd2680d Mon Sep 17 00:00:00 2001 From: Ayush Pateria Date: Tue, 6 Feb 2018 01:12:50 +0530 Subject: [PATCH 1/4] Fix StatefulSet set selector bug --- pkg/controller/history/controller_history.go | 36 +++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/pkg/controller/history/controller_history.go b/pkg/controller/history/controller_history.go index bf88063ef91..ec6107aac9c 100644 --- a/pkg/controller/history/controller_history.go +++ b/pkg/controller/history/controller_history.go @@ -18,6 +18,7 @@ package history import ( "bytes" + "encoding/json" "fmt" "hash/fnv" "sort" @@ -56,7 +57,7 @@ func ControllerRevisionName(prefix string, hash uint32) string { } // NewControllerRevision returns a ControllerRevision with a ControllerRef pointing to parent and indicating that -// parent is of parentKind. The ControllerRevision has labels matching selector, contains Data equal to data, and +// parent is of parentKind. The ControllerRevision has labels matching pod, contains Data equal to data, and // has a Revision equal to revision. The collisionCount is used when creating the name of the ControllerRevision // so the name is likely unique. If the returned error is nil, the returned ControllerRevision is valid. If the // returned error is not nil, the returned ControllerRevision is invalid for use. @@ -66,10 +67,7 @@ func NewControllerRevision(parent metav1.Object, data runtime.RawExtension, revision int64, collisionCount *int32) (*apps.ControllerRevision, error) { - labelMap, err := labels.ConvertSelectorToLabelsMap(selector.String()) - if err != nil { - return nil, err - } + labelMap := getPodLabelsFromData(data) blockOwnerDeletion := true isController := true cr := &apps.ControllerRevision{ @@ -454,3 +452,31 @@ func (fh *fakeHistory) ReleaseControllerRevision(parent metav1.Object, revision } return clone, fh.indexer.Update(clone) } + +func getPodLabelsFromData(data runtime.RawExtension) map[string]string { + var dat map[string]interface{} + labelMap := make(map[string]string) + if err := json.Unmarshal(data.Raw, &dat); err != nil { + return labelMap + } + spec, ok := dat["spec"].(map[string]interface{}) + if !ok { + return labelMap + } + template, ok := spec["template"].(map[string]interface{}) + if !ok { + return labelMap + } + metadata, ok := template["metadata"].(map[string]interface{}) + if !ok { + return labelMap + } + labels, ok := metadata["labels"].(map[string]interface{}) + if !ok { + return labelMap + } + for key, value := range labels { + labelMap[key] = value.(string) + } + return labelMap +} From 4f84a1cb7efe0800f7f01299737d9c23e2d7013b Mon Sep 17 00:00:00 2001 From: Ayush Pateria Date: Fri, 9 Feb 2018 14:54:18 +0530 Subject: [PATCH 2/4] Pass pod labels to controller revision --- pkg/controller/history/controller_history.go | 32 +------ .../history/controller_history_test.go | 90 +++++++++---------- .../statefulset/stateful_set_utils.go | 2 + 3 files changed, 49 insertions(+), 75 deletions(-) diff --git a/pkg/controller/history/controller_history.go b/pkg/controller/history/controller_history.go index ec6107aac9c..d987c4abdb3 100644 --- a/pkg/controller/history/controller_history.go +++ b/pkg/controller/history/controller_history.go @@ -18,7 +18,6 @@ package history import ( "bytes" - "encoding/json" "fmt" "hash/fnv" "sort" @@ -63,11 +62,12 @@ func ControllerRevisionName(prefix string, hash uint32) string { // returned error is not nil, the returned ControllerRevision is invalid for use. func NewControllerRevision(parent metav1.Object, parentKind schema.GroupVersionKind, + podLabels map[string]string, selector labels.Selector, data runtime.RawExtension, revision int64, collisionCount *int32) (*apps.ControllerRevision, error) { - labelMap := getPodLabelsFromData(data) + labelMap := podLabels blockOwnerDeletion := true isController := true cr := &apps.ControllerRevision{ @@ -452,31 +452,3 @@ func (fh *fakeHistory) ReleaseControllerRevision(parent metav1.Object, revision } return clone, fh.indexer.Update(clone) } - -func getPodLabelsFromData(data runtime.RawExtension) map[string]string { - var dat map[string]interface{} - labelMap := make(map[string]string) - if err := json.Unmarshal(data.Raw, &dat); err != nil { - return labelMap - } - spec, ok := dat["spec"].(map[string]interface{}) - if !ok { - return labelMap - } - template, ok := spec["template"].(map[string]interface{}) - if !ok { - return labelMap - } - metadata, ok := template["metadata"].(map[string]interface{}) - if !ok { - return labelMap - } - labels, ok := metadata["labels"].(map[string]interface{}) - if !ok { - return labelMap - } - for key, value := range labels { - labelMap[key] = value.(string) - } - return labelMap -} diff --git a/pkg/controller/history/controller_history_test.go b/pkg/controller/history/controller_history_test.go index e55cbd966a4..aabf1cd71d7 100644 --- a/pkg/controller/history/controller_history_test.go +++ b/pkg/controller/history/controller_history_test.go @@ -85,22 +85,22 @@ func TestRealHistory_ListControllerRevisions(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, nil) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, nil) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, nil) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 3, nil) + ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 3, nil) if err != nil { t.Fatal(err) } @@ -186,22 +186,22 @@ func TestFakeHistory_ListControllerRevisions(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, nil) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, nil) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, nil) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 3, nil) + ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 3, nil) if err != nil { t.Fatal(err) } @@ -312,17 +312,17 @@ func TestRealHistory_CreateControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -443,17 +443,17 @@ func TestFakeHistory_CreateControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -580,12 +580,12 @@ func TestRealHistory_UpdateControllerRevision(t *testing.T) { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -709,12 +709,12 @@ func TestFakeHistory_UpdateControllerRevision(t *testing.T) { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -803,22 +803,22 @@ func TestRealHistory_DeleteControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -914,22 +914,22 @@ func TestFakeHistory_DeleteControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1064,18 +1064,18 @@ func TestRealHistory_AdoptControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1178,18 +1178,18 @@ func TestFakeHistory_AdoptControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1334,18 +1334,18 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, nil) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, nil) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, nil) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } @@ -1465,18 +1465,18 @@ func TestFakeHistory_ReleaseControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1571,23 +1571,23 @@ func TestFindEqualRevisions(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1638,17 +1638,17 @@ func TestSortControllerRevisions(t *testing.T) { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss1Rev3, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev3, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } diff --git a/pkg/controller/statefulset/stateful_set_utils.go b/pkg/controller/statefulset/stateful_set_utils.go index d6b4189f11a..0469d7db009 100644 --- a/pkg/controller/statefulset/stateful_set_utils.go +++ b/pkg/controller/statefulset/stateful_set_utils.go @@ -316,8 +316,10 @@ func newRevision(set *apps.StatefulSet, revision int64, collisionCount *int32) ( if err != nil { return nil, err } + podLabels := set.Spec.Template.Labels cr, err := history.NewControllerRevision(set, controllerKind, + podLabels, selector, runtime.RawExtension{Raw: patch}, revision, From 1beed0f4c6ecf225a6a869bfd92d1ac1274a7370 Mon Sep 17 00:00:00 2001 From: Ayush Pateria Date: Tue, 20 Feb 2018 03:23:51 +0530 Subject: [PATCH 3/4] Remove unused code and modify tests to include set based selector --- pkg/controller/history/controller_history.go | 1 - .../history/controller_history_test.go | 210 ++++++------------ .../statefulset/stateful_set_utils.go | 5 - 3 files changed, 63 insertions(+), 153 deletions(-) diff --git a/pkg/controller/history/controller_history.go b/pkg/controller/history/controller_history.go index d987c4abdb3..1de109ed283 100644 --- a/pkg/controller/history/controller_history.go +++ b/pkg/controller/history/controller_history.go @@ -63,7 +63,6 @@ func ControllerRevisionName(prefix string, hash uint32) string { func NewControllerRevision(parent metav1.Object, parentKind schema.GroupVersionKind, podLabels map[string]string, - selector labels.Selector, data runtime.RawExtension, revision int64, collisionCount *int32) (*apps.ControllerRevision, error) { diff --git a/pkg/controller/history/controller_history_test.go b/pkg/controller/history/controller_history_test.go index aabf1cd71d7..7fcc3aa4578 100644 --- a/pkg/controller/history/controller_history_test.go +++ b/pkg/controller/history/controller_history_test.go @@ -77,30 +77,24 @@ func TestRealHistory_ListControllerRevisions(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) + sel1 := labels.Set(ss1.Spec.Template.Labels).AsSelector() + + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 3, nil) + ss1Orphan, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 3, nil) if err != nil { t.Fatal(err) } @@ -178,30 +172,23 @@ func TestFakeHistory_ListControllerRevisions(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) + sel1 := labels.Set(ss1.Spec.Template.Labels).AsSelector() + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 3, nil) + ss1Orphan, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 3, nil) if err != nil { t.Fatal(err) } @@ -304,25 +291,17 @@ func TestRealHistory_CreateControllerRevision(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -435,25 +414,17 @@ func TestFakeHistory_CreateControllerRevision(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -575,17 +546,12 @@ func TestRealHistory_UpdateControllerRevision(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss1.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -704,17 +670,13 @@ func TestFakeHistory_UpdateControllerRevision(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss1.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -795,30 +757,22 @@ func TestRealHistory_DeleteControllerRevision(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -906,30 +860,22 @@ func TestFakeHistory_DeleteControllerRevision(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1056,26 +1002,18 @@ func TestRealHistory_AdoptControllerRevision(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1170,26 +1108,18 @@ func TestFakeHistory_AdoptControllerRevision(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1326,26 +1256,18 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } @@ -1457,26 +1379,18 @@ func TestFakeHistory_ReleaseControllerRevision(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1563,31 +1477,23 @@ func TestFindEqualRevisions(t *testing.T) { ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) ss2.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - sel2, err := metav1.LabelSelectorAsSelector(ss2.Spec.Selector) - if err != nil { - t.Fatal(err) - } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, ss2.Spec.Template.Labels, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1633,22 +1539,17 @@ func TestSortControllerRevisions(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss1.Status.CollisionCount = new(int32) - sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) - if err != nil { - t.Fatal(err) - } - - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss1Rev3, err := NewControllerRevision(ss1, parentKind, sel1, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + ss1Rev3, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 3, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1682,6 +1583,14 @@ func TestSortControllerRevisions(t *testing.T) { } func newStatefulSet(replicas int, name string, uid types.UID, labels map[string]string) *apps.StatefulSet { + var testLabelKey string + var testLabelValue string + // Get the first label's key, value to be used for set based selector. + for key, value := range labels { + testLabelKey = key + testLabelValue = value + break + } return &apps.StatefulSet{ TypeMeta: metav1.TypeMeta{ Kind: "StatefulSet", @@ -1695,6 +1604,13 @@ func newStatefulSet(replicas int, name string, uid types.UID, labels map[string] Spec: apps.StatefulSetSpec{ Selector: &metav1.LabelSelector{ MatchLabels: labels, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: testLabelKey, + Operator: metav1.LabelSelectorOpIn, + Values: []string{testLabelValue}, + }, + }, }, Replicas: func() *int32 { i := int32(replicas); return &i }(), Template: v1.PodTemplateSpec{ diff --git a/pkg/controller/statefulset/stateful_set_utils.go b/pkg/controller/statefulset/stateful_set_utils.go index 0469d7db009..1cc97647752 100644 --- a/pkg/controller/statefulset/stateful_set_utils.go +++ b/pkg/controller/statefulset/stateful_set_utils.go @@ -312,15 +312,10 @@ func newRevision(set *apps.StatefulSet, revision int64, collisionCount *int32) ( if err != nil { return nil, err } - selector, err := metav1.LabelSelectorAsSelector(set.Spec.Selector) - if err != nil { - return nil, err - } podLabels := set.Spec.Template.Labels cr, err := history.NewControllerRevision(set, controllerKind, podLabels, - selector, runtime.RawExtension{Raw: patch}, revision, collisionCount) From a269491f1853f8e81c6bdeac254d4e2b3c369847 Mon Sep 17 00:00:00 2001 From: Ayush Pateria Date: Thu, 22 Feb 2018 19:19:06 +0530 Subject: [PATCH 4/4] Modify tests --- pkg/controller/history/controller_history.go | 9 +++-- .../history/controller_history_test.go | 38 ++++++++++--------- .../statefulset/stateful_set_utils.go | 3 +- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/pkg/controller/history/controller_history.go b/pkg/controller/history/controller_history.go index 1de109ed283..5e4e8f95c61 100644 --- a/pkg/controller/history/controller_history.go +++ b/pkg/controller/history/controller_history.go @@ -56,17 +56,20 @@ func ControllerRevisionName(prefix string, hash uint32) string { } // NewControllerRevision returns a ControllerRevision with a ControllerRef pointing to parent and indicating that -// parent is of parentKind. The ControllerRevision has labels matching pod, contains Data equal to data, and +// parent is of parentKind. The ControllerRevision has labels matching template labels, contains Data equal to data, and // has a Revision equal to revision. The collisionCount is used when creating the name of the ControllerRevision // so the name is likely unique. If the returned error is nil, the returned ControllerRevision is valid. If the // returned error is not nil, the returned ControllerRevision is invalid for use. func NewControllerRevision(parent metav1.Object, parentKind schema.GroupVersionKind, - podLabels map[string]string, + templateLabels map[string]string, data runtime.RawExtension, revision int64, collisionCount *int32) (*apps.ControllerRevision, error) { - labelMap := podLabels + labelMap := make(map[string]string) + for k, v := range templateLabels { + labelMap[k] = v + } blockOwnerDeletion := true isController := true cr := &apps.ControllerRevision{ diff --git a/pkg/controller/history/controller_history_test.go b/pkg/controller/history/controller_history_test.go index 7fcc3aa4578..455d6b67340 100644 --- a/pkg/controller/history/controller_history_test.go +++ b/pkg/controller/history/controller_history_test.go @@ -77,8 +77,10 @@ func TestRealHistory_ListControllerRevisions(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) - sel1 := labels.Set(ss1.Spec.Template.Labels).AsSelector() - + sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) + if err != nil { + t.Fatal(err) + } ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) @@ -172,7 +174,10 @@ func TestFakeHistory_ListControllerRevisions(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) - sel1 := labels.Set(ss1.Spec.Template.Labels).AsSelector() + sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) + if err != nil { + t.Fatal(err) + } ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) @@ -1539,6 +1544,7 @@ func TestSortControllerRevisions(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) ss1.Status.CollisionCount = new(int32) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) @@ -1583,13 +1589,15 @@ func TestSortControllerRevisions(t *testing.T) { } func newStatefulSet(replicas int, name string, uid types.UID, labels map[string]string) *apps.StatefulSet { - var testLabelKey string - var testLabelValue string - // Get the first label's key, value to be used for set based selector. + // Converting all the map-only selectors to set-based selectors. + var testMatchExpressions []metav1.LabelSelectorRequirement for key, value := range labels { - testLabelKey = key - testLabelValue = value - break + sel := metav1.LabelSelectorRequirement{ + Key: key, + Operator: metav1.LabelSelectorOpIn, + Values: []string{value}, + } + testMatchExpressions = append(testMatchExpressions, sel) } return &apps.StatefulSet{ TypeMeta: metav1.TypeMeta{ @@ -1603,14 +1611,10 @@ func newStatefulSet(replicas int, name string, uid types.UID, labels map[string] }, Spec: apps.StatefulSetSpec{ Selector: &metav1.LabelSelector{ - MatchLabels: labels, - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: testLabelKey, - Operator: metav1.LabelSelectorOpIn, - Values: []string{testLabelValue}, - }, - }, + // Purposely leaving MatchLabels nil, so to ensure it will break if any link + // in the chain ignores the set-based MatchExpressions. + MatchLabels: nil, + MatchExpressions: testMatchExpressions, }, Replicas: func() *int32 { i := int32(replicas); return &i }(), Template: v1.PodTemplateSpec{ diff --git a/pkg/controller/statefulset/stateful_set_utils.go b/pkg/controller/statefulset/stateful_set_utils.go index 1cc97647752..77f5809b82d 100644 --- a/pkg/controller/statefulset/stateful_set_utils.go +++ b/pkg/controller/statefulset/stateful_set_utils.go @@ -312,10 +312,9 @@ func newRevision(set *apps.StatefulSet, revision int64, collisionCount *int32) ( if err != nil { return nil, err } - podLabels := set.Spec.Template.Labels cr, err := history.NewControllerRevision(set, controllerKind, - podLabels, + set.Spec.Template.Labels, runtime.RawExtension{Raw: patch}, revision, collisionCount)