diff --git a/pkg/apply/applier.go b/pkg/apply/applier.go index 119953b..6a76482 100644 --- a/pkg/apply/applier.go +++ b/pkg/apply/applier.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/cli-utils/pkg/inventory" "sigs.k8s.io/cli-utils/pkg/kstatus/polling" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/ordering" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -182,7 +183,7 @@ func (a *Applier) prepareObjects(infos []*resource.Info) (*ResourceObjects, erro return nil, err } - sort.Sort(ResourceInfos(resources)) + sort.Sort(ordering.SortableInfos(resources)) if !validateNamespace(resources) { return nil, fmt.Errorf("objects have differing namespaces") diff --git a/pkg/apply/prune/prune.go b/pkg/apply/prune/prune.go index e048465..02e1f20 100644 --- a/pkg/apply/prune/prune.go +++ b/pkg/apply/prune/prune.go @@ -13,6 +13,7 @@ package prune import ( "fmt" + "sort" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -26,6 +27,7 @@ import ( "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/inventory" + "sigs.k8s.io/cli-utils/pkg/ordering" ) // PruneOptions encapsulates the necessary information to @@ -103,6 +105,11 @@ func (po *PruneOptions) Prune(currentObjects []*resource.Info, eventChannel chan } klog.V(4).Infof("prune %d currently applied objects", len(po.currentUids)) klog.V(4).Infof("prune %d previously applied objects", len(pastObjs)) + + // Sort the resources in reverse order using the same rules as is + // used for apply. + sort.Sort(sort.Reverse(ordering.SortableMetas(pastObjs))) + // Iterate through set of all previously applied objects. for _, past := range pastObjs { mapping, err := po.mapper.RESTMapping(past.GroupKind) diff --git a/pkg/apply/prune/prune_test.go b/pkg/apply/prune/prune_test.go index 8de7749..5762dfd 100644 --- a/pkg/apply/prune/prune_test.go +++ b/pkg/apply/prune/prune_test.go @@ -20,9 +20,9 @@ import ( var testNamespace = "test-inventory-namespace" var inventoryObjName = "test-inventory-obj" -var pod1Name = "pod-1" -var pod2Name = "pod-2" -var pod3Name = "pod-3" +var namespaceName = "namespace" +var pdbName = "pdb" +var roleName = "role" var testInventoryLabel = "test-app-label" @@ -40,58 +40,58 @@ var inventoryObj = unstructured.Unstructured{ }, } -var pod1 = unstructured.Unstructured{ +var namespace = unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "v1", - "kind": "Pod", + "kind": "Namespace", "metadata": map[string]interface{}{ - "name": pod1Name, + "name": namespaceName, "namespace": testNamespace, "uid": "uid1", }, }, } -var pod1Info = &resource.Info{ +var namespaceInfo = &resource.Info{ Namespace: testNamespace, - Name: pod1Name, - Object: &pod1, + Name: namespaceName, + Object: &namespace, } -var pod2 = unstructured.Unstructured{ +var pdb = unstructured.Unstructured{ Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Pod", + "apiVersion": "policy/v1beta1", + "kind": "PodDisruptionBudget", "metadata": map[string]interface{}{ - "name": pod2Name, + "name": pdbName, "namespace": testNamespace, "uid": "uid2", }, }, } -var pod2Info = &resource.Info{ +var pdbInfo = &resource.Info{ Namespace: testNamespace, - Name: pod2Name, - Object: &pod2, + Name: pdbName, + Object: &pdb, } -var pod3 = unstructured.Unstructured{ +var role = unstructured.Unstructured{ Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Pod", + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "Role", "metadata": map[string]interface{}{ - "name": pod3Name, + "name": roleName, "namespace": testNamespace, "uid": "uid3", }, }, } -var pod3Info = &resource.Info{ +var roleInfo = &resource.Info{ Namespace: testNamespace, - Name: pod3Name, - Object: &pod3, + Name: roleName, + Object: &role, } // Returns a inventory object with the inventory set from @@ -151,32 +151,32 @@ func TestPrune(t *testing.T) { isError: false, }, "Past and current objects are the same; no pruned objects": { - pastInfos: []*resource.Info{pod1Info, pod2Info}, - currentInfos: []*resource.Info{pod2Info, pod1Info}, + pastInfos: []*resource.Info{namespaceInfo, pdbInfo}, + currentInfos: []*resource.Info{pdbInfo, namespaceInfo}, prunedInfos: []*resource.Info{}, isError: false, }, "No past objects; no pruned objects": { pastInfos: []*resource.Info{}, - currentInfos: []*resource.Info{pod2Info, pod1Info}, + currentInfos: []*resource.Info{pdbInfo, namespaceInfo}, prunedInfos: []*resource.Info{}, isError: false, }, - "No current objects; all previous objects pruned": { - pastInfos: []*resource.Info{pod1Info, pod2Info, pod3Info}, + "No current objects; all previous objects pruned in correct order": { + pastInfos: []*resource.Info{namespaceInfo, pdbInfo, roleInfo}, currentInfos: []*resource.Info{}, - prunedInfos: []*resource.Info{pod1Info, pod2Info, pod3Info}, + prunedInfos: []*resource.Info{pdbInfo, roleInfo, namespaceInfo}, isError: false, }, "Omitted object is pruned": { - pastInfos: []*resource.Info{pod1Info, pod2Info}, - currentInfos: []*resource.Info{pod2Info, pod3Info}, - prunedInfos: []*resource.Info{pod1Info}, + pastInfos: []*resource.Info{namespaceInfo, pdbInfo}, + currentInfos: []*resource.Info{pdbInfo, roleInfo}, + prunedInfos: []*resource.Info{namespaceInfo}, isError: false, }, "Prevent delete lifecycle annotation stops pruning": { - pastInfos: []*resource.Info{preventDeleteInfo, pod2Info}, - currentInfos: []*resource.Info{pod2Info, pod3Info}, + pastInfos: []*resource.Info{preventDeleteInfo, pdbInfo}, + currentInfos: []*resource.Info{pdbInfo, roleInfo}, prunedInfos: []*resource.Info{}, isError: false, }, @@ -191,29 +191,42 @@ func TestPrune(t *testing.T) { // Set up the currently applied objects. currentInventoryInfo := createInventoryInfo("current-group", tc.currentInfos...) currentInfos := append(tc.currentInfos, currentInventoryInfo) + // Set up the fake dynamic client to recognize all objects, and the RESTMapper. + po.client = fake.NewSimpleDynamicClient(scheme.Scheme, + namespaceInfo.Object, pdbInfo.Object, roleInfo.Object) + po.mapper = testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme, + scheme.Scheme.PrioritizedVersionsAllGroups()...) // The event channel can not block; make sure its bigger than all // the events that can be put on it. eventChannel := make(chan event.Event, len(tc.pastInfos)+1) // Add one for inventory object - defer close(eventChannel) - // Set up the fake dynamic client to recognize all objects, and the RESTMapper. - po.client = fake.NewSimpleDynamicClient(scheme.Scheme, - pod1Info.Object, pod2Info.Object, pod3Info.Object) - po.mapper = testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme, - scheme.Scheme.PrioritizedVersionsAllGroups()...) - // Run the prune and validate. - err := po.Prune(currentInfos, eventChannel, Options{ - DryRun: true, - }) + err := func() error { + defer close(eventChannel) + // Run the prune and validate. + return po.Prune(currentInfos, eventChannel, Options{ + DryRun: true, + }) + }() + if !tc.isError { if err != nil { t.Fatalf("Unexpected error during Prune(): %#v", err) } - // Validate the prune events on the event channel. - expectedPruneEvents := len(tc.prunedInfos) + 1 // One extra for pruning inventory object - actualPruneEvents := len(eventChannel) - if expectedPruneEvents != actualPruneEvents { - t.Errorf("Expected (%d) prune events, got (%d)", - expectedPruneEvents, actualPruneEvents) + + var actualPruneEvents []event.Event + for e := range eventChannel { + actualPruneEvents = append(actualPruneEvents, e) + } + if want, got := len(tc.prunedInfos)+1, len(actualPruneEvents); want != got { + t.Errorf("Expected (%d) prune events, got (%d)", want, got) + } + + for i, info := range tc.prunedInfos { + e := actualPruneEvents[i] + expKind := info.Object.GetObjectKind().GroupVersionKind().Kind + actKind := e.PruneEvent.Object.GetObjectKind().GroupVersionKind().Kind + if expKind != actKind { + t.Errorf("Expected kind %s, got %s", expKind, actKind) + } } } else if err == nil { t.Fatalf("Expected error during Prune() but received none") diff --git a/pkg/apply/resource_infos.go b/pkg/ordering/sort.go similarity index 58% rename from pkg/apply/resource_infos.go rename to pkg/ordering/sort.go index 77812ce..70b0c70 100644 --- a/pkg/apply/resource_infos.go +++ b/pkg/ordering/sort.go @@ -1,30 +1,43 @@ // Copyright 2020 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 -package apply +package ordering import ( "sort" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/resource" + "sigs.k8s.io/cli-utils/pkg/object" ) -type ResourceInfos []*resource.Info +type SortableInfos []*resource.Info -var _ sort.Interface = ResourceInfos{} +var _ sort.Interface = SortableInfos{} -func (a ResourceInfos) Len() int { return len(a) } -func (a ResourceInfos) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a ResourceInfos) Less(i, j int) bool { - x := a[i].Object.GetObjectKind().GroupVersionKind() - o := a[j].Object.GetObjectKind().GroupVersionKind() - if !Equals(x, o) { - return IsLessThan(x, o) +func (a SortableInfos) Len() int { return len(a) } +func (a SortableInfos) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a SortableInfos) Less(i, j int) bool { + return less(object.InfoToObjMeta(a[i]), object.InfoToObjMeta(a[j])) +} + +type SortableMetas []object.ObjMetadata + +var _ sort.Interface = SortableMetas{} + +func (a SortableMetas) Len() int { return len(a) } +func (a SortableMetas) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a SortableMetas) Less(i, j int) bool { + return less(a[i], a[j]) +} + +func less(i, j object.ObjMetadata) bool { + if !Equals(i.GroupKind, j.GroupKind) { + return IsLessThan(i.GroupKind, j.GroupKind) } // In case of tie, compare the namespace and name combination so that the output // order is consistent irrespective of input order - return a[i].Namespace+a[i].Name < a[j].Namespace+a[j].Name + return i.Namespace+i.Name < j.Namespace+j.Name } // An attempt to order things to help k8s, e.g. @@ -70,13 +83,11 @@ func getIndexByKind(kind string) int { return m[kind] } -// Equals returns true if the GVK's have equal fields. -func Equals(x schema.GroupVersionKind, o schema.GroupVersionKind) bool { - return x.Group == o.Group && x.Version == o.Version && x.Kind == o.Kind +func Equals(x schema.GroupKind, o schema.GroupKind) bool { + return x.Group == o.Group && x.Kind == o.Kind } -// IsLessThan compares two GVK's as per orderFirst and orderLast, returns boolean result. -func IsLessThan(x schema.GroupVersionKind, o schema.GroupVersionKind) bool { +func IsLessThan(x schema.GroupKind, o schema.GroupKind) bool { indexI := getIndexByKind(x.Kind) indexJ := getIndexByKind(o.Kind) if indexI != indexJ { diff --git a/pkg/apply/resource_infos_test.go b/pkg/ordering/sort_test.go similarity index 82% rename from pkg/apply/resource_infos_test.go rename to pkg/ordering/sort_test.go index b46f4a5..eaf4384 100644 --- a/pkg/apply/resource_infos_test.go +++ b/pkg/ordering/sort_test.go @@ -1,7 +1,7 @@ // Copyright 2020 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 -package apply +package ordering import ( "sort" @@ -78,7 +78,7 @@ func TestResourceOrdering(t *testing.T) { } infos := []*resource.Info{&deploymentInfo, &configMapInfo, &namespaceInfo, &deploymentInfo2} - sort.Sort(ResourceInfos(infos)) + sort.Sort(SortableInfos(infos)) assert.Equal(t, infos[0].Name, "testspace") assert.Equal(t, infos[1].Name, "the-map") @@ -92,33 +92,29 @@ func TestResourceOrdering(t *testing.T) { } func TestGvkLessThan(t *testing.T) { - gvk1 := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Deployment", + gk1 := schema.GroupKind{ + Group: "", + Kind: "Deployment", } - gvk2 := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Namespace", + gk2 := schema.GroupKind{ + Group: "", + Kind: "Namespace", } - assert.Equal(t, IsLessThan(gvk1, gvk2), false) + assert.Equal(t, IsLessThan(gk1, gk2), false) } func TestGvkEquals(t *testing.T) { - gvk1 := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Deployment", + gk1 := schema.GroupKind{ + Group: "", + Kind: "Deployment", } - gvk2 := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Deployment", + gk2 := schema.GroupKind{ + Group: "", + Kind: "Deployment", } - assert.Equal(t, Equals(gvk1, gvk2), true) + assert.Equal(t, Equals(gk1, gk2), true) }