Fix wrong subnet in use existing VM flow#314
Fix wrong subnet in use existing VM flow#314AmitSahastra wants to merge 1 commit intospectro-masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where MAAS may select the wrong subnet for eth0 after VM commissioning/acquisition. The fix ensures network interfaces are verified and corrected before deployment, particularly in the "use existing VM" flow where a VM was already composed but needs to be deployed.
Changes:
- Adds a
subnetsMatchhelper function that compares subnets by CIDR network equality in addition to string matching, making subnet verification more robust - Adds network interface verification call in the
createVMViaMAASreuse path (when VM already exists) - Extends network verification to handle both
ReadyandAllocatedmachine states, movingAllocatedfrom the deploying state to pre-deployment state where it logically belongs - Updates Go toolchain version from 1.24.11 to 1.24.13
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/maas/machine/machine.go | Added subnetsMatch function for robust subnet comparison; added VerifyVMNetworkInterfaces call in existing VM flow to fix subnet mismatches before deployment |
| controllers/maasmachine_controller.go | Moved Allocated state handling from deploying case to ready case; added network verification for both Ready and Allocated states before deployment; updated comments and error messages |
| go.mod | Updated Go toolchain version to 1.24.13 |
| Makefile | Updated builder Go version to 1.24.13 and SPECTRO_VERSION timestamp |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| IMAGE_NAME := cluster-api-provider-maas-controller | ||
| REGISTRY ?= "us-east1-docker.pkg.dev/spectro-images/dev/${USER}/cluster-api" | ||
| SPECTRO_VERSION ?= 4.8.3-dev-12112025 | ||
| SPECTRO_VERSION ?= 4.8.3-dev-tmo-19022026 |
There was a problem hiding this comment.
The date format in this version string appears to be "19022026" which could be interpreted as February 19, 2026 (ddmmyyyy format). According to the system information, the current date is February 20, 2026, so this date would be yesterday. However, if this is intended to be a date in the past or a specific versioning convention, please ensure it's intentional. If it's meant to be today's date, it should be updated to "20022026".
| return actualNet.IP.Equal(expectedNet.IP) && actualNet.Mask != nil && expectedNet.Mask != nil && | ||
| string(actualNet.Mask) == string(expectedNet.Mask) |
There was a problem hiding this comment.
Comparing network masks using string conversion is problematic because net.IPMask is a byte slice and converting it to a string may not correctly compare the actual byte values. Use bytes.Equal() instead for proper byte-wise comparison. Change line 687-688 to: bytes.Equal(actualNet.Mask, expectedNet.Mask). You'll need to import the "bytes" package.
No description provided.