wait for CRD deletion; delete all objects created by install plan (#3664)

joelanford · web-flow · commit 4739c1fc5bfb · 2020-08-07T09:21:50.000-04:00
diff --git a/internal/cmd/operator-sdk/cleanup/cmd.go b/internal/cmd/operator-sdk/cleanup/cmd.go
@@ -39,6 +39,8 @@ func NewCmd() *cobra.Command {
 		Run: func(cmd *cobra.Command, args []string) {
 			u := operator.NewUninstall(cfg)
 			u.Package = args[0]
+			u.DeleteAll = true
+			u.DeleteOperatorGroupNames = []string{operator.SDKOperatorGroupName}
 
 			ctx, cancel := context.WithTimeout(cmd.Context(), timeout)
 			defer cancel()
diff --git a/internal/operator/uninstall.go b/internal/operator/uninstall.go
@@ -17,15 +17,17 @@ package operator
 import (
 	"context"
 	"fmt"
-	"sort"
 	"strings"
+	"time"
 
 	v1 "github.com/operator-framework/api/pkg/operators/v1"
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/kubectl/pkg/util/slice"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/yaml"
@@ -34,7 +36,11 @@ import (
 type Uninstall struct {
 	config *Configuration
 
-	Package string
+	Package                  string
+	DeleteAll                bool
+	DeleteCRDs               bool
+	DeleteOperatorGroups     bool
+	DeleteOperatorGroupNames []string
 }
 
 func NewUninstall(cfg *Configuration) *Uninstall {
@@ -44,6 +50,11 @@ func NewUninstall(cfg *Configuration) *Uninstall {
 }
 
 func (u *Uninstall) Run(ctx context.Context) error {
+	if u.DeleteAll {
+		u.DeleteCRDs = true
+		u.DeleteOperatorGroups = true
+	}
+
 	subs := v1alpha1.SubscriptionList{}
 	if err := u.config.Client.List(ctx, &subs, client.InNamespace(u.config.Namespace)); err != nil {
 		return fmt.Errorf("list subscriptions: %v", err)
@@ -69,82 +80,115 @@ func (u *Uninstall) Run(ctx context.Context) error {
 	if err := u.config.Client.Get(ctx, catsrcKey, catsrc); err != nil {
 		return fmt.Errorf("get catalog source: %v", err)
 	}
-
-	installPlanKey := types.NamespacedName{
-		Namespace: sub.Status.InstallPlanRef.Namespace,
-		Name:      sub.Status.InstallPlanRef.Name,
-	}
+	catsrc.SetGroupVersionKind(v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.CatalogSourceKind))
 
 	// Since the install plan is owned by the subscription, we need to
 	// read all of the resource references from the install plan before
 	// deleting the subscription.
-	deleteObjs, err := u.getInstallPlanResources(ctx, installPlanKey)
-	if err != nil {
-		return err
+	var crds, csvs, others []controllerutil.Object
+	if sub.Status.InstallPlanRef != nil {
+		ipKey := types.NamespacedName{
+			Namespace: sub.Status.InstallPlanRef.Namespace,
+			Name:      sub.Status.InstallPlanRef.Name,
+		}
+		var err error
+		crds, csvs, others, err = u.getInstallPlanResources(ctx, ipKey)
+		if err != nil {
+			return fmt.Errorf("get install plan resources: %v", err)
+		}
 	}
 
 	// Delete the subscription first, so that no further installs or upgrades
 	// of the operator occur while we're cleaning up.
-	if err := u.config.Client.Delete(ctx, sub); err != nil {
-		return fmt.Errorf("delete subscription %q: %v", sub.Name, err)
-	}
-	u.config.Log("subscription %q deleted\n", sub.Name)
-
-	// Ensure CustomResourceDefinitions are deleted first, so that the operator
-	// has a chance to handle CRs that have finalizers.
-	sort.SliceStable(deleteObjs, func(i, j int) bool {
-		return deleteObjs[i].GetObjectKind().GroupVersionKind().Kind == "CustomResourceDefinition"
-	})
-	for _, obj := range deleteObjs {
-		err := u.config.Client.Delete(ctx, obj)
-		if err != nil && !apierrors.IsNotFound(err) {
+	if err := u.deleteObjects(ctx, false, sub); err != nil {
+		return err
+	}
+
+	if u.DeleteCRDs {
+		// Ensure CustomResourceDefinitions are deleted next, so that the operator
+		// has a chance to handle CRs that have finalizers.
+		if err := u.deleteObjects(ctx, true, crds...); err != nil {
 			return err
 		}
-		if err == nil {
-			u.config.Log("%s %q deleted\n", strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), obj.GetName())
-		}
+	}
+
+	// Delete CSVs and all other objects created by the install plan.
+	objects := append(csvs, others...)
+	if err := u.deleteObjects(ctx, true, objects...); err != nil {
+		return err
 	}
 
 	// Delete the catalog source. This assumes that all underlying resources related
 	// to this catalog source have an owner reference to this catalog source so that
 	// they are automatically garbage-collected.
-	if err := u.config.Client.Delete(ctx, catsrc); err != nil {
-		return fmt.Errorf("delete catalog source: %v", err)
+	if err := u.deleteObjects(ctx, true, catsrc); err != nil {
+		return err
 	}
-	u.config.Log("catalogsource %q deleted\n", catsrc.Name)
 
 	// If this was the last subscription in the namespace and the operator group is
 	// the one we created, delete it
-	if len(subs.Items) == 1 {
-		ogs := v1.OperatorGroupList{}
-		if err := u.config.Client.List(ctx, &ogs, client.InNamespace(u.config.Namespace)); err != nil {
-			return fmt.Errorf("list operatorgroups: %v", err)
+	if u.DeleteOperatorGroups {
+		if err := u.config.Client.List(ctx, &subs, client.InNamespace(u.config.Namespace)); err != nil {
+			return fmt.Errorf("list subscriptions: %v", err)
 		}
-		for _, og := range ogs.Items {
-			if og.GetName() == SDKOperatorGroupName {
-				if err := u.config.Client.Delete(ctx, &og); err != nil {
-					return fmt.Errorf("delete operatorgroup %q: %v", og.Name, err)
+		if len(subs.Items) == 0 {
+			ogs := v1.OperatorGroupList{}
+			if err := u.config.Client.List(ctx, &ogs, client.InNamespace(u.config.Namespace)); err != nil {
+				return fmt.Errorf("list operatorgroups: %v", err)
+			}
+			for _, og := range ogs.Items {
+				og := og
+				if len(u.DeleteOperatorGroupNames) == 0 || slice.ContainsString(u.DeleteOperatorGroupNames, og.GetName(), nil) {
+					if err := u.deleteObjects(ctx, false, &og); err != nil {
+						return err
+					}
 				}
-				u.config.Log("operatorgroup %q deleted\n", og.Name)
 			}
 		}
 	}
+	return nil
+}
 
+func (u *Uninstall) deleteObjects(ctx context.Context, waitForDelete bool, objs ...controllerutil.Object) error {
+	for _, obj := range objs {
+		obj := obj
+		lowerKind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind)
+		if err := u.config.Client.Delete(ctx, obj); err != nil && !apierrors.IsNotFound(err) {
+			return fmt.Errorf("delete %s %q: %v", lowerKind, obj.GetName(), err)
+		} else if err == nil {
+			u.config.Log("%s %q deleted", lowerKind, obj.GetName())
+		}
+		if waitForDelete {
+			key, err := client.ObjectKeyFromObject(obj)
+			if err != nil {
+				return fmt.Errorf("get %s key: %v", lowerKind, err)
+			}
+			if err := wait.PollImmediateUntil(250*time.Millisecond, func() (bool, error) {
+				if err := u.config.Client.Get(ctx, key, obj); apierrors.IsNotFound(err) {
+					return true, nil
+				} else if err != nil {
+					return false, err
+				}
+				return false, nil
+			}, ctx.Done()); err != nil {
+				return fmt.Errorf("wait for %s deleted: %v", lowerKind, err)
+			}
+		}
+	}
 	return nil
 }
 
-func (u *Uninstall) getInstallPlanResources(ctx context.Context, installPlanKey types.NamespacedName) ([]controllerutil.Object, error) {
+func (u *Uninstall) getInstallPlanResources(ctx context.Context, installPlanKey types.NamespacedName) (crds, csvs, others []controllerutil.Object, err error) {
 	installPlan := &v1alpha1.InstallPlan{}
 	if err := u.config.Client.Get(ctx, installPlanKey, installPlan); err != nil {
-		return nil, fmt.Errorf("get install plan: %v", err)
+		return nil, nil, nil, fmt.Errorf("get install plan: %v", err)
 	}
 
-	var objs []controllerutil.Object
 	for _, step := range installPlan.Status.Plan {
-		obj := &unstructured.Unstructured{Object: map[string]interface{}{}}
 		lowerKind := strings.ToLower(step.Resource.Kind)
+		obj := &unstructured.Unstructured{Object: map[string]interface{}{}}
 		if err := yaml.Unmarshal([]byte(step.Resource.Manifest), &obj.Object); err != nil {
-			return nil, fmt.Errorf("parse %s manifest %q: %v", lowerKind, step.Resource.Name, err)
+			return nil, nil, nil, fmt.Errorf("parse %s manifest %q: %v", lowerKind, step.Resource.Name, err)
 		}
 		obj.SetGroupVersionKind(schema.GroupVersionKind{
 			Group:   step.Resource.Group,
@@ -159,7 +203,19 @@ func (u *Uninstall) getInstallPlanResources(ctx context.Context, installPlanKey
 		if step.Resource.Kind == "ServiceAccount" && obj.GetNamespace() == "" {
 			obj.SetNamespace(installPlanKey.Namespace)
 		}
-		objs = append(objs, obj)
+		switch step.Resource.Kind {
+		case "CustomResourceDefinition":
+			crds = append(crds, obj)
+		case "ClusterServiceVersion":
+			csvs = append(csvs, obj)
+		default:
+			// Skip non-CRD/non-CSV resources in the install plan that were not created by the install plan.
+			// This means we avoid deleting things like the default service account.
+			if step.Status != v1alpha1.StepStatusCreated {
+				continue
+			}
+			others = append(others, obj)
+		}
 	}
-	return objs, nil
+	return crds, csvs, others, nil
 }
diff --git a/test/integration/operator_olm_test.go b/test/integration/operator_olm_test.go
@@ -24,8 +24,12 @@ import (
 
 	apimanifests "github.com/operator-framework/api/pkg/manifests"
 	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	operator "github.com/operator-framework/operator-sdk/internal/olm/operator"
 	operator2 "github.com/operator-framework/operator-sdk/internal/operator"
@@ -106,12 +110,8 @@ func PackageManifestsAllNamespaces(t *testing.T) {
 		Version:      defaultOperatorVersion,
 	}
 	// Cleanup.
-	cfg := &operator2.Configuration{KubeconfigPath: kubeconfigPath}
-	assert.NoError(t, cfg.Load())
-	uninstall := operator2.NewUninstall(cfg)
-	uninstall.Package = defaultOperatorName
 	defer func() {
-		if err := doUninstall(uninstall, opcmd.Timeout); err != nil {
+		if err := doUninstall(t, kubeconfigPath, opcmd.Timeout); err != nil {
 			t.Fatal(err)
 		}
 	}()
@@ -169,24 +169,19 @@ func PackageManifestsBasic(t *testing.T) {
 		ManifestsDir: manifestsDir,
 		Version:      defaultOperatorVersion,
 	}
-	// Cleanup.
-	cfg := &operator2.Configuration{KubeconfigPath: kubeconfigPath}
-	assert.NoError(t, cfg.Load())
-	uninstall := operator2.NewUninstall(cfg)
-	uninstall.Package = defaultOperatorName
 
 	// "Remove operator before deploy"
-	assert.Error(t, doUninstall(uninstall, opcmd.Timeout))
+	assert.Error(t, doUninstall(t, kubeconfigPath, opcmd.Timeout))
 
 	// "Deploy operator"
 	assert.NoError(t, opcmd.Run())
 	// "Fail to deploy operator after deploy"
 	assert.Error(t, opcmd.Run())
 
 	// "Remove operator after deploy"
-	assert.NoError(t, doUninstall(uninstall, opcmd.Timeout))
+	assert.NoError(t, doUninstall(t, kubeconfigPath, opcmd.Timeout))
 	// "Remove operator after removal"
-	assert.Error(t, doUninstall(uninstall, opcmd.Timeout))
+	assert.Error(t, doUninstall(t, kubeconfigPath, opcmd.Timeout))
 }
 
 func PackageManifestsMultiplePackages(t *testing.T) {
@@ -269,20 +264,40 @@ func PackageManifestsMultiplePackages(t *testing.T) {
 		ManifestsDir: manifestsDir,
 		Version:      operatorVersion2,
 	}
-	// Cleanup.
-	cfg := &operator2.Configuration{KubeconfigPath: kubeconfigPath}
-	assert.NoError(t, cfg.Load())
-	uninstall := operator2.NewUninstall(cfg)
-	uninstall.Package = defaultOperatorName
 
 	// "Deploy operator"
 	assert.NoError(t, opcmd.Run())
 	// "Remove operator after deploy"
-	assert.NoError(t, doUninstall(uninstall, opcmd.Timeout))
+	assert.NoError(t, doUninstall(t, kubeconfigPath, opcmd.Timeout))
 }
 
-func doUninstall(u *operator2.Uninstall, timeout time.Duration) error {
+func doUninstall(t *testing.T, kubeconfigPath string, timeout time.Duration) error {
+	cfg := &operator2.Configuration{KubeconfigPath: kubeconfigPath}
+	cfg.Log = logrus.Infof
+	assert.NoError(t, cfg.Load())
+	uninstall := operator2.NewUninstall(cfg)
+	uninstall.DeleteAll = true
+	uninstall.DeleteOperatorGroupNames = []string{operator2.SDKOperatorGroupName}
+	uninstall.Package = defaultOperatorName
+
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
-	return u.Run(ctx)
+	if err := uninstall.Run(ctx); err != nil {
+		return err
+	}
+	return waitForPackageManifestConfigMapDeletion(ctx, cfg, defaultOperatorName)
+}
+
+func waitForPackageManifestConfigMapDeletion(ctx context.Context, cfg *operator2.Configuration, packageName string) error {
+	cfgmaps := corev1.ConfigMapList{}
+	opts := []client.ListOption{
+		client.InNamespace(cfg.Namespace),
+		client.MatchingLabels{"owner": "operator-sdk", "package-name": packageName},
+	}
+	return wait.PollImmediateUntil(250*time.Millisecond, func() (bool, error) {
+		if err := cfg.Client.List(ctx, &cfgmaps, opts...); err != nil {
+			return false, err
+		}
+		return len(cfgmaps.Items) == 0, nil
+	}, ctx.Done())
 }
