Merge pull request #655 from AlexanderYastrebov/deletion-selector

katyanna · web-flow · commit 9b6a926f6d01 · 2023-03-17T14:27:38.000+01:00
deletions: add selector
diff --git a/README.md b/README.md
@@ -140,6 +140,9 @@ pre_apply: # everything defined under here will be deleted before applying the m
   labels:
     foo: bar
   has_owner: false
+- namespace: kube-system
+  kind: deployment
+  selector: version != v1
 post_apply: # everything defined under here will be deleted after applying the manifests
 - namespace: kube-system
   kind: deployment
@@ -152,8 +155,9 @@ Whatever is defined in this file will be deleted pre/post applying the other
 manifest files, if the resource exists. If the resource has already been
 deleted previously it's treated as a no-op.
 
-A resource can be identified either by `name` or `labels`.
-It is an error if both or none of them are defined.
+A resource can be identified either by `name`,
+[`selector`](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors) or
+`labels` and only one of them should be defined.
 
 `namespace` can be left out, in which case it will default to `kube-system`.
 
diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go
@@ -69,6 +69,7 @@ type Resource struct {
 	Name      string `yaml:"name"`
 	Namespace string `yaml:"namespace"`
 	Kind      string `yaml:"kind"`
+	Selector  string `yaml:"selector"`
 	Labels    Labels `yaml:"labels"`
 	HasOwner  *bool  `yaml:"has_owner"`
 
@@ -84,16 +85,23 @@ func (r *Resource) Options() metav1.DeleteOptions {
 	}
 }
 
