Merge pull request #759 from dinhxuanvu/deployment-update

fix(deployment): Clean up orphaned deployments

Makefile

@@ -144,7 +144,7 @@ container-codegen:
 	docker rm temp-codegen
 
 container-mockgen:
-	docker build -t olm:mockgen -f mockgen.Dockerfile .
+	docker build -t olm:mockgen -f mockgen.Dockerfile . --no-cache
 	docker run --name temp-mockgen olm:mockgen /bin/true
 	docker cp temp-mockgen:/operator-lifecycle-manager/pkg/api/wrappers/wrappersfakes/. ./pkg/api/wrappers/wrappersfakes
 	docker cp temp-mockgen:/operator-lifecycle-manager/pkg/lib/operatorlister/operatorlisterfakes/. ./pkg/lib/operatorlister/operatorlisterfakes
@@ -176,7 +176,7 @@ release:
 	rm -rf manifests
 	mkdir manifests
 	cp -R deploy/ocp/manifests/$(ver)/. manifests
-	find ./manifests -type f -exec sed -i "/^#/d" {} \; 
+	find ./manifests -type f -exec sed -i "/^#/d" {} \;
 	find ./manifests -type f -exec sed -i "1{/---/d}" {} \;
 
 package: olmref=$(shell docker inspect --format='{{index .RepoDigests 0}}' quay.io/operator-framework/olm:$(ver))

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 err != nil && !apierrors.IsNotFound(err) {
+		return nil, 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"
 )
@@ -113,14 +114,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
@@ -144,9 +139,15 @@ func (i *StrategyDeploymentInstaller) checkForDeployments(deploymentSpecs []Stra
 		depNames = append(depNames, dep.Name)
 	}
 
-	existingDeployments, err := i.strategyClient.FindAnyDeploymentsMatchingNames(depNames)
+	// Check the owner is a CSV
+	csv, ok := i.owner.(*v1alpha1.ClusterServiceVersion)
+	if !ok {
+		return StrategyError{Reason: StrategyErrReasonComponentMissing, Message: fmt.Sprintf("owner %s is not a CSV", i.owner.GetName())}
+	}
+
+	existingDeployments, err := i.strategyClient.FindAnyDeploymentsMatchingLabels(ownerutil.CSVOwnerSelector(csv))
 	if err != nil {
-		return StrategyError{Reason: StrategyErrReasonComponentMissing, Message: fmt.Sprintf("error querying for %s: %s", depNames, err)}
+		return StrategyError{Reason: StrategyErrReasonComponentMissing, Message: fmt.Sprintf("error querying existing deployments for CSV %s: %s", csv.GetName(), err)}
 	}
 
 	// compare deployments to see if any need to be created/updated
@@ -181,3 +182,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
+}