Enhance scheduling to respect MachinePool Ready condition - #1536
Conversation
Signed-off-by: Lukas Frank <lukas.frank@sap.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe scheduler now filters out machine pools without ChangesMachine pool readiness scheduling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/controllers/compute/machine_scheduler_test.go (1)
596-639: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
MachinePoolReady=False.This test covers the missing-condition path only. Add a case with an existing
MachinePoolReadycondition whose status iscorev1.ConditionFalse. Verify that the machine remains unscheduled until the condition becomes true.🤖 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 `@internal/controllers/compute/machine_scheduler_test.go` around lines 596 - 639, Extend the test around the existing machine-pool readiness scenario to initialize a MachinePoolReady condition with status corev1.ConditionFalse before creating the machine. Verify the machine remains unscheduled while the condition is false, then update that condition to true and preserve the existing assertion that scheduling occurs only after the pool becomes ready.
🤖 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.
Nitpick comments:
In `@internal/controllers/compute/machine_scheduler_test.go`:
- Around line 596-639: Extend the test around the existing machine-pool
readiness scenario to initialize a MachinePoolReady condition with status
corev1.ConditionFalse before creating the machine. Verify the machine remains
unscheduled while the condition is false, then update that condition to true and
preserve the existing assertion that scheduling occurs only after the pool
becomes ready.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ce1eda7-19a1-41e6-9f37-797244a3f897
📒 Files selected for processing (2)
internal/controllers/compute/machine_scheduler.gointernal/controllers/compute/machine_scheduler_test.go
Signed-off-by: Lukas Frank <lukas.frank@sap.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
gonzolino
left a comment
There was a problem hiding this comment.
lgtm, one minor nit regarding the test description after adding the second test case.
Proposed Changes
MachinePoolReadyconditionFixes #1531
Summary by CodeRabbit