PCP-6208: Enhance MaaS machine handling during commissioning phase & add ip check before compose#324
PCP-6208: Enhance MaaS machine handling during commissioning phase & add ip check before compose#324Kun483 wants to merge 4 commits intospectro-masterfrom
Conversation
Kun483
commented
Mar 11, 2026
Kun483
commented
- Added a new error handling for machine commissioning state to improve clarity.
- Updated the reconcile logic to handle the commissioning state, allowing for retries of static IP configuration after commissioning completes.
There was a problem hiding this comment.
Pull request overview
Enhances MaaS machine reconciliation during commissioning by introducing a dedicated commissioning error and handling it in the controller to enable safe retries of static IP configuration once commissioning completes.
Changes:
- Add a new sentinel error (
ErrMachineCommissioning) returned when a machine is in MaaS “Commissioning” state during static IP configuration. - Update
MaasMachineReconcilerto detect this error, mark the machine as deploying, and requeue after a short delay.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/maas/machine/machine.go | Introduces ErrMachineCommissioning and returns it when static IP configuration is attempted during commissioning. |
| controllers/maasmachine_controller.go | Adds error-specific reconcile handling to requeue and update conditions while commissioning is in progress. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // Special handling for Commissioning state: skip static IP configuration to avoid blocking commissioning | ||
| if machineState == "Commissioning" { | ||
| s.scope.Info("Machine is commissioning, skipping static IP configuration to avoid interfering with commissioning process. Will configure after commissioning completes", "systemID", systemID) | ||
| // Return error to requeue - static IP will be configured after commissioning completes | ||
| return fmt.Errorf("machine is commissioning, static IP configuration will be retried after commissioning completes") | ||
| return ErrMachineCommissioning | ||
| } |
There was a problem hiding this comment.
Consider adding a unit test for this commissioning-path in setMachineStaticIP. Right now there are no tests asserting that when MAAS reports State() == "Commissioning", the method returns ErrMachineCommissioning (and that the error remains detectable via errors.Is after any wrapping). This is an important behavioral contract because the controller reconciliation flow depends on it for requeue behavior.
Kun483
changed the title
… for NetworkInterfaces, IPAddresses, Tags, VMHosts, and Subnets in client mock. Enhance static IP handling in machine service during VM composition.
…cileNormal method to streamline error handling during VM creation.
| github.com/onsi/gomega v1.36.3 | ||
| github.com/pkg/errors v0.9.1 | ||
| github.com/spectrocloud/maas-client-go v0.1.4-beta1 | ||
| github.com/spectrocloud/maas-client-go v0.1.4-beta2 |
There was a problem hiding this comment.
any reason to go for beta2 naming instead of updating minor version?
Also, maas client go release looks wrong, its a public repo we can avoid PCP taging there.
v0.1.4-beta2: Merge pull request #44 from spectrocloud/PCP-6208
There was a problem hiding this comment.
Got it. I'll add a new feature in maas-client-go and update the minor version
