INTEROP-9265: Add ACM operator upgrade step for OPP product upgrades

[amp-rh]

TL;DR

New step registry ref interop-opp-product-upgrade-acm that upgrades ACM via OLM subscription channel change and validates the operator reaches Succeeded phase. Wired into both OPP upgrade configs (4.22 and 5.0) after OCP platform upgrade, before downstream product upgrades.

What changed

• New step: interop/opp/product-upgrade/acm/ upgrades ACM by patching the Subscription to the target (or next available) channel, waits for CSV Succeeded, validates MCE co-upgrade, and confirms hub health
• Config integration: adds interop-opp-product-upgrade-acm ref + ACM_SUBSCRIPTION_NAMESPACE: ocm override to both upgrade workflow configs
• Parent directory: interop/opp/product-upgrade/ OWNERS establishes the namespace for future product upgrade steps (ACS, ODF, Quay)

Core files (review these first)

File	Lines	What to look at
.../acm/interop-opp-product-upgrade-acm-commands.sh	397	Upgrade logic: channel resolution, subscription patching, CSV wait, MCE co-upgrade, hub health checks, diagnostics on failure
.../acm/interop-opp-product-upgrade-acm-ref.yaml	49	Step definition: env vars (ACM_TARGET_CHANNEL, ACM_UPGRADE_TIMEOUT, ACM_SUBSCRIPTION_NAME, ACM_SUBSCRIPTION_NAMESPACE), 45m timeout, documentation

Mechanical/config files

File	Why it's routine
.../product-upgrade/OWNERS	Standard cspi-qe-ocp-lp team alias
.../product-upgrade/acm/OWNERS	Same
.../acm/interop-opp-product-upgrade-acm-ref.metadata.json	Generated metadata matching ref.yaml
.../stolostron-policy-collection-main__ocp4.22-upgrade.yaml	+2 lines: adds ref + env override
.../stolostron-policy-collection-main__ocp5.0-upgrade.yaml	+2 lines: adds ref + env override

Risk areas

• Channel auto-resolution (commands.sh ResolveTargetChannel): When ACM_TARGET_CHANNEL is empty, the script queries the packagemanifest for the next channel above the installed version. This is the most complex logic path; relies on release-X.Y channel naming convention.
• MCE co-upgrade wait: ACM upgrades trigger an MCE (MultiCluster Engine) upgrade. The script waits for MCE CSV Succeeded separately, which could extend total time beyond the 30m ACM_UPGRADE_TIMEOUT default. The step-level timeout is 45m to accommodate this.
• Namespace override: Configs set ACM_SUBSCRIPTION_NAMESPACE: ocm (not the default open-cluster-management) to match how install-operators deploys ACM in the OPP pipeline.

Testing

