diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 2fd825f0..2bc1ca73 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/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 diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index 2a043936..a70c775e 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -643,7 +643,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 diff --git a/internal/deployer/operator_olm.go b/internal/deployer/operator_olm.go index 662fbd08..913a390e 100644 --- a/internal/deployer/operator_olm.go +++ b/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. diff --git a/internal/k8s/resource.go b/internal/k8s/resource.go index eb54b521..25489dae 100644 --- a/internal/k8s/resource.go +++ b/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 +}