From 6e903321ea14339466940a8e6abb5b3262cb4773 Mon Sep 17 00:00:00 2001 From: Bhautik Vala Date: Fri, 1 May 2026 16:38:24 +0530 Subject: [PATCH 1/3] [patch] Add Fix for constraints not satisfiable issue for applySubscription --- src/mas/devops/olm.py | 110 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/src/mas/devops/olm.py b/src/mas/devops/olm.py index 1cea83b6..6277523b 100644 --- a/src/mas/devops/olm.py +++ b/src/mas/devops/olm.py @@ -9,6 +9,7 @@ # ***************************************************************************** import logging +import re from time import sleep from os import path from typing import Optional @@ -118,6 +119,55 @@ def getSubscription(dynClient: DynamicClient, namespace: str, packageName: str): return subscriptions.items[0] +def _getSubscriptionConstraintMessage(subscriptionResource): + conditions = getattr(getattr(subscriptionResource, "status", None), "conditions", []) + for condition in conditions: + conditionType = getattr(condition, "type", None) + reason = getattr(condition, "reason", None) + message = getattr(condition, "message", None) + if conditionType == "ResolutionFailed" and reason == "ConstraintsNotSatisfiable" and message is not None: + return message + return None + + +def _parseConstraintMessage(message: str): + existingCSVMatch = re.search(r"clusterserviceversion\s+([A-Za-z0-9.\-]+)\s+exists and is not referenced by a subscription", message) + requiredCSVMatch = re.search(r"subscription\s+[A-Za-z0-9.\-]+\s+requires\s+[^\s]+/([A-Za-z0-9.\-]+)", message) + + existingCSV = existingCSVMatch.group(1) if existingCSVMatch else None + requiredCSV = requiredCSVMatch.group(1) if requiredCSVMatch else None + + scenario = None + if existingCSV and requiredCSV: + existingVersion = existingCSV.rsplit(".v", 1)[-1] if ".v" in existingCSV else existingCSV + requiredVersion = requiredCSV.rsplit(".v", 1)[-1] if ".v" in requiredCSV else requiredCSV + if existingVersion > requiredVersion: + scenario = "catalog_behind" + else: + scenario = "marketplace_cache" + + return { + "scenario": scenario, + "existingCSV": existingCSV, + "requiredCSV": requiredCSV + } + + +def _deleteInstallPlansBySelector(installPlanAPI, namespace: str, labelSelector: str): + installPlans = installPlanAPI.get(label_selector=labelSelector, namespace=namespace) + for item in installPlans.items: + logger.warning(f"Deleting stale InstallPlan {item.metadata.name} in {namespace}") + installPlanAPI.delete(name=item.metadata.name, namespace=namespace) + + +def _deleteMarketplaceCatalogJobs(dynClient: DynamicClient, catalogSourceNamespace: str = "openshift-marketplace"): + jobsAPI = dynClient.resources.get(api_version="batch/v1", kind="Job") + jobs = jobsAPI.get(namespace=catalogSourceNamespace) + for job in jobs.items: + logger.warning(f"Deleting marketplace catalog cache job {job.metadata.name} in {catalogSourceNamespace}") + jobsAPI.delete(name=job.metadata.name, namespace=catalogSourceNamespace) + + def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str, packageChannel: Optional[str] = None, catalogSource: Optional[str] = None, catalogSourceNamespace: str = "openshift-marketplace", config: Optional[dict] = None, installMode: str = "OwnNamespace", installPlanApproval: Optional[str] = None, startingCSV: Optional[str] = None): """ Create or update an operator subscription in a namespace. @@ -322,9 +372,69 @@ def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str # Wait for Subscription to complete logger.debug(f"Waiting for Subscription {name} in {namespace}") + constraintRecoveryAttempted = False while True: subscriptionResource = subscriptionsAPI.get(name=name, namespace=namespace) state = getattr(subscriptionResource.status, "state", None) + constraintMessage = _getSubscriptionConstraintMessage(subscriptionResource) + + if constraintMessage is not None: + parsedConstraint = _parseConstraintMessage(constraintMessage) + existingCSV = parsedConstraint["existingCSV"] + requiredCSV = parsedConstraint["requiredCSV"] + scenario = parsedConstraint["scenario"] + + logger.warning(f"Subscription {name} in {namespace} hit ConstraintsNotSatisfiable: {constraintMessage}") + + if scenario == "catalog_behind": + logger.error( + f"Catalog appears behind for {packageName} in {namespace}: existing CSV {existingCSV} is newer than required CSV {requiredCSV}. " + f"Rebuild or republish the operator catalog, delete the stale Subscription and CSV, then retry." + ) + deleteSubscription(dynClient, namespace, packageName) + raise OLMException( + f"Catalog is behind for {packageName} in {namespace}. Rebuild the operator catalog, " + f"remove stale subscription/CSV, and retry. existingCSV={existingCSV}, requiredCSV={requiredCSV}" + ) + + elif scenario == "marketplace_cache": + if constraintRecoveryAttempted: + logger.error( + f"Constraint recovery already attempted for {packageName} in {namespace} but subscription is still stuck. " + f"Manual cleanup may be required. existingCSV={existingCSV}, requiredCSV={requiredCSV}" + ) + raise OLMException( + f"Subscription {name} in {namespace} is still stuck after automatic constraint recovery. " + f"existingCSV={existingCSV}, requiredCSV={requiredCSV}" + ) + + logger.warning( + f"Detected stale OLM resolution for {packageName} in {namespace}. " + f"Cleaning stale subscription, CSV, installplans and marketplace jobs before retrying." + ) + deleteSubscription(dynClient, namespace, packageName) + _deleteInstallPlansBySelector(installPlanAPI, namespace, labelSelector) + _deleteMarketplaceCatalogJobs(dynClient, catalogSourceNamespace) + + constraintRecoveryAttempted = True + sleep(30) + logger.info(f"Re-applying subscription {name} in {namespace} after constraint recovery") + try: + subscriptionsAPI.apply(body=subscription, namespace=namespace) + except Exception as e: + if "409" in str(e) or "AlreadyExists" in str(e): + logger.warning(f"Subscription {name} already exists and produced a conflict during recovery, retrying the apply") + subscriptionsAPI.apply(body=subscription, namespace=namespace) + else: + raise + sleep(30) + continue + + else: + logger.error(f"Unable to distinguish ConstraintsNotSatisfiable scenario for {packageName} in {namespace}") + raise OLMException( + f"Subscription {name} in {namespace} hit ConstraintsNotSatisfiable but scenario could not be determined: {constraintMessage}" + ) # When manual approval is used with startingCSV, the state will be "UpgradePending" # after the initial installation completes (indicating newer versions are available From 1820ad544ff15d5b32294de86d1cc811a128fc43 Mon Sep 17 00:00:00 2001 From: Bhautik Vala Date: Fri, 1 May 2026 19:08:26 +0530 Subject: [PATCH 2/3] [patch] Fix build error --- src/mas/devops/olm.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/mas/devops/olm.py b/src/mas/devops/olm.py index 6277523b..dd0f54ff 100644 --- a/src/mas/devops/olm.py +++ b/src/mas/devops/olm.py @@ -21,6 +21,7 @@ import yaml from .ocp import createNamespace +from .utils import isVersionEqualOrAfter logger = logging.getLogger(__name__) @@ -121,6 +122,9 @@ def getSubscription(dynClient: DynamicClient, namespace: str, packageName: str): def _getSubscriptionConstraintMessage(subscriptionResource): conditions = getattr(getattr(subscriptionResource, "status", None), "conditions", []) + if not isinstance(conditions, list): + return None + for condition in conditions: conditionType = getattr(condition, "type", None) reason = getattr(condition, "reason", None) @@ -137,14 +141,15 @@ def _parseConstraintMessage(message: str): existingCSV = existingCSVMatch.group(1) if existingCSVMatch else None requiredCSV = requiredCSVMatch.group(1) if requiredCSVMatch else None + existingVersion = existingCSV.rsplit(".v", 1)[-1] if existingCSV and ".v" in existingCSV else None + requiredVersion = requiredCSV.rsplit(".v", 1)[-1] if requiredCSV and ".v" in requiredCSV else None + scenario = None - if existingCSV and requiredCSV: - existingVersion = existingCSV.rsplit(".v", 1)[-1] if ".v" in existingCSV else existingCSV - requiredVersion = requiredCSV.rsplit(".v", 1)[-1] if ".v" in requiredCSV else requiredCSV - if existingVersion > requiredVersion: - scenario = "catalog_behind" - else: + if existingVersion and requiredVersion: + if isVersionEqualOrAfter(existingVersion, requiredVersion): scenario = "marketplace_cache" + else: + scenario = "catalog_behind" return { "scenario": scenario, From c1be4f930cb0afdea816362f4a6b4d1d52c4150f Mon Sep 17 00:00:00 2001 From: Bhautik Vala Date: Mon, 4 May 2026 11:54:54 +0530 Subject: [PATCH 3/3] [patch] Add tests for Subscription constraint's issue recovery --- test/src/test_olm_constraints.py | 257 +++++++++++++++++++++++++++++++ 1 file changed, 257 insertions(+) create mode 100644 test/src/test_olm_constraints.py diff --git a/test/src/test_olm_constraints.py b/test/src/test_olm_constraints.py new file mode 100644 index 00000000..dbad9821 --- /dev/null +++ b/test/src/test_olm_constraints.py @@ -0,0 +1,257 @@ +# ***************************************************************************** +# Copyright (c) 2024 IBM Corporation and other Contributors. +# +# All rights reserved. This program and the accompanying materials +# are made available under the terms of the Eclipse Public License v1.0 +# which accompanies this distribution, and is available at +# http://www.eclipse.org/legal/epl-v10.html +# +# ***************************************************************************** + +""" +Unit tests for OLM ConstraintsNotSatisfiable parsing and recovery handling. +""" + +import pytest +from unittest.mock import Mock, patch +from mas.devops import olm + + +class MockResource: + """Mock Kubernetes resource object""" + + def __init__(self, name, labels=None, owner_refs=None, csv_names=None, phase="Complete"): + self.metadata = Mock() + self.metadata.name = name + self.metadata.labels = labels or {} + self.metadata.ownerReferences = owner_refs or [] + + self.spec = Mock() + self.spec.clusterServiceVersionNames = csv_names or [] + + self.status = Mock() + self.status.phase = phase + + +class MockResourceList: + """Mock Kubernetes resource list""" + + def __init__(self, items): + self.items = items + self.status = Mock() + + +@pytest.fixture +def mock_dyn_client(): + client = Mock() + return client + + +@pytest.fixture +def mock_env(): + env = Mock() + template = Mock() + template.render.return_value = """ +apiVersion: operators.coreos.com/v1alpha1 +kind: Subscription +metadata: + name: test-operator + namespace: test-namespace +spec: + channel: stable + name: test-operator + source: test-catalog + sourceNamespace: openshift-marketplace +""" + env.get_template.return_value = template + return env + + +def _constraint_subscription(message, state="UpgradePending"): + subscription = Mock() + subscription.metadata.name = "test-operator" + subscription.status = Mock() + subscription.status.state = state + + condition = Mock() + condition.type = "ResolutionFailed" + condition.reason = "ConstraintsNotSatisfiable" + condition.message = message + + subscription.status.conditions = [condition] + return subscription + + +def test_get_subscription_constraint_message_returns_matching_message(): + message = ( + "clusterserviceversion test-operator.v1.0.0 exists and is not referenced by a subscription" + ) + subscription = _constraint_subscription(message) + + assert olm._getSubscriptionConstraintMessage(subscription) == message + + +def test_parse_constraint_message_marketplace_cache(): + parsed = olm._parseConstraintMessage( + "constraints not satisfiable: @existing/test-ns//test-operator.v2.0.0 and " + "@existing/test-ns//test-operator.v1.0.0 originate from package test-operator, " + "clusterserviceversion test-operator.v1.0.0 exists and is not referenced by a subscription, " + "subscription test-operator exists, subscription test-operator requires " + "@existing/test-ns//test-operator.v2.0.0" + ) + + assert parsed == { + "scenario": "marketplace_cache", + "existingCSV": "test-operator.v1.0.0", + "requiredCSV": "test-operator.v2.0.0" + } + + +def test_parse_constraint_message_catalog_behind(): + parsed = olm._parseConstraintMessage( + "constraints not satisfiable: @existing/test-ns//test-operator.v1.0.0 and " + "@existing/test-ns//test-operator.v2.0.0 originate from package test-operator, " + "clusterserviceversion test-operator.v2.0.0 exists and is not referenced by a subscription, " + "subscription test-operator exists, subscription test-operator requires " + "@existing/test-ns//test-operator.v1.0.0" + ) + + assert parsed == { + "scenario": "catalog_behind", + "existingCSV": "test-operator.v2.0.0", + "requiredCSV": "test-operator.v1.0.0" + } + + +@patch('mas.devops.olm._deleteMarketplaceCatalogJobs') +@patch('mas.devops.olm._deleteInstallPlansBySelector') +@patch('mas.devops.olm.deleteSubscription') +@patch('mas.devops.olm.createNamespace') +@patch('mas.devops.olm.ensureOperatorGroupExists') +@patch('mas.devops.olm.getPackageManifest') +@patch('mas.devops.olm.sleep') +def test_marketplace_cache_recovery_reapplies_subscription( + mock_sleep, mock_get_manifest, mock_ensure_og, mock_create_ns, mock_delete_subscription, + mock_delete_installplans, mock_delete_jobs, mock_dyn_client, mock_env +): + mock_get_manifest.return_value = Mock( + status=Mock(defaultChannel="stable", catalogSource="test-catalog") + ) + + sub_api = Mock() + healthy_subscription = Mock() + healthy_subscription.metadata.name = "test-operator" + healthy_subscription.status = Mock() + healthy_subscription.status.state = "AtLatestKnown" + healthy_subscription.status.conditions = [] + + sub_api.get.side_effect = [ + MockResourceList([]), + _constraint_subscription( + "constraints not satisfiable: @existing/test-ns//test-operator.v2.0.0 and " + "@existing/test-ns//test-operator.v1.0.0 originate from package test-operator, " + "clusterserviceversion test-operator.v1.0.0 exists and is not referenced by a subscription, " + "subscription test-operator exists, subscription test-operator requires " + "@existing/test-ns//test-operator.v2.0.0" + ), + healthy_subscription + ] + sub_api.apply.return_value = Mock() + + install_plan_api = Mock() + install_plan_api.get.return_value = MockResourceList([ + MockResource( + name="install-plan-1", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v1.0.0"], + phase="Complete" + ) + ]) + + def get_resource_api(**kwargs): + api_version = kwargs["api_version"] + kind = kwargs["kind"] + return { + ("operators.coreos.com/v1alpha1", "Subscription"): sub_api, + ("operators.coreos.com/v1alpha1", "InstallPlan"): install_plan_api, + }[(api_version, kind)] + + mock_dyn_client.resources.get.side_effect = get_resource_api + + with patch('mas.devops.olm.Environment', return_value=mock_env): + result = olm.applySubscription( + mock_dyn_client, + "test-namespace", + "test-operator", + packageChannel="stable" + ) + + assert result == healthy_subscription + mock_delete_subscription.assert_called_once_with(mock_dyn_client, "test-namespace", "test-operator") + mock_delete_installplans.assert_called_once_with( + install_plan_api, + "test-namespace", + "operators.coreos.com/test-operator.test-namespace" + ) + mock_delete_jobs.assert_called_once_with(mock_dyn_client, "openshift-marketplace") + assert sub_api.apply.call_count == 2 + + +@patch('mas.devops.olm.deleteSubscription') +@patch('mas.devops.olm.createNamespace') +@patch('mas.devops.olm.ensureOperatorGroupExists') +@patch('mas.devops.olm.getPackageManifest') +@patch('mas.devops.olm.sleep') +def test_catalog_behind_raises_and_cleans_subscription( + mock_sleep, mock_get_manifest, mock_ensure_og, mock_create_ns, mock_delete_subscription, + mock_dyn_client, mock_env +): + mock_get_manifest.return_value = Mock( + status=Mock(defaultChannel="stable", catalogSource="test-catalog") + ) + + sub_api = Mock() + sub_api.get.side_effect = [ + MockResourceList([]), + _constraint_subscription( + "constraints not satisfiable: @existing/test-ns//test-operator.v1.0.0 and " + "@existing/test-ns//test-operator.v2.0.0 originate from package test-operator, " + "clusterserviceversion test-operator.v2.0.0 exists and is not referenced by a subscription, " + "subscription test-operator exists, subscription test-operator requires " + "@existing/test-ns//test-operator.v1.0.0" + ) + ] + sub_api.apply.return_value = Mock() + + install_plan_api = Mock() + install_plan_api.get.return_value = MockResourceList([ + MockResource( + name="install-plan-1", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v1.0.0"], + phase="Complete" + ) + ]) + + def get_resource_api(**kwargs): + api_version = kwargs["api_version"] + kind = kwargs["kind"] + return { + ("operators.coreos.com/v1alpha1", "Subscription"): sub_api, + ("operators.coreos.com/v1alpha1", "InstallPlan"): install_plan_api, + }[(api_version, kind)] + + mock_dyn_client.resources.get.side_effect = get_resource_api + + with patch('mas.devops.olm.Environment', return_value=mock_env): + with pytest.raises(olm.OLMException, match="Catalog is behind"): + olm.applySubscription( + mock_dyn_client, + "test-namespace", + "test-operator", + packageChannel="stable" + ) + + mock_delete_subscription.assert_called_once_with(mock_dyn_client, "test-namespace", "test-operator") + +# Made with Bob