Repository: tektoncd/operator
Pull request #595 (2022-01-27 10:32:07)

Removes obsolete installersets created during upgrades
after the introduction of installerSet, it was observed that
multiple installerSet are getting created during upgrades,
this adds logic to remove those installeSet.
as we have now changed labels on installerSets and query them
compared to by their name which we used to save in status.

so this deletes any older installerSet which doesn't have the
new label of type.

Signed-off-by: Shivam Mukhade <smukhade@redhat.com>

# Changes

<!-- 🎉🎉🎉 Thank you for the PR!!! 🎉🎉🎉 -->

<!-- Describe your changes here- ideally you can get that description straight from
your descriptive commit message(s)! -->

# Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

- [x] Includes [tests](https://github.com/tektoncd/community/blob/master/standards.md#principles) (if functionality changed/added)
- [ ] Includes [docs](https://github.com/tektoncd/community/blob/master/standards.md#principles) (if user facing)
- [x] Commit messages follow [commit message best practices](https://github.com/tektoncd/community/blob/master/standards.md#commit-messages)

_See [the contribution guide](https://github.com/tektoncd/operator/blob/master/CONTRIBUTING.md) for more details._

# Release Notes

<!--
Describe any user facing changes here, or delete this block.

Examples of user facing changes:
- API changes
- Bug fixes
- Any changes in behavior
- Changes requiring upgrade notices or deprecation warnings

For pull requests with a release note:

```release-note
Your release note here
```

For pull requests that require additional action from users switching to the new release, include the string "action required" (case insensitive) in the release note:

```release-note
action required: your release note here
```

For pull requests that don't need to be mentioned at release time, use the `/release-note-none` Prow command to add the `release-note-none` label to the PR. You can also write the string "NONE" as a release note in your PR description:

```release-note
NONE
```
-->


tekton-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **sm43**
You can assign the PR to them by writing `/assign @sm43` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=tektoncd%2Foperator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/tektoncd/operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["sm43"]} -->
tekton-robot: The following is the coverage report on the affected files.
Say `/test pull-tekton-operator-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/reconciler/kubernetes/tektoninstallerset/query.go](https://storage.cloud.google.com/tekton-prow/pr-logs/pull/tektoncd_operator/595/pull-tekton-operator-go-coverage/1486648276303220738/artifacts/line-cov.html#file0) | 83.3% | 81.5% | -1.9

nikhil-thomas: /approve
tekton-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/tektoncd/operator/pull/595#issuecomment-1023143547" title="Approved">nikhil-thomas</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=tektoncd%2Foperator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/tektoncd/operator/blob/main/OWNERS)~~ [nikhil-thomas]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
concaf: /lgtm
pradeepitm12: /lgtm

Patch:
diff --git a/pkg/reconciler/kubernetes/tektoninstallerset/query.go b/pkg/reconciler/kubernetes/tektoninstallerset/query.go
index 4327c1eed..3476d19f3 100644
--- a/pkg/reconciler/kubernetes/tektoninstallerset/query.go
+++ b/pkg/reconciler/kubernetes/tektoninstallerset/query.go
@@ -22,6 +22,8 @@ import (
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
 	clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
 )
 
 func CurrentInstallerSetName(ctx context.Context, client clientset.Interface, labelSelector string) (string, error) {
@@ -51,3 +53,40 @@ func CurrentInstallerSetName(ctx context.Context, client clientset.Interface, la
 	}
 	return "", v1alpha1.RECONCILE_AGAIN_ERR
 }
+
+// CleanUpObsoleteResources cleans up obsolete resources
+// this is required because after TektonInstallerSet were introduced
+// it was observed that during upgrade multiple installerSets were
+// getting created
+// now that we have label based query and we have new labels
+// this cleanup is just to make sure we delete all older installerSets
+// from the cluster
+func CleanUpObsoleteResources(ctx context.Context, client clientset.Interface, createdBy string) error {
+
+	labelSelector := labels.NewSelector()
+	createdReq, _ := labels.NewRequirement(CreatedByKey, selection.Equals, []string{createdBy})
+	if createdReq != nil {
+		labelSelector = labelSelector.Add(*createdReq)
+	}
+
+	list, err := client.OperatorV1alpha1().TektonInstallerSets().List(ctx, v1.ListOptions{LabelSelector: labelSelector.String()})
+	if err != nil {
+		return err
+	}
+
+	if len(list.Items) == 0 {
+		return nil
+	}
+
+	for _, i := range list.Items {
+		// check if installerSet has InstallerSetType label
+		// if it doesn't exist then delete it
+		if _, ok := i.Labels[InstallerSetType]; !ok {
+			err := client.OperatorV1alpha1().TektonInstallerSets().Delete(ctx, i.Name, v1.DeleteOptions{})
+			if err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
diff --git a/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go b/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go
index 9057f5dd4..2cb6fdc10 100644
--- a/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go
+++ b/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go
@@ -120,6 +120,10 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel
 	// Pass the object through defaulting
 	tp.SetDefaults(ctx)
 
+	if err := tektoninstallerset.CleanUpObsoleteResources(ctx, r.operatorClientSet, createdByValue); err != nil {
+		return err
+	}
+
 	if err := r.targetNamespaceCheck(ctx, tp); err != nil {
 		return err
 	}
diff --git a/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go b/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go
index 97e7a46fa..50fb71a4b 100644
--- a/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go
+++ b/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go
@@ -134,6 +134,10 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tt *v1alpha1.TektonTrigg
 	// Pass the object through defaulting
 	tt.SetDefaults(ctx)
 
+	if err := tektoninstallerset.CleanUpObsoleteResources(ctx, r.operatorClientSet, createdByValue); err != nil {
+		return err
+	}
+
 	if err := r.extension.PreReconcile(ctx, tt); err != nil {
 		tt.Status.MarkPreReconcilerFailed(fmt.Sprintf("PreReconciliation failed: %s", err.Error()))
 		return err


Test patch:
diff --git a/pkg/reconciler/kubernetes/tektoninstallerset/query_test.go b/pkg/reconciler/kubernetes/tektoninstallerset/query_test.go
index 218527059..ee5390080 100644
--- a/pkg/reconciler/kubernetes/tektoninstallerset/query_test.go
+++ b/pkg/reconciler/kubernetes/tektoninstallerset/query_test.go
@@ -116,3 +116,42 @@ func TestCurrentInstallerSetNameWithDuplicates(t *testing.T) {
 	assert.Error(t, err, v1alpha1.RECONCILE_AGAIN_ERR.Error())
 	assert.Equal(t, name, "")
 }
+
+func TestCleanUpObsoleteResources(t *testing.T) {
+
+	iSets := v1alpha1.TektonInstallerSetList{
+		Items: []v1alpha1.TektonInstallerSet{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "pipeline-1",
+					Labels: map[string]string{
+						CreatedByKey: "Abc",
+					},
+				},
+			},
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "pipeline-2",
+					Labels: map[string]string{
+						CreatedByKey:     "Abc",
+						InstallerSetType: "pipeline",
+					},
+				},
+			},
+		},
+	}
+
+	// initially there are 2 installerSet
+	client := fake.NewSimpleClientset(&iSets)
+
+	err := CleanUpObsoleteResources(context.TODO(), client, "Abc")
+	assert.NilError(t, err)
+
+	// now only one installerSet should exist
+	// which doesn't have InstallerSetType label
+	is, err := client.OperatorV1alpha1().TektonInstallerSets().List(context.TODO(), metav1.ListOptions{})
+	assert.NilError(t, err)
+
+	// pipeline-1 is obsolete resources, so must be deleted
+	assert.Equal(t, is.Items[0].Name, "pipeline-2")
+}