feat(chaos): integrate operator-chaos L3 SDK tests for both controllers - #834
Conversation
|
Skipping CI for Draft Pull Request. |
📝 WalkthroughWalkthroughChangesThe PR adds operator-chaos validation for both notebook controllers: CI trigger expansion, experiment YAML validation, new Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant operator-chaos
participant make test-chaos
GitHubActions->>operator-chaos: validate chaos/experiments/*.yaml
GitHubActions->>make test-chaos: run notebook-controller and odh-notebook-controller chaos suites
Supply-chain findings
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #834 +/- ##
==========================================
+ Coverage 63.75% 64.69% +0.94%
==========================================
Files 15 15
Lines 2974 2974
==========================================
+ Hits 1896 1924 +28
+ Misses 895 874 -21
+ Partials 183 176 -7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
/group-test |
|
/group-test |
|
/group-test |
|
/group-test |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/notebook-controller/chaostests/suite_test.go (1)
28-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffGinkgo v1 is unsupported upstream.
Ginkgo v1 (
github.com/onsi/ginkgo) reached its end of maintenance and the project states "you are using Ginkgo V2 (V1 is no longer supported - see here for the migration guide)". Building new chaos test infrastructure on an unsupported major version means no further bugfixes/security patches for the test framework itself, and blocks future dependency upgrades that assume v2 (e.g., transitive packages that dot-import ginkgo/v2 will conflict at init time).Also applies to: 36-38
🤖 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 `@components/notebook-controller/chaostests/suite_test.go` around lines 28 - 29, Update the chaos test suite to use Ginkgo v2 instead of the unsupported v1 dot-imports. In suite_test.go, replace the current ginkgo/gomega imports used by the suite setup and any related helpers with the v2 packages and adjust any affected symbols such as RunSpecs, RegisterFailHandler, and Expect to their v2-compatible usage. Make the same migration anywhere else in the chaos test package that still imports the old Ginkgo v1 symbols so the suite compiles cleanly on a single supported major version.
🤖 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 `@components/odh-notebook-controller/README.md`:
- Around line 216-218: Update the maintenance directive in the README snippet to
match the coding guideline by changing the non-committal “consider adding”
wording to an imperative “add” in the sentence about new sub-reconcilers or API
operations, so the instruction clearly points to adding corresponding
ChaosClient SDK test scenarios in chaostests/chaos_test.go.
---
Nitpick comments:
In `@components/notebook-controller/chaostests/suite_test.go`:
- Around line 28-29: Update the chaos test suite to use Ginkgo v2 instead of the
unsupported v1 dot-imports. In suite_test.go, replace the current ginkgo/gomega
imports used by the suite setup and any related helpers with the v2 packages and
adjust any affected symbols such as RunSpecs, RegisterFailHandler, and Expect to
their v2-compatible usage. Make the same migration anywhere else in the chaos
test package that still imports the old Ginkgo v1 symbols so the suite compiles
cleanly on a single supported major version.
🪄 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: 41d7f33a-968b-49c3-83c6-bd718e6d9a0d
⛔ Files ignored due to path filters (2)
components/notebook-controller/go.sumis excluded by!**/*.sum,!**/*.sumcomponents/odh-notebook-controller/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (20)
.github/workflows/operator_chaos_validation.yamlAGENTS.mdchaos/experiments/deployment-scale-zero.yamlchaos/experiments/network-partition.yamlchaos/experiments/pod-kill.yamlchaos/experiments/rbac-revoke.yamlchaos/experiments/webhook-disrupt.yamlcomponents/notebook-controller/Makefilecomponents/notebook-controller/README.mdcomponents/notebook-controller/chaostests/chaos_test.gocomponents/notebook-controller/chaostests/suite_test.gocomponents/notebook-controller/go.modcomponents/odh-notebook-controller/Makefilecomponents/odh-notebook-controller/README.mdcomponents/odh-notebook-controller/chaostests/chaos_test.gocomponents/odh-notebook-controller/chaostests/suite_test.gocomponents/odh-notebook-controller/controllers/notebook_controller_test.gocomponents/odh-notebook-controller/controllers/notebook_mlflow_test.gocomponents/odh-notebook-controller/controllers/suite_test.gocomponents/odh-notebook-controller/go.mod
✅ Files skipped from review due to trivial changes (3)
- chaos/experiments/rbac-revoke.yaml
- components/notebook-controller/README.md
- components/odh-notebook-controller/controllers/suite_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- chaos/experiments/pod-kill.yaml
- chaos/experiments/webhook-disrupt.yaml
- components/odh-notebook-controller/controllers/notebook_controller_test.go
- components/odh-notebook-controller/controllers/notebook_mlflow_test.go
- .github/workflows/operator_chaos_validation.yaml
- chaos/experiments/deployment-scale-zero.yaml
- chaos/experiments/network-partition.yaml
- components/notebook-controller/go.mod
- components/odh-notebook-controller/go.mod
|
/group-test |
|
/group-test |
Add Level 3 operator-chaos integration with ChaosClient SDK tests for both the ODH and upstream notebook reconcilers, using isolated envtest environments (no controller manager) for deterministic fault injection. - Add operator-chaos/pkg/sdk as a Go dependency in both controllers; sync k8s API versions to v0.35.2 in notebook-controller - Create chaostests/ packages with isolated envtest setup (API server + etcd only) enabling deterministic Create/Delete fault testing - ODH controller: 10 Ginkgo specs covering Get, List, Create, Update, Delete faults, transient recovery, and intermittent errors - Upstream controller: 7 Ginkgo specs covering Get, List, Create faults, transient recovery, and intermittent errors (no Update-no-drift or Delete tests as the reconciler always detects StatefulSet drift and does not handle finalizers) - Add 5 experiment YAMLs under chaos/experiments/ adapted from upstream (pod-kill, network-partition, webhook-disrupt, rbac-revoke, deployment-scale-zero) - Add `make test-chaos` Makefile targets with -coverpkg=./controllers/... for accurate coverage attribution - Extend CI workflow to validate experiments, run chaos SDK tests, and upload coverage to Codecov with a dedicated `chaos` flag - Update AGENTS.md and component READMEs with chaos testing documentation Co-authored-by: Cursor <cursoragent@cursor.com>
|
/group-test |
harshad16
left a comment
There was a problem hiding this comment.
The core design - isolated envtest, per-component L3 SDK tests, CI wiring.
Thank you for working on this.
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: harshad16 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 |
d886335
into
opendatahub-io:main
|
/group-test |
Jira: RHOAIENG-70591
Summary
Add Level 3 operator-chaos integration with ChaosClient SDK tests for
both the ODH and upstream notebook reconcilers, using isolated envtest
environments (no controller manager) for deterministic fault injection.
What changed
sync k8s API versions to v0.35.2 in notebook-controller
etcd only) enabling deterministic Create/Delete fault testing
Delete faults, transient recovery, and intermittent errors
transient recovery, and intermittent errors (no Update-no-drift or
Delete tests as the reconciler always detects StatefulSet drift and
does not handle finalizers)
(pod-kill, network-partition, webhook-disrupt, rbac-revoke,
deployment-scale-zero)
make test-chaosMakefile targets with -coverpkg=./controllers/...for accurate coverage attribution
upload coverage to Codecov with a dedicated
chaosflagCo-authored-by: Cursor cursoragent@cursor.com
Adds Level 3 of the operator-chaos shift-left integration for workbenches, building on the L1+L2 foundation merged in #832. This follows the pattern established in model-registry-operator PR #525.
Test plan
make test-chaos— 6 passed, 0 failedmake test(full ODH suite) — 130 passed, 0 failed (no regressions)make test(upstream notebook-controller) — all passedmake chaos-validate— knowledge model valid, preflight passedSummary by CodeRabbit
make test-chaostargets to run ChaosClient SDK resilience tests locally for both components.