• ci/prow/ci-operator-registry passes (step ref validation)
• ci/prow/step-registry-shellcheck passes
• ci/prow/owners passes (OWNERS file validation)
• Rehearsal runs fail due to OperatorGroup issue in the upgrade workflow (tracked in sibling PR #82920), not related to this PR's code

Jira

• INTEROP-9265: Implement ACM operator upgrade step
• INTEROP-9381: Implement ACM operator upgrade and health validation
• Parent: INTEROP-8941: Operator Upgrade Testing

[openshift-ci-robot]

@amp-rh: This pull request references INTEROP-9265 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Add step registry entry at interop/opp/product-upgrade/acm/ that upgrades ACM via OLM subscription channel change and validates the operator reaches Succeeded phase.

What it does:

• Identifies current ACM version via CSV
• Patches the Subscription to the target channel (or resolves next available)
• Waits for the new CSV to reach Succeeded phase
• Validates MCE (MultiCluster Engine) co-upgrade completion
• Confirms hub health (MCH phase, policy propagator, managed clusters)

This step gates downstream product upgrades (ACS, ODF, Quay) in the OPP coordinated product upgrade workflow (INTEROP-8941).

Jira

• INTEROP-9265: Implement ACM operator upgrade step
• Parent: INTEROP-8941: Operator Upgrade Testing

Files

• ci-operator/step-registry/interop/opp/product-upgrade/OWNERS
• ci-operator/step-registry/interop/opp/product-upgrade/acm/OWNERS
• ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh
• ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-ref.yaml
• ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-ref.metadata.json

Test plan

• ci/prow/ci-operator-registry passes (step ref validation)
• ci/prow/step-registry-shellcheck passes
• ci/prow/owners passes (OWNERS file validation)
• Prow rehearsal confirms step loads without error

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds a CI step-registry entry that upgrades the ACM operator through an OLM subscription channel change, approves InstallPlans, waits for completion, validates MCE and hub health, and collects diagnostics and summary artifacts.

Changes

ACM operator upgrade

Layer / File(s)	Summary
Step contract and ownership ci-operator/step-registry/interop/opp/product-upgrade/...	Defines ACM step metadata, runtime settings, environment variables, documentation, and approver/reviewer ownership.
Subscription upgrade orchestration ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh	Resolves the target channel, patches the subscription, handles InstallPlans, polls CSV completion, and collects failure diagnostics.
Health validation and test integration ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh, ci-operator/config/stolostron/policy-collection/*	Validates MCE and ACM hub health, writes upgrade results, and adds the step to OCP 4.22 and OCP 5.0 upgrade tests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI as ACM upgrade step
  participant Subscription as OLM subscription
  participant InstallPlan
  participant CSV as ACM CSV
  participant Cluster as ACM cluster
  CI->>Subscription: Resolve and patch target channel
  Subscription-->>InstallPlan: Create or expose upgrade InstallPlan
  CI->>InstallPlan: Approve manual InstallPlan
  InstallPlan->>CSV: Install upgraded CSV
  CI->>CSV: Wait for Succeeded
  CI->>Cluster: Validate MCE and hub health
  Cluster-->>CI: Return health status

Loading

Suggested reviewers: shakyav, chaclark1974

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	The PR changes only OWNERS, YAML, JSON, and Bash files; searches found no Ginkgo test declarations or test titles to evaluate.
Test Structure And Quality	✅ Passed	The patch adds only YAML, JSON, OWNERS, and Bash files; it contains no Ginkgo test files, It blocks, or Ginkgo assertions to review.
Microshift Test Compatibility	✅ Passed	The PR adds a shell step, YAML, metadata, OWNERS, and configuration entries. No new Ginkgo tests or It/Describe/Context/When constructs were found.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR changes only YAML configuration and a Bash step; no new Ginkgo e2e tests or multi-node test assumptions are present.
Topology-Aware Scheduling Compatibility	✅ Passed	The PR adds an OLM upgrade script and CI step references only. It adds no manifests, replicas, affinity, topology spread, node selectors, tolerations, or PDB constraints.
Ote Binary Stdout Contract	✅ Passed	The PR changes only OWNERS, YAML, JSON, and a Bash step; it adds no OTE Go binary, main/suite setup, or process-level stdout writes.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR adds a Bash OLM upgrade step and YAML configuration, not Ginkgo tests; searches found no Ginkgo constructs, IPv4 assumptions, or external connectivity calls.
No-Weak-Crypto	✅ Passed	The complete 457-line PR addition scan found no MD5, SHA1, DES, RC4, Blowfish, ECB, crypto API, custom crypto, or secret/token comparison usage.
Container-Privileges	✅ Passed	The complete PR diff adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or root settings.
No-Sensitive-Data-In-Logs	✅ Passed	The new script logs only ACM resource names, versions, channels, phases, counts, and bounded status diagnostics; it does not log secrets, tokens, credentials, PII, customer data, or pod logs.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding an ACM operator upgrade step for OPP product upgrades.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[redhat-chai-bot]

Code Review: ACM Operator Upgrade Step

Overall this is a well-structured step with clean function decomposition, proper artifact collection, defensive error handling in diagnostics, and thorough post-upgrade validation (MCE + hub health). The ref YAML documentation with exit codes is excellent. A few issues and suggestions below.

---

Bug: set -e + inherit_errexit makes empty-value checks unreachable

The script sets set -euo pipefail and shopt -s inherit_errexit. With inherit_errexit, command substitutions inherit set -e, so if oc get inside a helper function returns non-zero (e.g. subscription doesn't exist yet, transient API error), the subshell exits immediately, which then triggers set -e in the calling scope — before the subsequent empty-string check is ever reached.

Affected call sites (non-exhaustive):

# Main section — if oc get fails, script exits before the -z check:
current_csv="$(get_current_csv)"
if [[ -z "${current_csv}" ]]; then  # ← never reached on oc failure
    echo >&2 "ERROR: No ACM subscription found or no currentCSV set"
    exit 3
fi

# wait_for_csv_succeeded — transient API blip kills the polling loop:
new_csv="$(get_current_csv)"
if [[ -z "${new_csv}" ]]; then  # ← never reached
    sleep 10
    continue
fi

This pattern repeats for get_current_channel, get_csv_phase, and get_installed_version.

Suggested fix — add || true at call sites that expect empty output as a valid result:

current_csv="$(get_current_csv || true)"
if [[ -z "${current_csv}" ]]; then
    ...
fi

Or equivalently, add || true inside the helper functions after oc get ... 2>/dev/null || true. This is the more robust approach since it fixes every caller at once.

---

Bug: Inconsistent jsonpath for InstallPlan reference

The script uses two different jsonpaths for the same field:

# Line ~298 ("same channel" check) — correct, current API field:
-o jsonpath='{.status.installPlanRef.name}'

# Line ~316 (post-channel-patch polling loop) — deprecated field:
-o jsonpath='{.status.installplan.name}'

Per the OLM SubscriptionStatus type definition, .status.installplan (JSON tag of the Install field) is explicitly marked DEPRECATED in favor of .status.installPlanRef. Both fields are currently populated by OLM for backward compatibility, but only installPlanRef includes the full object reference (with namespace and resourceVersion).

Fix: Use .status.installPlanRef.name consistently in both locations.

---

Suggestion: parse_timeout should warn on unrecognized formats

parse_timeout returns 0 seconds for any input it can't parse (e.g. 30min, abc). This would cause wait_for_csv_succeeded to timeout immediately with no indication that the timeout value was invalid.

parse_timeout() {
    local input="$1"
    # ... existing cases ...
    else
        echo >&2 "WARNING: Unrecognized timeout format '${input}', defaulting to 30m"
        minutes=30
    fi
    echo "$(( minutes * 60 + seconds ))"
}

---

Suggestion: Write upgrade result to SHARED_DIR for downstream steps

The PR description says this step "gates downstream product upgrades (ACS, ODF, Quay) in the OPP coordinated product upgrade workflow." Downstream steps may need to know the post-upgrade ACM version and channel. Consider writing the result to SHARED_DIR:

echo "${new_version}" > "${SHARED_DIR}/acm-upgraded-version"
echo "${target_channel}" > "${SHARED_DIR}/acm-upgraded-channel"

This follows the standard inter-step communication pattern in the step registry and keeps the 1MB flat-file constraint.

---

Minor: validate_hub_health pipefail edge case

cluster_count="$(oc get managedclusters --no-headers 2>/dev/null | wc -l || echo 0)"

If oc get fails (API unreachable, CRD not found), wc -l outputs 0 from empty input, but pipefail makes the pipeline fail, triggering || echo 0. The command substitution captures output from both wc -l and echo 0, resulting in "0\n0". Since this is only used for informational logging and validate_hub_health always returns 0 regardless, the impact is cosmetic (double 0 in the log). A cleaner pattern:

cluster_count="$(oc get managedclusters --no-headers 2>/dev/null | wc -l)" || cluster_count=0

---

Positive notes

• shopt -s inherit_errexit is a nice strictness addition (though it creates the interaction noted above).
• The collect_diagnostics trap with || true on every oc call is solid — it won't interfere with the original exit code.
• The resolve_target_channel version comparison logic correctly finds the minimum-next-version across unsorted channel lists.
• MCE co-upgrade validation with displayName + label fallback is a good defensive pattern.
• The ref YAML env documentation and exit code table are well-structured.

Thanks for a solid contribution. The set -e / inherit_errexit interaction is the most important item — it could cause silent failures in production runs.

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (1)

ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh (1)

79-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Empty spec.sourceNamespace degrades into a misleading error.

If the Subscription omits sourceNamespace, the packagemanifest query runs against the current context namespace and returns nothing, surfacing as "No channels found" rather than the real cause. Default it to openshift-marketplace and validate package_name too.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh`
around lines 79 - 97, Update the subscription lookup flow to default an empty
catalog_namespace from spec.sourceNamespace to openshift-marketplace before
querying packagemanifest. Also validate package_name immediately after
retrieving it and return a clear error when it is empty, before executing the
channels lookup; preserve the existing no-channels handling for valid package
manifests.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh`:
- Around line 303-312: Update the same-channel logic in the target_channel check
to verify that the referenced InstallPlan represents a pending upgrade, rather
than treating any installPlanRef.name as pending. Inspect the referenced
InstallPlan’s spec.clusterServiceVersionNames and status.phase, or compare the
subscription’s currentCSV and installedCSV, and exit 0 only when no upgrade is
actually available; preserve the existing upgrade flow when one is pending.
- Around line 36-40: Update get_current_csv and the corresponding subscription
lookups around the callers near lines 290-297 to explicitly handle oc failures.
Ensure command substitution failures are caught without triggering set -e, then
return an empty result so the existing “No ACM subscription found” handling
executes and exits with status 3.
- Around line 180-193: Update parse_timeout to reject inputs that match none of
its supported formats instead of echoing 0. After the existing parsing branches,
return a nonzero status and emit a clear error for invalid values such as
“1h30m” or “45min”; preserve the current seconds conversion for valid minute,
second, hour, and numeric inputs.
- Around line 143-178: Update wait_for_csv_succeeded to ignore the pre-upgrade
CSV by requiring get_current_csv to differ from the previously installed
current_csv before accepting a Succeeded phase. Preserve the existing timeout,
failure handling, and polling behavior, and only return success after the
replacement CSV reports Succeeded.
- Around line 323-351: Update the InstallPlan lookup in the retry loop to read
`.status.installPlanRef.name`, matching the earlier subscription lookup. Scope
the fallback `oc get installplan` query to InstallPlans whose owner reference
name matches `${ACM_SUBSCRIPTION_NAME}` before selecting the newest result, so
the approval logic only targets the ACM Subscription’s InstallPlan.

---

Nitpick comments:
In
`@ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh`:
- Around line 79-97: Update the subscription lookup flow to default an empty
catalog_namespace from spec.sourceNamespace to openshift-marketplace before
querying packagemanifest. Also validate package_name immediately after
retrieving it and return a clear error when it is empty, before executing the
channels lookup; preserve the existing no-channels handling for valid package
manifests.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: e4f7e528-1fa1-4acd-9866-6113eb749e67

📥 Commits

Reviewing files that changed from the base of the PR and between e38f568 and e828887.

📒 Files selected for processing (5)

• ci-operator/step-registry/interop/opp/product-upgrade/OWNERS
• ci-operator/step-registry/interop/opp/product-upgrade/acm/OWNERS
• ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-commands.sh
• ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-ref.metadata.json
• ci-operator/step-registry/interop/opp/product-upgrade/acm/interop-opp-product-upgrade-acm-ref.yaml

[redhat-chai-bot]

Review of fix commit 024e404

Reviewed the full PR diff with a focus on the two bug fixes in the latest push. Both fixes are correct and well-implemented.

---

✅ Fix 1: CSV name gating in wait_for_csv_succeeded

The function now takes previous_csv as a parameter and skips poll iterations where currentCSV still matches the pre-upgrade value:

if [[ -z "${new_csv}" || "${new_csv}" == "${previous_csv}" ]]; then
    sleep 10
    continue
fi

This correctly prevents the race where OLM hasn't yet swapped the CSV after the channel patch — the old CSV is already Succeeded, so the loop would have returned immediately and reported a successful upgrade that never happened. The call site on line 389 passes "${current_csv}" correctly.

✅ Fix 2: InstallPlan phase check for same-channel upgrades

The same-channel path now fetches the referenced InstallPlan's status.phase instead of relying on the mere presence of installPlanRef.name:

plan_phase="$(oc get installplan "${install_plan}" \
    -n "${ACM_SUBSCRIPTION_NAMESPACE}" \
    -o jsonpath='{.status.phase}' 2>/dev/null || true)"
if [[ "${plan_phase}" == "Complete" ]]; then
    echo "InstallPlan ${install_plan} already complete; no pending upgrade"
    exit 0
fi

This fixes the stale-ref problem: installPlanRef.name persists after the previously-applied InstallPlan completes, so checking .name != "" alone would almost never take the "nothing to do" early exit. Gating on phase == Complete is the correct approach.

Other items confirmed addressed

• parse_timeout fallback: Now logs WARNING: Unrecognized timeout format and defaults to 30m instead of silently yielding 0 seconds. Good.
• SHARED_DIR output: Writes acm-upgraded-version and acm-upgraded-channel for downstream steps, guarded by [[ -n "${SHARED_DIR:-}" ]]. Follows the standard inter-step communication pattern.
• Consistent installPlanRef.name: All three subscription lookups (lines 335, 362, and the same-channel check) now use .status.installPlanRef.name consistently.

---

Minor remaining observation (non-blocking)

validate_hub_health pipefail edge case (line 301):

cluster_count="$(oc get managedclusters --no-headers 2>/dev/null | wc -l || echo 0)"

With pipefail, if oc get fails, the pipeline exit status is non-zero, so || echo 0 fires. But wc -l already output 0 from the empty stdin, so the command substitution captures both outputs ("0\n0"). Since this value is only used in an informational log line and validate_hub_health always returns 0 regardless, the impact is purely cosmetic (a double 0 in the log). A slightly cleaner pattern if you want to tidy it up in a follow-up:

cluster_count="$(oc get managedclusters --no-headers 2>/dev/null | wc -l)" || cluster_count=0

Same applies to available_count on line 302.

---

Overall: both fixes correctly address the identified bugs, the code is clean and well-structured, and CI previously passed. LGTM pending the current CI run on this push.

[redhat-chai-bot]

Readiness Review for External Product Team Review

Verified this PR against INTEROP-9265 acceptance criteria and the parent epic INTEROP-8941 (Automated Product Upgrade Testing).

Linkage

• ✅ PR title references INTEROP-9265
• ✅ Jira ticket has remote link to this PR
• ✅ openshift-ci-robot confirmed valid Jira reference

Acceptance Criteria Coverage

AC	Status	Evidence
Step registry entry in openshift/release	✅	5 files at ci-operator/step-registry/interop/opp/product-upgrade/acm/
Script changes ACM subscription channel	✅	ResolveTargetChannel + oc patch subscription
Monitor operator upgrade via CSV status	✅	WaitForCsvSucceeded polling loop with timeout
Validate MCE upgrade completion	✅	ValidateMceUpgrade with displayName + label fallback
Confirm hub-to-managed-cluster connectivity	✅	ValidateHubHealth checks MCH phase, policy propagator, managed clusters
Structured exit codes	✅	0=success, 1=failure, 2=timeout, 3=precondition
Artifact collection on failure	✅	CollectDiagnostics EXIT trap + upgrade summary to ARTIFACT_DIR
ACM upgrade succeeds in CI	⏳	Tracked separately in INTEROP-9381 (wiring into periodic job)

Code Quality

• All review feedback addressed (2 bugs fixed in commit 024e404: CSV race condition, stale InstallPlan detection)
• All 5 CodeRabbit review threads resolved
• Only one non-blocking cosmetic item remains (pipefail double-0 in validate_hub_health log output)
• OWNERS files correctly assign cspi-qe-ocp-lp
• Ref YAML well-documented with env vars, timeouts, and exit codes

CI Status

• 10/11 checks passed (all substantive checks green)
• tide pending (normal merge bot state)
• Passed: ci-operator-registry, step-registry-shellcheck, owners, yamllint, ci-operator-config, core-valid, step-registry-metadata

Epic Alignment

This PR correctly implements the ACM upgrade step as the first product in the OPP coordinated upgrade sequence (ACM → ACS → ODF → Quay), consistent with the epic's defined dependency order. The step writes upgrade results to SHARED_DIR for downstream step consumption.

Verdict: Ready for external product team review. The implementation is complete against all verifiable acceptance criteria, review feedback has been addressed, and CI is green.

---

AI-generated. Review for accuracy.

[amp-rh]

Ownership Consideration for Long-Term Sustainability

The implementation is well-structured — clean function decomposition, proper artifact collection, defensive error handling, and thorough post-upgrade validation (MCE + hub health). The exit code documentation in the ref YAML is excellent. Technically solid work.

That said, I want to flag a long-term sustainability concern: this step contains ~387 lines of deep ACM domain knowledge (subscription channel resolution, MCE co-upgrade validation, MultiClusterHub phase monitoring, policy propagator health, managed cluster availability). As ACM's architecture evolves — changes to MCE coupling, new hub CRDs, subscription model changes — maintaining this step will require ACM team expertise.

Current precedent in the step registry:
Other product teams own their operator lifecycle steps directly:

• ci-operator/step-registry/openshift/hypershift/ — Hypershift team
• ci-operator/step-registry/openshift/lvm-operator/ — LVM team
• ci-operator/step-registry/redhat-appstudio/installer/ — AppStudio team

DPTP policy (DPTP-1522) explicitly states: "We expect these kinds of installation steps to be owned and maintained by SMEs."

Suggestion for structuring toward eventual handoff:

1. The step path (interop/opp/product-upgrade/acm/) is fine for now
2. Consider updating OWNERS to include ACM team reviewers alongside cspi-qe-ocp-lp, so the ACM team builds familiarity with the code
3. The interop team's orchestration chain that calls this step (sequencing ACM before ACS/ODF/Quay) should remain interop-owned
4. Over time, transfer OWNERS approver authority to the ACM team

This isn't a merge blocker — the work is correct, valuable, and needed now. It's a heads-up that we should plan for the ACM team to take ownership of the step content while the interop team retains ownership of the multi-product orchestration layer.

We'll discuss at the next OPP sync. See also: OSSM-14040 (precedent for interop-to-product ownership transition), ACM-3702 (ACM accepted CI step ownership).

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amp-rh
Once this PR has been reviewed and has the lgtm label, please assign gparvin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• ci-operator/config/stolostron/policy-collection/OWNERS
• ci-operator/step-registry/interop/opp/OWNERS [amp-rh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[amp-rh]

/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp4.22-upgrade-interop-opp-upgrade-aws

[amp-rh]

/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp5.0-upgrade-interop-opp-upgrade-aws

[openshift-merge-bot]

@amp-rh: your /pj-rehearse request was not processed because the request waited in queue for longer than 5 minutes. Please retry in a few minutes.

[openshift-merge-bot]

@amp-rh: your /pj-rehearse request was not processed because the request waited in queue for longer than 5 minutes. Please retry in a few minutes.

[amp-rh]

/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp4.22-upgrade-interop-opp-upgrade-aws

[amp-rh]

/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp5.0-upgrade-interop-opp-upgrade-aws

[openshift-merge-bot]

@amp-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

[openshift-merge-bot]

@amp-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

[openshift-ci]

@amp-rh: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/rehearse/periodic-ci-stolostron-policy-collection-main-ocp4.22-upgrade-interop-opp-upgrade-aws	834e514	link	unknown	/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp4.22-upgrade-interop-opp-upgrade-aws
ci/rehearse/periodic-ci-stolostron-policy-collection-main-ocp5.0-upgrade-interop-opp-upgrade-aws	834e514	link	unknown	/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp5.0-upgrade-interop-opp-upgrade-aws

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[amp-rh]

Both rehearsal failures are pre-existing and unrelated to PR changes. The ocp4.22 job fails in install-operators (rhacs-operator deployment timeout), and the ocp5.0 job fails in interop-opp-preflight (jq install failure). Both production jobs have 100% failure rate with identical errors across all recent runs. The new ACM operator upgrade step introduced by this PR was never reached in either rehearsal.

[amp-rh]

/pj-rehearse ack

[openshift-merge-bot]

@amp-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

[amp-rh]

Note: This PR is also included in combined PR #83405, which bundles all 8 OPP Q3 pipeline improvements into a single review target. Either this PR or #83405 can be merged (not both).

[openshift-merge-bot]

[REHEARSALNOTIFIER]
@amp-rh: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name	Repo	Type	Reason
periodic-ci-stolostron-policy-collection-main-ocp4.22-upgrade-interop-opp-upgrade-aws	N/A	periodic	Ci-operator config changed
periodic-ci-stolostron-policy-collection-main-ocp5.0-upgrade-interop-opp-upgrade-aws	N/A	periodic	Ci-operator config changed

Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.