More robust OLM deployment check (#133)

Co-authored-by: Moritz Clasmeier <mclasmeier@redhat.com>

Author: mclasmeier

internal/deployer/deploy_via_operator.go

@@ -55,7 +55,10 @@ func (d *Deployer) ensureOperatorDeployed(ctx context.Context) error {
 	}
 
 	// Detect current operator deployment mode
-	operatorExists, currentMode := d.detectOperatorDeploymentMode(ctx)
+	operatorExists, currentMode, err := d.detectOperatorDeploymentMode(ctx)
+	if err != nil {
+		return fmt.Errorf("detecting operator deployment mode: %w", err)
+	}
 	needsDeployment := false
 	needsTeardown := false
 

internal/deployer/operator.go

@@ -642,7 +642,10 @@ func (d *Deployer) teardownOperatorNonOLM(ctx context.Context) error {
 
 // teardownOperator removes the operator if it exists, detecting the deployment mode automatically.
 func (d *Deployer) teardownOperator(ctx context.Context) error {
-	operatorExists, operatorMode := d.detectOperatorDeploymentMode(ctx)
+	operatorExists, operatorMode, err := d.detectOperatorDeploymentMode(ctx)
+	if err != nil {
+		return fmt.Errorf("detecting operator deployment mode: %w", err)
+	}
 	if !operatorExists {
 		d.logger.Dim("No operator deployment found, skipping operator teardown")
 		return nil

internal/deployer/operator_olm.go

@@ -335,35 +335,34 @@ func (d *Deployer) waitForCSVSuccess(ctx context.Context) error {
 
 // detectOperatorDeploymentMode detects how the operator is currently deployed.
 // Returns (operatorExists bool, isOLM OperatorDeploymentMode)
-func (d *Deployer) detectOperatorDeploymentMode(ctx context.Context) (bool, OperatorDeploymentMode) {
+func (d *Deployer) detectOperatorDeploymentMode(ctx context.Context) (bool, OperatorDeploymentMode, error) {
+	const olmOwnerLabel = "olm.owner"
+
 	// First, check if a Subscription exists (OLM-specific resource)
 	_, err := d.runKubectl(ctx, k8s.KubectlOptions{
 		Args: []string{"get", "subscription", subscriptionName, "-n", operatorNamespace},
 	})
 	if err == nil {
-		return true, OperatorModeOLM
+		return true, OperatorModeOLM, nil
 	}
 
-	// If no subscription, check if operator deployment exists.
-	_, err = d.runKubectl(ctx, k8s.KubectlOptions{
-		Args: []string{"get", "deployment", operatorDeploymentName, "-n", operatorNamespace},
-	})
-	if err == nil {
-		// Deployment exists - check if it has OLM owner labels.
-		result, err := d.runKubectl(ctx, k8s.KubectlOptions{
-			Args: []string{"get", "deployment", operatorDeploymentName, "-n", operatorNamespace, "-o", "jsonpath={.metadata.labels}"},
-		})
-		// TODO(ROX-34499): This is not very robust. Better retrieve a specific label in the `get`
-		// command?
-		if err == nil && strings.Contains(result.Stdout, "olm.owner") {
-			return true, OperatorModeOLM
-		}
-		// Deployment exists without OLM labels = non-OLM deployment.
-		return true, OperatorModeNonOLM
+	// If no subscription, check if operator deployment exists/if it has the expected OLM label.
+	labelValue, err := k8s.RetrieveClusterResourceLabel(ctx, d.logger, operatorNamespace, "deployment", operatorDeploymentName, olmOwnerLabel)
+	if k8s.IsResourceNotFound(err) {
+		// No operator deployment found.
+		return false, OperatorModeNonOLM, nil
+	}
+	if err != nil {
+		return false, OperatorModeNonOLM, err
+	}
+
+	if labelValue == "" {
+		// Deployment exists without OLM labels -> non-OLM deployment.
+		return true, OperatorModeNonOLM, nil
 	}
 
-	// No operator found.
-	return false, OperatorModeNonOLM
+	// Label set -> OLM deployment.
+	return true, OperatorModeOLM, nil
 }
 
 // teardownOperatorOLM removes the operator when installed via OLM.

internal/k8s/resource.go

@@ -64,7 +64,7 @@ func RetrieveResourceFromCluster(ctx context.Context, log *logger.Logger, namesp
 
 // IsResourceNotFound checks if an error is a resource not found error.
 func IsResourceNotFound(err error) bool {
-	return err == ErrResourceNotFound
+	return errors.Is(err, ErrResourceNotFound)
 }
 
 // ResourceNotOwnedByName checks if the given unstructured object does not have an owner reference with the specified owner name.
@@ -80,3 +80,33 @@ func ResourceNotOwnedByName(obj *unstructured.Unstructured, ownerName string) bo
 	}
 	return true
 }
+
+// RetrieveClusterResourceLabel retrieves the specified label from a Kubernetes resource from the cluster.
+//
+// Parameters: Same as for RetrieveResourceFromCluster, plus
+//   - label: The label to retrieve.
+//
+// Returns:
+//   - string: the label value -- if the label is not found, an empty string is returned without error.
+//   - error: nil if successful, error otherwise (including ErrResourceNotFound for not found resources)
+func RetrieveClusterResourceLabel(ctx context.Context, log *logger.Logger, namespace, resourceType, resourceName, label string) (string, error) {
+	u, err := RetrieveResourceFromCluster(ctx, log, namespace, resourceType, resourceName)
+	if err != nil {
+		return "", fmt.Errorf("retrieving resource %s/%s from namespace %s: %w", resourceType, resourceName, namespace, err)
+	}
+
+	val, found, err := unstructured.NestedFieldCopy(u.Object, "metadata", "labels", label)
+	if err != nil {
+		return "", fmt.Errorf("extracting label value: %w", err)
+	}
+	if !found {
+		return "", nil
+	}
+
+	valStr, ok := val.(string)
+	if !ok {
+		return "", fmt.Errorf("unexpected type for label %q (%T)", label, val)
+	}
+
+	return valStr, nil
+}