Add a provision healthcheck job per region periodic job#80138
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: roivaz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds periodic CI healthchecks for ARO-HCP: new ci-operator config scheduling regional provision workflows, a provision-healthcheck workflow with ownership metadata, and a provision script change to apply MSI mock SP overrides only when leased. ChangesARO-HCP Periodic Healthcheck
Sequence DiagramsequenceDiagram
participant Cron
participant CI_Operator
participant ProvisionWorkflow as aro_hcp_provision_healthcheck
participant ProvisionScript as aro_hcp_provision_environment
Cron->>CI_Operator: trigger scheduled job (region)
CI_Operator->>ProvisionWorkflow: start workflow (pre/test/post)
ProvisionWorkflow->>ProvisionScript: run provision step (may apply MSI mock overrides)
ProvisionWorkflow->>ProvisionScript: run post steps (gather/deprovision)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh`:
- Around line 100-109: When LEASED_MSI_MOCK_SP is set, the script currently
reads MSI_MOCK_CLIENT_ID, MSI_MOCK_PRINCIPAL_ID, and MSI_MOCK_CERT_NAME from
dev-infrastructure/openshift-ci/msi-mock-pool.yaml and blindly writes them to
OVERRIDE_CONFIG_FILE; if the SP key is missing the yq reads yield empty/"null"
and you must fail fast. After populating MSI_MOCK_CLIENT_ID,
MSI_MOCK_PRINCIPAL_ID, and MSI_MOCK_CERT_NAME, validate that none are empty or
equal to "null" (check variables MSI_MOCK_CLIENT_ID, MSI_MOCK_PRINCIPAL_ID,
MSI_MOCK_CERT_NAME) and if any validation fails, print an error including
LEASED_MSI_MOCK_SP and the missing field(s) and exit with a non‑zero status
instead of writing the override. Ensure the check runs before the yq -i override
to prevent propagating bad values.
🪄 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: Enterprise
Run ID: d3ca1e2e-0cc5-4495-9a70-9105c4e03629
⛔ Files ignored due to path filters (2)
ci-operator/jobs/Azure/ARO-HCP/Azure-ARO-HCP-main-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/Azure/ARO-HCP/Azure-ARO-HCP-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (4)
ci-operator/config/Azure/ARO-HCP/Azure-ARO-HCP-main__periodic-healthcheck.yamlci-operator/step-registry/aro-hcp/provision-healthcheck/OWNERSci-operator/step-registry/aro-hcp/provision-healthcheck/aro-hcp-provision-healthcheck-workflow.yamlci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh
roivaz
force-pushed
the
implement-provision-healthchecks
branch
from
7900ba0 to
94869cc
Compare
|
/pj-rehearse pull-ci-Azure-ARO-HCP-main-e2e-parallel |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
ci-operator/config/Azure/ARO-HCP/Azure-ARO-HCP-main__periodic-healthcheck.yaml (1)
77-77: ⚡ Quick winUse more restrictive permissions than
chmod 777.Setting
chmod 777makes the directory world-writable, allowing any process to modify the code. Consider using more restrictive permissions (e.g.,chmod 755for read/execute by all, orchmod 775if group-write is needed).🔒 Proposed fix for more restrictive permissions
- RUN chmod 777 /opt/app-root/src/github.com/Azure/ARO-HCP + RUN chmod 755 /opt/app-root/src/github.com/Azure/ARO-HCP🤖 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/config/Azure/ARO-HCP/Azure-ARO-HCP-main__periodic-healthcheck.yaml` at line 77, The Dockerfile line using the command "RUN chmod 777 /opt/app-root/src/github.com/Azure/ARO-HCP" grants world-writable permissions; change it to a more restrictive mode such as "chmod 755" (or "chmod 775" if group-write is intentionally required) and, if appropriate, ensure ownership is set to the intended runtime user via chown (e.g., chown -R <user>:<group> on the same path) so only the owner (and optionally group) can write while others can only read/execute.
🤖 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/config/Azure/ARO-HCP/Azure-ARO-HCP-main__periodic-healthcheck.yaml`:
- Line 71: The Dockerfile sets USER root (the "USER root" directive) with no
justification—either document why root is necessary (e.g., required privileged
steps for make/cleanup) or change to a non-root user; preferably revert to the
same non-root "src" base image user and adjust ownership/permissions for build
artifacts (use chown/chmod during image build) so make and cleanup can run
without root, and add a short comment explaining any remaining need for USER
root if you must keep it.
- Around line 1-84: The dockerfile_literal used for the aro-hcp-e2e-tools image
is running as root and uses an overly-permissive chmod 777; change the
Dockerfile literal to create/use a non-root user (e.g., add a dedicated
user/group and switch via USER) and replace chmod 777 with least-permission
adjustments (chown to the new user and chmod only what’s required, e.g., 0755 or
0700) or document/justify why root and 777 are necessary; update the
dockerfile_literal block (the FROM src ... USER root ... RUN chmod 777 ...
sequence) accordingly so the image no longer runs as root and files have
restrictive permissions.
---
Nitpick comments:
In
`@ci-operator/config/Azure/ARO-HCP/Azure-ARO-HCP-main__periodic-healthcheck.yaml`:
- Line 77: The Dockerfile line using the command "RUN chmod 777
/opt/app-root/src/github.com/Azure/ARO-HCP" grants world-writable permissions;
change it to a more restrictive mode such as "chmod 755" (or "chmod 775" if
group-write is intentionally required) and, if appropriate, ensure ownership is
set to the intended runtime user via chown (e.g., chown -R <user>:<group> on the
same path) so only the owner (and optionally group) can write while others can
only read/execute.
🪄 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: Enterprise
Run ID: a4273532-06c4-40ad-94db-4409043d8f9f
⛔ Files ignored due to path filters (2)
ci-operator/jobs/Azure/ARO-HCP/Azure-ARO-HCP-main-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/Azure/ARO-HCP/Azure-ARO-HCP-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (5)
ci-operator/config/Azure/ARO-HCP/Azure-ARO-HCP-main__periodic-healthcheck.yamlci-operator/step-registry/aro-hcp/provision-healthcheck/OWNERSci-operator/step-registry/aro-hcp/provision-healthcheck/aro-hcp-provision-healthcheck-workflow.metadata.jsonci-operator/step-registry/aro-hcp/provision-healthcheck/aro-hcp-provision-healthcheck-workflow.yamlci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh
✅ Files skipped from review due to trivial changes (3)
- ci-operator/step-registry/aro-hcp/provision-healthcheck/aro-hcp-provision-healthcheck-workflow.metadata.json
- ci-operator/step-registry/aro-hcp/provision-healthcheck/aro-hcp-provision-healthcheck-workflow.yaml
- ci-operator/step-registry/aro-hcp/provision-healthcheck/OWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh
roivaz
force-pushed
the
implement-provision-healthchecks
branch
from
94869cc to
3cd68fb
Compare
|
/pj-rehearse pull-ci-Azure-ARO-HCP-main-e2e-parallel |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-Azure-ARO-HCP-main-periodic-healthcheck-provision-centralus |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
roivaz
force-pushed
the
implement-provision-healthchecks
branch
from
3cd68fb to
e975b81
Compare
|
/pj-rehearse ci/rehearse/periodic-ci-Azure-ARO-HCP-main-periodic-healthcheck-provision-centralus |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-Azure-ARO-HCP-main-periodic-healthcheck-provision-centralus |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-Azure-ARO-HCP-main-periodic-healthcheck-provision-centralus |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
roivaz
force-pushed
the
implement-provision-healthchecks
branch
from
e975b81 to
800fc79
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-Azure-ARO-HCP-main-periodic-healthcheck-provision-centralus |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-Azure-ARO-HCP-main-periodic-healthcheck-provision-westus3 |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-Azure-ARO-HCP-main-periodic-healthcheck-provision-canadacentral |
|
@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@roivaz: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
1 participant
Summary by CodeRabbit
This PR updates OpenShift CI configuration and the ci-operator step-registry for the Azure/ARO-HCP repository to add periodic per-region provision healthchecks, a corresponding workflow and ownership metadata, and a targeted provisioning script change.
What changed in practical terms
CI job configuration (ci-operator/config/Azure/ARO-HCP/Azure-ARO-HCP-main__periodic-healthcheck.yaml)
New workflow and ownership (ci-operator/step-registry/aro-hcp/provision-healthcheck/)
Provision script change (ci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh)
Scope and impact