Adopt authoritative AKS Machine goals

[Wenxuan W (wenxuan0923)]

Summary

• Treat an existing AKS Machine goal as authoritative during bootstrap instead of overwriting it from local config.
• Resolve Kubernetes version, max pods, custom labels, taints, and image-GC thresholds from one effective Machine goal for bootstrap, repave, and restart.
• Persist complete current and previous applied goals while retaining legacy scalar projections for downgrade compatibility.
• Keep legacy-only state partial and merge it with local config during restart so unknown labels, taints, and kubelet settings are preserved.
• Resolve major.minor Kubernetes aliases to the returned exact patch before artifact resolution.

Validation

• make check
• go test -race ./pkg/aksmachine ./pkg/daemon

Stack

This is PR 3 of the Machine-goal split and builds on merged PRs #275 and #276. Label/taint in-place acknowledgement and preflight Machine-goal checks remain separate follow-up PRs.

[Copilot]

Pull request overview

This PR updates the agent to treat the AKS-provided Machine goal as authoritative during bootstrap and to consistently resolve an “effective goal” (remote goal overlaid with local scalar defaults) for bootstrap, repave, and restart. It also extends daemon state persistence to store full current/previous applied goals while keeping legacy scalar projections for downgrade compatibility, and resolves major.minor Kubernetes aliases to an exact patch version before artifact resolution.

Changes:

• Adopt and validate the remote Machine goal during bootstrap (instead of overwriting it from local config), while filling missing scalar defaults from local config.
• Persist complete applied/previous applied Machine goals in daemon state, plus legacy projections for older binaries, and validate state on load/save.
• Centralize goal-state resolution to overlay machine-owned settings onto config for goal rendering (bootstrap/repave/restart), including alias-to-patch Kubernetes version resolution.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/daemon/test_helpers_test.go	Adds a shared daemon test helper for building complete Machine goals.
pkg/daemon/state.go	Persists full applied/previous goals, validates state, and maintains legacy projections for compatibility.
pkg/daemon/state_test.go	Updates and expands state persistence/compatibility tests for new state model.
pkg/daemon/repave_reconciler.go	Uses observed version from complete goals when available.
pkg/daemon/repave_reconciler_test.go	Updates repave tests for complete-goal state tracking.
pkg/daemon/reconcile_test.go	Updates decision tests to use complete-goal state shape.
pkg/daemon/nodeoperator.go	Resolves effective goal for restart/repave via new daemon goalstate helper.
pkg/daemon/nodeoperator_test.go	Updates node operator tests for new restart/repave goal resolution behavior.
pkg/daemon/goalstate.go	New helper to overlay machine-owned settings onto config before goal resolution.
pkg/daemon/goalstate_test.go	Adds tests verifying machine-goal overlay and restart goal behavior.
pkg/cmd/start/start.go	Uses daemon ResolveMachineGoalState with the effective bootstrap goal.
pkg/aksmachine/types.go	Adds Validate/ValidateEffective and EffectiveGoal overlay logic for Machine vs local goals.
pkg/aksmachine/types_test.go	Adds coverage for EffectiveGoal behavior and updates existing tests for new helpers.
pkg/aksmachine/test_helpers_test.go	Adds shared test goal builder for aksmachine package tests.
pkg/aksmachine/ensure.go	Changes bootstrap registration to adopt remote goal (with local scalar fill) when present.
pkg/aksmachine/ensure_test.go	Updates ensure tests to reflect adopting remote goals and server-normalized values.
pkg/aksmachine/client_incluster.go	Adopts pre-created machine from in-cluster endpoint without version-match enforcement.
pkg/aksmachine/client_incluster_test.go	Updates in-cluster client tests for expanded goal shape and adoption behavior.
pkg/aksmachine/client_armapi.go	Resolves major.minor version aliases to exact patch using current orchestrator version.
pkg/aksmachine/client_armapi_test.go	Adds test for minor-version alias resolution and updates validation tests.
hack/e2e/lib/upgrade-drift.sh	Passes maxPods to controller machine-goal fixtures for drift/upgrade scenarios.
hack/e2e/lib/node-join-token.sh	Publishes maxPods into machine goal fixture for token-join flow.
hack/e2e/lib/controller.sh	Makes controller fixture machine JSON accept maxPods as a parameter.
hack/demo/aks-flex-node-upgrade.sh	Updates demo machine-goal editing to avoid injecting managed label.
docs/design/in-cluster-machine.md	Updates design doc to reflect remote-goal authoritative bootstrap and alias resolution.
docs/design.md	Updates design overview to match expanded daemon state model.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

