Make ARM Machine goals authoritative and acknowledge RP node updates

docs/design.md

@@ -152,7 +152,7 @@ See [AKS RP And Flex Node Agent Interaction](design/agent-and-aks.md) for the de
 
 ## State And Idempotency
 
-The agent persists local daemon state so it can recover after restart, reboot, or partial failure. Persisted state includes the applied Kubernetes/settings version and active nspawn machine side.
+The agent persists local daemon state so it can recover after restart, reboot, or partial failure. Persisted state includes the current and previous applied goals and the active nspawn machine side.
 
 The current state model separates desired state, applied state, and runtime discovery:
 

docs/design/in-cluster-machine.md

@@ -27,18 +27,18 @@ Status updates use a separate patch model because the agent operation status is
 
 ## Bootstrap flow
 
-The local bootstrap configuration is authoritative while `aks-flex-node start` is running:
+The local bootstrap configuration seeds a Machine when one does not already exist. Once the endpoint returns a Machine, its complete goal is authoritative for bootstrap:
 
 1. `NewMachineClient` selects the in-cluster backend without a supplied Kubernetes REST config.
 2. The client builds a REST config from the bootstrap token or configured exec credential.
 3. `EnsureMachine` reads the machine through the Kubernetes service proxy.
 4. If the machine is absent, the client sends a PUT using the local bootstrap goal.
-5. If its Kubernetes version differs, the client sends a PUT that overwrites the remote goal with the local version.
-6. If its Kubernetes version already matches, local bootstrap settings remain authoritative; remote settings other than the ETag do not replace them.
-7. The returned ETag becomes the reconciliation baseline for the locally applied goal.
-8. The daemon state is seeded from that ETag before host or nspawn state is mutated. A later ETag change is treated as a new remote goal.
+5. Whether read or created, the returned Machine is validated and its complete goal replaces the local bootstrap goal. This includes the Kubernetes version, node settings, and ETag-backed settings version.
+6. The daemon state is seeded from the accepted Machine goal before host or nspawn state is mutated. A later ETag change is treated as a new remote goal.
 
-The ConfigMap-backed controller is read-only: it accepts mutation requests but returns the pre-created machine. Its fixture must therefore already match the local bootstrap version. When machine registration is required, a mismatch fails bootstrap before host mutation.
+The ConfigMap-backed controller is read-only: it accepts mutation requests but returns the pre-created Machine. The agent adopts that returned goal even when it differs from local bootstrap configuration. The returned Machine must contain a valid Kubernetes version and settings version.
+
+When machine registration is required, a read, create, or validation failure stops bootstrap before host mutation. When registration is optional, the same failure is logged and bootstrap continues from the local goal; its settings version remains empty until a valid Machine goal is later observed.
 
 ## Daemon flow
 

docs/usages/operations.md

@@ -4,7 +4,7 @@ This guide summarizes common host and cluster operations for AKS Flex Node.
 
 ## Preflight
 
