From 4152a916097e5a953883b6f907eb8737f1caeea1 Mon Sep 17 00:00:00 2001 From: Sean Sullivan Date: Tue, 14 Jan 2020 10:34:53 -0800 Subject: [PATCH] Add hash as suffix to grouping object name --- cmd/kubectl/kubectlcobra/grouping.go | 43 +++++- cmd/kubectl/kubectlcobra/grouping_test.go | 175 +++++++++++++++------- 2 files changed, 166 insertions(+), 52 deletions(-) diff --git a/cmd/kubectl/kubectlcobra/grouping.go b/cmd/kubectl/kubectlcobra/grouping.go index 4b11bee51..2f50881bc 100644 --- a/cmd/kubectl/kubectlcobra/grouping.go +++ b/cmd/kubectl/kubectlcobra/grouping.go @@ -9,6 +9,7 @@ import ( "hash/fnv" "sort" "strconv" + "strings" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -75,7 +76,8 @@ func sortGroupingObject(infos []*resource.Info) bool { func addInventoryToGroupingObj(infos []*resource.Info) error { // Iterate through the objects (infos), creating an Inventory struct - // as metadata for the object, or if it's the grouping object, store it. + // as metadata for each object, or if it's the grouping object, store it. + var groupingInfo *resource.Info var groupingObj *unstructured.Unstructured inventoryMap := map[string]string{} for _, info := range infos { @@ -90,6 +92,7 @@ func addInventoryToGroupingObj(infos []*resource.Info) error { if !ok { return fmt.Errorf("Grouping object is not an Unstructured: %#v", groupingObj) } + groupingInfo = info } else { if obj == nil { return fmt.Errorf("Creating inventory; object is nil") @@ -125,11 +128,16 @@ func addInventoryToGroupingObj(infos []*resource.Info) error { if err != nil { return err } + // Add the hash as a suffix to the grouping object's name. + invHashStr := strconv.FormatUint(uint64(invHash), 16) + if err := addSuffixToName(groupingInfo, invHashStr); err != nil { + return err + } annotations := groupingObj.GetAnnotations() if annotations == nil { annotations = map[string]string{} } - annotations[GroupingHash] = strconv.FormatUint(uint64(invHash), 16) + annotations[GroupingHash] = invHashStr groupingObj.SetAnnotations(annotations) } return nil @@ -211,3 +219,34 @@ func mapKeysToSlice(m map[string]string) []string { } return s } + +// addSuffixToName adds the passed suffix (usually a hash) as a suffix +// to the name of the passed object stored in the Info struct. Returns +// an error if the object is not "*unstructured.Unstructured" or if the +// name stored in the object differs from the name in the Info struct. +func addSuffixToName(info *resource.Info, suffix string) error { + + if info == nil { + return fmt.Errorf("Nil resource.Info") + } + suffix = strings.TrimSpace(suffix) + if len(suffix) == 0 { + return fmt.Errorf("Passed empty suffix") + } + + accessor, _ := meta.Accessor(info.Object) + name := accessor.GetName() + if name != info.Name { + return fmt.Errorf("Grouping object (%s) and resource.Info (%s) have different names\n", name, info.Name) + } + // Error if name alread has suffix. + suffix = "-" + suffix + if strings.HasSuffix(name, suffix) { + return fmt.Errorf("Name already has suffix: %s\n", name) + } + name += suffix + accessor.SetName(name) + info.Name = name + + return nil +} diff --git a/cmd/kubectl/kubectlcobra/grouping_test.go b/cmd/kubectl/kubectlcobra/grouping_test.go index 71dfabaeb..f2cca5ad8 100644 --- a/cmd/kubectl/kubectlcobra/grouping_test.go +++ b/cmd/kubectl/kubectlcobra/grouping_test.go @@ -5,6 +5,7 @@ package kubectlcobra import ( + "fmt" "testing" corev1 "k8s.io/api/core/v1" @@ -35,12 +36,6 @@ var groupingObj = unstructured.Unstructured{ }, } -var groupingInfo = &resource.Info{ - Namespace: testNamespace, - Name: groupingObjName, - Object: &groupingObj, -} - var pod1 = unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "v1", @@ -161,7 +156,7 @@ func TestFindGroupingObject(t *testing.T) { name: "", }, { - infos: []*resource.Info{groupingInfo}, + infos: []*resource.Info{copyGroupingInfo()}, exists: true, name: groupingObjName, }, @@ -176,7 +171,7 @@ func TestFindGroupingObject(t *testing.T) { name: "", }, { - infos: []*resource.Info{pod1Info, pod2Info, groupingInfo, pod3Info}, + infos: []*resource.Info{pod1Info, pod2Info, copyGroupingInfo(), pod3Info}, exists: true, name: groupingObjName, }, @@ -206,7 +201,7 @@ func TestSortGroupingObject(t *testing.T) { sorted: false, }, { - infos: []*resource.Info{groupingInfo}, + infos: []*resource.Info{copyGroupingInfo()}, sorted: true, }, { @@ -218,23 +213,23 @@ func TestSortGroupingObject(t *testing.T) { sorted: false, }, { - infos: []*resource.Info{groupingInfo, pod1Info}, + infos: []*resource.Info{copyGroupingInfo(), pod1Info}, sorted: true, }, { - infos: []*resource.Info{pod1Info, groupingInfo}, + infos: []*resource.Info{pod1Info, copyGroupingInfo()}, sorted: true, }, { - infos: []*resource.Info{pod1Info, pod2Info, groupingInfo, pod3Info}, + infos: []*resource.Info{pod1Info, pod2Info, copyGroupingInfo(), pod3Info}, sorted: true, }, { - infos: []*resource.Info{pod1Info, pod2Info, pod3Info, groupingInfo}, + infos: []*resource.Info{pod1Info, pod2Info, pod3Info, copyGroupingInfo()}, sorted: true, }, { - infos: []*resource.Info{groupingInfo, pod1Info, pod2Info, pod3Info}, + infos: []*resource.Info{copyGroupingInfo(), pod1Info, pod2Info, pod3Info}, sorted: true, }, } @@ -274,7 +269,7 @@ func TestAddRetrieveInventoryToFromGroupingObject(t *testing.T) { }, // Grouping object without other objects is OK. { - infos: []*resource.Info{groupingInfo, nilInfo}, + infos: []*resource.Info{copyGroupingInfo(), nilInfo}, isError: true, }, { @@ -282,25 +277,25 @@ func TestAddRetrieveInventoryToFromGroupingObject(t *testing.T) { isError: true, }, { - infos: []*resource.Info{groupingInfo}, + infos: []*resource.Info{copyGroupingInfo()}, expected: []*Inventory{}, isError: false, }, // More than one grouping object is an error. { - infos: []*resource.Info{groupingInfo, groupingInfo}, + infos: []*resource.Info{copyGroupingInfo(), copyGroupingInfo()}, expected: []*Inventory{}, isError: true, }, // More than one grouping object is an error. { - infos: []*resource.Info{groupingInfo, pod1Info, groupingInfo}, + infos: []*resource.Info{copyGroupingInfo(), pod1Info, copyGroupingInfo()}, expected: []*Inventory{}, isError: true, }, // Basic test case: one grouping object, one pod. { - infos: []*resource.Info{groupingInfo, pod1Info}, + infos: []*resource.Info{copyGroupingInfo(), pod1Info}, expected: []*Inventory{ &Inventory{ Namespace: testNamespace, @@ -314,7 +309,7 @@ func TestAddRetrieveInventoryToFromGroupingObject(t *testing.T) { isError: false, }, { - infos: []*resource.Info{pod1Info, groupingInfo}, + infos: []*resource.Info{pod1Info, copyGroupingInfo()}, expected: []*Inventory{ &Inventory{ Namespace: testNamespace, @@ -328,7 +323,7 @@ func TestAddRetrieveInventoryToFromGroupingObject(t *testing.T) { isError: false, }, { - infos: []*resource.Info{pod1Info, pod2Info, groupingInfo, pod3Info}, + infos: []*resource.Info{pod1Info, pod2Info, copyGroupingInfo(), pod3Info}, expected: []*Inventory{ &Inventory{ Namespace: testNamespace, @@ -358,7 +353,7 @@ func TestAddRetrieveInventoryToFromGroupingObject(t *testing.T) { isError: false, }, { - infos: []*resource.Info{pod1Info, pod2Info, pod3Info, groupingInfo}, + infos: []*resource.Info{pod1Info, pod2Info, pod3Info, copyGroupingInfo()}, expected: []*Inventory{ &Inventory{ Namespace: testNamespace, @@ -388,7 +383,7 @@ func TestAddRetrieveInventoryToFromGroupingObject(t *testing.T) { isError: false, }, { - infos: []*resource.Info{groupingInfo, pod1Info, pod2Info, pod3Info}, + infos: []*resource.Info{copyGroupingInfo(), pod1Info, pod2Info, pod3Info}, expected: []*Inventory{ &Inventory{ Namespace: testNamespace, @@ -426,37 +421,117 @@ func TestAddRetrieveInventoryToFromGroupingObject(t *testing.T) { } if !test.isError { if err != nil { - t.Errorf("Received error when expecting none (%s)\n", err) - } else { - retrieved, err := retrieveInventoryFromGroupingObj(test.infos) - if err != nil { - t.Errorf("Error retrieving inventory: %s\n", err) - } - if len(test.expected) != len(retrieved) { - t.Errorf("Expected inventory for %d resources, actual %d", - len(test.expected), len(retrieved)) - } - for _, expected := range test.expected { - found := false - for _, actual := range retrieved { - if expected.Equals(actual) { - found = true - } - } - if !found { - t.Errorf("Expected inventory (%s) not found", expected) + t.Fatalf("Received error when expecting none (%s)\n", err) + } + retrieved, err := retrieveInventoryFromGroupingObj(test.infos) + if err != nil { + t.Fatalf("Error retrieving inventory: %s\n", err) + } + if len(test.expected) != len(retrieved) { + t.Errorf("Expected inventory for %d resources, actual %d", + len(test.expected), len(retrieved)) + } + for _, expected := range test.expected { + found := false + for _, actual := range retrieved { + if expected.Equals(actual) { + found = true + continue } } - // If the grouping object has an inventory, check the - // grouping object has an inventory hash. - groupingInfo, exists := findGroupingObject(test.infos) - if exists && len(test.expected) > 0 { - invHash := retrieveInventoryHash(groupingInfo) - if len(invHash) == 0 { - t.Errorf("Grouping object missing inventory hash") - } + if !found { + t.Errorf("Expected inventory (%s) not found", expected) + } + } + // If the grouping object has an inventory, check the + // grouping object has an inventory hash. + groupingInfo, exists := findGroupingObject(test.infos) + if exists && len(test.expected) > 0 { + invHash := retrieveInventoryHash(groupingInfo) + if len(invHash) == 0 { + t.Errorf("Grouping object missing inventory hash") } } } } } + +func TestAddSuffixToName(t *testing.T) { + tests := []struct { + info *resource.Info + suffix string + expected string + isError bool + }{ + // Nil info should return error. + { + info: nil, + suffix: "", + expected: "", + isError: true, + }, + // Empty suffix should return error. + { + info: copyGroupingInfo(), + suffix: "", + expected: "", + isError: true, + }, + // Empty suffix should return error. + { + info: copyGroupingInfo(), + suffix: " \t", + expected: "", + isError: true, + }, + { + info: copyGroupingInfo(), + suffix: "hashsuffix", + expected: groupingObjName + "-hashsuffix", + isError: false, + }, + } + + for _, test := range tests { + //t.Errorf("%#v [%s]", test.info, test.suffix) + err := addSuffixToName(test.info, test.suffix) + if test.isError { + if err == nil { + t.Errorf("Should have produced an error, but returned none.") + } + } + if !test.isError { + if err != nil { + t.Fatalf("Received error when expecting none (%s)\n", err) + } + actualName, err := getObjectName(test.info.Object) + if err != nil { + t.Fatalf("Error getting object name: %s", err) + } + if actualName != test.info.Name { + t.Errorf("Object name (%s) does not match info name (%s)\n", actualName, test.info.Name) + } + if test.expected != actualName { + t.Errorf("Expected name (%s), got (%s)\n", test.expected, actualName) + } + } + } +} + +func getObjectName(obj runtime.Object) (string, error) { + u, ok := obj.(*unstructured.Unstructured) + if !ok { + return "", fmt.Errorf("Grouping object is not Unstructured format") + } + return u.GetName(), nil +} + +func copyGroupingInfo() *resource.Info { + groupingObjCopy := groupingObj.DeepCopy() + var groupingInfo = &resource.Info{ + Namespace: testNamespace, + Name: groupingObjName, + Object: groupingObjCopy, + } + return groupingInfo +}