feat: add AUTO link mode and LinkSubnetWithMode for MAAS link_subnet

[AmitSahastra]

No description provided.

[Copilot]

Pull request overview

This pull request adds support for configurable MAAS link modes when linking subnets to network interfaces, specifically introducing the "auto" mode as a default for eth0 and providing an extensible API for custom link mode configuration.

Changes:

• Adds a new InterfaceLinkModes field to VMConfig API that allows per-interface link mode configuration (auto, dhcp, static, link_up)
• Implements network interface verification and correction before VM deployment to ensure correct subnet assignment
• Updates state handling logic to treat "Allocated" state similarly to "Ready" state for network verification
• Updates dependency versions including Go toolchain and maas-client-go library

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/maas/machine/machine.go	Adds resolveLinkMode function, updates VerifyVMNetworkInterfaces to use configurable link modes, adds subnetsMatch helper for robust subnet comparison, and updates fixInterfaceSubnet to use LinkSubnetWithMode
api/v1beta1/maasmachine_types.go	Adds InterfaceLinkModes map field to VMConfig for configuring per-interface link modes
api/v1beta1/zz_generated.deepcopy.go	Auto-generated DeepCopy code for new InterfaceLinkModes field and existing Parent field
controllers/maasmachine_controller.go	Updates state handling to treat Allocated state like Ready state, adds network verification before deployment for both states
go.mod	Updates maas-client-go dependency version and Go toolchain version
go.sum	Updates checksums for maas-client-go dependency
Makefile	Updates BUILDER_GOLANG_VERSION and SPECTRO_VERSION

Comments suppressed due to low confidence (4)

pkg/maas/machine/machine.go:710

• The null checks for actualNet.Mask and expectedNet.Mask are redundant. The net.ParseCIDR function guarantees that the returned IPNet has a non-nil Mask field when it succeeds (err is nil). The checks actualNet != nil and expectedNet != nil are also redundant for the same reason. Consider simplifying to: return actualNet.IP.Equal(expectedNet.IP) && bytes.Equal(actualNet.Mask, expectedNet.Mask)

		if err1 == nil && err2 == nil && actualNet != nil && expectedNet != nil {
			return actualNet.IP.Equal(expectedNet.IP) && actualNet.Mask != nil && expectedNet.Mask != nil &&
				bytes.Equal(actualNet.Mask, expectedNet.Mask)

go.mod:85

• The replace directive for github.com/spectrocloud/maas-client-go appears redundant since it replaces the module with the same version (v0.1.5-pre) that's already specified in the require section. This replace directive might be necessary if the version is unpublished or requires special resolution, but if not, consider removing it to simplify the go.mod file.

	github.com/spectrocloud/maas-client-go => github.com/spectrocloud/maas-client-go v0.1.5-pre

pkg/maas/machine/machine.go:772

• The comment references "VMConfig.Eth0LinkMode/Eth1LinkMode" but the actual field is "VMConfig.InterfaceLinkModes" (a map). Update the comment to reference "VMConfig.InterfaceLinkModes" instead.

	// Link the subnet with the given mode (from VMConfig.Eth0LinkMode/Eth1LinkMode or defaults).

go.mod:14

• The version "v0.1.5-pre" uses a non-standard pre-release identifier. Standard semver pre-release versions should include a dot followed by the pre-release identifier (e.g., "v0.1.5-pre.1" or "v0.1.5-alpha"). While this may work with Go modules, consider following semver conventions for clarity. Additionally, verify that this specific version exists in the maas-client-go repository.

	github.com/spectrocloud/maas-client-go v0.1.5-pre

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The resolveLinkMode function always returns a non-empty string (either a configured mode or a default mode). This means the condition "if linkMode != ''" on line 774 will always be true, making the else block (lines 778-782) unreachable dead code. Consider removing the conditional check and the else branch, keeping only the LinkSubnetWithMode call.

Suggested change

	// When linkMode is empty we use LinkSubnet (client default: auto when no IP).
	if linkMode != "" {
	if err := ifaceClient.LinkSubnetWithMode(ctx, expectedSubnetIdentifier, linkMode, ""); err != nil {
	return fmt.Errorf("failed to link subnet %s to %s (mode=%s): %w", expectedSubnetIdentifier, ifaceName, linkMode, err)
	}
	} else {
	if err := ifaceClient.LinkSubnet(ctx, expectedSubnetIdentifier, ""); err != nil {
	return fmt.Errorf("failed to link subnet %s to %s: %w", expectedSubnetIdentifier, ifaceName, err)
	}
	// resolveLinkMode always returns a non-empty mode, so we always use LinkSubnetWithMode.
	if err := ifaceClient.LinkSubnetWithMode(ctx, expectedSubnetIdentifier, linkMode, ""); err != nil {
	return fmt.Errorf("failed to link subnet %s to %s (mode=%s): %w", expectedSubnetIdentifier, ifaceName, linkMode, err)

[Copilot]

The new resolveLinkMode function and the modified VerifyVMNetworkInterfaces function that now uses it lack test coverage. Consider adding unit tests to verify: 1) default mode selection for eth0 (auto) vs other interfaces (dhcp), 2) custom mode selection from InterfaceLinkModes map, 3) validation of allowed modes, and 4) handling of invalid/empty mode values.