Skip to content

DNM: swap to rhel 8 for testing - #82644

Open
jbpratt wants to merge 3 commits into
openshift:mainfrom
jbpratt:patch-2
Open

DNM: swap to rhel 8 for testing#82644
jbpratt wants to merge 3 commits into
openshift:mainfrom
jbpratt:patch-2

Conversation

@jbpratt

@jbpratt jbpratt commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

Updates OpenShift CI’s Quay OMR disconnected install/upgrade testing scripts to support a RHEL 8-based (DNM) environment:

  • Dedicated host provisioning (OMR v2): Switches AMI discovery to select official RHEL 8 hourly AWS images instead of RHEL 9, including updating the AMI resolution failure message for the chosen region.
  • OMR v2 install step: Adjusts the embedded health-check curl command to be compatible with older curl on RHEL 8 by removing --retry-all-errors while preserving the existing retry behavior and health endpoint.
  • OMR v2 → v3 upgrade helper (OMR v3 upgrade-from-v2): Changes the migrate-to-v3 helper from a rootless migration to a root / cgroups-v1 experiment flow—using /var/lib/quay, stopping user-level quay-* services, running the migration via sudo, and ensuring health checking and certificate installation use root-owned paths (with sudo for service status and /healthz calls).

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

OMR v2 provisioning now selects official RHEL 8 hourly AMIs and uses an older-curl-compatible health check. OMR v3 upgrades now use a root-based migration flow with /var/lib/quay, sudo-managed operations, and updated completion messaging.

Changes

Quay OMR compatibility and migration

Layer / File(s) Summary
RHEL 8 provisioning and health checks
ci-operator/step-registry/quay/omr-v2/host-provision/..., ci-operator/step-registry/quay/omr-v2/install/...
AMI discovery and diagnostics now target official RHEL 8 images, while the health check removes the curl option unavailable on RHEL 8 and retains its retry settings.
Root OMR v3 migration
ci-operator/step-registry/quay/omr-v3/upgrade-from-v2/...
The migration helper stops user services, runs migration with sudo into /var/lib/quay, uses root-owned health and certificate paths, and updates the success message for the cgroups-v1 experiment.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: aroyoredhat, marcusk19

Sequence Diagram(s)

sequenceDiagram
  participant UserServices
  participant MigrateToV3
  participant QuayInstaller
  participant RootData
  UserServices->>MigrateToV3: stop quay services
  MigrateToV3->>QuayInstaller: run sudo migration
  QuayInstaller->>RootData: write migration data and logs
  MigrateToV3->>RootData: check health and install certificates
Loading
🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and correctly reflects the RHEL 8 testing change, even though the PR also includes other related install and upgrade updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 PR only changes shell step-registry scripts; no Ginkgo test titles or dynamic test names were added or modified.
Test Structure And Quality ✅ Passed PR changes only step-registry shell scripts; no Ginkgo test files or It blocks were modified, so the test-quality check is not applicable.
Microshift Test Compatibility ✅ Passed No new Ginkgo tests were added; the PR only changes shell step scripts, and none contain MicroShift-affected OpenShift API usage.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR only changes three ci-operator shell scripts; no Ginkgo/e2e test files or It/Describe/Context additions were introduced.
Topology-Aware Scheduling Compatibility ✅ Passed The PR only changes CI step shell scripts for OMR host/install/upgrade logic; no manifests, controllers, replicas, affinities, or node selectors were added.
Ote Binary Stdout Contract ✅ Passed PR only changes ci-operator shell steps; no OTE binary main/TestMain/suite-setup stdout paths were touched.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS: The PR changes only a shell step-registry script; no new Ginkgo tests were added, and the modified commands use the cluster mirror registry/hostname without IPv4-only assumptions.
No-Weak-Crypto ✅ Passed Touched scripts only change AMI, curl retry, and migration paths; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or secret comparisons were introduced.
Container-Privileges ✅ Passed The PR only changes shell scripts; no container/K8s manifests were added or edited, and the files contain no privileged/hostNetwork/SYS_ADMIN/allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs ✅ Passed The diff adds no secret-bearing logs; new messages only mention AMI/region and sanitized artifacts, while credential/hostname outputs are unchanged and still redacted.
✨ 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

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

@jbpratt

