NO-JIRA: test(scheduler): add pair loss recovery unit coverage#7848
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
WalkthroughAdds a test helper Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
enxebre
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@enxebre: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
area/hypershift-operator
|
/auto-cc |
sdminonne
left a comment
sdminonne
left a comment
There was a problem hiding this comment.
It took me a while to understand the way this is designed but it looks OK.
thanks!
|
/lgtm |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, sdminonne 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 |
Test Resultse2e-aws
Failed TestsTotal failed tests: 2
e2e-aks
|
|
/retest |
|
/verified by unit tests |
|
/retest |
Add scheduler-and-sizer test cases for scheduled clusters that lose one node in a pair, covering both successful same-pair replacement and placeholder creation when no same-pair replacement is available. Made-with: Cursor
enxebre
force-pushed
the
agarcial/scheduler-pair-recovery-tests
branch
from
a82d2e0 to
7c7fee2
Compare
|
@enxebre: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hypershift-operator/controllers/scheduler/aws/dedicated_request_serving_nodes_test.go (1)
575-590: Optional: bindpairConfigMapto the exact test HC instance.Using a separate
hostedcluster()call foradditionalObjectscan drift fromhcif fixtures evolve (name/namespace overrides). Consider deriving the ConfigMap from the samehcobject per case to keep fixtures coupled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hypershift-operator/controllers/scheduler/aws/dedicated_request_serving_nodes_test.go` around lines 575 - 590, The test uses pairConfigMap(hostedcluster(), "id1") which can diverge from the test case's hc fixture; update the additionalObjects to derive the ConfigMap from the same hosted cluster instance used by the test (e.g., call pairConfigMap(hc, "id1") or pairConfigMap(hostedcluster(scheduledHC), "id1") depending on the case) so the ConfigMap is bound to the exact hosted cluster under test; adjust occurrences in the table-driven cases where pairConfigMap is created from a fresh hostedcluster() to instead reference the case's hc variable or the same hostedcluster(...) call used for that case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@hypershift-operator/controllers/scheduler/aws/dedicated_request_serving_nodes_test.go`:
- Around line 575-590: The test uses pairConfigMap(hostedcluster(), "id1") which
can diverge from the test case's hc fixture; update the additionalObjects to
derive the ConfigMap from the same hosted cluster instance used by the test
(e.g., call pairConfigMap(hc, "id1") or
pairConfigMap(hostedcluster(scheduledHC), "id1") depending on the case) so the
ConfigMap is bound to the exact hosted cluster under test; adjust occurrences in
the table-driven cases where pairConfigMap is created from a fresh
hostedcluster() to instead reference the case's hc variable or the same
hostedcluster(...) call used for that case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87d42736-53be-4203-8d81-84cf1fc444b5
📒 Files selected for processing (1)
hypershift-operator/controllers/scheduler/aws/dedicated_request_serving_nodes_test.go
|
/verified by unit test @enxebre |
openshift-ci-robot
added
the
verified
|
@enxebre: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
Scheduling tests matching the |
|
/retest |
|
@enxebre: all tests passed! 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. |
5 participants
Summary
DedicatedServingComponentSchedulerAndSizerwhen a scheduled hosted cluster loses one node in its assigned pairTest plan
go test ./hypershift-operator/controllers/scheduler/aws -run TestHostedClusterSchedulerAndSizer -count=1Made with Cursor
Summary by CodeRabbit