-Run preflight before mutating the host. The command validates the config, resolves the nspawn goal state, and checks host prerequisites, API server reachability, rootfs image reachability, and bootstrap artifact sources.
+Run preflight before mutating the host. The command validates the config, reads any existing AKS Machine, resolves the effective nspawn goal state, and checks host prerequisites, API server reachability, rootfs image reachability, and bootstrap artifact sources. An existing Machine is authoritative. When its goal differs from local configuration, preflight reports an error and validates bootstrap inputs derived from the Machine goal. If the Machine does not exist, preflight keeps the original config-only behavior.
 
 ```bash
 aks-flex-node preflight --config /etc/aks-flex-node/config.json

hack/e2e/lib/upgrade-drift.sh

@@ -56,8 +56,22 @@ _remote_active_machine_snapshot() {
   remote_exec "${vm_ip}" 'bash -s' <<'REMOTE'
 set +e
 state_file="/etc/aks-flex-node/daemon-state.json"
-machine="$(sudo sed -n 's/.*"activeMachine"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "${state_file}" 2>/dev/null)"
-applied_settings_version="$(sudo sed -n 's/.*"appliedSettingsVersion"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "${state_file}" 2>/dev/null)"
+state_snapshot="$(sudo python3 - "${state_file}" <<'PY' 2>/dev/null
+import json
+import sys
+
+try:
+    with open(sys.argv[1], encoding="utf-8") as state_file:
+        state = json.load(state_file)
+    machine = state.get("activeMachine", "")
+    applied_goal = state.get("appliedGoal") or {}
+    settings_version = applied_goal.get("settingsVersion", "")
+    print(f"{machine}|{settings_version}")
+except (OSError, json.JSONDecodeError, AttributeError):
+    print("|")
+PY
+)"
+IFS='|' read -r machine applied_settings_version <<<"${state_snapshot}"
 state=""
 version=""
 if [[ -n "${machine}" ]]; then

pkg/aksmachine/client_armapi.go

@@ -77,7 +77,7 @@ func (c *armMachineClient) Create(ctx context.Context, desired GoalState) (*Mach
 	if err := c.validateMachineIdentity(resp.Machine); err != nil {
 		return nil, err
 	}
-	result := machineFromARM(resp.Machine, desired)
+	result := machineFromARM(resp.Machine)
 	result.ID = c.machineID.String()
 	result.Name = c.machineID.Name
 	return result, nil
@@ -103,7 +103,7 @@ func (c *armMachineClient) Get(ctx context.Context) (*Machine, error) {
 	if err := c.validateMachineIdentity(resp.Machine); err != nil {
 		return nil, err
 	}
-	result := machineFromARM(resp.Machine, GoalState{})
+	result := machineFromARM(resp.Machine)
 	result.ID = c.machineID.String()
 	result.Name = c.machineID.Name
 	return result, nil
@@ -256,8 +256,8 @@ func (c *armMachineClient) validateMachineIdentity(machine armcontainerservice.M
 	return nil
 }
 
-func machineFromARM(machine armcontainerservice.Machine, fallback GoalState) *Machine {
-	result := &Machine{Goal: fallback}
+func machineFromARM(machine armcontainerservice.Machine) *Machine {
+	result := &Machine{}
 	if machine.ID != nil {
 		result.ID = *machine.ID
 	}
@@ -298,9 +298,6 @@ func machineFromARM(machine armcontainerservice.Machine, fallback GoalState) *Ma
 	if properties.ETag != nil {
 		result.Goal.SettingsVersion = *properties.ETag
 	}
-	if result.Goal.SettingsVersion == "" {
-		result.Goal.SettingsVersion = result.Goal.KubernetesVersion
-	}
 	if properties.ProvisioningState != nil {
 		result.Status.ProvisioningState = ProvisioningState(*properties.ProvisioningState)
 	}

pkg/aksmachine/client_armapi_test.go

@@ -317,7 +317,7 @@ func TestMachineFromARM(t *testing.T) {
 			},
 			ProvisioningState: ptr("Succeeded"),
 		},
-	}, GoalState{SettingsVersion: "fallback-settings"})
+	})
 
 	if machine.ID != "machine-id" || machine.Name != "node1" {
 		t.Fatalf("machine identity = %#v", machine)
@@ -408,13 +408,13 @@ func TestMachineFromARMUsesCurrentOrchestratorVersionFallback(t *testing.T) {
 				CurrentOrchestratorVersion: &currentVersion,
 			},
 		},
-	}, GoalState{})
+	})
 
 	if machine.Goal.KubernetesVersion != currentVersion {
 		t.Fatalf("KubernetesVersion = %q, want %q", machine.Goal.KubernetesVersion, currentVersion)
 	}
-	if machine.Goal.SettingsVersion != currentVersion {
-		t.Fatalf("SettingsVersion = %q, want %q", machine.Goal.SettingsVersion, currentVersion)
+	if machine.Goal.SettingsVersion != "" {
+		t.Fatalf("SettingsVersion = %q, want empty without an ETag", machine.Goal.SettingsVersion)
 	}
 }
 

pkg/aksmachine/client_incluster.go

@@ -130,7 +130,7 @@ func (c *clusterEndpointClient) Create(ctx context.Context, desired GoalState) (
 
 	if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusMethodNotAllowed || resp.StatusCode == http.StatusNoContent {
 		c.logger.Debug("cluster endpoint did not apply machine create request; verifying pre-created machine", "status", resp.Status)
-		return c.adoptExistingMachine(ctx, desired)
+		return c.adoptExistingMachine(ctx)
 	}
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
 		return nil, clusterEndpointHTTPError("create machine through cluster endpoint", requestURL, resp)
@@ -140,7 +140,7 @@ func (c *clusterEndpointClient) Create(ctx context.Context, desired GoalState) (
 		return nil, fmt.Errorf("read cluster endpoint machine create response: %w", err)
 	}
 	if strings.TrimSpace(string(data)) == "" {
-		return c.adoptExistingMachine(ctx, desired)
+		return c.adoptExistingMachine(ctx)
 	}
 	machine, err := machineFromEndpointJSON(data)
 	if err != nil {
@@ -152,33 +152,17 @@ func (c *clusterEndpointClient) Create(ctx context.Context, desired GoalState) (
 	if machine.Name == "" {
 		machine.Name = c.nodeName
 	}
-	if err := validateAdoptedMachine(machine, desired); err != nil {
-		return nil, err
-	}
 	return machine, nil
 }
 
-func (c *clusterEndpointClient) adoptExistingMachine(ctx context.Context, desired GoalState) (*Machine, error) {
+func (c *clusterEndpointClient) adoptExistingMachine(ctx context.Context) (*Machine, error) {
 	machine, err := c.Get(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("verify pre-created machine from cluster endpoint: %w", err)
 	}
-	if err := validateAdoptedMachine(machine, desired); err != nil {
-		return nil, err
-	}
 	return machine, nil
 }
 
-func validateAdoptedMachine(machine *Machine, desired GoalState) error {
-	if machine == nil {
-		return fmt.Errorf("cluster endpoint returned nil machine")
-	}
-	if desired.KubernetesVersion != "" && machine.Goal.KubernetesVersion != "" && machine.Goal.KubernetesVersion != desired.KubernetesVersion {
-		return fmt.Errorf("pre-created machine Kubernetes version %q does not match desired %q", machine.Goal.KubernetesVersion, desired.KubernetesVersion)
-	}
-	return nil
-}
-
 func (c *clusterEndpointClient) Get(ctx context.Context) (*Machine, error) {
 	requestURL := c.machineURL(c.nodeName)
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, requestURL.String(), nil)
@@ -279,7 +263,7 @@ func machineFromEndpointJSON(data []byte) (*Machine, error) {
 	if err := json.Unmarshal(data, &armMachine); err != nil {
 		return nil, fmt.Errorf("decode cluster endpoint machine response: %w", err)
 	}
-	return machineFromARM(armMachine, GoalState{}), nil
+	return machineFromARM(armMachine), nil
 }
 
 func joinEndpointURLPath(base string, elem ...string) string {

pkg/aksmachine/client_incluster_test.go

@@ -134,7 +134,7 @@ func TestClusterEndpointCreateSendsMutation(t *testing.T) {
 	}
 }
 
-func TestClusterEndpointCreateVerifiesPrecreatedMachine(t *testing.T) {
+func TestClusterEndpointCreateAdoptsReturnedMachine(t *testing.T) {
 	t.Parallel()
 
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
@@ -144,9 +144,12 @@ func TestClusterEndpointCreateVerifiesPrecreatedMachine(t *testing.T) {
 	defer server.Close()
 
 	client := newTestClusterEndpointClient(t, server.URL, "node1")
-	_, err := client.Create(context.Background(), GoalState{KubernetesVersion: "1.35.0", SettingsVersion: "42"})
-	if err == nil || !strings.Contains(err.Error(), "Kubernetes version") {
-		t.Fatalf("Create() error = %v, want version mismatch", err)
+	machine, err := client.Create(context.Background(), GoalState{KubernetesVersion: "1.35.0", SettingsVersion: "local"})
+	if err != nil {
+		t.Fatalf("Create() error = %v", err)
+	}
+	if machine.Goal.KubernetesVersion != "1.34.0" || machine.Goal.SettingsVersion != "42" {
+		t.Fatalf("Create() machine goal = %#v, want returned machine goal", machine.Goal)
 	}
 }
 

pkg/aksmachine/ensure.go

@@ -17,10 +17,8 @@ type ensureMachineTask struct {
 }
 
 // EnsureMachine returns a task that ensures this machine is registered in AKS.
-// Local configuration remains authoritative during bootstrap. When the remote
-// Kubernetes version already matches, the task adopts only the remote ETag as
-// the reconciliation baseline; other remote settings do not replace the local
-// goal. Subsequent ETag changes are handled by the daemon as new remote goals.
+// Local bootstrap configuration seeds a new Machine. Once a Machine exists, its
+// complete goal is authoritative for Node creation and later reconciliation.
 func EnsureMachine(machines MachineClient, goal *GoalState, require bool, logger *slog.Logger) phases.Task {
 	return &ensureMachineTask{machines: machines, goal: goal, require: require, logger: logger}
 }
@@ -30,25 +28,8 @@ func (t *ensureMachineTask) Name() string { return "ensure-machine" }
 func (t *ensureMachineTask) Do(ctx context.Context) error {
 	machine, err := t.machines.Get(ctx)
 	if err == nil {
-		if machine != nil && machine.Goal.KubernetesVersion == t.goal.KubernetesVersion {
-			t.logger.Info("machine already registered, skipping")
-			return t.adoptSettingsVersion(machine, "get machine")
-		}
-
-		remoteVersion := ""
-		if machine != nil {
-			remoteVersion = machine.Goal.KubernetesVersion
-		}
-		t.logger.Info(
-			"updating registered machine from local bootstrap config",
-			"remoteKubernetesVersion", remoteVersion,
-			"localKubernetesVersion", t.goal.KubernetesVersion,
-		)
-		machine, err = t.machines.Create(ctx, *t.goal)
-		if err != nil {
-			return t.handleError("update machine", err)
-		}
-		return t.adoptSettingsVersion(machine, "update machine")
+		t.logger.Info("ARM machine already registered, adopting remote goal")
+		return t.adoptGoal(machine, "get machine")
 	}
 
 	var notFound *NotFoundError
@@ -59,26 +40,15 @@ func (t *ensureMachineTask) Do(ctx context.Context) error {
 	if err != nil {
 		return t.handleError("create machine", err)
 	}
-	return t.adoptSettingsVersion(machine, "create machine")
+	return t.adoptGoal(machine, "create machine")
 }
 
-func (t *ensureMachineTask) adoptSettingsVersion(machine *Machine, operation string) error {
-	if machine == nil {
-		return t.handleError(operation, fmt.Errorf("AKS returned a nil machine"))
-	}
-	if machine.Goal.KubernetesVersion != t.goal.KubernetesVersion {
-		return t.handleError(
-			operation,
-			fmt.Errorf(
-				"AKS machine Kubernetes version %q does not match local bootstrap version %q",
-				machine.Goal.KubernetesVersion,
-				t.goal.KubernetesVersion,
-			),
-		)
-	}
-	if machine.Goal.SettingsVersion != "" {
-		t.goal.SettingsVersion = machine.Goal.SettingsVersion
+func (t *ensureMachineTask) adoptGoal(machine *Machine, operation string) error {
+	if err := machine.Validate(); err != nil {
+		return t.handleError(operation, fmt.Errorf("AKS returned an invalid machine: %w", err))
 	}
+
+	*t.goal = machine.Goal
 	return nil
 }
 

pkg/aksmachine/ensure_test.go

@@ -102,7 +102,7 @@ func TestEnsureMachineCreatesAndAdoptsSettingsVersion(t *testing.T) {
 	}
 }
 
-func TestEnsureMachineAdoptsExistingSettingsVersionWithoutReplacingLocalGoal(t *testing.T) {
+func TestEnsureMachineAdoptsExistingGoal(t *testing.T) {
 	t.Parallel()
 
 	goal := GoalState{
@@ -135,63 +135,57 @@ func TestEnsureMachineAdoptsExistingSettingsVersionWithoutReplacingLocalGoal(t *
 	if client.createCalls != 0 {
 		t.Fatalf("Create() calls = %d, want 0", client.createCalls)
 	}
-	if goal.SettingsVersion != "etag-42" {
-		t.Fatalf("SettingsVersion = %q, want etag-42", goal.SettingsVersion)
+	if goal.SettingsVersion != "etag-42" || goal.KubernetesVersion != "1.35.1" {
+		t.Fatalf("version goal = %#v", goal)
 	}
-	if goal.MaxPods != 30 || goal.NodeLabels["source"] != "local" || goal.NodeTaints[0] != "local=true:NoSchedule" {
-		t.Fatalf("local goal was replaced by remote settings: %#v", goal)
+	if goal.MaxPods != 110 || goal.NodeLabels["source"] != "remote" || goal.NodeTaints[0] != "remote=true:NoSchedule" {
+		t.Fatalf("remote goal was not adopted: %#v", goal)
 	}
-	if goal.KubeletConfig.ImageGCHighThreshold != 85 || goal.KubeletConfig.ImageGCLowThreshold != 80 {
-		t.Fatalf("local kubelet config was replaced by remote settings: %#v", goal.KubeletConfig)
+	if goal.KubeletConfig.ImageGCHighThreshold != 70 || goal.KubeletConfig.ImageGCLowThreshold != 60 {
+		t.Fatalf("remote kubelet config was not adopted: %#v", goal.KubeletConfig)
 	}
 }
 
-func TestEnsureMachineUpdatesMismatchedVersion(t *testing.T) {
+func TestEnsureMachineAdoptsExistingMismatchedVersion(t *testing.T) {
 	t.Parallel()
 
 	goal := GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "1.35.1"}
 	client := &ensureMachineClient{
-		machine: &Machine{Goal: GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "etag-old"}},
-		createResult: &Machine{Goal: GoalState{
-			KubernetesVersion: "1.35.1",
-			SettingsVersion:   "etag-new",
+		machine: &Machine{Goal: GoalState{
+			KubernetesVersion: "1.34.0",
+			SettingsVersion:   "etag-remote",
+			NodeLabels:        map[string]string{"source": "remote"},
+			NodeTaints:        []string{"remote=true:NoSchedule"},
 		}},
 	}
 	task := EnsureMachine(client, &goal, true, slog.New(slog.NewTextHandler(io.Discard, nil)))
 
 	if err := task.Do(context.Background()); err != nil {
 		t.Fatalf("Do() error = %v", err)
 	}
-	if client.createCalls != 1 {
-		t.Fatalf("Create() calls = %d, want 1", client.createCalls)
+	if client.createCalls != 0 {
+		t.Fatalf("Create() calls = %d, want 0", client.createCalls)
 	}
-	if client.createdGoal.KubernetesVersion != "1.35.1" {
-		t.Fatalf("Create() goal = %#v", client.createdGoal)
+	if goal.KubernetesVersion != "1.34.0" || goal.SettingsVersion != "etag-remote" {
+		t.Fatalf("version goal = %#v", goal)
 	}
-	if goal.SettingsVersion != "etag-new" {
-		t.Fatalf("SettingsVersion = %q, want etag-new", goal.SettingsVersion)
+	if goal.NodeLabels["source"] != "remote" || goal.NodeTaints[0] != "remote=true:NoSchedule" {
+		t.Fatalf("remote goal was not adopted: %#v", goal)
 	}
 }
 
-func TestEnsureMachineRejectsUnchangedRemoteVersionAfterUpdate(t *testing.T) {
+func TestEnsureMachineRejectsMissingSettingsVersion(t *testing.T) {
 	t.Parallel()
 
-	goal := GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "1.35.1"}
+	goal := GoalState{KubernetesVersion: "1.35.1"}
 	client := &ensureMachineClient{
-		machine: &Machine{Goal: GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "etag-old"}},
-		createResult: &Machine{Goal: GoalState{
-			KubernetesVersion: "1.34.0",
-			SettingsVersion:   "etag-old",
-		}},
+		machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}},
 	}
 	task := EnsureMachine(client, &goal, true, slog.New(slog.NewTextHandler(io.Discard, nil)))
 
 	err := task.Do(context.Background())
-	if err == nil || !strings.Contains(err.Error(), `AKS machine Kubernetes version "1.34.0" does not match local bootstrap version "1.35.1"`) {
-		t.Fatalf("Do() error = %v, want version mismatch", err)
-	}
-	if goal.SettingsVersion != "1.35.1" {
-		t.Fatalf("SettingsVersion = %q, want local fallback", goal.SettingsVersion)
+	if err == nil || !strings.Contains(err.Error(), "goal settings version is empty") {
+		t.Fatalf("Do() error = %v, want missing settings version", err)
 	}
 }