jbpratt commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-quay-quay-master-omr-v2-upgrade-v3-disconnected-install

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/quay/omr-v2/host-provision/quay-omr-v2-host-provision-commands.sh`:
- Line 105: Update the documentation in the host-provision step reference for
OMR_V2_AMI_ID and the provisioned host to state RHEL 8 instead of RHEL 9,
ensuring explicit AMI inputs and generated guidance match the RHEL 8 discovery
behavior.
🪄 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: 345f9648-bfcf-4c21-b89b-929a94396aa2

📥 Commits

Reviewing files that changed from the base of the PR and between 64e7ba1 and a8b30b8.

📒 Files selected for processing (1)
  • ci-operator/step-registry/quay/omr-v2/host-provision/quay-omr-v2-host-provision-commands.sh

--owners 309956199498 \
--filters \
'Name=name,Values=RHEL-9.*_HVM-*-x86_64-*-Hourly2-GP3' \
'Name=name,Values=RHEL-8.*_HVM-*-x86_64-*-Hourly2-GP3' \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Update the step contract to RHEL 8.

The script now discovers RHEL 8 AMIs, but ci-operator/step-registry/quay/omr-v2/host-provision/quay-omr-v2-host-provision-ref.yaml still documents OMR_V2_AMI_ID and the provisioned host as RHEL 9. Update that file’s documentation to RHEL 8 so explicit AMI inputs and generated guidance match the new behavior.

🤖 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/quay/omr-v2/host-provision/quay-omr-v2-host-provision-commands.sh`
at line 105, Update the documentation in the host-provision step reference for
OMR_V2_AMI_ID and the provisioned host to state RHEL 8 instead of RHEL 9,
ensuring explicit AMI inputs and generated guidance match the RHEL 8 discovery
behavior.

@openshift-ci
openshift-ci Bot requested review from Marcusk19 and aroyoredhat July 29, 2026 20:33
@jbpratt

jbpratt commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

/label wip

@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbpratt

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2026
@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@jbpratt: The label(s) /label wip cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, ux-approved, no-qe, rebase/manual, cluster-config-api-changed, run-integration-tests, verified, ready-for-human-review, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/skip-dependent-bug-check, jira/valid-bug, ok-to-test, priority/ci-critical, stability-fix-approved, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

Details

In response to this:

/label wip

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.

@harishsurf

Copy link
Copy Markdown
Contributor

/hold

@jbpratt

jbpratt commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

/label do-not-merge/work-in-progress

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2026
@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@jbpratt: The label(s) /label do-not-merge/work-in-progress cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, ux-approved, no-qe, rebase/manual, cluster-config-api-changed, run-integration-tests, verified, ready-for-human-review, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/skip-dependent-bug-check, jira/valid-bug, ok-to-test, priority/ci-critical, stability-fix-approved, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

Details

In response to this:

/label do-not-merge/work-in-progress

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.

Signed-off-by: Brady Pratt <[email protected]>
@jbpratt

jbpratt commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-quay-quay-master-omr-v2-upgrade-v3-disconnected-install

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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

@jbpratt

