Repository navigation
Add operator-chaos upgrade analysis workflow - #128
openshift-merge-bot[bot] merged 1 commit into
Conversation
|
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: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces a shift-left upgrade validation gate for the MLflow operator. It adds a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Security findingsPinned operator-chaos CLI binary (range_2a4fcc44e103, range_0676aa583abd)
Sparse checkout of base branch (range_2a4fcc44e103) Breaking change detection logic (range_b20c2bc91c34, range_3e9c529a168f)
Exit behavior & workflow failure semantics 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/operator-chaos.yml:
- Around line 150-159: The "Update PR comment" step can fail with HTTP 403 for
forked PRs because the reduced GITHUB_TOKEN is read-only; modify the step (named
"Update PR comment") to skip execution when the PR is from a fork by guarding it
with an if condition that checks github.event.pull_request.head.repo.fork (e.g.,
only run when that value is false), keep the existing env (GH_TOKEN, PR_NUMBER)
and the call to .github/scripts/operator-chaos-pr-comment.sh unchanged so
non-fork PRs still update the comment.
- Around line 14-16: The workflow-level permissions block should be moved to the
specific job to follow least-privilege; remove the top-level "permissions:"
block and add the same permissions under the job named "operator-chaos"
(jobs.operator-chaos.permissions) so only that job gets contents: read and
issues: write instead of applying to all jobs.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 220ece1d-aafc-4fde-9e2f-566c043b21fe
📒 Files selected for processing (6)
.github/scripts/operator-chaos-pr-comment.sh.github/scripts/operator-chaos-summary.sh.github/workflows/operator-chaos.ymlAGENTS.mdREADME.mdchaos/knowledge/mlflow.yaml
ccbd1c5 to
4fd4adf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/operator-chaos-pr-comment.yml (2)
129-136: 💤 Low valueAdd
if: always()so PR comment is posted even when upstream steps fail.If "Write operator-chaos summary" fails unexpectedly, this step is skipped and the PR gets no feedback. Match the pattern used for "Fail on operator-chaos errors" (line 139).
Suggested change
- name: Update PR comment + if: always() env: GH_TOKEN: ${{ github.token }}🤖 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 @.github/workflows/operator-chaos-pr-comment.yml around lines 129 - 136, The "Update PR comment" workflow step is currently skipped when earlier steps fail; add the conditional so it always runs by inserting if: always() under the step name to match the "Fail on operator-chaos errors" pattern. Update the step with name "Update PR comment" (the one that runs bash .github/scripts/operator-chaos-pr-comment.sh) to include if: always() so the PR comment is posted even when "Write operator-chaos summary" or other upstream steps fail.
95-101: ⚡ Quick winVersion drift: hardcoded SHA diverges from
OPERATOR_CHAOS_VERSIONenv var in the other workflow.Line 100 hardcodes
9e6ac9668b9aaca2f0f2ddf169867862b7925b80while.github/workflows/operator-chaos.ymldefines this asenv.OPERATOR_CHAOS_VERSION. Future updates will require changing both files, risking inconsistency.Define a shared env var or anchor this to a single source of truth.
Suggested change
+env: + OPERATOR_CHAOS_VERSION: "9e6ac9668b9aaca2f0f2ddf169867862b7925b80" + jobs: update-pr-comment:Then at line 100:
- GOBIN="${RUNNER_TEMP}/bin" go install "github.com/opendatahub-io/operator-chaos/cmd/operator-chaos@9e6ac9668b9aaca2f0f2ddf169867862b7925b80" + GOBIN="${RUNNER_TEMP}/bin" go install "github.com/opendatahub-io/operator-chaos/cmd/operator-chaos@${OPERATOR_CHAOS_VERSION}"🤖 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 @.github/workflows/operator-chaos-pr-comment.yml around lines 95 - 101, The workflow hardcodes the operator-chaos commit SHA in the go install command (the literal "9e6ac9...b80") which diverges from the OPERATOR_CHAOS_VERSION env var used elsewhere; update the run step so it uses the shared OPERATOR_CHAOS_VERSION environment variable instead of the hardcoded SHA (adjust the GOBIN/go install line that references the operator-chaos module), or move OPERATOR_CHAOS_VERSION into this workflow's env and reference it consistently to keep a single source of truth.
🤖 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 @.github/scripts/operator-chaos-run-checks.sh:
- Around line 88-99: The script runs bare operator-chaos diff / diff-crds
commands that may exit non-zero and trigger set -e termination (e.g., the
unguarded JSON captures that write to knowledge-diff.json and similar for CRDs);
change these to be guarded so failures don't short-circuit the script: either
run them via the existing run_capture wrapper (use run_capture "VAR" path
operator-chaos ...) or execute the command in a way that preserves its
stdout/stderr to the JSON file but prevents set -e from exiting (capture the
exit code with || true or via a subshell and then record that exit code into a
variable), and ensure the downstream logic uses the guarded exit code rather
than allowing the bare command to abort execution (apply same fix to the
BASE_KNOWLEDGE_FILE block, the JSON redirect to
"${OUTPUT_DIR}/knowledge-diff.json", and the corresponding diff-crds JSON
capture).
In @.github/workflows/operator-chaos-pr-comment.yml:
- Around line 22-26: The workflow currently unsafely indexes
github.event.workflow_run.pull_requests[0] (used in the concurrency group name
and PR_NUMBER) which can be empty; replace that pattern by resolving the PR
number via a lookup using github.event.workflow_run.head_sha (e.g., call the
GitHub REST/GraphQL API to find the PR whose head.sha matches) store the result
into a workflow output/variable (e.g., PR_NUMBER) and use that variable in the
concurrency group name (operator-chaos-pr-comment-${{ env.PR_NUMBER }} or
similar) and anywhere else instead of pull_requests[0]; remove all direct
references to pull_requests[0] so the job no longer assumes the array is
populated.
---
Nitpick comments:
In @.github/workflows/operator-chaos-pr-comment.yml:
- Around line 129-136: The "Update PR comment" workflow step is currently
skipped when earlier steps fail; add the conditional so it always runs by
inserting if: always() under the step name to match the "Fail on operator-chaos
errors" pattern. Update the step with name "Update PR comment" (the one that
runs bash .github/scripts/operator-chaos-pr-comment.sh) to include if: always()
so the PR comment is posted even when "Write operator-chaos summary" or other
upstream steps fail.
- Around line 95-101: The workflow hardcodes the operator-chaos commit SHA in
the go install command (the literal "9e6ac9...b80") which diverges from the
OPERATOR_CHAOS_VERSION env var used elsewhere; update the run step so it uses
the shared OPERATOR_CHAOS_VERSION environment variable instead of the hardcoded
SHA (adjust the GOBIN/go install line that references the operator-chaos
module), or move OPERATOR_CHAOS_VERSION into this workflow's env and reference
it consistently to keep a single source of truth.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: a5e6d9af-6905-4e10-98de-b137c52073af
📒 Files selected for processing (8)
.github/scripts/operator-chaos-pr-comment.sh.github/scripts/operator-chaos-run-checks.sh.github/scripts/operator-chaos-summary.sh.github/workflows/operator-chaos-pr-comment.yml.github/workflows/operator-chaos.ymlAGENTS.mdREADME.mdchaos/knowledge/mlflow.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/scripts/operator-chaos-pr-comment.sh
- chaos/knowledge/mlflow.yaml
7f8f7cc to
e696f55
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/operator-chaos-pr-comment.yml:
- Around line 14-17: The workflow currently grants workflow-level permissions
including "issues: write" which is broader than necessary; move the top-level
permissions block into the specific job's definition and grant only the
least-privilege scope required (e.g., add a "permissions:" map under the job
that sets "issues: write" and other entries as needed and remove the
workflow-level "permissions:" block) so that "issues: write" is scoped to the
job instead of the entire workflow.
- Around line 84-128: The workflow currently checks out untrusted pr-head and
then runs .github/scripts/operator-chaos-run-checks.sh which reads
pr-head/chaos/knowledge/mlflow.yaml and copies the PR CRD via cp
"${TARGET_CRD_PATH}" (which follows symlinks); add a symlink-rejection step
before invoking operator-chaos-run-checks.sh that scans the pr-head inputs
(e.g., pr-head/chaos/knowledge and pr-head/config/crd/...) for any symbolic
links and fails the job if any are found; implement this check either as a small
shell step in the workflow (run a find ... -type l and exit 1 if results) or
incorporate it at the start of operator-chaos-run-checks.sh (before cp
"${TARGET_CRD_PATH}") to prevent following malicious symlinks.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: a654b2bb-8dc2-40cb-9ab6-895353918321
📒 Files selected for processing (8)
.github/scripts/operator-chaos-pr-comment.sh.github/scripts/operator-chaos-run-checks.sh.github/scripts/operator-chaos-summary.sh.github/workflows/operator-chaos-pr-comment.yml.github/workflows/operator-chaos.ymlAGENTS.mdREADME.mdchaos/knowledge/mlflow.yaml
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (4)
- chaos/knowledge/mlflow.yaml
- .github/workflows/operator-chaos.yml
- .github/scripts/operator-chaos-summary.sh
- .github/scripts/operator-chaos-run-checks.sh
e696f55 to
9b05cb9
Compare
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 @.github/workflows/operator-chaos-pr-comment.yml:
- Around line 28-31: The workflow checks out the repo initially without ref and
later runs bash .github/scripts/operator-chaos-*.sh from that initial checkout,
causing scripts to be from the workflow default ref instead of the base commit
in steps.pr_meta.outputs.base_sha; update the "Checkout base branch assets"
sparse checkout (the checkout step used to fetch base-branch/... assets) to also
include the .github/scripts path and change the run steps that call bash
.github/scripts/operator-chaos-*.sh to invoke
base-branch/.github/scripts/operator-chaos-*.sh (i.e., execute the scripts from
the checked-out base_sha) while keeping the initial checkout (actions/checkout)
if still needed for setup-go's go-version-file: go.mod.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: edddf823-561c-45a3-a7ba-a73cc97c4cbe
📒 Files selected for processing (8)
.github/scripts/operator-chaos-pr-comment.sh.github/scripts/operator-chaos-run-checks.sh.github/scripts/operator-chaos-summary.sh.github/workflows/operator-chaos-pr-comment.yml.github/workflows/operator-chaos.ymlAGENTS.mdREADME.mdchaos/knowledge/mlflow.yaml
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/scripts/operator-chaos-pr-comment.sh
- .github/workflows/operator-chaos.yml
- chaos/knowledge/mlflow.yaml
- .github/scripts/operator-chaos-summary.sh
- .github/scripts/operator-chaos-run-checks.sh
9b05cb9 to
e3fab5f
Compare
Introduce a repo-local MLflow operator-chaos knowledge model and a PR workflow that validates it, diffs the checked-in CRD, and previews upgrade simulations. Fail fast on breaking knowledge or CRD changes so upgrade risks block PRs without maintaining a separate PR-comment reporting flow. Signed-off-by: Humair Khan <HumairAK@users.noreply.github.com>
e3fab5f to
c578b38
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mprahl 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 |
00a212e
into
opendatahub-io:main
Integrate operator-chaos GitHub Actions workflow for automated upgrade validation at PR time. Implements Level 1 maturity (minimum requirement for GA components). Changes: - Add .github/workflows/operator-chaos.yml * Triggers on PRs modifying api/, controllers/, config/, or knowledge * Installs operator-chaos tool from opendatahub-io/operator-chaos * Validates knowledge model structure * Detects breaking changes in FeastOperator CRD schema * Runs upgrade simulation in dry-run mode * Fails PR if breaking changes detected without migration path - Add chaos/knowledge/feast.yaml * Defines operator metadata (name, namespace, repository) * Lists managed resources for feast-operator-controller-manager component * Lists managed resources for feast component (operator, CRDs, jobs) * Includes steady-state checks for Deployment availability * Configures recovery timeouts and reconcile cycles - Update README.md * Document operator-chaos validation in Development section * Link to operator-chaos repository for reference References: - Template: mlflow-operator PR opendatahub-io/mlflow-operator#128 - Tool: https://github.com/opendatahub-io/operator-chaos - Maturity level: L1 (lightweight, breaking change detection) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Integrate operator-chaos GitHub Actions workflow for automated upgrade validation at PR time. Implements Level 1 maturity (minimum requirement for GA components). Changes: - Add .github/workflows/operator-chaos.yml * Triggers on PRs modifying api/, controllers/, config/, or knowledge * Installs operator-chaos tool from opendatahub-io/operator-chaos * Validates knowledge model structure * Detects breaking changes in FeastOperator CRD schema * Runs upgrade simulation in dry-run mode * Fails PR if breaking changes detected without migration path - Add chaos/knowledge/feast.yaml * Defines operator metadata (name, namespace, repository) * Lists managed resources for feast-operator-controller-manager component * Lists managed resources for feast component (operator, CRDs, jobs) * Includes steady-state checks for Deployment availability * Configures recovery timeouts and reconcile cycles - Update README.md * Document operator-chaos validation in Development section * Link to operator-chaos repository for reference References: - Template: mlflow-operator PR opendatahub-io/mlflow-operator#128 - Tool: https://github.com/opendatahub-io/operator-chaos - Maturity level: L1 (lightweight, breaking change detection) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Srihari <svenkata@redhat.com>
Integrate operator-chaos GitHub Actions workflow for automated upgrade validation at PR time. Implements Level 1 maturity (minimum requirement for GA components). Changes: - Add .github/workflows/operator-chaos.yml * Triggers on PRs modifying api/, controllers/, config/, or knowledge * Installs operator-chaos tool from opendatahub-io/operator-chaos * Validates knowledge model structure * Detects breaking changes in FeastOperator CRD schema * Runs upgrade simulation in dry-run mode * Fails PR if breaking changes detected without migration path - Add chaos/knowledge/feast.yaml * Defines operator metadata (name, namespace, repository) * Lists managed resources for feast-operator-controller-manager component * Lists managed resources for feast component (operator, CRDs, jobs) * Includes steady-state checks for Deployment availability * Configures recovery timeouts and reconcile cycles - Update README.md * Document operator-chaos validation in Development section * Link to operator-chaos repository for reference References: - Template: mlflow-operator PR opendatahub-io/mlflow-operator#128 - Tool: https://github.com/opendatahub-io/operator-chaos - Maturity level: L1 (lightweight, breaking change detection) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Srihari <svenkata@redhat.com>
Integrate operator-chaos GitHub Actions workflow for automated upgrade validation at PR time. Implements Level 1 maturity (minimum requirement for GA components). Changes: - Add .github/workflows/operator-chaos.yml * Triggers on PRs modifying api/, controllers/, config/, or knowledge * Installs operator-chaos tool from opendatahub-io/operator-chaos * Validates knowledge model structure * Detects breaking changes in FeastOperator CRD schema * Runs upgrade simulation in dry-run mode * Fails PR if breaking changes detected without migration path - Add chaos/knowledge/feast.yaml * Defines operator metadata (name, namespace, repository) * Lists managed resources for feast-operator-controller-manager component * Lists managed resources for feast component (operator, CRDs, jobs) * Includes steady-state checks for Deployment availability * Configures recovery timeouts and reconcile cycles - Update README.md * Document operator-chaos validation in Development section * Link to operator-chaos repository for reference References: - Template: mlflow-operator PR opendatahub-io/mlflow-operator#128 - Tool: https://github.com/opendatahub-io/operator-chaos - Maturity level: L1 (lightweight, breaking change detection) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Srihari <svenkata@redhat.com>
Integrate operator-chaos GitHub Actions workflow for automated upgrade validation at PR time. Implements Level 1 maturity (minimum requirement for GA components). Changes: - Add .github/workflows/operator-chaos.yml * Triggers on PRs modifying api/, controllers/, config/, or knowledge * Installs operator-chaos tool from opendatahub-io/operator-chaos * Validates knowledge model structure * Detects breaking changes in FeastOperator CRD schema * Runs upgrade simulation in dry-run mode * Fails PR if breaking changes detected without migration path - Add chaos/knowledge/feast.yaml * Defines operator metadata (name, namespace, repository) * Lists managed resources for feast-operator-controller-manager component * Lists managed resources for feast component (operator, CRDs, jobs) * Includes steady-state checks for Deployment availability * Configures recovery timeouts and reconcile cycles - Update README.md * Document operator-chaos validation in Development section * Link to operator-chaos repository for reference References: - Template: mlflow-operator PR opendatahub-io/mlflow-operator#128 - Tool: https://github.com/opendatahub-io/operator-chaos - Maturity level: L1 (lightweight, breaking change detection) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Srihari <svenkata@redhat.com>
Integrate operator-chaos GitHub Actions workflow for automated upgrade validation at PR time. Implements Level 1 maturity (minimum requirement for GA components). Changes: - Add .github/workflows/operator-chaos.yml * Triggers on PRs modifying api/, controllers/, config/, or knowledge * Installs operator-chaos tool from opendatahub-io/operator-chaos * Validates knowledge model structure * Detects breaking changes in FeastOperator CRD schema * Runs upgrade simulation in dry-run mode * Fails PR if breaking changes detected without migration path - Add chaos/knowledge/feast.yaml * Defines operator metadata (name, namespace, repository) * Lists managed resources for feast-operator-controller-manager component * Lists managed resources for feast component (operator, CRDs, jobs) * Includes steady-state checks for Deployment availability * Configures recovery timeouts and reconcile cycles - Update README.md * Document operator-chaos validation in Development section * Link to operator-chaos repository for reference References: - Template: mlflow-operator PR opendatahub-io/mlflow-operator#128 - Tool: https://github.com/opendatahub-io/operator-chaos - Maturity level: L1 (lightweight, breaking change detection) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Srihari <svenkata@redhat.com>
Summary
This adds a repo-local
operator-chaosknowledge model for MLflow and a PR workflow that:operator-chaos preflight --localsimulate-upgrade --dry-runThe workflow now fails fast on breaking knowledge or CRD changes, and intentionally uses the workflow logs as the review surface instead of a separate PR comment flow.
Test plan
go run ./cmd/operator-chaos validate --knowledge /home/hukhan/projects/github/rhoai/mlflow-operator/chaos/knowledge/mlflow.yamlgo run ./cmd/operator-chaos preflight --knowledge /home/hukhan/projects/github/rhoai/mlflow-operator/chaos/knowledge/mlflow.yaml --localgo run ./cmd/operator-chaos diff --source /home/hukhan/projects/github/rhoai/mlflow-operator/chaos/knowledge --target /home/hukhan/projects/github/rhoai/mlflow-operator/chaos/knowledgego run ./cmd/operator-chaos diff-crds --source-crds /home/hukhan/projects/github/rhoai/mlflow-operator/config/crd/bases --target-crds /home/hukhan/projects/github/rhoai/mlflow-operator/config/crd/basesgo run ./cmd/operator-chaos simulate-upgrade --source /home/hukhan/projects/github/rhoai/mlflow-operator/chaos/knowledge --target /home/hukhan/projects/github/rhoai/mlflow-operator/chaos/knowledge --dry-rungo run github.com/rhysd/actionlint/cmd/actionlint@f5adaf2a03a0a2557568caae2546fa04a05460dc .github/workflows/operator-chaos.ymlbash -n .github/scripts/operator-chaos-summary.shbash -n .github/scripts/operator-chaos-pr-comment.shSummary by CodeRabbit
New Features
Documentation