Skip to content

[IUO] Install plan upgrade test #4394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hmeir wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[IUO] Install plan upgrade test #4394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,8 @@ def determine_upgrade_stream(current_version, target_version):
return UpgradeStreams.Y_STREAM
elif current_cnv_version.micro < target_cnv_version.micro:
return UpgradeStreams.Z_STREAM
elif target_cnv_version.minor % 2 == 0 and target_cnv_version.minor - 2 == current_cnv_version.minor:
return UpgradeStreams.EUS
elif HOTFIX_STR in current_version:
# if we reach here, this is an upgrade out of hotfix to next z-stream
return UpgradeStreams.Z_STREAM
Expand Down
4 changes: 2 additions & 2 deletions tests/install_upgrade_operators/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ def hco_version_scope_class(admin_client, hco_namespace):


@pytest.fixture()
def disabled_default_sources_in_operatorhub(admin_client, installing_cnv):
if installing_cnv:
def disabled_default_sources_in_operatorhub(admin_client, installing_cnv, is_production_source):
if installing_cnv or is_production_source:
yield
else:
with disable_default_sources_in_operatorhub(admin_client=admin_client):
Expand Down
32 changes: 28 additions & 4 deletions tests/install_upgrade_operators/product_upgrade/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

from tests.install_upgrade_operators.constants import WORKLOAD_UPDATE_STRATEGY_KEY_NAME, WORKLOADUPDATEMETHODS
from tests.install_upgrade_operators.product_upgrade.utils import (
approve_cnv_upgrade_install_plan,
extract_ocp_version_from_ocp_image,
get_alerts_fired_during_upgrade,
get_all_cnv_alerts,
Expand All @@ -35,6 +34,7 @@
KONFLUX_MIRROR_BASE_URL,
apply_konflux_idms,
idms_has_all_mirrors,
wait_for_install_plan,
wait_for_operator_condition,
)
from tests.upgrade_params import EUS
Expand All @@ -55,6 +55,7 @@
get_subscription,
)
from utilities.operator import (
approve_install_plan,
get_machine_config_pool_by_name,
get_machine_config_pools_conditions,
update_image_in_catalog_source,
Expand Down Expand Up @@ -112,16 +113,21 @@ def updated_konflux_idms(
cnv_source,
required_konflux_mirrors,
is_disconnected_cluster,
is_production_source,
active_machine_config_pools,
machine_config_pools_conditions,
):
"""Ensures the Konflux IDMS contains the required mirror entries for the CNV upgrade target version."""
if is_production_source:
LOGGER.info("IDMS updates skipped for production source.")
return

if is_disconnected_cluster:
LOGGER.warning("Skip applying IDMS in a disconnected setup.")
return

if cnv_source == HOTFIX_STR:
LOGGER.info("IDMS updates skipped as upgrading using production source/upgrade to hotfix")
LOGGER.info("IDMS updates skipped for hotfix upgrade.")
return
Comment on lines 125 to 131
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

CRITICAL: This reverses the IDMS setup condition.

updated_konflux_idms used to apply mirrors only for disconnected clusters, which is where the IDMS is needed. The new early return on is_disconnected_cluster skips the patch in exactly that environment, so the install-plan precheck and later upgrade flow run without the required mirror configuration.

Proposed fix 
-    if is_disconnected_cluster:
-        LOGGER.warning("Skip applying IDMS in a disconnected setup.")
+    if not is_disconnected_cluster:
+        LOGGER.info("Connected setup; skipping Konflux IDMS update.")
         return
🧰 Tools
🪛 Ruff (0.15.9)

[warning] 141-141: Logging statement uses f-string

(G004)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/install_upgrade_operators/product_upgrade/conftest.py` around lines 136
- 142, The change reversed the intended logic so updated_konflux_idms no longer
runs for disconnected clusters: ensure the function runs only when
is_disconnected_cluster is True and iib_build_info.get("pipeline") ==
KONFLUX_PIPELINE; i.e., remove or invert the early return that currently exits
when is_disconnected_cluster is True and instead only return when NOT
disconnected OR when the pipeline is not KONFLUX_PIPELINE so
updated_konflux_idms is invoked for disconnected Konflux installs.


idms = ImageDigestMirrorSet(name=KONFLUX_IDMS_NAME, client=admin_client)
Expand All @@ -143,7 +149,12 @@ def updated_custom_hco_catalog_source_image(
admin_client,
cnv_image_url,
is_disconnected_cluster,
is_production_source,
):
if is_production_source:
LOGGER.info("Custom catalog source update skipped for production source.")
return

image_url = cnv_image_url
if is_disconnected_cluster:
image_info = get_oc_image_info(image=image_url, pull_secret=generate_openshift_pull_secret_file())
Expand All @@ -169,13 +180,22 @@ def updated_cnv_subscription_source(cnv_subscription_scope_session, cnv_registry


@pytest.fixture()
def approved_cnv_upgrade_install_plan(admin_client, hco_namespace, hco_target_csv_name, is_production_source):
approve_cnv_upgrade_install_plan(
def approved_cnv_upgrade_install_plan(
admin_client,
hco_namespace,
hco_target_csv_name,
is_production_source,
cnv_subscription_scope_session,
):
install_plan = wait_for_install_plan(
client=admin_client,
hco_namespace=hco_namespace.name,
hco_target_csv_name=hco_target_csv_name,
is_production_source=is_production_source,
cnv_subscription=cnv_subscription_scope_session,
)
LOGGER.info(f"Approve the upgrade install plan {install_plan.name} to trigger the upgrade.")
approve_install_plan(install_plan=install_plan)


@pytest.fixture()
Expand Down Expand Up @@ -532,6 +552,7 @@ def source_eus_to_non_eus_cnv_upgraded(
eus_cnv_upgrade_path,
hyperconverged_resource_scope_function,
updated_cnv_subscription_source,
cnv_subscription_scope_session,
):
for version, cnv_image in sorted(eus_cnv_upgrade_path["non-eus"].items()):
LOGGER.info(f"Cnv upgrade to version {version} using image: {cnv_image}")
Expand All @@ -541,6 +562,7 @@ def source_eus_to_non_eus_cnv_upgraded(
cr_name=hyperconverged_resource_scope_function.name,
hco_namespace=hco_namespace,
cnv_target_version=version.lstrip("v"),
cnv_subscription=cnv_subscription_scope_session,
)
LOGGER.info("Successfully performed cnv upgrades from source EUS to non-EUS version.")

Expand All @@ -552,6 +574,7 @@ def non_eus_to_target_eus_cnv_upgraded(
eus_cnv_upgrade_path,
hyperconverged_resource_scope_function,
updated_cnv_subscription_source,
cnv_subscription_scope_session,
):
version, cnv_image = next(iter(eus_cnv_upgrade_path[EUS].items()))
LOGGER.info(f"Cnv upgrade to version {version} using image: {cnv_image}")
Expand All @@ -561,6 +584,7 @@ def non_eus_to_target_eus_cnv_upgraded(
cr_name=hyperconverged_resource_scope_function.name,
hco_namespace=hco_namespace,
cnv_target_version=version.lstrip("v"),
cnv_subscription=cnv_subscription_scope_session,
)


Expand Down
73 changes: 47 additions & 26 deletions tests/install_upgrade_operators/product_upgrade/test_upgrade.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,49 @@
verify_upgrade_cnv,
verify_upgrade_ocp,
)
from tests.install_upgrade_operators.utils import wait_for_install_plan
from tests.upgrade_params import IUO_UPGRADE_TEST_DEPENDENCY_NODE_ID

pytestmark = pytest.mark.usefixtures(
"nodes_taints_before_upgrade",
"nodes_labels_before_upgrade",
)
pytestmark = [
pytest.mark.product_upgrade_test,
pytest.mark.sno,
pytest.mark.upgrade,
pytest.mark.upgrade_custom,
pytest.mark.usefixtures(
"nodes_taints_before_upgrade",
"nodes_labels_before_upgrade",
),
]
LOGGER = logging.getLogger(__name__)


@pytest.mark.product_upgrade_test
@pytest.mark.sno
@pytest.mark.upgrade
@pytest.mark.upgrade_custom
@pytest.mark.gating
@pytest.mark.cnv_upgrade
@pytest.mark.order("first")
@pytest.mark.polarion("CNV-12451")
@pytest.mark.usefixtures(
"cnv_upgrade_stream",
"disabled_default_sources_in_operatorhub",
"updated_konflux_idms",
"updated_custom_hco_catalog_source_image",
"updated_cnv_subscription_source",
)
def test_cnv_upgrade_install_plan_creation(
admin_client,
hco_namespace,
hco_target_csv_name,
is_production_source,
cnv_subscription_scope_session,
):
wait_for_install_plan(
client=admin_client,
hco_namespace=hco_namespace.name,
hco_target_csv_name=hco_target_csv_name,
is_production_source=is_production_source,
cnv_subscription=cnv_subscription_scope_session,
)
Comment on lines +25 to +49
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

HIGH: This precheck is ordered first, but the downstream CNV tests now depend on its side effects.

After moving disabled_default_sources_in_operatorhub, updated_konflux_idms, updated_custom_hco_catalog_source_image, and updated_cnv_subscription_source onto this test, test_cnv_upgrade_process() and test_production_source_cnv_upgrade_process() are no longer independently runnable. @pytest.mark.order("first") only changes order; it does not stop later tests from running without this setup when the precheck is deselected, rerun selectively, or fails. Keep the shared setup fixtures on the upgrade tests too, or add an explicit dependency contract for the test-to-test gating.

As per coding guidelines "Tests MUST be independent; use pytest-dependency ONLY when test B requires side effects from test A; when using @pytest.mark.dependency, a comment explaining WHY the dependency exists is REQUIRED".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/install_upgrade_operators/product_upgrade/test_upgrade.py` around lines
25 - 49, The test test_cnv_upgrade_install_plan_creation now declares shared
setup fixtures (disabled_default_sources_in_operatorhub, updated_konflux_idms,
updated_custom_hco_catalog_source_image, updated_cnv_subscription_source) and is
marked `@pytest.mark.order`("first"), which makes downstream tests
(test_cnv_upgrade_process, test_production_source_cnv_upgrade_process) dependent
on its side-effects; to fix, restore independence by removing those shared
fixtures from test_cnv_upgrade_install_plan_creation and re-attaching them to
the actual upgrade tests (test_cnv_upgrade_process and
test_production_source_cnv_upgrade_process), or explicitly declare a
pytest-dependency on those downstream tests using
`@pytest.mark.dependency`(depends=["test_cnv_upgrade_install_plan_creation"]) on
each dependent test and add a one-line comment explaining WHY the dependency
exists (that these tests require the catalog/operatorhub changes as
side-effects); ensure you keep the `@pytest.mark.order` only if ordering is still
needed.



class TestUpgrade:
@pytest.mark.ocp_upgrade
@pytest.mark.polarion("CNV-8381")
Expand Down Expand Up @@ -53,29 +83,21 @@ def test_cnv_upgrade_process(
cnv_target_version,
cnv_upgrade_stream,
fired_alerts_before_upgrade,
disabled_default_sources_in_operatorhub,
updated_konflux_idms,
updated_custom_hco_catalog_source_image,
updated_cnv_subscription_source,
approved_cnv_upgrade_install_plan,
started_cnv_upgrade,
created_target_hco_csv,
related_images_from_target_csv,
upgraded_cnv,
):
"""
Test the CNV upgrade process (using OSBS/fbc sources). The main steps of the test are:
Test the CNV upgrade process. The main steps of the test are:

1. Disable the default sources in operatorhub in order to be able to upgrade usg a custom catalog source.
2. Generate a new ICSP for the IIB image being used.
3. Update HCO CatalogSource with the image being used.
4. Update the CNV Subscription source.
5. Wait for the upgrade InstallPlan to be created and approve it.
6. Wait until the upgrade has finished:
6.1. Wait for CSV to be created and reach status SUCCEEDED.
6.2. Wait for HCO OperatorCondition to reach status Upgradeable=True.
6.3. Wait until all the pods have been replaced.
6.4. Wait until HCO is stable and its version is updated.
1. Approve the upgrade InstallPlan (created by test_cnv_upgrade_install_plan_creation).
2. Wait until the upgrade has finished:
2.1. Wait for CSV to be created and reach status SUCCEEDED.
2.2. Wait for HCO OperatorCondition to reach status Upgradeable=True.
2.3. Wait until all the pods have been replaced.
2.4. Wait until HCO is stable and its version is updated.
"""
verify_upgrade_cnv(
client=admin_client,
Expand All @@ -94,7 +116,6 @@ def test_production_source_cnv_upgrade_process(
cnv_target_version,
cnv_upgrade_stream,
fired_alerts_before_upgrade,
updated_cnv_subscription_source,
approved_cnv_upgrade_install_plan,
started_cnv_upgrade,
created_target_hco_csv,
Expand All @@ -103,8 +124,8 @@ def test_production_source_cnv_upgrade_process(
):
"""
Test the CNV upgrade process using the production source.
The main steps of the test are the same as for osbs/fbc source,
but it is not needed to disable the default sources, create a new ICSP or update the HCO CatalogSource.
The main steps are the same as for custom source,
but source configuration is handled by test_cnv_upgrade_install_plan_creation.
"""
verify_upgrade_cnv(
client=admin_client,
Expand Down
21 changes: 6 additions & 15 deletions tests/install_upgrade_operators/product_upgrade/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from ocp_resources.machine_config_pool import MachineConfigPool
from ocp_resources.namespace import Namespace
from ocp_resources.resource import Resource, ResourceEditor
from ocp_resources.subscription import Subscription
from packaging.version import Version
from pyhelper_utils.shell import run_command
from timeout_sampler import TimeoutExpiredError, TimeoutSampler
Expand Down Expand Up @@ -344,19 +345,6 @@ def verify_upgrade_cnv(client, hco_namespace, expected_images):
)


def approve_cnv_upgrade_install_plan(client, hco_namespace, hco_target_csv_name, is_production_source):
LOGGER.info("Get the upgrade install plan.")
install_plan = wait_for_install_plan(
client=client,
hco_namespace=hco_namespace,
hco_target_csv_name=hco_target_csv_name,
is_production_source=is_production_source,
)

LOGGER.info(f"Approve the upgrade install plan {install_plan.name} to trigger the upgrade.")
approve_install_plan(install_plan=install_plan)


def wait_for_cluster_version_stable_conditions(admin_client):
wait_for_consistent_resource_conditions(
dynamic_client=admin_client,
Expand Down Expand Up @@ -631,6 +619,7 @@ def perform_cnv_upgrade(
cr_name: str,
hco_namespace: Namespace,
cnv_target_version: str,
cnv_subscription: Subscription,
) -> None:
hco_target_csv_name = get_hco_csv_name_by_version(cnv_target_version=cnv_target_version)

Expand All @@ -641,13 +630,15 @@ def perform_cnv_upgrade(
catalog_source_name=HCO_CATALOG_SOURCE,
cr_name=cr_name,
)
LOGGER.info("Approving CNV InstallPlan")
approve_cnv_upgrade_install_plan(
install_plan = wait_for_install_plan(
client=admin_client,
hco_namespace=hco_namespace.name,
hco_target_csv_name=hco_target_csv_name,
is_production_source=False,
cnv_subscription=cnv_subscription,
)
LOGGER.info(f"Approve the upgrade install plan {install_plan.name} to trigger the upgrade.")
approve_install_plan(install_plan=install_plan)
LOGGER.info("Waiting for target CSV")
target_csv = wait_for_hco_csv_creation(
admin_client=admin_client, hco_namespace=hco_namespace.name, hco_target_csv_name=hco_target_csv_name
Expand Down
26 changes: 17 additions & 9 deletions tests/install_upgrade_operators/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,18 @@
from ocp_resources.node import Node
from ocp_resources.operator_condition import OperatorCondition
from ocp_resources.resource import Resource, ResourceEditor
from ocp_resources.subscription import Subscription
from timeout_sampler import TimeoutExpiredError, TimeoutSampler

from tests.install_upgrade_operators.constants import KEY_PATH_SEPARATOR
from utilities.constants import (
HCO_SUBSCRIPTION,
PRODUCTION_CATALOG_SOURCE,
TIMEOUT_1MIN,
TIMEOUT_5SEC,
TIMEOUT_10SEC,
TIMEOUT_30MIN,
TIMEOUT_40MIN,
)
from utilities.infra import get_subscription
from utilities.operator import wait_for_mcp_update_completion

LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -68,7 +67,21 @@ def wait_for_install_plan(
hco_namespace: str,
hco_target_csv_name: str,
is_production_source: bool,
cnv_subscription: Subscription,
) -> Any:
"""Waits for the upgrade InstallPlan to be created and returns it.

Args:
client: Kubernetes dynamic client.
hco_namespace: HCO namespace name.
hco_target_csv_name: Expected target CSV name.
is_production_source: Whether upgrading from production source.
cnv_subscription: CNV subscription resource.

Returns:
The matching InstallPlan resource.
"""
LOGGER.info(f"Waiting for the upgrade install plan. hco_target_csv_name: {hco_target_csv_name}")
install_plan_sampler = TimeoutSampler(
wait_timeout=TIMEOUT_40MIN,
sleep=TIMEOUT_10SEC,
Expand All @@ -81,16 +94,11 @@ def wait_for_install_plan(
hco_namespace=hco_namespace,
hco_target_version=hco_target_csv_name,
)
subscription = get_subscription(
admin_client=client,
namespace=hco_namespace,
subscription_name=HCO_SUBSCRIPTION,
)
install_plan_name_in_subscription = None
try:
for install_plan_samples in install_plan_sampler:
# wait for the install plan to be created and updated in the subscription.
install_plan_name_in_subscription = getattr(subscription.instance.status.installplan, "name", None)
install_plan_name_in_subscription = getattr(cnv_subscription.instance.status.installplan, "name", None)
Comment on lines 99 to +101
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

CRITICAL: Guard the subscription InstallPlan lookup while it is still unset.

Line 101 still dereferences .status.installplan before getattr() runs. On the first sampler iterations the Subscription often has no InstallPlan yet, so this raises AttributeError instead of continuing to wait.

Proposed fix 
-            install_plan_name_in_subscription = getattr(cnv_subscription.instance.status.installplan, "name", None)
+            subscription_status = getattr(cnv_subscription.instance, "status", None)
+            install_plan_ref = getattr(subscription_status, "installplan", None)
+            install_plan_name_in_subscription = getattr(install_plan_ref, "name", None)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for install_plan_samples in install_plan_sampler:
# wait for the install plan to be created and updated in the subscription.
install_plan_name_in_subscription = getattr(subscription.instance.status.installplan, "name", None)
install_plan_name_in_subscription = getattr(cnv_subscription.instance.status.installplan, "name", None)
for install_plan_samples in install_plan_sampler:
# wait for the install plan to be created and updated in the subscription.
subscription_status = getattr(cnv_subscription.instance, "status", None)
install_plan_ref = getattr(subscription_status, "installplan", None)
install_plan_name_in_subscription = getattr(install_plan_ref, "name", None)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/install_upgrade_operators/utils.py` around lines 99 - 101, The code
dereferences cnv_subscription.instance.status.installplan before getattr runs
causing AttributeError when status or installplan is None; update the assignment
for install_plan_name_in_subscription inside the install_plan_sampler loop to
safely traverse the nested attributes (e.g., check for None or use nested
getattr calls) so you only access .name when installplan exists on
cnv_subscription.instance.status; target the install_plan_sampler loop and the
variable install_plan_name_in_subscription to implement the guarded lookup.

for ip in install_plan_samples:
# If we find a not-approved install plan that is associated with production catalogsource, we need
# to delete it. Deleting the install plan associated with production catalogsource, would cause
Expand All @@ -115,7 +123,7 @@ def wait_for_install_plan(
):
return ip
LOGGER.info(
f"Subscription: {subscription.name}, is associated with install plan:"
f"Subscription: {cnv_subscription.name}, is associated with install plan:"
f" {install_plan_name_in_subscription}"
)

Expand Down
2 changes: 1 addition & 1 deletion tests/virt/upgrade/test_upgrade_virt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class TestUpgradeVirt:
@pytest.mark.ocp_upgrade
@pytest.mark.sno
@pytest.mark.polarion("CNV-2974")
@pytest.mark.order("first")
@pytest.mark.order("second")
@pytest.mark.dependency(name=VMS_RUNNING_BEFORE_UPGRADE_TEST_NODE_ID)
def test_is_vm_running_before_upgrade(self, vms_for_upgrade, linux_boot_time_before_upgrade):
for vm in vms_for_upgrade:
Expand Down
1 change: 1 addition & 0 deletions utilities/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ class UpgradeStreams:
X_STREAM = "x-stream"
Y_STREAM = "y-stream"
Z_STREAM = "z-stream"
EUS = "eus"


IMAGE_CRON_STR = "image-cron"
Expand Down
Loading