Rework cleanupOrphanedDeployments

Signed-off-by: Vu Dinh <[email protected]>

pkg/api/wrappers/deployment_install_client.go

@@ -8,6 +8,7 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
@@ -25,6 +26,7 @@ type InstallStrategyDeploymentInterface interface {
 	DeleteDeployment(name string) error
 	GetServiceAccountByName(serviceAccountName string) (*corev1.ServiceAccount, error)
 	FindAnyDeploymentsMatchingNames(depNames []string) ([]*appsv1.Deployment, error)
+	FindAnyDeploymentsMatchingLabels(label labels.Selector) ([]*appsv1.Deployment, error)
 }
 
 type InstallStrategyDeploymentClientForNamespace struct {
@@ -118,3 +120,13 @@ func (c *InstallStrategyDeploymentClientForNamespace) FindAnyDeploymentsMatching
 	}
 	return deployments, nil
 }
+
+func (c *InstallStrategyDeploymentClientForNamespace) FindAnyDeploymentsMatchingLabels(label labels.Selector) ([]*appsv1.Deployment, error) {
+	deployments, err := c.opLister.AppsV1().DeploymentLister().Deployments(c.Namespace).List(label)
+	// Any errors other than !exists are propagated up
+	if !apierrors.IsNotFound(err) {
+		return deployments, err
+	}
+
+	return deployments, nil
+}

pkg/controller/install/deployment.go

@@ -7,6 +7,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	rbac "k8s.io/api/rbac/v1"
 
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
 )
@@ -111,14 +112,8 @@ func (i *StrategyDeploymentInstaller) Install(s Strategy) error {
 		return err
 	}
 
-	if i.previousStrategy != nil {
-		previous, ok := i.previousStrategy.(*StrategyDetailsDeployment)
-		if !ok {
-			return fmt.Errorf("couldn't parse old install %s strategy with deployment installer", previous.GetStrategyName())
-		}
-		return i.cleanupPrevious(strategy, previous)
-	}
-	return nil
+	// Clean up orphaned deployments
+	return i.cleanupOrphanedDeployments(strategy.DeploymentSpecs)
 }
 
 // CheckInstalled can return nil (installed), or errors
@@ -179,3 +174,39 @@ func (i *StrategyDeploymentInstaller) checkForDeployments(deploymentSpecs []Stra
 	}
 	return nil
 }
+
+// Clean up orphaned deployments after reinstalling deployments process
+func (i *StrategyDeploymentInstaller) cleanupOrphanedDeployments(deploymentSpecs []StrategyDeploymentSpec) error {
+	// Map of deployments
+	depNames := map[string]string{}
+	for _, dep := range deploymentSpecs {
+		depNames[dep.Name] = dep.Name
+	}
+
+	// Check the owner is a CSV
+	csv, ok := i.owner.(*v1alpha1.ClusterServiceVersion)
+	if !ok {
+		return fmt.Errorf("owner %s is not a CSV", i.owner.GetName())
+	}
+
+	// Get existing deployments in CSV's namespace and owned by CSV
+	existingDeployments, err := i.strategyClient.FindAnyDeploymentsMatchingLabels(ownerutil.CSVOwnerSelector(csv))
+	if err != nil {
+		return err
+	}
+
+	// compare existing deployments to deployments in CSV's spec to see if any need to be deleted
+	for _, d := range existingDeployments {
+		if _, exists := depNames[d.GetName()]; !exists {
+			if ownerutil.IsOwnedBy(d, i.owner) {
+				log.Infof("found an orphaned deployment %s in namespace %s", d.GetName(), i.owner.GetNamespace())
+				if err := i.strategyClient.DeleteDeployment(d.GetName()); err != nil {
+					log.Warnf("error cleaning up deployment %s", d.GetName())
+					return err
+				}
+			}
+		}
+	}
+
+	return nil
+}

pkg/controller/operators/olm/operator.go

@@ -906,10 +906,6 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
 			}
 		}
 
-		// Clean up deployments in case some become orphaned
-		if cleanupErr := a.cleanupOrphanedDeployments(logger, out); cleanupErr == nil {
-			logger.WithField("strategy", out.Spec.InstallStrategy.StrategyName).Infof("clean up deployments successful")
-		}
 	case v1alpha1.CSVPhaseSucceeded:
 		installer, strategy, _ := a.parseStrategiesAndUpdateStatus(out)
 		if strategy == nil {
@@ -1374,43 +1370,3 @@ func (a *Operator) ensureDeploymentAnnotations(logger *logrus.Entry, csv *v1alph
 	}
 	return utilerrors.NewAggregate(updateErrs)
 }
-
-// Clean up orphaned deployments after reinstalling deployments process
-func (a *Operator) cleanupOrphanedDeployments(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) error {
-	// Extract the InstallStrategy for the deployment
-	strategy, err := a.resolver.UnmarshalStrategy(csv.Spec.InstallStrategy)
-	if err != nil {
-		logger.Warn("could not parse install strategy while cleaning up CSV deployment")
-		return nil
-	}
-
-	// Assume the strategy is for a deployment
-	strategyDetailsDeployment, ok := strategy.(*install.StrategyDetailsDeployment)
-	if !ok {
-		logger.Warnf("could not cast install strategy as type %T", strategyDetailsDeployment)
-		return nil
-	}
-
-	depNames := map[string]string{}
-	for _, dep := range strategyDetailsDeployment.DeploymentSpecs {
-		depNames[dep.Name] = dep.Name
-	}
-	// Get existing deployments in CSV's namespace and owned by CSV
-	existingDeployments, err := a.lister.AppsV1().DeploymentLister().Deployments(csv.GetNamespace()).List(ownerutil.CSVOwnerSelector(csv))
-	if err != nil {
-		return err
-	}
-
-	// compare existing deployments to deployments in CSV's spec to see if any need to be deleted
-	for _, d := range existingDeployments {
-		if _, exists := depNames[d.GetName()]; !exists {
-			logger.Infof("found an orphaned deployment %s in namespace %s", d.GetName(), csv.GetNamespace())
-			if err := a.OpClient.DeleteDeployment(csv.GetNamespace(), d.GetName(), &metav1.DeleteOptions{}); err != nil {
-				logger.Warnf("error cleaning up deployment %s", d.GetName())
-				return err
-			}
-		}
-	}
-
-	return nil
-}