hack/demo/aks-flex-node-upgrade.sh:134

• This does not migrate Machine entries created by the previous demo version: those already contain kubernetes.azure.com/managed, and // {} preserves the existing map. Because Machine goals are now adopted as custom labels, the demo continues publishing an AKS-owned label instead of keeping the goal custom-only. Preserve custom entries but delete the AKS-owned keys when updating the goal.

      .properties.kubernetes.nodeLabels = (.properties.kubernetes.nodeLabels // {})

[Wenxuan W (wenxuan0923)]

Good catch. Present values are now validated before checking the ETag.

[hbc (bcho)]

will this result in nil maxPods? MaxPods is required by the kubelet settings, can we set a default or error out when nil is returned from goal state?

[Wenxuan W (wenxuan0923)]

Good point. MaxPods is required now, so it cannot be nil here.

[hbc (bcho)]

nit: maybe create a helper function for similar backfill logic in max pods / ImageGCHighThreshold

[Wenxuan W (wenxuan0923)]

Yep, added a helper and reused it for all three fields.

[hbc (bcho)]

giving these values are required, why do we want to change to pointer type here?

[Wenxuan W (wenxuan0923)]

Agreed. Required fields are values again. Pointers are only used for the raw API response to detect omitted fields.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pkg/daemon/daemon.go:196

• The only test for this new bootstrap-blocking path exercises the early return without a token. The token branch creates a credential provider, starts it, and immediately cancels it; regressions in provider creation, error propagation, or cleanup would therefore be uncovered. Please make provider creation injectable and add tests for successful preparation, constructor failure, and cancellation.

	_, stop, err := daemonRESTConfigProvider(ctx, cfg, bootstrapRestCfg)
	if err != nil {
		return err
	}
	stop()

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pkg/aksmachine/client_armapi.go:282

• A major.minor goal is left unresolved when currentOrchestratorVersion is absent, and resolveKubernetesVersionAlias also returns the alias unchanged when the current version does not match. Machine.Validate accepts that nonempty alias, so bootstrap/repave passes (for example) 1.35 to artifact resolution even though this codebase requires a full patch version for Kubernetes downloads. Require a matching exact currentOrchestratorVersion whenever the desired version is an alias, and reject the Machine response otherwise.

			if kubernetes.CurrentOrchestratorVersion != nil {
				result.Goal.KubernetesVersion = resolveKubernetesVersionAlias(
					result.Goal.KubernetesVersion,
					*kubernetes.CurrentOrchestratorVersion,
				)

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pkg/aksmachine/client_armapi.go:283

• A major.minor goal remains unresolved when currentOrchestratorVersion is absent or does not match (for example, desired 1.35 with current 1.34.9). Machine.Validate accepts the resulting 1.35, so bootstrap or repave can pass that alias into artifact resolution instead of the promised exact patch. Reject such a response (or defer reconciliation) unless a matching exact current version is available.

			if kubernetes.CurrentOrchestratorVersion != nil {
				result.Goal.KubernetesVersion = resolveKubernetesVersionAlias(
					result.Goal.KubernetesVersion,
					*kubernetes.CurrentOrchestratorVersion,
				)
			}

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (3)

pkg/aksmachine/types.go:19

• MaxPods does not actually preserve API presence: an ARM response containing an explicit maxPods: 0 is converted to the same value as an omitted field. Validate therefore accepts it and EffectiveGoal silently substitutes the local value, even though complete goals require max pods to be positive. Preserve presence (for example with a pointer) so only omission defaults and an explicit zero is rejected.

	MaxPods           int                  `json:"maxPods,omitempty"`

pkg/aksmachine/types.go:26

• ImageGCHighThreshold has the same presence-loss problem: an explicit API value of 0 becomes indistinguishable from omission, passes MachineGoal.Validate, and is replaced by the local threshold. Since a complete goal requires this value to be positive, preserve field presence and reject an explicit zero rather than treating it as absent.

	ImageGCHighThreshold int  `json:"imageGCHighThreshold,omitempty"`

pkg/aksmachine/client_armapi.go:319

• For a major.minor desired version, a missing or nonmatching currentOrchestratorVersion falls through and returns the unresolved alias. That alias passes Machine.Validate and reaches artifact resolution, so the PR does not guarantee the exact patch described in its validation contract. Reject such a response (or return an error from conversion) unless currentOrchestratorVersion supplies a matching patch.

	if len(strings.Split(desiredVersion, ".")) == 2 && strings.HasPrefix(currentVersion, desiredVersion+".") {
		return currentVersion
	}
	return desiredVersion