Merge pull request #207 from mortent/OrderResourceForPrune

Prune/destroy resources in reverse order from apply

Author: k8s-ci-robot

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")

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)

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)
-			// 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)
+				namespaceInfo.Object, pdbInfo.Object, roleInfo.Object)
 			po.mapper = testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme,
 				scheme.Scheme.PrioritizedVersionsAllGroups()...)
-			// Run the prune and validate.
-			err := po.Prune(currentInfos, eventChannel, Options{
-				DryRun: true,
-			})
+			// 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
+			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")

pkg/apply/resource_infos.go renamed to 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 {

pkg/apply/resource_infos_test.go renamed to 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)
 }