+func (r *Resource) LabelSelector() string {
+	if r.Selector != "" {
+		return r.Selector
+	}
+	return metav1.FormatLabelSelector(&metav1.LabelSelector{MatchLabels: r.Labels})
+}
+
 func (r *Resource) LogFields() logrus.Fields {
 	fields := logrus.Fields{
 		"kind": r.Kind,
 	}
 	if r.Namespace != "" {
 		fields["namespace"] = r.Namespace
 	}
-	if len(r.Labels) > 0 {
-		fields["selector"] = metav1.FormatLabelSelector(&metav1.LabelSelector{MatchLabels: r.Labels})
-	}
+
+	fields["selector"] = r.LabelSelector()
+
 	if r.HasOwner != nil {
 		fields["has_owner"] = fmt.Sprintf("%t", *r.HasOwner)
 	}
@@ -252,19 +260,27 @@ func (c *ClientsCollection) deleteIfFound(ctx context.Context, logger *logrus.En
 func (c *ClientsCollection) DeleteResource(ctx context.Context, logger *logrus.Entry, deletion *Resource) error {
 	logger = logger.WithFields(deletion.LogFields())
 
-	// identify the resource to be deleted either by name or
-	// labels. name AND labels cannot be defined at the same time,
-	// but one of them MUST be defined.
-	if deletion.Name != "" && len(deletion.Labels) > 0 {
-		return fmt.Errorf("only one of 'name' or 'labels' must be specified")
+	// identify the resource to be deleted either by name, selector or labels.
+	// Only one of them must be defined.
+	resourceIdentifiers := 0
+	if deletion.Name != "" {
+		resourceIdentifiers++
+	}
+	if deletion.Selector != "" {
+		resourceIdentifiers++
+	}
+	if len(deletion.Labels) > 0 {
+		resourceIdentifiers++
 	}
 
-	if deletion.Name == "" && len(deletion.Labels) == 0 {
-		return fmt.Errorf("either name or labels must be specified to identify a resource")
+	if resourceIdentifiers == 0 {
+		return fmt.Errorf("either 'name', 'selector' or 'labels' must be specified to identify a resource")
+	} else if resourceIdentifiers > 1 {
+		return fmt.Errorf("only one of 'name', 'selector' or 'labels' must be specified to identify a resource")
 	}
 
-	if deletion.HasOwner != nil && len(deletion.Labels) == 0 {
-		return fmt.Errorf("'has_owner' requires 'labels' to be specified")
+	if deletion.HasOwner != nil && deletion.Selector == "" && len(deletion.Labels) == 0 {
+		return fmt.Errorf("'has_owner' requires 'selector' or 'labels' to be specified")
 	}
 
 	if deletion.Name != "" {
@@ -273,37 +289,33 @@ func (c *ClientsCollection) DeleteResource(ctx context.Context, logger *logrus.E
 			return err
 		}
 		return c.deleteIfFound(ctx, logger, deletion.Kind, deletion.Namespace, deletion.Name, deletion.Options())
-	} else if len(deletion.Labels) > 0 {
-		items, err := c.ListResources(ctx, deletion)
+	}
+
+	items, err := c.ListResources(ctx, deletion)
+	if err != nil {
+		return err
+	}
+
+	if len(items) == 0 {
+		logger.Infof("No matching %s resources found", deletion.Kind)
+	}
+
+	for _, item := range items {
+		err := c.overrideDeletionProtection(ctx, logger, deletion.Kind, deletion.Namespace, deletion.Name)
 		if err != nil {
 			return err
 		}
-
-		if len(items) == 0 {
-			logger.Infof("No matching %s resources found", deletion.Kind)
-		}
-
-		for _, item := range items {
-			err := c.overrideDeletionProtection(ctx, logger, deletion.Kind, deletion.Namespace, deletion.Name)
-			if err != nil {
-				return err
-			}
-			err = c.deleteIfFound(ctx, logger, deletion.Kind, item.GetNamespace(), item.GetName(), deletion.Options())
-			if err != nil {
-				return err
-			}
+		err = c.deleteIfFound(ctx, logger, deletion.Kind, item.GetNamespace(), item.GetName(), deletion.Options())
+		if err != nil {
+			return err
 		}
 	}
 
 	return nil
 }
 
 func (c *ClientsCollection) ListResources(ctx context.Context, rsrc *Resource) ([]unstructured.Unstructured, error) {
-	items, err := c.List(ctx, rsrc.Kind, rsrc.Namespace, metav1.ListOptions{
-		LabelSelector: metav1.FormatLabelSelector(&metav1.LabelSelector{
-			MatchLabels: rsrc.Labels,
-		}),
-	})
+	items, err := c.List(ctx, rsrc.Kind, rsrc.Namespace, metav1.ListOptions{LabelSelector: rsrc.LabelSelector()})
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/kubernetes/kubernetes_test.go b/pkg/kubernetes/kubernetes_test.go
@@ -196,6 +196,64 @@ func TestPerformDeletion(t *testing.T) {
 			},
 			expectDeleted: []string{},
 		},
+		{
+			name: "delete by selector (single label)",
+			deletion: &Resource{
+				Namespace: "ns-foo",
+				Kind:      "Deployment",
+				Selector:  "app=foo",
+			},
+			expectDeleted: []string{
+				"/namespaces/ns-foo/deployments/foo-app",
+				"/namespaces/ns-foo/deployments/foo-app-com",
+				"/namespaces/ns-foo/deployments/foo-app-com-env",
+			},
+		},
+		{
+			name: "delete by selector (multiple labels)",
+			deletion: &Resource{
+				Namespace: "ns-foo",
+				Kind:      "Deployment",
+				Selector:  "app=foo,com=foo",
+			},
+			expectDeleted: []string{
+				"/namespaces/ns-foo/deployments/foo-app-com",
+				"/namespaces/ns-foo/deployments/foo-app-com-env",
+			},
+		},
+		{
+			name: "delete by selector (label not equal)",
+			deletion: &Resource{
+				Namespace: "ns-foo",
+				Kind:      "Deployment",
+				Selector:  "app=foo, env != foo",
+			},
+			expectDeleted: []string{
+				"/namespaces/ns-foo/deployments/foo-app",
+				"/namespaces/ns-foo/deployments/foo-app-com",
+			},
+		},
+		{
+			name: "delete by selector (label not in)",
+			deletion: &Resource{
+				Namespace: "ns-foo",
+				Kind:      "Deployment",
+				Selector:  "app=foo, env notin (foo, bar)",
+			},
+			expectDeleted: []string{
+				"/namespaces/ns-foo/deployments/foo-app",
+				"/namespaces/ns-foo/deployments/foo-app-com",
+			},
+		},
+		{
+			name: "delete by selector (no match)",
+			deletion: &Resource{
+				Namespace: "ns-foo",
+				Kind:      "Deployment",
+				Selector:  "app=nomatch",
+			},
+			expectDeleted: []string{},
+		},
 		{
 			name: "delete replicasets without owner",
 			deletion: &Resource{
@@ -210,7 +268,7 @@ func TestPerformDeletion(t *testing.T) {
 			},
 		},
 		{
-			name: "delete replicasets with owner",
+			name: "delete replicasets with owner and labels",
 			deletion: &Resource{
 				Namespace: "ns-foo",
 				Kind:      "ReplicaSet",
@@ -222,6 +280,19 @@ func TestPerformDeletion(t *testing.T) {
 				"/namespaces/ns-foo/replicasets/foo-app-com-env-0002",
 			},
 		},
+		{
+			name: "delete replicasets with owner and selector",
+			deletion: &Resource{
+				Namespace: "ns-foo",
+				Kind:      "ReplicaSet",
+				Selector:  "app=foo,com=foo",
+				HasOwner:  &yes,
+			},
+			expectDeleted: []string{
+				"/namespaces/ns-foo/replicasets/foo-app-com-0002",
+				"/namespaces/ns-foo/replicasets/foo-app-com-env-0002",
+			},
+		},
 		{
 			name: "delete replicasets regardless owner",
 			deletion: &Resource{
@@ -239,32 +310,63 @@ func TestPerformDeletion(t *testing.T) {
 		},
 		// Errors
 		{
-			name: "both name or labels are specified",
+			name: "all of name, selector and labels are specified",
+			deletion: &Resource{
+				Name:      "foo",
+				Labels:    map[string]string{"foo": "bar"},
+				Selector:  "foo=bar",
+				Namespace: "default",
+				Kind:      "Deployment",
+			},
+			expectError: "only one of 'name', 'selector' or 'labels' must be specified to identify a resource",
+		},
+		{
+			name: "both name and labels are specified",
+			deletion: &Resource{
+				Name:      "foo",
+				Labels:    map[string]string{"foo": "bar"},
+				Namespace: "default",
+				Kind:      "Deployment",
+			},
+			expectError: "only one of 'name', 'selector' or 'labels' must be specified to identify a resource",
+		},
+		{
+			name: "both name and selector are specified",
 			deletion: &Resource{
 				Name:      "foo",
+				Selector:  "foo=bar",
+				Namespace: "default",
+				Kind:      "Deployment",
+			},
+			expectError: "only one of 'name', 'selector' or 'labels' must be specified to identify a resource",
+		},
+		{
+			name: "both selector and labels are specified",
+			deletion: &Resource{
 				Labels:    map[string]string{"foo": "bar"},
+				Selector:  "foo=bar",
 				Namespace: "default",
 				Kind:      "Deployment",
 			},
-			expectError: "only one of 'name' or 'labels' must be specified",
+			expectError: "only one of 'name', 'selector' or 'labels' must be specified to identify a resource",
 		},
 		{
-			name: "neither name or labels are specified",
+			name: "none of name, selector or labels are specified",
 			deletion: &Resource{
 				Namespace: "default",
 				Kind:      "Deployment",
 			},
-			expectError: "either name or labels must be specified to identify a resource",
+			expectError: "either 'name', 'selector' or 'labels' must be specified to identify a resource",
 		},
 		{
-			name: "has_owner without labels",
+			name: "has_owner without selector or labels",
 			deletion: &Resource{
 				Name:      "foo",
 				Namespace: "default",
 				Kind:      "Deployment",
 				HasOwner:  &yes,
 			},
-			expectError: "'has_owner' requires 'labels' to be specified",
+			expectError: "'has_owner' requires 'selector' or 'labels' to be specified",
 		},
 		{
 			name: "list error",
diff --git a/provisioner/clusterpy_test.go b/provisioner/clusterpy_test.go
@@ -68,6 +68,13 @@ pre_apply:
     baz: qux
   has_owner: true
 `
+
+	deletionsContent4 = `
+pre_apply:
+- namespace: kube-system
+  kind: Deployment
+  selector: version != v1
+`
 )
 
 func TestGetInfrastructureID(t *testing.T) {
@@ -249,7 +256,7 @@ func TestPropagateConfigItemsToNodePool(tt *testing.T) {
 	} {
 		cluster := &api.Cluster{
 			ConfigItems: tc.cluster,
-			NodePools:   []*api.NodePool{&api.NodePool{ConfigItems: tc.nodePool}},
+			NodePools:   []*api.NodePool{{ConfigItems: tc.nodePool}},
 		}
 
 		p := clusterpyProvisioner{}
@@ -305,6 +312,7 @@ func TestParseDeletions(t *testing.T) {
 			{Path: "deletions.yaml", Contents: []byte(deletionsContent)},
 			{Path: "deletions.yaml", Contents: []byte(deletionsContent2)},
 			{Path: "deletions.yaml", Contents: []byte(deletionsContent3)},
+			{Path: "deletions.yaml", Contents: []byte(deletionsContent4)},
 		},
 	}
 
@@ -322,6 +330,7 @@ func TestParseDeletions(t *testing.T) {
 			{Name: "foobar-pre", Namespace: "templated", Kind: "deployment"},
 			{Name: "has-no-owner-pre", HasOwner: &no, Namespace: "kube-system", Kind: "ReplicaSet", Labels: map[string]string{"foo": "bar", "baz": "qux"}},
 			{Name: "require-owner-pre", HasOwner: &yes, Namespace: "kube-system", Kind: "ReplicaSet", Labels: map[string]string{"foo": "bar", "baz": "qux"}},
+			{Namespace: "kube-system", Kind: "Deployment", Selector: "version != v1"},
 		},
 		PostApply: []*kubernetes.Resource{
 			{Name: "secretary-post", Namespace: "kube-system", Kind: "deployment"},