jbpratt commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-quay-quay-master-omr-v2-upgrade-v3-disconnected-install

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@jbpratt: 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
pull-ci-quay-mirror-registry-main-omr-v2-disconnected-install quay/mirror-registry presubmit Registry content changed
periodic-ci-quay-quay-master-omr-v2-upgrade-v3-disconnected-install N/A periodic Registry content 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh`:
- Around line 154-162: Update the certificate installation flow after the health
check to use /var/lib/quay/ssl.cert as the source and copy it to ca.crt in the
existing cert_dir, replacing the rootCA.pem source path. Keep the migrated
root-owned registry paths used by the health check and podman authentication
consistent.
🪄 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: 1248d4fd-34b1-4756-9ca1-cfb084e38a36

📥 Commits

Reviewing files that changed from the base of the PR and between a2fabe3 and 5c4d6be.

📒 Files selected for processing (1)
  • ci-operator/step-registry/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh

Comment on lines +154 to +162
sudo curl --retry 20 --retry-delay 3 --retry-all-errors \
--silent --show-error --fail \
--cacert "/home/$(id -un)/quay-install/quay-rootCA/rootCA.pem" \
--cacert "${data_dir}/rootCA.pem" \
"https://${registry_endpoint}/healthz" >/dev/null
cert_dir="${work_dir}/certs"
auth_file="${work_dir}/registry-auth.json"
install -d -m 0700 "${cert_dir}"
install -m 0600 \
"/home/$(id -un)/quay-install/quay-rootCA/rootCA.pem" \
sudo install -m 0644 \
"${data_dir}/rootCA.pem" \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for cert filename conventions used across the quay step-registry scripts,
# and whether rootCA.pem appears anywhere else (e.g. already-established for `migrate`).
rg -n 'rootCA\.pem|ssl\.cert' ci-operator/step-registry/quay --glob '*.sh'

Repository: openshift/release

Length of output: 2100


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect relevant script sections and related files to understand data_dir definition
# and whether rootCA.pem is produced elsewhere in this path before being read.
for f in \
  ci-operator/step-registry/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh \
  ci-operator/step-registry/quay/omr-v3/gather/quay-omr-v3-gather-commands.sh \
  ci-operator/step-registry/quay/omr-v3/install/quay-omr-v3-install-commands.sh \
  ci-operator/step-registry/quay/omr-v3/install/quay-omr-v3-setup-commands.sh \
  ci-operator/step-registry/quay/omr-v3/install/quay-omr-v3-pre-commands.sh
do
  if [ -f "$f" ]; then
    echo "===== $f ====="
    wc -l "$f"
    rg -n 'data_dir|migrate|ssl\.cert|rootCA\.pem|quay-rootCA|migrate|healthz|\$curl|SUDO|sudo' "$f" || true
  fi
done

echo "===== outline ====="
ast-grep outline ci-operator/step-registry/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh || true

echo "===== relevant upgrade excerpts ====="
sed -n '1,230p' ci-operator/quay/step-registry/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh 2>/dev/null || \
sed -n '1,230p' ci-operator/step-registry/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh

Repository: openshift/release

Length of output: 10567


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the upgrade script around the certificate usage and nearby migration/setup steps.
file="ci-operator/step-registry/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh"
echo "===== $file line count ====="
wc -l "$file"

echo "===== $file relevant excerpts ====="
sed -n '1,230p' "$file"

echo "===== search for data_dir and migrate in upgrade ===="
rg -n 'data_dir|quay migrate|migrate|quay/ssl\.cert|quay-rootCA/rootCA\.pem' "$file"

Repository: openshift/release

Length of output: 8411


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "===== repository files mentioning ssl.cert ====="
rg -n 'ssl\.cert|rootCA\.pem|quay-rootCA' . --glob '*.go' --glob '*.py' --glob '*.sh' --glob '*.yaml' --glob '*.yml' || true

echo "===== step registry quay files ====="
git ls-files 'ci-operator/step-registry/quay/**/*'

Repository: openshift/release

Length of output: 13574


🌐 Web query:

Quay OMR migration -data-dir rootCA.pem ssl.cert

💡 Result:

When working with Red Hat Quay, including migrations from mirror-registry (OMR) or standalone deployments, the rootCA.pem and ssl.cert files are central to configuring secure TLS communication [1][2]. In typical standalone and OMR configurations, the following approach is used: 1. SSL/TLS Certificate Files: Red Hat Quay expects the server certificate (ssl.cert) and private key (ssl.key) to be present in the configuration directory [1][3][4]. 2. Root CA Handling: The rootCA.pem file represents your Certificate Authority [1][2]. To ensure full certificate chain validation, it is common practice to append the contents of rootCA.pem to ssl.cert [1][2][4]: cat rootCA.pem >> ssl.cert 3. Deployment Contexts: - Standalone/OMR: Certificates are manually managed in the configuration directory and referenced by the config.yaml file [2][4]. - OpenShift (Operator): Custom certificates are typically applied by creating a configBundleSecret in OpenShift and referencing it within the QuayRegistry custom resource, often using the extra_ca_cert field to include additional CAs if necessary [5]. - Migrations: If you are migrating an OMR installation to a Go-based Quay binary (via the quay migrate command), the migration process is designed to checkpoint and copy existing TLS certificates and storage from the old environment to the new deployment [6]. Always ensure that any client or host needing to access the registry has the rootCA.pem imported into its system-wide trust store (e.g., /etc/pki/ca-trust/source/anchors/ followed by update-ca-trust) to prevent SSL verification errors [7][4].

Citations:


Use /var/lib/quay/ssl.cert for the migrated root-owned registry.

The migration runs quay migrate -data-dir /var/lib/quay, but the same root-owned install/gather flows read /var/lib/quay/ssl.cert for the registry CA. Expect this path to copy ssl.cert to ca.crt instead of a rootCA.pem file in /var/lib/quay; otherwise the health check and podman login rely on a missing file.

🤖 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/quay/omr-v3/upgrade-from-v2/quay-omr-v3-upgrade-from-v2-commands.sh`
around lines 154 - 162, Update the certificate installation flow after the
health check to use /var/lib/quay/ssl.cert as the source and copy it to ca.crt
in the existing cert_dir, replacing the rootCA.pem source path. Keep the
migrated root-owned registry paths used by the health check and podman
authentication consistent.

@openshift-ci

openshift-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

@jbpratt: The following test 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-quay-quay-master-omr-v2-upgrade-v3-disconnected-install 5c4d6be link unknown /pj-rehearse periodic-ci-quay-quay-master-omr-v2-upgrade-v3-disconnected-install

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants