Support Claw CR idling#1281
Conversation
alexeykazakov
requested review from
MatousJobanek
and removed request for
jrosental and
metlos
|
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:
WalkthroughAdds a Claw CustomResourceDefinition and documents its retrieval; implements MemberAwaitility.WaitForClaw and fixes a WaitForAAP context usage; integrates Claw creation and idle-state verification into e2e workloads and TestIdlerAndPriorityClass. ChangesClaw resource definition and e2e testing
Sequence DiagramsequenceDiagram
participant Test as TestIdlerAndPriorityClass
participant PrepareWorkloads as prepareWorkloads
participant CreateClaw as createClaw
participant WaitHelper as WaitForClaw
participant ClawAPI as Claw Resource
Test->>PrepareWorkloads: prepare test resources
PrepareWorkloads->>CreateClaw: create Claw workload
CreateClaw->>ClawAPI: create Claw resource (idle: false)
CreateClaw->>ClawAPI: create Deployment owned by Claw
Test->>WaitHelper: wait for Claw idle state
WaitHelper->>ClawAPI: poll spec.idle
ClawAPI-->>WaitHelper: return Claw object when idle: true
Test->>Test: verify workload idled
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
documentation
![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 `@testsupport/wait/member.go`:
- Around line 1548-1561: In WaitForClaw's PollUntilContextTimeout callback,
replace context.Background() with the provided ctx when calling
clawRes.Namespace(...).Get so the API request is cancelled on timeout, and when
calling unstructured.NestedBool check and use the returned found bool (e.g.,
idled, found, err := unstructured.NestedBool(...)); if err return true,err, if
!found return false,nil to continue polling, otherwise compare expectedIdled ==
idled and return that result.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: e2ac546f-613b-4ca3-851f-e6cd9f5879d9
📒 Files selected for processing (4)
deploy/crds/README.adocdeploy/crds/claw.sandbox.redhat.com_claws.yamltest/e2e/parallel/user_workloads_test.gotestsupport/wait/member.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
deploy/crds/README.adocdeploy/crds/claw.sandbox.redhat.com_claws.yamltestsupport/wait/member.gotest/e2e/parallel/user_workloads_test.go
🪛 GitHub Check: SonarCloud Code Analysis
testsupport/wait/member.go
[warning] 1550-1550: Use the available context parameter 'ctx' instead of creating a new background context.
🔇 Additional comments (3)
deploy/crds/claw.sandbox.redhat.com_claws.yaml (1)
1-40: LGTM!deploy/crds/README.adoc (1)
13-16: LGTM!test/e2e/parallel/user_workloads_test.go (1)
104-107: LGTM!Also applies to: 188-190, 218-218, 255-299
coderabbitai
Bot
removed
documentation
coderabbitai
Bot
added
documentation
| // Create a Claw workload only in the dev namespace (the one being idled). | ||
| // Not added to prepareWorkloads to avoid exceeding the ClusterResourceQuota pod limit | ||
| // when workloads are created in multiple namespaces for the same user. | ||
| clawDeployment := createClaw(t, memberAwait, "test-idler-claw", idler.Name) | ||
| podsToIdle, err := memberAwait.WaitForPods(t, idler.Name, len(podsToIdle)+int(*clawDeployment.Spec.Replicas), | ||
| wait.PodRunning(), wait.WithPodLabel("idler", "idler"), wait.WithSandboxPriorityClass()) | ||
| require.NoError(t, err) | ||
|
|
There was a problem hiding this comment.
just a detail - could have been done as part of prepareWorkloads in the same way as it's done for other workload types to verify that it ignores the "noise" namespace for claw workloads
There was a problem hiding this comment.
Had to revert it due to the pod quota issue if we create it in all namespaces in multi-namespace tiers. We alerready create too many resources in the test namespaces.
There was a problem hiding this comment.
We should adjust that (or refactor the test so it uses more accounts) because this would block us from adding more workloads to the test suite
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MatousJobanek, metlos, xcoulon 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 |
coderabbitai
Bot
removed
documentation
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 `@testsupport/wait/member.go`:
- Line 17: gofmt failure due to improperly formatted imports in
testsupport/wait/member.go; run gofmt (or go fmt) on that file to fix import
ordering/formatting so the import block (including
"github.com/codeready-toolchain/toolchain-e2e/testsupport/cleanup") conforms to
gofmt rules and removes any stray blank lines or incorrect grouping.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 30b1b07b-41c0-49ba-973d-c9c0009fb705
📒 Files selected for processing (2)
test/e2e/parallel/user_workloads_test.gotestsupport/wait/member.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
testsupport/wait/member.gotest/e2e/parallel/user_workloads_test.go
🪛 GitHub Actions: ci-build / 0_GolangCI Lint.txt
testsupport/wait/member.go
[error] 17-17: golangci-lint: File is not properly formatted (gofmt)
🪛 GitHub Actions: ci-build / GolangCI Lint
testsupport/wait/member.go
[error] 17-17: gofmt formatting issue: file is not properly formatted (gofmt) at line 17.
🪛 GitHub Check: GolangCI Lint
testsupport/wait/member.go
[failure] 17-17:
File is not properly formatted (gofmt)
🔇 Additional comments (2)
testsupport/wait/member.go (1)
1419-1427: LGTM!test/e2e/parallel/user_workloads_test.go (1)
47-50: LGTM!Also applies to: 177-180
coderabbitai
Bot
added
documentation
|
|
/retest |
alexeykazakov
merged commit 8a09e02
into
codeready-toolchain:master
4 participants
Paired with codeready-toolchain/member-operator#745
Summary by CodeRabbit
New Features
Documentation
Tests