From 44b711f5fb3373cff3368e1f9ffd980b2a783cc2 Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Fri, 14 Aug 2026 19:58:20 -0700 Subject: [PATCH 1/7] Adopt authoritative AKS Machine goals --- docs/design.md | 2 +- docs/design/in-cluster-machine.md | 14 +-- hack/demo/aks-flex-node-upgrade.sh | 2 +- pkg/aksmachine/client_armapi.go | 15 +++ pkg/aksmachine/client_armapi_test.go | 20 ++++ pkg/aksmachine/client_incluster.go | 22 +---- pkg/aksmachine/client_incluster_test.go | 29 ++++-- pkg/aksmachine/ensure.go | 61 +++--------- pkg/aksmachine/ensure_test.go | 79 +++++----------- pkg/aksmachine/test_helpers_test.go | 13 +++ pkg/aksmachine/types.go | 60 +++++++++++- pkg/aksmachine/types_test.go | 40 +++++++- pkg/cmd/start/start.go | 2 +- pkg/daemon/goalstate.go | 81 ++++++++++++++++ pkg/daemon/goalstate_test.go | 102 ++++++++++++++++++++ pkg/daemon/nodeoperator.go | 43 ++++----- pkg/daemon/nodeoperator_test.go | 38 ++++++-- pkg/daemon/reconcile_test.go | 7 +- pkg/daemon/repave_reconciler.go | 10 +- pkg/daemon/repave_reconciler_test.go | 15 ++- pkg/daemon/state.go | 73 ++++++++++++-- pkg/daemon/state_test.go | 121 +++++++++++++++++++++--- pkg/daemon/test_helpers_test.go | 15 +++ 23 files changed, 656 insertions(+), 208 deletions(-) create mode 100644 pkg/aksmachine/test_helpers_test.go create mode 100644 pkg/daemon/goalstate.go create mode 100644 pkg/daemon/goalstate_test.go create mode 100644 pkg/daemon/test_helpers_test.go diff --git a/docs/design.md b/docs/design.md index da72834b..1de9970a 100644 --- a/docs/design.md +++ b/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 Machine goals and the active nspawn machine side. The current state model separates desired state, applied state, and runtime discovery: diff --git a/docs/design/in-cluster-machine.md b/docs/design/in-cluster-machine.md index 3d46a2ac..f5ce53bb 100644 --- a/docs/design/in-cluster-machine.md +++ b/docs/design/in-cluster-machine.md @@ -15,7 +15,7 @@ The controller serves the `armcontainerservice.Machine` JSON shape from the `kub "orchestratorVersion": "1.34.0", "maxPods": 110, "nodeLabels": { - "kubernetes.azure.com/managed": "false" + "workload": "edge" } }, "provisioningState": "Succeeded" @@ -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 goal replaces the local bootstrap goal. This includes Kubernetes version, max pods, custom labels, taints, kubelet image-GC thresholds, and the ETag-backed settings version. Scalar defaults omitted by the API retain their validated local bootstrap values. +6. The daemon resolves nspawn settings and seeds its state from that same effective goal before mutating the host. 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. +When `orchestratorVersion` is a `major.minor` alias, the returned `currentOrchestratorVersion` supplies the exact patch used for artifact resolution. + +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. When registration is required, a read, create, or validation failure stops bootstrap before host mutation. When registration is optional, bootstrap continues with the local goal. ## Daemon flow diff --git a/hack/demo/aks-flex-node-upgrade.sh b/hack/demo/aks-flex-node-upgrade.sh index b85073c2..66b9a163 100755 --- a/hack/demo/aks-flex-node-upgrade.sh +++ b/hack/demo/aks-flex-node-upgrade.sh @@ -131,7 +131,7 @@ update_machine_goal() { .properties.eTag = $settings | .properties.kubernetes = (.properties.kubernetes // {}) | .properties.kubernetes.orchestratorVersion = $version | - .properties.kubernetes.nodeLabels = (.properties.kubernetes.nodeLabels // {"kubernetes.azure.com/managed":"false"}) + .properties.kubernetes.nodeLabels = (.properties.kubernetes.nodeLabels // {}) ' <<<"${current_json}" > "${tmp}" if [[ -z "${cm_json}" ]]; then diff --git a/pkg/aksmachine/client_armapi.go b/pkg/aksmachine/client_armapi.go index f6c2327b..428da5b1 100644 --- a/pkg/aksmachine/client_armapi.go +++ b/pkg/aksmachine/client_armapi.go @@ -275,6 +275,12 @@ func machineFromARM(machine armcontainerservice.Machine, defaultID, defaultName kubernetes := properties.Kubernetes if kubernetes.OrchestratorVersion != nil { result.Goal.KubernetesVersion = *kubernetes.OrchestratorVersion + if kubernetes.CurrentOrchestratorVersion != nil { + result.Goal.KubernetesVersion = resolveKubernetesVersionAlias( + result.Goal.KubernetesVersion, + *kubernetes.CurrentOrchestratorVersion, + ) + } } if kubernetes.MaxPods != nil { result.Goal.MaxPods = int(*kubernetes.MaxPods) @@ -303,6 +309,15 @@ func machineFromARM(machine armcontainerservice.Machine, defaultID, defaultName return result } +func resolveKubernetesVersionAlias(desired, current string) string { + desiredVersion := strings.TrimPrefix(strings.TrimSpace(desired), "v") + currentVersion := strings.TrimPrefix(strings.TrimSpace(current), "v") + if len(strings.Split(desiredVersion, ".")) == 2 && strings.HasPrefix(currentVersion, desiredVersion+".") { + return current + } + return desired +} + func stringMapFromPointers(values map[string]*string) map[string]string { result := make(map[string]string, len(values)) for key, value := range values { diff --git a/pkg/aksmachine/client_armapi_test.go b/pkg/aksmachine/client_armapi_test.go index 0644171d..b12720d7 100644 --- a/pkg/aksmachine/client_armapi_test.go +++ b/pkg/aksmachine/client_armapi_test.go @@ -261,6 +261,7 @@ func TestGoalStateValidate(t *testing.T) { name: "negative image GC high threshold", goal: GoalState{ KubernetesVersion: "1.35.1", + MaxPods: 110, KubeletConfig: KubeletConfig{ ImageGCHighThreshold: -1, }, @@ -271,6 +272,7 @@ func TestGoalStateValidate(t *testing.T) { name: "negative image GC low threshold", goal: GoalState{ KubernetesVersion: "1.35.1", + MaxPods: 110, KubeletConfig: KubeletConfig{ ImageGCLowThreshold: -1, }, @@ -415,6 +417,24 @@ func TestMachineFromARMDoesNotUseCurrentOrchestratorVersionAsGoal(t *testing.T) } } +func TestMachineFromARMResolvesMinorVersionAlias(t *testing.T) { + t.Parallel() + + machine := machineFromARM(armcontainerservice.Machine{ + Properties: &armcontainerservice.MachineProperties{ + ETag: ptr("42"), + Kubernetes: &armcontainerservice.MachineKubernetesProfile{ + OrchestratorVersion: ptr("1.35"), + CurrentOrchestratorVersion: ptr("1.35.2"), + }, + }, + }, "", "") + + if machine.Goal.KubernetesVersion != "1.35.2" { + t.Fatalf("KubernetesVersion = %q, want resolved patch 1.35.2", machine.Goal.KubernetesVersion) + } +} + func TestMachineFromARMDoesNotSynthesizeSettingsVersion(t *testing.T) { t.Parallel() diff --git a/pkg/aksmachine/client_incluster.go b/pkg/aksmachine/client_incluster.go index 3b518a7a..23d6c2fc 100644 --- a/pkg/aksmachine/client_incluster.go +++ b/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 err := machine.Validate(); err != nil { - return fmt.Errorf("cluster endpoint returned invalid machine: %w", err) - } - 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) diff --git a/pkg/aksmachine/client_incluster_test.go b/pkg/aksmachine/client_incluster_test.go index 77df721d..a950110b 100644 --- a/pkg/aksmachine/client_incluster_test.go +++ b/pkg/aksmachine/client_incluster_test.go @@ -70,7 +70,11 @@ func TestMachineFromEndpointJSONUsesARMModel(t *testing.T) { "name": "node1", "properties": { "eTag": "42", - "kubernetes": {"orchestratorVersion": "1.34.0"}, + "kubernetes": { + "orchestratorVersion": "1.34.0", + "maxPods": 110, + "kubeletConfig": {"imageGcHighThreshold": 85, "imageGcLowThreshold": 80} + }, "provisioningState": "Succeeded" } }`)) @@ -93,7 +97,11 @@ func TestMachineFromEndpointJSONRejectsMissingETag(t *testing.T) { _, err := machineFromEndpointJSON([]byte(`{ "properties": { - "kubernetes": {"orchestratorVersion": "1.34.0"} + "kubernetes": { + "orchestratorVersion": "1.34.0", + "maxPods": 110, + "kubeletConfig": {"imageGcHighThreshold": 85, "imageGcLowThreshold": 80} + } } }`)) if err == nil || !strings.Contains(err.Error(), "goal settings version is empty") { @@ -137,29 +145,32 @@ func TestClusterEndpointCreateSendsMutation(t *testing.T) { return } w.Header().Set("Content-Type", "application/json") - _, _ = fmt.Fprint(w, `{"properties":{"eTag":"42","kubernetes":{"orchestratorVersion":"1.34.0"}}}`) + _, _ = fmt.Fprint(w, `{"properties":{"eTag":"42","kubernetes":{"orchestratorVersion":"1.34.0","maxPods":110,"kubeletConfig":{"imageGcHighThreshold":85,"imageGcLowThreshold":80}}}}`) })) defer server.Close() client := newTestClusterEndpointClient(t, server.URL, "node1") - if _, err := client.Create(context.Background(), GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "42"}); err != nil { + if _, err := client.Create(context.Background(), testGoal("1.34.0", "42")); err != nil { t.Fatalf("Create() error = %v", err) } } -func TestClusterEndpointCreateVerifiesPrecreatedMachine(t *testing.T) { +func TestClusterEndpointCreateAdoptsPrecreatedMachine(t *testing.T) { t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") - _, _ = fmt.Fprint(w, `{"properties":{"eTag":"42","kubernetes":{"orchestratorVersion":"1.34.0"}}}`) + _, _ = fmt.Fprint(w, `{"properties":{"eTag":"42","kubernetes":{"orchestratorVersion":"1.34.0","maxPods":110,"kubeletConfig":{"imageGcHighThreshold":85,"imageGcLowThreshold":80}}}}`) })) 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(), testGoal("1.35.0", "local")) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if machine.Goal.KubernetesVersion != "1.34.0" || machine.Goal.SettingsVersion != "42" { + t.Fatalf("Create() goal = %#v, want pre-created Machine goal", machine.Goal) } } diff --git a/pkg/aksmachine/ensure.go b/pkg/aksmachine/ensure.go index f08178c6..decc3849 100644 --- a/pkg/aksmachine/ensure.go +++ b/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 configuration seeds a Machine when one does not exist. A Machine +// returned by AKS is authoritative for bootstrap 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} } @@ -33,22 +31,16 @@ func (t *ensureMachineTask) Do(ctx context.Context) error { return t.handleError("get machine", err) } - switch { - case remoteMachine == nil: + switch remoteMachine { + case nil: remoteMachine, err = t.createRemoteMachineFromGoal(ctx) if err != nil { return t.handleError("create machine", err) } - case machineGoalHasDrift(remoteMachine.Goal, *t.goal): - remoteMachine, err = t.updateRemoteMachineFromGoal(ctx, remoteMachine) - if err != nil { - return t.handleError("update machine", err) - } default: - t.logger.Info("machine already registered, skipping") + t.logger.Info("machine already registered, adopting remote goal") } - t.applyGoalStateWithRemoteMachineSettingsVersion(remoteMachine) - return nil + return t.applyRemoteMachineGoal(remoteMachine) } func (t *ensureMachineTask) fetchRemoteMachine(ctx context.Context) (*Machine, error) { @@ -71,47 +63,18 @@ func (t *ensureMachineTask) createRemoteMachineFromGoal(ctx context.Context) (*M if err != nil { return nil, err } - if err := validateMachineForGoal(machine, *t.goal); err != nil { - return nil, err + if err := machine.Validate(); err != nil { + return nil, fmt.Errorf("AKS returned an invalid machine: %w", err) } return machine, nil } -func (t *ensureMachineTask) updateRemoteMachineFromGoal(ctx context.Context, current *Machine) (*Machine, error) { - t.logger.Info( - "updating registered machine from local bootstrap config", - "remoteKubernetesVersion", current.Goal.KubernetesVersion, - "localKubernetesVersion", t.goal.KubernetesVersion, - ) - machine, err := t.machines.Create(ctx, *t.goal) +func (t *ensureMachineTask) applyRemoteMachineGoal(machine *Machine) error { + effectiveGoal, err := EffectiveGoal(machine.Goal, *t.goal) if err != nil { - return nil, err - } - if err := validateMachineForGoal(machine, *t.goal); err != nil { - return nil, err - } - return machine, nil -} - -func (t *ensureMachineTask) applyGoalStateWithRemoteMachineSettingsVersion(machine *Machine) { - t.goal.SettingsVersion = machine.Goal.SettingsVersion -} - -func machineGoalHasDrift(remote, desired GoalState) bool { - return remote.KubernetesVersion != desired.KubernetesVersion -} - -func validateMachineForGoal(machine *Machine, goal GoalState) error { - if err := machine.Validate(); err != nil { - return fmt.Errorf("AKS returned an invalid machine: %w", err) - } - if machine.Goal.KubernetesVersion != goal.KubernetesVersion { - return fmt.Errorf( - "AKS machine Kubernetes version %q does not match local bootstrap version %q", - machine.Goal.KubernetesVersion, - goal.KubernetesVersion, - ) + return t.handleError("apply machine goal", err) } + *t.goal = effectiveGoal return nil } diff --git a/pkg/aksmachine/ensure_test.go b/pkg/aksmachine/ensure_test.go index 3e8b0750..65b30a7d 100644 --- a/pkg/aksmachine/ensure_test.go +++ b/pkg/aksmachine/ensure_test.go @@ -28,7 +28,7 @@ func TestEnsureMachineCreateFailure(t *testing.T) { t.Parallel() client := &ensureMachineClient{createErr: errors.New("boom")} - goal := GoalState{KubernetesVersion: "1.35.1"} + goal := testGoal("1.35.1", "") task := EnsureMachine(client, &goal, tt.require, slog.New(slog.NewTextHandler(io.Discard, nil))) err := task.Do(context.Background()) @@ -64,7 +64,7 @@ func TestEnsureMachineGetFailure(t *testing.T) { t.Parallel() client := &ensureMachineClient{getErr: errors.New("boom")} - goal := GoalState{KubernetesVersion: "1.35.1"} + goal := testGoal("1.35.1", "") task := EnsureMachine(client, &goal, tt.require, slog.New(slog.NewTextHandler(io.Discard, nil))) err := task.Do(context.Background()) @@ -84,11 +84,10 @@ func TestEnsureMachineGetFailure(t *testing.T) { func TestEnsureMachineCreatesAndAdoptsSettingsVersion(t *testing.T) { t.Parallel() - goal := GoalState{KubernetesVersion: "1.35.1"} - client := &ensureMachineClient{createResult: &Machine{Goal: GoalState{ - KubernetesVersion: "1.35.1", - SettingsVersion: "etag-created", - }}} + goal := testGoal("1.35.1", "") + createdGoal := testGoal("1.35.1", "etag-created") + createdGoal.MaxPods = 42 + client := &ensureMachineClient{createResult: &Machine{Goal: createdGoal}} task := EnsureMachine(client, &goal, true, slog.New(slog.NewTextHandler(io.Discard, nil))) if err := task.Do(context.Background()); err != nil { @@ -100,9 +99,12 @@ func TestEnsureMachineCreatesAndAdoptsSettingsVersion(t *testing.T) { if goal.SettingsVersion != "etag-created" { t.Fatalf("SettingsVersion = %q, want etag-created", goal.SettingsVersion) } + if goal.MaxPods != 42 { + t.Fatalf("MaxPods = %d, want server-normalized value 42", goal.MaxPods) + } } -func TestEnsureMachineAdoptsExistingSettingsVersionWithoutReplacingLocalGoal(t *testing.T) { +func TestEnsureMachineAdoptsExistingGoal(t *testing.T) { t.Parallel() goal := GoalState{ @@ -137,60 +139,30 @@ func TestEnsureMachineAdoptsExistingSettingsVersionWithoutReplacingLocalGoal(t * if goal.SettingsVersion != "etag-42" { t.Fatalf("SettingsVersion = %q, want etag-42", goal.SettingsVersion) } - 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"} - client := &ensureMachineClient{ - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "etag-old"}}, - createResult: &Machine{Goal: GoalState{ - KubernetesVersion: "1.35.1", - SettingsVersion: "etag-new", - }}, - } + goal := testGoal("1.35.1", "") + remoteGoal := testGoal("1.34.0", "etag-remote") + client := &ensureMachineClient{machine: &Machine{Goal: remoteGoal}} 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.createdGoal.KubernetesVersion != "1.35.1" { - t.Fatalf("Create() goal = %#v", client.createdGoal) - } - if goal.SettingsVersion != "etag-new" { - t.Fatalf("SettingsVersion = %q, want etag-new", goal.SettingsVersion) - } -} - -func TestEnsureMachineRejectsUnchangedRemoteVersionAfterUpdate(t *testing.T) { - t.Parallel() - - 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", - }}, - } - 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 client.createCalls != 0 { + t.Fatalf("Create() calls = %d, want 0", client.createCalls) } - if goal.SettingsVersion != "" { - t.Fatalf("SettingsVersion = %q, want empty before a valid Machine response", goal.SettingsVersion) + if goal.KubernetesVersion != "1.34.0" || goal.SettingsVersion != "etag-remote" { + t.Fatalf("goal = %#v, want remote version and settings version", goal) } } @@ -203,10 +175,10 @@ func TestEnsureMachineRejectsInvalidExistingMachine(t *testing.T) { wantErr string }{ "best effort preserves local goal after missing settings version": { - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}}, + machine: &Machine{Goal: testGoal("1.35.1", "")}, }, "required rejects missing settings version": { - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}}, + machine: &Machine{Goal: testGoal("1.35.1", "")}, require: true, wantErr: "goal settings version is empty", }, @@ -221,7 +193,8 @@ func TestEnsureMachineRejectsInvalidExistingMachine(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - goal := GoalState{KubernetesVersion: "1.35.1", NodeLabels: map[string]string{"source": "local"}} + goal := testGoal("1.35.1", "") + goal.NodeLabels = map[string]string{"source": "local"} client := &ensureMachineClient{machine: tt.machine, getResultSet: true} task := EnsureMachine(client, &goal, tt.require, slog.New(slog.NewTextHandler(io.Discard, nil))) diff --git a/pkg/aksmachine/test_helpers_test.go b/pkg/aksmachine/test_helpers_test.go new file mode 100644 index 00000000..ad0bb254 --- /dev/null +++ b/pkg/aksmachine/test_helpers_test.go @@ -0,0 +1,13 @@ +package aksmachine + +func testGoal(kubernetesVersion, settingsVersion string) GoalState { + return GoalState{ + KubernetesVersion: kubernetesVersion, + SettingsVersion: settingsVersion, + MaxPods: 110, + KubeletConfig: KubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: 80, + }, + } +} diff --git a/pkg/aksmachine/types.go b/pkg/aksmachine/types.go index 35b1f4b8..78e65ebf 100644 --- a/pkg/aksmachine/types.go +++ b/pkg/aksmachine/types.go @@ -43,6 +43,36 @@ func (g GoalState) validate() error { if g.KubeletConfig.ImageGCLowThreshold < 0 { return fmt.Errorf("image GC low threshold must be non-negative") } + if g.KubeletConfig.ImageGCHighThreshold > 100 { + return fmt.Errorf("image GC high threshold must be less than or equal to 100") + } + if g.KubeletConfig.ImageGCLowThreshold > 100 { + return fmt.Errorf("image GC low threshold must be less than or equal to 100") + } + if g.KubeletConfig.ImageGCHighThreshold != 0 && g.KubeletConfig.ImageGCLowThreshold >= g.KubeletConfig.ImageGCHighThreshold { + return fmt.Errorf("image GC low threshold must be less than image GC high threshold") + } + return nil +} + +// Validate verifies the values present in a goal. SettingsVersion is validated +// by Machine because local bootstrap goals do not have an ETag until persisted. +func (g GoalState) Validate() error { + return g.validate() +} + +// ValidateEffective verifies that omitted API defaults have been resolved and +// the goal has every scalar setting needed to render a node. +func (g GoalState) ValidateEffective() error { + if err := g.Validate(); err != nil { + return err + } + if g.MaxPods == 0 { + return fmt.Errorf("max pods is empty") + } + if g.KubeletConfig.ImageGCHighThreshold == 0 { + return fmt.Errorf("image GC high threshold is empty") + } return nil } @@ -59,12 +89,38 @@ func GoalStateFromConfig(cfg *config.Config) (GoalState, error) { ImageGCLowThreshold: cfg.Node.Kubelet.ImageGCLowThreshold, }, } - if err := goal.validate(); err != nil { + if err := goal.ValidateEffective(); err != nil { return GoalState{}, err } return goal, nil } +func cloneGoalState(goal GoalState) GoalState { + cloned := goal + cloned.NodeLabels = maps.Clone(goal.NodeLabels) + cloned.NodeTaints = slices.Clone(goal.NodeTaints) + return cloned +} + +// EffectiveGoal overlays a Machine goal on a complete local goal. AKS owns the +// desired values; the local goal only fills scalar fields omitted by the API. +func EffectiveGoal(machine, local GoalState) (GoalState, error) { + effective := cloneGoalState(machine) + if effective.MaxPods == 0 { + effective.MaxPods = local.MaxPods + } + if effective.KubeletConfig.ImageGCHighThreshold == 0 { + effective.KubeletConfig.ImageGCHighThreshold = local.KubeletConfig.ImageGCHighThreshold + } + if effective.KubeletConfig.ImageGCLowThreshold == 0 { + effective.KubeletConfig.ImageGCLowThreshold = local.KubeletConfig.ImageGCLowThreshold + } + if err := effective.ValidateEffective(); err != nil { + return GoalState{}, fmt.Errorf("validate effective goal: %w", err) + } + return effective, nil +} + type ProvisioningState string const ( @@ -96,7 +152,7 @@ func (m *Machine) Validate() error { if m == nil { return fmt.Errorf("machine is nil") } - if err := m.Goal.validate(); err != nil { + if err := m.Goal.Validate(); err != nil { return fmt.Errorf("goal: %w", err) } if m.Goal.SettingsVersion == "" { diff --git a/pkg/aksmachine/types_test.go b/pkg/aksmachine/types_test.go index b6ab345f..c8aebdf2 100644 --- a/pkg/aksmachine/types_test.go +++ b/pkg/aksmachine/types_test.go @@ -98,11 +98,11 @@ func TestMachineValidate(t *testing.T) { wantErr: "kubernetes version is empty", }, "missing settings version": { - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}}, + machine: &Machine{Goal: testGoal("1.35.1", "")}, wantErr: "goal settings version is empty", }, "complete machine": { - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "42"}}, + machine: &Machine{Goal: testGoal("1.35.1", "42")}, }, } @@ -123,3 +123,39 @@ func TestMachineValidate(t *testing.T) { }) } } + +func TestEffectiveGoal(t *testing.T) { + t.Parallel() + + local := testGoal("1.34.0", "") + local.MaxPods = 30 + local.NodeLabels = map[string]string{"source": "local"} + local.NodeTaints = []string{"local=true:NoSchedule"} + local.KubeletConfig.ImageGCHighThreshold = 90 + local.KubeletConfig.ImageGCLowThreshold = 75 + machine := GoalState{ + KubernetesVersion: "1.35.0", + SettingsVersion: "42", + NodeLabels: map[string]string{}, + NodeTaints: []string{}, + } + + effective, err := EffectiveGoal(machine, local) + if err != nil { + t.Fatalf("EffectiveGoal() error = %v", err) + } + if effective.KubernetesVersion != "1.35.0" || effective.SettingsVersion != "42" || effective.MaxPods != 30 { + t.Fatalf("effective versions/maxPods = %#v", effective) + } + if len(effective.NodeLabels) != 0 || len(effective.NodeTaints) != 0 { + t.Fatalf("effective collections = %#v, want authoritative empty collections", effective) + } + if effective.KubeletConfig.ImageGCHighThreshold != 90 || effective.KubeletConfig.ImageGCLowThreshold != 75 { + t.Fatalf("effective kubelet config = %#v", effective.KubeletConfig) + } + + effective.NodeLabels["source"] = "changed" + if _, ok := machine.NodeLabels["source"]; ok { + t.Fatal("EffectiveGoal returned Machine-owned label map") + } +} diff --git a/pkg/cmd/start/start.go b/pkg/cmd/start/start.go index 23e112c5..deb220e1 100644 --- a/pkg/cmd/start/start.go +++ b/pkg/cmd/start/start.go @@ -76,7 +76,7 @@ func runStart(ctx context.Context, cfg *config.Config, logger *slog.Logger) erro return err } - _, gs, containerImageArchives, err := config.ResolveMachineGoalState(ctx, logger, cfg, machineName) + _, gs, containerImageArchives, err := daemon.ResolveMachineGoalState(ctx, logger, cfg, machineName, &goal) if err != nil { return fmt.Errorf("bootstrap failed to resolve goal state: %w", err) } diff --git a/pkg/daemon/goalstate.go b/pkg/daemon/goalstate.go new file mode 100644 index 00000000..76e9f5b4 --- /dev/null +++ b/pkg/daemon/goalstate.go @@ -0,0 +1,81 @@ +package daemon + +import ( + "context" + "fmt" + "log/slog" + "maps" + "slices" + + agentconfig "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/goalstates" + + "github.com/Azure/AKSFlexNode/pkg/aksmachine" + "github.com/Azure/AKSFlexNode/pkg/config" +) + +// ResolveMachineGoalState overlays AKS Machine-owned settings on the local +// host configuration before resolving the nspawn goal. +func ResolveMachineGoalState( + ctx context.Context, + log *slog.Logger, + cfg *config.Config, + machineName string, + goal *aksmachine.GoalState, +) (*agentconfig.AgentConfig, *goalstates.MachineGoalState, *goalstates.ContainerImageArchiveStaging, error) { + effectiveGoal, err := effectiveMachineGoal(cfg, goal) + if err != nil { + return nil, nil, nil, err + } + resolvedConfig := cfg.DeepCopy() + if resolvedConfig == nil { + return nil, nil, nil, fmt.Errorf("copy config for machine goal") + } + resolvedConfig.Components.Kubernetes = effectiveGoal.KubernetesVersion + resolvedConfig.Node.MaxPods = effectiveGoal.MaxPods + resolvedConfig.Node.Labels = maps.Clone(effectiveGoal.NodeLabels) + resolvedConfig.Node.Taints = slices.Clone(effectiveGoal.NodeTaints) + resolvedConfig.Node.Kubelet.ImageGCHighThreshold = effectiveGoal.KubeletConfig.ImageGCHighThreshold + resolvedConfig.Node.Kubelet.ImageGCLowThreshold = effectiveGoal.KubeletConfig.ImageGCLowThreshold + return config.ResolveMachineGoalState(ctx, log, resolvedConfig, machineName) +} + +func effectiveMachineGoal(cfg *config.Config, goal *aksmachine.GoalState) (*aksmachine.GoalState, error) { + localGoal, err := aksmachine.GoalStateFromConfig(cfg) + if err != nil { + return nil, fmt.Errorf("build local machine goal: %w", err) + } + if goal == nil { + return &localGoal, nil + } + effectiveGoal, err := aksmachine.EffectiveGoal(*goal, localGoal) + if err != nil { + return nil, err + } + return &effectiveGoal, nil +} + +func goalForRestart(cfg *config.Config, state *State) (*aksmachine.GoalState, error) { + if state != nil && state.AppliedGoal != nil { + goal := cloneGoalState(*state.AppliedGoal) + if err := goal.ValidateEffective(); err != nil { + return nil, fmt.Errorf("validate persisted restart goal: %w", err) + } + return goal, nil + } + + goal, err := aksmachine.GoalStateFromConfig(cfg) + if err != nil { + return nil, fmt.Errorf("build restart goal from config: %w", err) + } + if state != nil { + goal.SettingsVersion = state.AppliedSettingsVersion + if state.AppliedKubernetesVersion != "" { + goal.KubernetesVersion = state.AppliedKubernetesVersion + } + } + if err := goal.ValidateEffective(); err != nil { + return nil, fmt.Errorf("validate legacy restart goal: %w", err) + } + return &goal, nil +} diff --git a/pkg/daemon/goalstate_test.go b/pkg/daemon/goalstate_test.go new file mode 100644 index 00000000..c50b4dba --- /dev/null +++ b/pkg/daemon/goalstate_test.go @@ -0,0 +1,102 @@ +package daemon + +import ( + "log/slog" + "maps" + "testing" + + "github.com/Azure/AKSFlexNode/pkg/config" +) + +func TestResolveMachineGoalStateUsesCompleteMachineGoal(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ + Azure: config.AzureConfig{TargetAgentPoolName: "flexnode-edge"}, + Components: config.ComponentsConfig{Kubernetes: "1.34.0"}, + Node: config.NodeConfig{ + MaxPods: 30, + Labels: map[string]string{"source": "config"}, + Taints: []string{"config=true:NoSchedule"}, + Kubelet: config.KubeletConfig{ImageGCHighThreshold: 90, ImageGCLowThreshold: 75}, + }, + } + goal := testMachineGoal("1.35.1", "42") + goal.MaxPods = 50 + goal.NodeLabels = map[string]string{"source": "machine"} + goal.NodeTaints = []string{"machine=true:NoExecute"} + goal.KubeletConfig.ImageGCHighThreshold = 70 + goal.KubeletConfig.ImageGCLowThreshold = 60 + + agentCfg, _, _, err := ResolveMachineGoalState(t.Context(), slog.Default(), cfg, "kube1", &goal) + if err != nil { + t.Fatalf("ResolveMachineGoalState: %v", err) + } + if agentCfg.Cluster.Version != "1.35.1" { + t.Fatalf("Cluster.Version = %q, want 1.35.1", agentCfg.Cluster.Version) + } + if got := agentCfg.Kubelet.Configuration["maxPods"]; got != 50 { + t.Fatalf("maxPods = %v, want 50", got) + } + if got := agentCfg.Kubelet.Configuration["imageGCHighThresholdPercent"]; got != 70 { + t.Fatalf("imageGCHighThresholdPercent = %v, want 70", got) + } + if got := agentCfg.Kubelet.Configuration["imageGCLowThresholdPercent"]; got != 60 { + t.Fatalf("imageGCLowThresholdPercent = %v, want 60", got) + } + if agentCfg.Kubelet.Labels["source"] != "machine" { + t.Fatalf("Kubelet.Labels = %#v, want Machine labels", agentCfg.Kubelet.Labels) + } + if len(agentCfg.Kubelet.RegisterWithTaints) != 1 || agentCfg.Kubelet.RegisterWithTaints[0] != "machine=true:NoExecute" { + t.Fatalf("RegisterWithTaints = %#v", agentCfg.Kubelet.RegisterWithTaints) + } + if cfg.Components.Kubernetes != "1.34.0" || cfg.Node.MaxPods != 30 || cfg.Node.Labels["source"] != "config" { + t.Fatalf("base config was mutated: %#v", cfg) + } +} + +func TestGoalForRestartLegacyStatePreservesConfigSettings(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ + Components: config.ComponentsConfig{Kubernetes: "1.34.0"}, + Node: config.NodeConfig{ + MaxPods: 30, + Labels: map[string]string{"source": "config"}, + Taints: []string{"config=true:NoSchedule"}, + Kubelet: config.KubeletConfig{ImageGCHighThreshold: 90, ImageGCLowThreshold: 75}, + }, + } + state := &State{AppliedSettingsVersion: "42", AppliedKubernetesVersion: "1.35.1"} + + goal, err := goalForRestart(cfg, state) + if err != nil { + t.Fatalf("goalForRestart: %v", err) + } + if goal.KubernetesVersion != "1.35.1" || goal.SettingsVersion != "42" || goal.MaxPods != 30 { + t.Fatalf("goal versions/maxPods = %#v", goal) + } + if !maps.Equal(goal.NodeLabels, cfg.Node.Labels) || len(goal.NodeTaints) != 1 || goal.NodeTaints[0] != cfg.Node.Taints[0] { + t.Fatalf("legacy restart goal lost config settings: %#v", goal) + } + if goal.KubeletConfig.ImageGCHighThreshold != 90 || goal.KubeletConfig.ImageGCLowThreshold != 75 { + t.Fatalf("legacy restart kubelet config = %#v", goal.KubeletConfig) + } +} + +func TestGoalForRestartClonesCompleteGoal(t *testing.T) { + t.Parallel() + + applied := testMachineGoal("1.35.1", "42") + applied.NodeLabels = map[string]string{"source": "machine"} + state := &State{AppliedGoal: &applied} + + goal, err := goalForRestart(&config.Config{}, state) + if err != nil { + t.Fatalf("goalForRestart: %v", err) + } + goal.NodeLabels["source"] = "changed" + if state.AppliedGoal.NodeLabels["source"] != "machine" { + t.Fatal("goalForRestart returned state-owned label map") + } +} diff --git a/pkg/daemon/nodeoperator.go b/pkg/daemon/nodeoperator.go index 0a082cc7..4230142b 100644 --- a/pkg/daemon/nodeoperator.go +++ b/pkg/daemon/nodeoperator.go @@ -38,11 +38,11 @@ func (o *nspawnNodeOperator) RestartNode(ctx context.Context, log *slog.Logger) return err } - cfg := o.cfg.DeepCopy() - if active.State.AppliedKubernetesVersion != "" { - cfg.Components.Kubernetes = active.State.AppliedKubernetesVersion + goal, err := goalForRestart(o.cfg, active.State) + if err != nil { + return err } - _, gs, containerImageArchives, err := config.ResolveMachineGoalState(ctx, log, cfg, active.Name) + _, gs, containerImageArchives, err := ResolveMachineGoalState(ctx, log, o.cfg, active.Name, goal) if err != nil { return fmt.Errorf("resolve goal state for node restart: %w", err) } @@ -52,7 +52,7 @@ func (o *nspawnNodeOperator) RestartNode(ctx context.Context, log *slog.Logger) nodestop.StopNode(log, active.Name), nodestart.StartNode(log, gs.NodeStart), nodestart.WaitForKubelet(log, active.Name), - npd.Start(log, cfg, gs.NodeStart), + npd.Start(log, o.cfg, gs.NodeStart), ).Do(ctx) } @@ -90,20 +90,24 @@ func (o *nspawnNodeOperator) ApplyGoalState(ctx context.Context, log *slog.Logge if err != nil { return nil, err } + effectiveGoal, err := effectiveMachineGoal(cfg, &goal) + if err != nil { + return nil, err + } oldMachine := active.Name newMachine := goalstates.AlternateMachine(oldMachine) log.Info("starting nspawn machine goal-state apply", "oldMachine", oldMachine, "newMachine", newMachine, "settingsVersion", goal.SettingsVersion, - "kubernetesVersion", cfg.Components.Kubernetes, + "kubernetesVersion", effectiveGoal.KubernetesVersion, ) - _, gs, containerImageArchives, err := config.ResolveMachineGoalState(ctx, log, cfg, newMachine) + _, gs, containerImageArchives, err := ResolveMachineGoalState(ctx, log, cfg, newMachine, effectiveGoal) if err != nil { return nil, fmt.Errorf("resolve goal state for repave: %w", err) } - newState := nextAppliedState(active.State, goal, &activeMachine{Name: newMachine}) + newState := nextAppliedState(active.State, *effectiveGoal, &activeMachine{Name: newMachine}) tasks := phases.Serial(log, nodestop.StopNode(log, oldMachine), @@ -144,15 +148,6 @@ func (o *nspawnNodeOperator) configForGoalState(ctx context.Context, log *slog.L cfg.Node.Kubelet.CACertData = data.CACertData } } - // MaxPods is immutable in AKS, so the startup configuration remains its - // authoritative source rather than reapplying the value from each goal. - if goal.KubernetesVersion != "" && cfg.Components.Kubernetes != goal.KubernetesVersion { - log.Info("updated Kubernetes version for repave", - "oldVersion", cfg.Components.Kubernetes, - "newVersion", goal.KubernetesVersion, - ) - cfg.Components.Kubernetes = goal.KubernetesVersion - } return cfg, nil } @@ -166,17 +161,19 @@ func (o *nspawnNodeOperator) StopDaemon(ctx context.Context, log *slog.Logger) e func nextAppliedState(current *State, goal aksmachine.GoalState, active *activeMachine) *State { next := &State{ - AppliedSettingsVersion: goal.SettingsVersion, - AppliedKubernetesVersion: goal.KubernetesVersion, - PreviousSettingsVersion: "", - PreviousKubernetesVersion: "", + AppliedGoal: cloneGoalState(goal), } if current != nil { - next.PreviousSettingsVersion = current.AppliedSettingsVersion - next.PreviousKubernetesVersion = current.AppliedKubernetesVersion + if current.AppliedGoal != nil { + next.PreviousAppliedGoal = cloneGoalState(*current.AppliedGoal) + } else if current.AppliedKubernetesVersion != "" { + next.PreviousSettingsVersion = current.AppliedSettingsVersion + next.PreviousKubernetesVersion = current.AppliedKubernetesVersion + } } if active != nil { next.ActiveMachine = active.Name } + next.populateLegacyFields() return next } diff --git a/pkg/daemon/nodeoperator_test.go b/pkg/daemon/nodeoperator_test.go index c8f3f8a8..f5182138 100644 --- a/pkg/daemon/nodeoperator_test.go +++ b/pkg/daemon/nodeoperator_test.go @@ -24,11 +24,11 @@ func TestFindActiveMachine(t *testing.T) { wantErr bool }{ "kube1": { - state: &State{ActiveMachine: goalstates.NSpawnMachineKube1}, + state: &State{AppliedKubernetesVersion: "1.34.0", ActiveMachine: goalstates.NSpawnMachineKube1}, want: goalstates.NSpawnMachineKube1, }, "kube2": { - state: &State{ActiveMachine: goalstates.NSpawnMachineKube2}, + state: &State{AppliedKubernetesVersion: "1.34.0", ActiveMachine: goalstates.NSpawnMachineKube2}, want: goalstates.NSpawnMachineKube2, }, "missing state": { @@ -104,8 +104,8 @@ func TestConfigForGoalStateRefreshesBootstrapData(t *testing.T) { if got.Node.Kubelet.CACertData != "bmV3" { t.Fatalf("kubelet CA data = %q", got.Node.Kubelet.CACertData) } - if got.Components.Kubernetes != "1.36.2" { - t.Fatalf("Kubernetes version = %q", got.Components.Kubernetes) + if got.Components.Kubernetes != "1.35.0" { + t.Fatalf("base Kubernetes version = %q", got.Components.Kubernetes) } if cfg.Azure.BootstrapToken.Token != "oldtok.0123456789abcdef" { t.Fatal("original config bootstrap token was mutated") @@ -114,9 +114,6 @@ func TestConfigForGoalStateRefreshesBootstrapData(t *testing.T) { "refreshed AKS bootstrap data for repave", "updated bootstrap token for repave", "updated kubelet CA data for repave", - "updated Kubernetes version for repave", - "oldVersion=1.35.0", - "newVersion=1.36.2", } { if !strings.Contains(logs.String(), message) { t.Errorf("logs did not contain %q: %s", message, logs.String()) @@ -272,6 +269,33 @@ func discardLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) } +func TestNextAppliedStateRotatesCompleteGoals(t *testing.T) { + t.Parallel() + + currentGoal := testMachineGoal("1.34.0", "41") + currentGoal.NodeLabels = map[string]string{"source": "old"} + current := &State{AppliedGoal: ¤tGoal, ActiveMachine: goalstates.NSpawnMachineKube1} + nextGoal := testMachineGoal("1.35.0", "42") + nextGoal.NodeLabels = map[string]string{"source": "new"} + + got := nextAppliedState(current, nextGoal, &activeMachine{Name: goalstates.NSpawnMachineKube2}) + if got.AppliedGoal == nil || got.AppliedGoal.SettingsVersion != "42" || got.AppliedGoal.NodeLabels["source"] != "new" { + t.Fatalf("AppliedGoal = %#v", got.AppliedGoal) + } + if got.PreviousAppliedGoal == nil || got.PreviousAppliedGoal.SettingsVersion != "41" || got.PreviousAppliedGoal.NodeLabels["source"] != "old" { + t.Fatalf("PreviousAppliedGoal = %#v", got.PreviousAppliedGoal) + } + if got.AppliedSettingsVersion != "42" || got.PreviousSettingsVersion != "41" || got.ActiveMachine != goalstates.NSpawnMachineKube2 { + t.Fatalf("state = %#v", got) + } + + nextGoal.NodeLabels["source"] = "mutated" + currentGoal.NodeLabels["source"] = "mutated" + if got.AppliedGoal.NodeLabels["source"] != "new" || got.PreviousAppliedGoal.NodeLabels["source"] != "old" { + t.Fatal("nextAppliedState retained caller-owned maps") + } +} + type testStateStore struct { state *State } diff --git a/pkg/daemon/reconcile_test.go b/pkg/daemon/reconcile_test.go index 43090acc..b5e4d4fe 100644 --- a/pkg/daemon/reconcile_test.go +++ b/pkg/daemon/reconcile_test.go @@ -11,10 +11,11 @@ import ( func TestDecide(t *testing.T) { t.Parallel() - goal := aksmachine.GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "42"} + goal := testMachineGoal("1.34.0", "42") machine := machineSnapshot{machine: &aksmachine.Machine{Goal: goal}} - applied := &State{AppliedSettingsVersion: "42", AppliedKubernetesVersion: "1.34.0"} - stale := &State{AppliedSettingsVersion: "41", AppliedKubernetesVersion: "1.33.0"} + applied := &State{AppliedGoal: cloneGoalState(goal)} + staleGoal := testMachineGoal("1.33.0", "41") + stale := &State{AppliedGoal: &staleGoal} node := nodeSnapshot{node: &corev1.Node{}} missingNode := nodeSnapshot{} deleteNode := nodeSnapshot{node: &corev1.Node{Spec: corev1.NodeSpec{Taints: []corev1.Taint{deletionTaint()}}}} diff --git a/pkg/daemon/repave_reconciler.go b/pkg/daemon/repave_reconciler.go index 0b96f167..bf9cbffe 100644 --- a/pkg/daemon/repave_reconciler.go +++ b/pkg/daemon/repave_reconciler.go @@ -223,7 +223,7 @@ func (r *repaveReconciler) applyGoalState(ctx context.Context, state *State, goa _ = r.patchStatus(ctx, aksmachine.ProvisioningStateFailed, stateObservedVersion(state), err.Error()) return err } - return r.patchStatus(ctx, aksmachine.ProvisioningStateSucceeded, newState.AppliedSettingsVersion, "machine goal state applied") + return r.patchStatus(ctx, aksmachine.ProvisioningStateSucceeded, stateObservedVersion(newState), "machine goal state applied") } func (r *repaveReconciler) resetDelete(ctx context.Context) error { @@ -278,10 +278,7 @@ func decide(machine machineSnapshot, node nodeSnapshot, state *State) decision { } func goalApplied(goal aksmachine.GoalState, state *State) bool { - if state == nil { - return false - } - return goal.SettingsVersion != "" && state.AppliedSettingsVersion == goal.SettingsVersion + return goal.SettingsVersion != "" && stateObservedVersion(state) == goal.SettingsVersion } func hasDeletionSignal(taints []corev1.Taint) bool { @@ -297,6 +294,9 @@ func stateObservedVersion(state *State) string { if state == nil { return "" } + if state.AppliedGoal != nil { + return state.AppliedGoal.SettingsVersion + } return state.AppliedSettingsVersion } diff --git a/pkg/daemon/repave_reconciler_test.go b/pkg/daemon/repave_reconciler_test.go index f8323568..88676c10 100644 --- a/pkg/daemon/repave_reconciler_test.go +++ b/pkg/daemon/repave_reconciler_test.go @@ -18,8 +18,17 @@ import ( func TestRepaveReconcilerApplyGoalState(t *testing.T) { t.Parallel() - machines := &fakeMachineClient{machine: &aksmachine.Machine{Goal: aksmachine.GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "42"}}} - operator := &fakeNodeOperator{state: &State{AppliedSettingsVersion: "41", AppliedKubernetesVersion: "1.33.0", ActiveMachine: "kube1"}, newState: &State{AppliedSettingsVersion: "42", AppliedKubernetesVersion: "1.34.0", PreviousSettingsVersion: "41", PreviousKubernetesVersion: "1.33.0", ActiveMachine: "kube2"}} + goal := testMachineGoal("1.34.0", "42") + previousGoal := testMachineGoal("1.33.0", "41") + machines := &fakeMachineClient{machine: &aksmachine.Machine{Goal: goal}} + operator := &fakeNodeOperator{ + state: &State{AppliedGoal: &previousGoal, ActiveMachine: "kube1"}, + newState: &State{ + AppliedGoal: &goal, + PreviousAppliedGoal: &previousGoal, + ActiveMachine: "kube2", + }, + } repaves := newTestRepaveReconciler(t, machines, fakeClient(), operator) if err := repaves.reconcileOnce(context.Background()); err != nil { @@ -28,7 +37,7 @@ func TestRepaveReconcilerApplyGoalState(t *testing.T) { if !operator.applied { t.Fatal("ApplyGoalState was not called") } - if operator.state.AppliedSettingsVersion != "42" || operator.state.PreviousSettingsVersion != "41" || operator.state.ActiveMachine != "kube2" { + if stateObservedVersion(operator.state) != "42" || operator.state.PreviousAppliedGoal == nil || operator.state.PreviousAppliedGoal.SettingsVersion != "41" || operator.state.ActiveMachine != "kube2" { t.Fatalf("state = %#v", operator.state) } if got := machines.status.ProvisioningState; got != aksmachine.ProvisioningStateSucceeded { diff --git a/pkg/daemon/state.go b/pkg/daemon/state.go index 3c16a4fc..28e3cbf0 100644 --- a/pkg/daemon/state.go +++ b/pkg/daemon/state.go @@ -7,8 +7,10 @@ import ( "encoding/json" "errors" "fmt" + "maps" "os" "path/filepath" + "slices" "strings" "github.com/Azure/AKSFlexNode/pkg/aksmachine" @@ -23,14 +25,40 @@ const ( stateFileName = "daemon-state.json" ) -// State records the last safely applied AKS machine goal and the previous -// known-good goal needed for rollback-oriented reconciliation. +// State records the current and previous safely applied AKS Machine goals and +// the active nspawn machine. type State struct { + AppliedGoal *aksmachine.GoalState `json:"appliedGoal,omitempty"` + PreviousAppliedGoal *aksmachine.GoalState `json:"previousAppliedGoal,omitempty"` + + // Deprecated: these projections keep state readable by older agent binaries. + // AppliedGoal and PreviousAppliedGoal are authoritative when present. AppliedSettingsVersion string `json:"appliedSettingsVersion,omitempty"` AppliedKubernetesVersion string `json:"appliedKubernetesVersion,omitempty"` PreviousSettingsVersion string `json:"previousSettingsVersion,omitempty"` PreviousKubernetesVersion string `json:"previousKubernetesVersion,omitempty"` - ActiveMachine string `json:"activeMachine,omitempty"` + + ActiveMachine string `json:"activeMachine,omitempty"` +} + +func (s *State) validate() error { + if s == nil { + return fmt.Errorf("daemon state is nil") + } + if s.AppliedGoal == nil && s.AppliedKubernetesVersion == "" { + return fmt.Errorf("daemon state applied goal is missing") + } + if s.AppliedGoal != nil { + if err := s.AppliedGoal.ValidateEffective(); err != nil { + return fmt.Errorf("daemon state applied goal: %w", err) + } + } + if s.PreviousAppliedGoal != nil { + if err := s.PreviousAppliedGoal.ValidateEffective(); err != nil { + return fmt.Errorf("daemon state previous applied goal: %w", err) + } + } + return nil } type saveStateTask struct { @@ -58,10 +86,26 @@ func (t *saveStateTask) Do(ctx context.Context) error { } func SeededState(goal aksmachine.GoalState) *State { - return &State{ - AppliedSettingsVersion: goal.SettingsVersion, - AppliedKubernetesVersion: goal.KubernetesVersion, - ActiveMachine: goalstates.NSpawnMachineKube1, + state := &State{AppliedGoal: cloneGoalState(goal), ActiveMachine: goalstates.NSpawnMachineKube1} + state.populateLegacyFields() + return state +} + +func cloneGoalState(goal aksmachine.GoalState) *aksmachine.GoalState { + cloned := goal + cloned.NodeLabels = maps.Clone(goal.NodeLabels) + cloned.NodeTaints = slices.Clone(goal.NodeTaints) + return &cloned +} + +func (s *State) populateLegacyFields() { + if s.AppliedGoal != nil { + s.AppliedSettingsVersion = s.AppliedGoal.SettingsVersion + s.AppliedKubernetesVersion = s.AppliedGoal.KubernetesVersion + } + if s.PreviousAppliedGoal != nil { + s.PreviousSettingsVersion = s.PreviousAppliedGoal.SettingsVersion + s.PreviousKubernetesVersion = s.PreviousAppliedGoal.KubernetesVersion } } @@ -77,6 +121,9 @@ func activeMachineFromStore(ctx context.Context, store stateStore) (*activeMachi if state == nil { return nil, fmt.Errorf("daemon state is missing active machine") } + if err := state.validate(); err != nil { + return nil, err + } if !validActiveMachine(state.ActiveMachine) { return nil, fmt.Errorf("daemon state active machine %q is invalid", state.ActiveMachine) } @@ -125,14 +172,20 @@ func (s *fileStateStore) Load(context.Context) (*State, error) { if err := json.Unmarshal(data, &state); err != nil { return nil, fmt.Errorf("decode daemon state %s: %w", s.path, err) } + if err := state.validate(); err != nil { + return nil, fmt.Errorf("validate daemon state %s: %w", s.path, err) + } + state.populateLegacyFields() return &state, nil } func (s *fileStateStore) Save(_ context.Context, state *State) error { - if state == nil { - return fmt.Errorf("daemon state is nil") + if err := state.validate(); err != nil { + return err } - data, err := json.MarshalIndent(state, "", " ") + stateForPersistence := *state + stateForPersistence.populateLegacyFields() + data, err := json.MarshalIndent(&stateForPersistence, "", " ") if err != nil { return fmt.Errorf("marshal daemon state: %w", err) } diff --git a/pkg/daemon/state_test.go b/pkg/daemon/state_test.go index 620b221a..7259be08 100644 --- a/pkg/daemon/state_test.go +++ b/pkg/daemon/state_test.go @@ -2,12 +2,11 @@ package daemon import ( "context" + "encoding/json" "os" "path/filepath" "strings" "testing" - - "github.com/Azure/AKSFlexNode/pkg/aksmachine" ) func TestFileStateStoreSaveLoad(t *testing.T) { @@ -18,12 +17,12 @@ func TestFileStateStoreSaveLoad(t *testing.T) { t.Fatalf("newFileStateStore: %v", err) } want := &State{ - AppliedSettingsVersion: "42", - AppliedKubernetesVersion: "1.34.0", - PreviousSettingsVersion: "41", - PreviousKubernetesVersion: "1.33.0", - ActiveMachine: "kube2", + AppliedGoal: cloneGoalState(testMachineGoal("1.34.0", "42")), + PreviousAppliedGoal: cloneGoalState(testMachineGoal("1.33.0", "41")), + ActiveMachine: "kube2", } + want.AppliedGoal.NodeLabels = map[string]string{"workload": "flex"} + want.AppliedGoal.NodeTaints = []string{"dedicated=flex:NoSchedule"} if err := store.Save(context.Background(), want); err != nil { t.Fatalf("Save: %v", err) @@ -32,9 +31,22 @@ func TestFileStateStoreSaveLoad(t *testing.T) { if err != nil { t.Fatalf("Load: %v", err) } - if got.AppliedSettingsVersion != want.AppliedSettingsVersion || got.ActiveMachine != want.ActiveMachine { + if got.ActiveMachine != want.ActiveMachine || got.AppliedGoal == nil || got.AppliedGoal.NodeLabels["workload"] != "flex" || + got.PreviousAppliedGoal == nil || got.PreviousAppliedGoal.SettingsVersion != "41" || + got.AppliedSettingsVersion != "42" || got.AppliedKubernetesVersion != "1.34.0" { t.Fatalf("state = %#v, want %#v", got, want) } + persistedData, err := os.ReadFile(store.path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + var persisted State + if err := json.Unmarshal(persistedData, &persisted); err != nil { + t.Fatalf("Unmarshal persisted state: %v", err) + } + if persisted.AppliedSettingsVersion != "42" || persisted.PreviousSettingsVersion != "41" { + t.Fatalf("legacy state projections = %#v", persisted) + } } func TestFileStateStoreLoadMissing(t *testing.T) { @@ -53,6 +65,80 @@ func TestFileStateStoreLoadMissing(t *testing.T) { } } +func TestFileStateStoreLoadCompatibility(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + data string + check func(*testing.T, *State) + }{ + "legacy state remains partial": { + data: `{ + "appliedSettingsVersion":"42", + "appliedKubernetesVersion":"1.34.0", + "previousSettingsVersion":"41", + "previousKubernetesVersion":"1.33.0", + "activeMachine":"kube1" + }`, + check: func(t *testing.T, state *State) { + t.Helper() + if state.AppliedGoal != nil || state.PreviousAppliedGoal != nil { + t.Fatalf("legacy goals were fabricated: %#v", state) + } + if state.AppliedSettingsVersion != "42" || state.AppliedKubernetesVersion != "1.34.0" { + t.Fatalf("legacy projections = %#v", state) + } + }, + }, + "complete goals override stale projections": { + data: `{ + "appliedGoal":{"kubernetesVersion":"1.34.0","settingsVersion":"42","maxPods":110,"kubeletConfig":{"imageGCHighThreshold":85,"imageGCLowThreshold":80}}, + "previousAppliedGoal":{"kubernetesVersion":"1.33.0","settingsVersion":"41","maxPods":110,"kubeletConfig":{"imageGCHighThreshold":85,"imageGCLowThreshold":80}}, + "appliedSettingsVersion":"stale", + "appliedKubernetesVersion":"1.99.0", + "previousSettingsVersion":"stale", + "previousKubernetesVersion":"1.98.0", + "activeMachine":"kube2" + }`, + check: func(t *testing.T, state *State) { + t.Helper() + if state.AppliedGoal == nil || state.AppliedGoal.SettingsVersion != "42" || state.PreviousAppliedGoal == nil { + t.Fatalf("complete goals = %#v", state) + } + if state.AppliedSettingsVersion != "42" || state.AppliedKubernetesVersion != "1.34.0" || + state.PreviousSettingsVersion != "41" || state.PreviousKubernetesVersion != "1.33.0" { + t.Fatalf("legacy projections were not corrected: %#v", state) + } + }, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "state.json") + store, err := newFileStateStore(path) + if err != nil { + t.Fatalf("newFileStateStore: %v", err) + } + data := []byte(tt.data) + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if err := os.WriteFile(path+".sha256", []byte(checksum(data)+"\n"), 0o600); err != nil { + t.Fatalf("WriteFile checksum: %v", err) + } + + state, err := store.Load(t.Context()) + if err != nil { + t.Fatalf("Load: %v", err) + } + tt.check(t, state) + }) + } +} + func TestFileStateStoreChecksumMismatch(t *testing.T) { t.Parallel() @@ -61,7 +147,7 @@ func TestFileStateStoreChecksumMismatch(t *testing.T) { if err != nil { t.Fatalf("newFileStateStore: %v", err) } - if err := store.Save(context.Background(), &State{AppliedSettingsVersion: "42"}); err != nil { + if err := store.Save(context.Background(), &State{AppliedSettingsVersion: "42", AppliedKubernetesVersion: "1.34.0"}); err != nil { t.Fatalf("Save: %v", err) } if err := os.WriteFile(path, []byte(`{"appliedSettingsVersion":"43"}`), 0o600); err != nil { @@ -102,7 +188,7 @@ func TestFileStateStoreDelete(t *testing.T) { if err != nil { t.Fatalf("newFileStateStore: %v", err) } - if err := store.Save(context.Background(), &State{AppliedSettingsVersion: "42"}); err != nil { + if err := store.Save(context.Background(), &State{AppliedSettingsVersion: "42", AppliedKubernetesVersion: "1.34.0"}); err != nil { t.Fatalf("Save: %v", err) } if err := store.Delete(context.Background()); err != nil { @@ -119,7 +205,9 @@ func TestFileStateStoreDelete(t *testing.T) { func TestSeededState(t *testing.T) { t.Parallel() - state := SeededState(aksmachine.GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "42"}) + goal := testMachineGoal("1.34.0", "42") + goal.NodeLabels = map[string]string{"workload": "flex"} + state := SeededState(goal) if state.AppliedSettingsVersion != "42" { t.Fatalf("AppliedSettingsVersion = %q, want 42", state.AppliedSettingsVersion) } @@ -132,6 +220,13 @@ func TestSeededState(t *testing.T) { if state.PreviousSettingsVersion != "" || state.PreviousKubernetesVersion != "" { t.Fatalf("previous state = %#v, want empty", state) } + if state.AppliedGoal == nil || state.AppliedGoal.NodeLabels["workload"] != "flex" { + t.Fatalf("AppliedGoal = %#v", state.AppliedGoal) + } + goal.NodeLabels["workload"] = "changed" + if state.AppliedGoal.NodeLabels["workload"] != "flex" { + t.Fatal("SeededState retained caller-owned label map") + } } func TestSaveStateValidation(t *testing.T) { @@ -161,11 +256,11 @@ func TestActiveMachineFromStore(t *testing.T) { wantErr bool }{ "kube1": { - state: &State{ActiveMachine: "kube1"}, + state: &State{AppliedKubernetesVersion: "1.34.0", ActiveMachine: "kube1"}, want: "kube1", }, "kube2": { - state: &State{ActiveMachine: "kube2"}, + state: &State{AppliedKubernetesVersion: "1.34.0", ActiveMachine: "kube2"}, want: "kube2", }, "missing state": { diff --git a/pkg/daemon/test_helpers_test.go b/pkg/daemon/test_helpers_test.go new file mode 100644 index 00000000..8ba5b7fc --- /dev/null +++ b/pkg/daemon/test_helpers_test.go @@ -0,0 +1,15 @@ +package daemon + +import "github.com/Azure/AKSFlexNode/pkg/aksmachine" + +func testMachineGoal(kubernetesVersion, settingsVersion string) aksmachine.GoalState { + return aksmachine.GoalState{ + KubernetesVersion: kubernetesVersion, + SettingsVersion: settingsVersion, + MaxPods: 110, + KubeletConfig: aksmachine.KubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: 80, + }, + } +} From 3c0aa0397c81c66cd190332a2b9dff7ead3f7cea Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Mon, 17 Aug 2026 10:03:50 -0700 Subject: [PATCH 2/7] Align E2E Machine max pods with node config --- hack/e2e/lib/controller.sh | 13 +++++++------ hack/e2e/lib/node-join-token.sh | 2 +- hack/e2e/lib/upgrade-drift.sh | 8 ++++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/hack/e2e/lib/controller.sh b/hack/e2e/lib/controller.sh index 67175fb1..39af3ce8 100644 --- a/hack/e2e/lib/controller.sh +++ b/hack/e2e/lib/controller.sh @@ -332,7 +332,7 @@ ensure_flex_controller() { } _render_machine_json() { - local node_name="$1" kubernetes_version="$2" settings_version="$3" + local node_name="$1" kubernetes_version="$2" settings_version="$3" max_pods="$4" local cluster_id machine_id cluster_id="$(state_get cluster_id)" machine_id="${cluster_id}/agentPools/${E2E_TARGET_AGENT_POOL_NAME}/machines/${node_name}" @@ -342,6 +342,7 @@ _render_machine_json() { --arg name "${node_name}" \ --arg kubernetesVersion "${kubernetes_version}" \ --arg eTag "${settings_version}" \ + --argjson maxPods "${max_pods}" \ '{ id: $id, name: $name, @@ -351,7 +352,7 @@ _render_machine_json() { provisioningState: "Succeeded", kubernetes: { orchestratorVersion: $kubernetesVersion, - maxPods: 110, + maxPods: $maxPods, nodeLabels: {}, nodeTaints: [], kubeletConfig: { @@ -364,11 +365,11 @@ _render_machine_json() { } _machine_configmap_upsert_unlocked() { - local node_name="$1" kubernetes_version="$2" settings_version="$3" + local node_name="$1" kubernetes_version="$2" settings_version="$3" max_pods="$4" local machine_file patch machine_file="${E2E_WORK_DIR}/machine-${node_name}.json" - _render_machine_json "${node_name}" "${kubernetes_version}" "${settings_version}" > "${machine_file}" + _render_machine_json "${node_name}" "${kubernetes_version}" "${settings_version}" "${max_pods}" > "${machine_file}" if ! kubectl -n "${E2E_CONTROLLER_NAMESPACE}" get configmap "${E2E_MACHINE_CONFIGMAP}" >/dev/null 2>&1; then kubectl -n "${E2E_CONTROLLER_NAMESPACE}" create configmap "${E2E_MACHINE_CONFIGMAP}" >/dev/null fi @@ -379,8 +380,8 @@ _machine_configmap_upsert_unlocked() { } machine_configmap_upsert() { - local node_name="$1" kubernetes_version="${2:-${E2E_KUBERNETES_VERSION}}" settings_version="${3:-${kubernetes_version}}" - with_cluster_lock _machine_configmap_upsert_unlocked "${node_name}" "${kubernetes_version}" "${settings_version}" + local node_name="$1" kubernetes_version="${2:-${E2E_KUBERNETES_VERSION}}" settings_version="${3:-${kubernetes_version}}" max_pods="${4:-110}" + with_cluster_lock _machine_configmap_upsert_unlocked "${node_name}" "${kubernetes_version}" "${settings_version}" "${max_pods}" } _machine_configmap_delete_unlocked() { diff --git a/hack/e2e/lib/node-join-token.sh b/hack/e2e/lib/node-join-token.sh index 5414d2e1..a35ddd15 100644 --- a/hack/e2e/lib/node-join-token.sh +++ b/hack/e2e/lib/node-join-token.sh @@ -90,7 +90,7 @@ node_join_token() { mv "${config_file}.tmp" "${config_file}" # Step 3: Publish the AKS Machine goal and deploy the agent. - machine_configmap_upsert "$(state_get token_vm_name)" "${E2E_KUBERNETES_VERSION}" "${E2E_KUBERNETES_VERSION}" + machine_configmap_upsert "$(state_get token_vm_name)" "${E2E_KUBERNETES_VERSION}" "${E2E_KUBERNETES_VERSION}" "${E2E_KUBELET_MAX_PODS}" _deploy_and_start_agent "${vm_ip}" "${config_file}" "aks-flex-node-token" log_success "Token node joined in $(timer_elapsed "${start}")s" diff --git a/hack/e2e/lib/upgrade-drift.sh b/hack/e2e/lib/upgrade-drift.sh index dd7686ab..c7727eea 100644 --- a/hack/e2e/lib/upgrade-drift.sh +++ b/hack/e2e/lib/upgrade-drift.sh @@ -102,12 +102,16 @@ _ensure_mode_joined() { _trigger_mode_repave() { local mode="$1" desired_version="$2" settings_version="$3" - local vm_ip vm_name + local vm_ip vm_name max_pods vm_ip="$(_mode_vm_ip "${mode}")" vm_name="$(_mode_vm_name "${mode}")" + max_pods="110" + if [[ "${mode}" == "token" ]]; then + max_pods="${E2E_KUBELET_MAX_PODS}" + fi log_info "Updating controller machine goal for ${mode} node to Kubernetes ${desired_version} (${settings_version})" - machine_configmap_upsert "${vm_name}" "${desired_version}" "${settings_version}" + machine_configmap_upsert "${vm_name}" "${desired_version}" "${settings_version}" "${max_pods}" remote_exec "${vm_ip}" 'sudo systemctl status aks-flex-node-agent.service --no-pager -l || true' log_info "Deleting Kubernetes Node ${vm_name} to trigger ${mode} repave" From da95e42d6d157aad9313d4026a5a81adad173442 Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Mon, 17 Aug 2026 12:57:09 -0700 Subject: [PATCH 3/7] Document westcentralus FlexNode E2E validation --- ...exnode-pr277-westcentralus-e2e-20260817.md | 945 ++++++++++++++++++ 1 file changed, 945 insertions(+) create mode 100644 reports/flexnode-pr277-westcentralus-e2e-20260817.md diff --git a/reports/flexnode-pr277-westcentralus-e2e-20260817.md b/reports/flexnode-pr277-westcentralus-e2e-20260817.md new file mode 100644 index 00000000..acede251 --- /dev/null +++ b/reports/flexnode-pr277-westcentralus-e2e-20260817.md @@ -0,0 +1,945 @@ +# FlexNode PR #277 West Central US E2E Report + +## Status + +- Overall result: **PASS WITH RP CUSTOM-LABEL ISOLATION FINDING** +- Current phase: complete +- Started: `2026-08-17T18:53:51Z` +- Last updated: `2026-08-17T20:02:00Z` + +## Scope + +- Subscription: `8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8` +- Region: `westcentralus` +- Target regional AKS RP release: `v20260807` +- Agent changes: + - PR [#275](https://github.com/Azure/AKSFlexNode/pull/275): AKS-owned node labels + - PR [#276](https://github.com/Azure/AKSFlexNode/pull/276): strict Machine goal responses + - PR [#277](https://github.com/Azure/AKSFlexNode/pull/277): authoritative Machine goals +- Agent branch: `wenx/authoritative-machine-goals` +- Agent commit: `3c0aa0397c81c66cd190332a2b9dff7ead3f7cea` +- Test host: real Azure VM with system-assigned managed identity +- Control path: Azure CLI `aks-preview` FlexNodes commands + +## Test Objectives + +1. Create an AKS cluster and FlexNodes pool using Azure CLI. +2. Pre-create an RP Machine whose goal deliberately differs from pool bootstrap/local config. +3. Start the PR #277 agent on a real Azure VM and prove the existing RP Machine goal is authoritative. +4. Verify Machine custom labels contain only customer labels and exclude these AKS-owned labels: + - `kubernetes.azure.com/managed` + - `kubernetes.azure.com/agentpool` + - `kubernetes.azure.com/mode` + - `kubernetes.azure.com/nodepool-type` +5. Verify the Kubernetes Node contains the four AKS-owned labels plus the Machine custom labels. +6. Verify the effective Machine goal controls Kubernetes version, `maxPods`, labels, taints, and persisted daemon state. +7. Restart the nspawn node/agent and prove the authoritative applied goal survives restart. + +## Test Constraints + +- This test does not validate workload networking. The external VM is not provisioned by the AKS RP and may not receive a production CNI configuration. +- Secret responses such as bootstrap tokens and CA data are never written to this report. +- Commands are recorded with non-secret identifiers. Sensitive response fields are reduced to booleans or redacted summaries. + +## Execution Log + +### Step 1: Verify Local Source and Tooling + +- Result: **PASS** + +Command: + +```bash +git status --short --branch +git log --oneline --decorate --max-count=8 +git rev-parse HEAD +``` + +Response: + +```text +## wenx/authoritative-machine-goals...origin/wenx/authoritative-machine-goals +3c0aa03 Align E2E Machine max pods with node config +44b711f Adopt authoritative AKS Machine goals +072fe11 Validate AKS Machine goal responses (#276) +981f1cd Add AKS-owned labels to Flex Nodes (#275) +3c0aa0397c81c66cd190332a2b9dff7ead3f7cea +``` + +Command: + +```bash +az version +``` + +Response, relevant fields: + +```json +{ + "azure-cli": "2.86.0", + "extensions": { + "aks-preview": "22.0.0b1" + } +} +``` + +### Step 2: Verify Subscription and Preview Features + +- Result: **PASS** + +Command: + +```bash +az account show \ + --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --query '{id:id,name:name,state:state,tenantId:tenantId,userType:user.type}' \ + -o json +``` + +Response: + +```json +{ + "id": "8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8", + "name": "Azure Container Service - Test (AKS Standalone)", + "state": "Enabled", + "tenantId": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "userType": "user" +} +``` + +Commands: + +```bash +az feature show --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --namespace Microsoft.ContainerService --name AKSFlexNodePreview \ + --query '{name:name,state:properties.state}' -o json + +az feature show --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --namespace Microsoft.ContainerService --name PutMachinePreview \ + --query '{name:name,state:properties.state}' -o json +``` + +Responses: + +```json +{"name":"Microsoft.ContainerService/AKSFlexNodePreview","state":"Registered"} +{"name":"Microsoft.ContainerService/PutMachinePreview","state":"Registered"} +``` + +### Step 3: Verify Regional Kubernetes and VM Capacity + +- Result: **PASS** + +Command: + +```bash +az aks get-versions \ + --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --location westcentralus \ + --query 'values[*].patchVersions.keys(@)[]' -o json +``` + +Response summary: + +```text +Available versions include 1.35.7 and 1.36.3. +Selected test version: 1.35.7. +``` + +Commands: + +```bash +az vm list-skus --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --location westcentralus --size Standard_D4s_v5 \ + --resource-type virtualMachines -o json + +az vm list-skus --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --location westcentralus --size Standard_D2s_v5 \ + --resource-type virtualMachines -o json + +az vm list-usage --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --location westcentralus -o json +``` + +Response summary: + +```text +Standard_D4s_v5: unrestricted, 4 vCPU, 16 GiB +Standard_D2s_v5: unrestricted, 2 vCPU, 8 GiB +Standard DSv5 quota: 100 vCPU available +Regional total quota: 2300 vCPU available +``` + +### Step 4: Verify Azure CLI FlexNodes Command Surface + +- Result: **PASS** + +Commands: + +```bash +az aks nodepool add -h +az aks machine add -h +az aks nodepool get-bootstrap-data -h +``` + +Response summary: + +```text +az aks nodepool add supports --vm-set-type FlexNodes. +az aks machine add supports machine name, Kubernetes version, max pods, labels, and taints. +az aks nodepool get-bootstrap-data is available from aks-preview. +``` + +### Step 5: Build the PR Agent Artifact + +- Result: **PASS** +- Release publication decision: no public alpha release was needed; the exact PR binary will be copied to the test VM. + +Command: + +```bash +BUILD_DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" +GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build \ + -ldflags "-X github.com/Azure/AKSFlexNode/pkg/cmd/version.Version=v0.1.7-alpha.277 \ + -X github.com/Azure/AKSFlexNode/pkg/cmd/version.GitCommit=3c0aa0397c81c66cd190332a2b9dff7ead3f7cea \ + -X github.com/Azure/AKSFlexNode/pkg/cmd/version.BuildTime=${BUILD_DATE} -w -s" \ + -o /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64 \ + ./cmd/aks-flex-node + +tar -C /tmp/opencode/fn277-wcu-artifacts -czf \ + /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64.tar.gz \ + aks-flex-node-linux-amd64 + +sha256sum /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64.tar.gz +/tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64 version +``` + +Response: + +```text +47909047b39973069cb9ecc5f60cfae83429deb5b8a8df42bf605540ad759bfe aks-flex-node-linux-amd64.tar.gz +AKS Flex Node Agent +Version: v0.1.7-alpha.277 +Git Commit: 3c0aa0397c81c66cd190332a2b9dff7ead3f7cea +Build Time: 2026-08-17T19:05:31Z +``` + +### Step 6: Create the Azure Resource Group and Network + +- Result: **PASS** + +Commands: + +```bash +az account set --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 + +az group create --name fn277-wcu-20260817 --location westcentralus \ + --tags purpose=flexnode-pr277-e2e owner=wenxuan pr=277 \ + agentCommit=3c0aa03 rpRelease=v20260807 + +az network vnet create -g fn277-wcu-20260817 -n fn277-vnet \ + --location westcentralus --address-prefixes 10.247.0.0/16 \ + --subnet-name aks-subnet --subnet-prefixes 10.247.0.0/22 + +az network nsg create -g fn277-wcu-20260817 -n fn277-vm-nsg \ + --location westcentralus + +az network vnet subnet create -g fn277-wcu-20260817 \ + --vnet-name fn277-vnet --name flex-subnet \ + --address-prefixes 10.247.4.0/24 --network-security-group fn277-vm-nsg +``` + +Response summary: + +```text +Resource group provisioning: Succeeded +VNet: 10.247.0.0/16 +AKS subnet: 10.247.0.0/22 +Flex VM subnet: 10.247.4.0/24 +``` + +### Step 7: Create the AKS Cluster + +- Result: **PASS** + +Command: + +```bash +az aks create \ + --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --resource-group fn277-wcu-20260817 \ + --name fn277wcu \ + --location westcentralus \ + --kubernetes-version 1.35.7 \ + --nodepool-name systempool \ + --node-count 1 \ + --node-vm-size Standard_D2s_v5 \ + --network-plugin azure \ + --network-plugin-mode overlay \ + --vnet-subnet-id \ + --service-cidr 10.248.0.0/16 \ + --dns-service-ip 10.248.0.10 \ + --enable-managed-identity \ + --ssh-key-value ~/.ssh/id_rsa.pub +``` + +Response, relevant fields: + +```json +{ + "currentVersion": "1.35.7", + "location": "westcentralus", + "name": "fn277wcu", + "networkPlugin": "azure", + "networkPluginMode": "overlay", + "state": "Succeeded", + "systemPool": { + "count": 1, + "name": "systempool", + "state": "Succeeded", + "vmSize": "Standard_D2s_v5" + } +} +``` + +### Step 8: Create the FlexNodes Pool and Inspect Bootstrap Data + +- Result: **PASS** + +Command: + +```bash +az aks nodepool add \ + --resource-group fn277-wcu-20260817 \ + --cluster-name fn277wcu \ + --name flexpool \ + --vm-set-type FlexNodes \ + --mode User \ + --kubernetes-version 1.35.7 \ + --max-pods 75 \ + --labels pool-source=bootstrap-stale remove-after-bootstrap=true \ + --node-taints pool-source=bootstrap-stale:NoSchedule \ + --max-unavailable 30% +``` + +Response, relevant fields: + +```json +{ + "currentVersion": "1.35.7", + "labels": { + "pool-source": "bootstrap-stale", + "remove-after-bootstrap": "true" + }, + "maxPods": 75, + "mode": "User", + "name": "flexpool", + "state": "Succeeded", + "taints": ["pool-source=bootstrap-stale:NoSchedule"], + "type": "FlexNodes" +} +``` + +Command: + +```bash +az aks nodepool get-bootstrap-data \ + -g fn277-wcu-20260817 --cluster-name fn277wcu -n flexpool \ + --query '{targetPool:azure.targetAgentPoolName,kubernetes:components.kubernetes,maxPods:node.maxPods,labels:node.labels,taints:node.taints,hasToken:length(azure.bootstrapToken.token) > `0`,hasCACert:length(node.kubelet.caCertData) > `0`}' \ + -o json +``` + +Sanitized response: + +```json +{ + "hasCACert": true, + "hasToken": true, + "kubernetes": "1.35.7", + "labels": { + "pool-source": "bootstrap-stale", + "remove-after-bootstrap": "true" + }, + "maxPods": 75, + "taints": ["pool-source=bootstrap-stale:NoSchedule"], + "targetPool": "flexpool" +} +``` + +Observation: pool bootstrap data contains only the configured customer labels. It does not contain the four AKS-owned labels. + +### Step 9: Pre-create the Authoritative RP Machine + +- Result: **PASS WITH RP RESPONSE OBSERVATION** + +Command: + +```bash +az aks machine add \ + -g fn277-wcu-20260817 \ + --cluster-name fn277wcu \ + --nodepool-name flexpool \ + --machine-name fn277vm \ + --kubernetes-version 1.35.7 \ + --max-pods 61 \ + --labels source=machine-authoritative machine-only=true \ + --node-taints source=machine-authoritative:NoSchedule +``` + +Response, relevant fields: + +```json +{ + "name": "fn277vm", + "properties": { + "eTag": "91f22a47-dee8-4ab4-ba67-337abdb82b76", + "kubernetes": { + "currentOrchestratorVersion": "1.35.7", + "maxPods": 61, + "nodeLabels": { + "kubernetes.azure.com/managed": "false", + "machine-only": "true", + "source": "machine-authoritative" + }, + "nodeName": "fn277vm", + "nodeTaints": ["source=machine-authoritative:NoSchedule"], + "orchestratorVersion": "1.35.7" + }, + "provisioningState": "Succeeded" + } +} +``` + +Observation: the CLI request contained only `source` and `machine-only`; the RP response added `kubernetes.azure.com/managed=false`. `machine show` and `machine list` returned the same expanded label map. The remaining three AKS-owned labels were not present in the ARM Machine response. + +The Kubernetes Node did not exist before VM bootstrap: + +```text +Error from server (NotFound): nodes "fn277vm" not found +``` + +### Step 10: Create the Real Azure VM and Assign AKS Access + +- Result: **PASS** + +Commands: + +```bash +az network nsg rule create -g fn277-wcu-20260817 \ + --nsg-name fn277-vm-nsg --name AllowSSHFromOperator \ + --priority 100 --direction Inbound --access Allow --protocol Tcp \ + --source-address-prefixes 67.168.38.253/32 \ + --destination-port-ranges 22 + +az vm create \ + -g fn277-wcu-20260817 -n fn277vm --location westcentralus \ + --image Ubuntu2404 --size Standard_D4s_v5 \ + --admin-username azureuser --ssh-key-values ~/.ssh/id_rsa.pub \ + --vnet-name fn277-vnet --subnet flex-subnet --nsg "" \ + --assign-identity --public-ip-sku Standard --os-disk-size-gb 64 \ + --security-type TrustedLaunch +``` + +Response summary: + +```text +VM private IP: 10.247.4.4 +VM public IP: 20.168.179.165 +VM state: running +System-assigned principal: a7183dd9-4799-4b8b-b09a-10889758d431 +``` + +Command: + +```bash +az role assignment create \ + --assignee-object-id a7183dd9-4799-4b8b-b09a-10889758d431 \ + --assignee-principal-type ServicePrincipal \ + --role "Azure Kubernetes Service Contributor Role" \ + --scope +``` + +Response summary: + +```text +Role assignment ID: d03807f8-20da-4369-a646-8dd218d41122 +Scope: AKS cluster fn277wcu +``` + +### Step 11: Stage and Verify the PR Agent on the VM + +- Result: **PASS** + +Commands: + +```bash +scp -i ~/.ssh/id_rsa \ + /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64.tar.gz \ + azureuser@20.168.179.165:/tmp/aks-flex-node-linux-amd64.tar.gz + +scp -i ~/.ssh/id_rsa scripts/bootstrap.sh \ + azureuser@20.168.179.165:/tmp/bootstrap.sh + +ssh -i ~/.ssh/id_rsa azureuser@20.168.179.165 \ + 'sha256sum /tmp/aks-flex-node-linux-amd64.tar.gz; bash -n /tmp/bootstrap.sh' + +ssh -i ~/.ssh/id_rsa azureuser@20.168.179.165 \ + 'curl -fsS -H Metadata:true "http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fmanagement.azure.com%2F" | jq -r '"'"'if (.access_token | length) > 0 then "token-acquired" else "missing-token" end'"'"'' +``` + +Response: + +```text +47909047b39973069cb9ecc5f60cfae83429deb5b8a8df42bf605540ad759bfe /tmp/aks-flex-node-linux-amd64.tar.gz +token-acquired +``` + +An initial artifact-verification command ran concurrently with the copy and observed the files before transfer completion. The serial retry above passed; no bootstrap mutation had started. + +### Step 12: Bootstrap with Deliberately Conflicting Local Settings + +- Result: **PASS** + +Command, non-secret form: + +```bash +sudo bash /tmp/bootstrap.sh \ + --auth msi \ + --fetch-bootstrap-data \ + --cluster-resource-id \ + --agent-pool-name flexpool \ + --agent-url file:///tmp/aks-flex-node-linux-amd64.tar.gz \ + --agent-sha256 47909047b39973069cb9ecc5f60cfae83429deb5b8a8df42bf605540ad759bfe \ + --config-overrides '{ + "agent":{"nodeName":"fn277vm","logLevel":"debug"}, + "node":{ + "maxPods":47, + "labels":{"pool-source":"bootstrap-overridden","local-only":"true"}, + "taints":["local-source=bootstrap-stale:NoSchedule"], + "kubelet":{ + "nodeIP":"10.247.4.4", + "imageGCHighThreshold":90, + "imageGCLowThreshold":75 + } + } + }' +``` + +Response summary: + +```text +bootstrap: fetching fresh bootstrap data from AKS RP +bootstrap: rendered config at /etc/aks-flex-node/config.json +preflight: all required checks passed +level=INFO msg="machine already registered, adopting remote goal" +level=INFO msg="operation completed successfully" operation=bootstrap +``` + +Installed binary: + +```text +Version: v0.1.7-alpha.277 +Git Commit: 3c0aa0397c81c66cd190332a2b9dff7ead3f7cea +``` + +Service state: + +```text +aks-flex-node-agent.service: active, enabled +nspawn machine: kube1 +kubelet: active +containerd: active +``` + +### Step 13: Validate Initial RP Machine Authority + +- Result: **PASS WITH RP CUSTOM-LABEL ISOLATION FINDING** + +The persisted local config intentionally remains stale: + +```json +{ + "configMaxPods": 47, + "configLabels": { + "local-only": "true", + "pool-source": "bootstrap-overridden", + "remove-after-bootstrap": "true" + }, + "configTaints": ["local-source=bootstrap-stale:NoSchedule"], + "imageGC": {"high": 90, "low": 75}, + "kubernetes": "1.35.7", + "nodeIP": "10.247.4.4" +} +``` + +The daemon state adopted the RP Machine goal: + +```json +{ + "appliedGoal": { + "kubernetesVersion": "1.35.7", + "settingsVersion": "91f22a47-dee8-4ab4-ba67-337abdb82b76", + "maxPods": 61, + "nodeLabels": { + "kubernetes.azure.com/managed": "false", + "machine-only": "true", + "source": "machine-authoritative" + }, + "nodeTaints": ["source=machine-authoritative:NoSchedule"], + "kubeletConfig": { + "imageGCHighThreshold": 90, + "imageGCLowThreshold": 75 + } + }, + "activeMachine": "kube1" +} +``` + +Generated kubelet settings: + +```text +maxPods: 61 +imageGCHighThresholdPercent: 90 +imageGCLowThresholdPercent: 75 +--node-ip=10.247.4.4 +--node-labels=kubernetes.azure.com/agentpool=flexpool,kubernetes.azure.com/managed=false,kubernetes.azure.com/mode=user,kubernetes.azure.com/nodepool-type=FlexNodes,machine-only=true,source=machine-authoritative +--register-with-taints=source=machine-authoritative:NoSchedule +``` + +Kubernetes Node result: + +```json +{ + "uid": "78179c06-2b60-4cb5-aeee-f16dadad3fed", + "maxPods": "61", + "kubeletVersion": "v1.35.7", + "serverOwnedLabels": { + "kubernetes.azure.com/managed": "false", + "kubernetes.azure.com/agentpool": "flexpool", + "kubernetes.azure.com/mode": "user", + "kubernetes.azure.com/nodepool-type": "FlexNodes" + }, + "machineLabels": { + "source": "machine-authoritative", + "machine-only": "true" + }, + "staleLabels": { + "pool-source": null, + "remove-after-bootstrap": null, + "local-only": null + } +} +``` + +The Machine taint was present; stale pool/local taints were absent. The Node registered but remained `Ready=False` because no CNI configuration was installed on this externally provisioned VM. Workload networking was intentionally outside this test scope. + +Conclusions: + +1. The pre-created RP Machine, not pool bootstrap data or local config overrides, controlled Kubernetes version, `maxPods`, labels, and taints. +2. The agent added all four AKS-owned labels to the Node. +3. The original CLI request did not specify any AKS-owned label. However, the RP create/show/list response injected `kubernetes.azure.com/managed=false` into `properties.kubernetes.nodeLabels`, and the agent persisted that returned value in `appliedGoal.nodeLabels`. Therefore strict server-owned/custom-label separation does **not** hold in the current westcentralus RP response contract. +4. The other three AKS-owned labels were absent from ARM Machine labels and added only at kubelet registration. + +### Step 14: Validate Restart Persistence + +- Result: **PASS** + +Commands: + +```bash +sudo systemctl restart systemd-nspawn@kube1.service +sudo systemctl -M kube1 is-active kubelet containerd +sudo systemctl restart aks-flex-node-agent.service +sudo systemctl is-active aks-flex-node-agent.service +``` + +Response: + +```text +kubelet: active +containerd: active +aks-flex-node-agent.service: active +``` + +Daemon state checksum before and after restart: + +```text +de7638b2c4764e3d865261668ef4cae994583e8da7a3174cecebfc21f16aed33 +de7638b2c4764e3d865261668ef4cae994583e8da7a3174cecebfc21f16aed33 +``` + +Post-restart assertions: + +```text +Node UID unchanged: 78179c06-2b60-4cb5-aeee-f16dadad3fed +Active nspawn machine unchanged: kube1 +Machine ETag unchanged: 91f22a47-dee8-4ab4-ba67-337abdb82b76 +Node maxPods unchanged: 61 +Kubelet version unchanged: v1.35.7 +Machine custom labels and taint preserved +All four AKS-owned Node labels preserved +Stale pool/local metadata remained absent +``` + +The daemon repeatedly selected `ReportSucceeded` after restart, with no goal apply or repave. + +### Step 15: Inspect Regional Cluster Integration + +- Result: **INFORMATIONAL** + +Commands: + +```bash +kubectl api-resources --api-group=unbounded-cloud.io -o wide +kubectl get clusterrole,clusterrolebinding -l kubernetes.azure.com/managedby=aks -o name +``` + +Response summary: + +```text +No unbounded-cloud.io MachineOperation API was installed. +No dedicated AKS Flex daemon RBAC was present. +The agent logged: Machina MachineOperation API not found; using noop machine operation reconciler. +``` + +This means the regional test can validate authoritative bootstrap, daemon state, and restart persistence, but not PR #4's future in-place acknowledgement path. + +### Step 16: Update the RP Machine Goal + +- Result: **PASS** + +Command: + +```bash +az aks machine update \ + -g fn277-wcu-20260817 \ + --cluster-name fn277wcu \ + --nodepool-name flexpool \ + --machine-name fn277vm \ + --labels source=machine-updated update-only=true \ + --node-taints source=machine-updated:NoExecute +``` + +Response, relevant fields: + +```json +{ + "name": "fn277vm", + "properties": { + "eTag": "30f40af4-ef1a-4969-a06d-dfe46e80b2d5", + "kubernetes": { + "currentOrchestratorVersion": "1.35.7", + "maxPods": 61, + "nodeLabels": { + "kubernetes.azure.com/managed": "false", + "source": "machine-updated", + "update-only": "true" + }, + "nodeTaints": ["source=machine-updated:NoExecute"], + "orchestratorVersion": "1.35.7" + }, + "provisioningState": "Succeeded" + } +} +``` + +Before deleting the Node: + +```text +Live Node UID remained 78179c06-2b60-4cb5-aeee-f16dadad3fed. +Live Node retained source=machine-authoritative and machine-only=true. +Daemon applied ETag remained 91f22a47-dee8-4ab4-ba67-337abdb82b76. +Agent decision changed to WaitForNodeSignal. +``` + +Conclusion: PR #277 correctly did not acknowledge or apply the changed goal in place. That behavior remains in the separate PR #4 scope. + +### Step 17: Trigger and Validate Blue-Green Repave + +- Result: **PASS WITH ONE TRANSIENT RETRY** + +Command: + +```bash +kubectl delete node fn277vm --wait=false +``` + +Response: + +```text +node "fn277vm" deleted +``` + +Observed transition: + +```text +19:42:36 active=kube1 appliedETag=91f22a47-... node=absent +19:43:21 active=kube2 appliedETag=30f40af4-... nodeUID=a5bbaf76-... source=machine-updated update-only=true maxPods=61 +``` + +Agent log sequence: + +```text +decision=WaitForNodeSignal reason="goal state differs but node deletion trigger is absent" +decision=ApplyGoalState reason="node deletion observed and goal state is not applied" +refresh bootstrap data for repave: ... dial tcp ... i/o timeout +decision=ApplyGoalState reason="node deletion observed and goal state is not applied" +refreshed AKS bootstrap data for repave +starting nspawn machine goal-state apply oldMachine=kube1 newMachine=kube2 settingsVersion=30f40af4-... kubernetesVersion=1.35.7 +completed task=save-daemon-state status=ok +``` + +The first `listBootstrapData` call hit a transient ARM network timeout. Controller-runtime retried immediately and the second call succeeded without manual intervention. + +Final daemon state: + +```json +{ + "appliedGoal": { + "kubernetesVersion": "1.35.7", + "settingsVersion": "30f40af4-ef1a-4969-a06d-dfe46e80b2d5", + "maxPods": 61, + "nodeLabels": { + "kubernetes.azure.com/managed": "false", + "source": "machine-updated", + "update-only": "true" + }, + "nodeTaints": ["source=machine-updated:NoExecute"], + "kubeletConfig": { + "imageGCHighThreshold": 90, + "imageGCLowThreshold": 75 + } + }, + "previousAppliedGoal": { + "kubernetesVersion": "1.35.7", + "settingsVersion": "91f22a47-dee8-4ab4-ba67-337abdb82b76", + "maxPods": 61, + "nodeLabels": { + "kubernetes.azure.com/managed": "false", + "machine-only": "true", + "source": "machine-authoritative" + }, + "nodeTaints": ["source=machine-authoritative:NoSchedule"] + }, + "activeMachine": "kube2" +} +``` + +Final Kubernetes Node: + +```json +{ + "uid": "a5bbaf76-07f7-497c-bbb8-9d0b57c266c8", + "maxPods": "61", + "kubeletVersion": "v1.35.7", + "labels": { + "kubernetes.azure.com/managed": "false", + "kubernetes.azure.com/agentpool": "flexpool", + "kubernetes.azure.com/mode": "user", + "kubernetes.azure.com/nodepool-type": "FlexNodes", + "source": "machine-updated", + "update-only": "true", + "machine-only": null, + "pool-source": null, + "local-only": null + }, + "taint": "source=machine-updated:NoExecute" +} +``` + +The recreated Node remained `Ready=False` only because the externally provisioned VM had no CNI configuration. + +## Test Matrix + +| Test | Result | Evidence | +| --- | --- | --- | +| Azure CLI FlexNodes pool create/show/bootstrap | PASS | `flexpool` Succeeded; bootstrap data returned stale pool goal | +| Azure CLI Machine create/show/list | PASS | synchronous Machine with ETag `91f22a47-...` | +| Real Azure VM + MSI bootstrap | PASS | PR binary installed; AKS role scoped to cluster | +| Existing RP Machine authoritative at first bootstrap | PASS | local maxPods 47 vs applied/Node maxPods 61; Machine metadata won | +| Stale pool/local labels and taints excluded from Node | PASS | all deliberately stale keys absent | +| Four AKS-owned labels present on Node | PASS | managed, agentpool, mode, nodepool-type verified | +| CLI Machine create request used customer labels only | PASS | Request specified only `source` and `machine-only` | +| Server-owned labels absent from Machine custom-label response | **FAIL / RP FINDING** | RP injected `kubernetes.azure.com/managed=false` into Machine `nodeLabels` | +| Complete applied goal persistence | PASS | daemon state stores Machine ETag, maxPods, labels, taints, GC thresholds | +| Legacy scalar projections written | PASS | applied settings/Kubernetes fields match full goal | +| Nspawn and daemon restart persistence | PASS | state checksum and Node UID/settings unchanged | +| Machine update waits for Node deletion | PASS | agent selected `WaitForNodeSignal` | +| Machine goal repave | PASS | kube1→kube2, new ETag, new UID, updated metadata | +| Previous applied goal rotation | PASS | old full goal retained in `previousAppliedGoal` | +| Bootstrap-data refresh on repave | PASS WITH RETRY | first ARM call timed out; automatic retry succeeded | +| Workload networking / Node Ready | NOT IN SCOPE | no CNI installed on external VM | +| In-place label/taint acknowledgement | NOT AVAILABLE | regional cluster lacks MachineOperation CRD; PR #4 is separate | + +## Findings + +1. **RP custom-label isolation:** `az aks machine add` requested only `source` and `machine-only`, but create/show/list/raw ARM responses included `kubernetes.azure.com/managed=false` under `properties.kubernetes.nodeLabels`. The agent therefore persisted that RP-returned label in its complete applied goal. The other three AKS-owned labels remained outside the Machine response and were added only at kubelet registration. +2. **No regional MachineOperation integration:** westcentralus did not install `unbounded-cloud.io` CRDs or dedicated Flex daemon RBAC for this cluster, so PR #4 acknowledgement could not be tested. +3. **External VM CNI:** the Node remains `Ready=False` with `NetworkPluginNotReady`. This was expected because workload networking was outside the requested goal-authority test and the VM is not RP-provisioned. +4. **Transient ARM timeout:** one repave bootstrap-data refresh timed out, then succeeded through the existing reconciliation retry. + +## Overall Result + +PRs #275, #276, and #277 passed the requested real-VM goal-authority validation: + +- the existing RP Machine controlled first bootstrap despite conflicting pool/local config; +- the exact RP `maxPods`, labels, taints, version, and ETag reached kubelet, daemon state, and the Node; +- all four AKS-owned labels were present on the Node; +- full applied state survived restart; +- a real CLI Machine update was applied through blue-green repave and rotated the previous complete goal. + +The only requested assertion that did not hold was strict server-owned/custom-label separation in the ARM Machine response because the westcentralus RP adds `kubernetes.azure.com/managed=false` to `properties.kubernetes.nodeLabels`. + +## Test Resources + +| Resource | Planned value | +| --- | --- | +| Resource group | `fn277-wcu-20260817` | +| AKS cluster | `fn277wcu` | +| System pool | `systempool` | +| FlexNodes pool | `flexpool` | +| Azure VM / Machine / Node | `fn277vm` | +| VNet | `fn277-vnet` | +| AKS subnet | `aks-subnet` | +| Flex VM subnet | `flex-subnet` | +| Kubernetes version | `1.35.7` | +| Pool max pods | `75` | +| Machine max pods | `61` | +| Initial Machine custom labels | `source=machine-authoritative`, `machine-only=true` | +| Initial Machine taint | `source=machine-authoritative:NoSchedule` | +| Deliberately stale local labels | `pool-source=bootstrap-overridden`, `local-only=true` | + +## Cleanup + +- Result: **PASS** + +Commands: + +```bash +az group delete \ + --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --name fn277-wcu-20260817 \ + --yes --no-wait + +az group wait \ + --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --name fn277-wcu-20260817 \ + --deleted --interval 15 --timeout 1800 + +az group exists \ + --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --name fn277-wcu-20260817 + +az resource list \ + --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ + --tag purpose=flexnode-pr277-e2e \ + -o json +``` + +Responses: + +```text +Resource group exists: false +Tagged resources remaining: [] +``` From cac8bdbb07e11d1a8d71214ae10170c336929d4e Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Mon, 17 Aug 2026 14:56:01 -0700 Subject: [PATCH 4/7] Revert "Document westcentralus FlexNode E2E validation" --- ...exnode-pr277-westcentralus-e2e-20260817.md | 945 ------------------ 1 file changed, 945 deletions(-) delete mode 100644 reports/flexnode-pr277-westcentralus-e2e-20260817.md diff --git a/reports/flexnode-pr277-westcentralus-e2e-20260817.md b/reports/flexnode-pr277-westcentralus-e2e-20260817.md deleted file mode 100644 index acede251..00000000 --- a/reports/flexnode-pr277-westcentralus-e2e-20260817.md +++ /dev/null @@ -1,945 +0,0 @@ -# FlexNode PR #277 West Central US E2E Report - -## Status - -- Overall result: **PASS WITH RP CUSTOM-LABEL ISOLATION FINDING** -- Current phase: complete -- Started: `2026-08-17T18:53:51Z` -- Last updated: `2026-08-17T20:02:00Z` - -## Scope - -- Subscription: `8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8` -- Region: `westcentralus` -- Target regional AKS RP release: `v20260807` -- Agent changes: - - PR [#275](https://github.com/Azure/AKSFlexNode/pull/275): AKS-owned node labels - - PR [#276](https://github.com/Azure/AKSFlexNode/pull/276): strict Machine goal responses - - PR [#277](https://github.com/Azure/AKSFlexNode/pull/277): authoritative Machine goals -- Agent branch: `wenx/authoritative-machine-goals` -- Agent commit: `3c0aa0397c81c66cd190332a2b9dff7ead3f7cea` -- Test host: real Azure VM with system-assigned managed identity -- Control path: Azure CLI `aks-preview` FlexNodes commands - -## Test Objectives - -1. Create an AKS cluster and FlexNodes pool using Azure CLI. -2. Pre-create an RP Machine whose goal deliberately differs from pool bootstrap/local config. -3. Start the PR #277 agent on a real Azure VM and prove the existing RP Machine goal is authoritative. -4. Verify Machine custom labels contain only customer labels and exclude these AKS-owned labels: - - `kubernetes.azure.com/managed` - - `kubernetes.azure.com/agentpool` - - `kubernetes.azure.com/mode` - - `kubernetes.azure.com/nodepool-type` -5. Verify the Kubernetes Node contains the four AKS-owned labels plus the Machine custom labels. -6. Verify the effective Machine goal controls Kubernetes version, `maxPods`, labels, taints, and persisted daemon state. -7. Restart the nspawn node/agent and prove the authoritative applied goal survives restart. - -## Test Constraints - -- This test does not validate workload networking. The external VM is not provisioned by the AKS RP and may not receive a production CNI configuration. -- Secret responses such as bootstrap tokens and CA data are never written to this report. -- Commands are recorded with non-secret identifiers. Sensitive response fields are reduced to booleans or redacted summaries. - -## Execution Log - -### Step 1: Verify Local Source and Tooling - -- Result: **PASS** - -Command: - -```bash -git status --short --branch -git log --oneline --decorate --max-count=8 -git rev-parse HEAD -``` - -Response: - -```text -## wenx/authoritative-machine-goals...origin/wenx/authoritative-machine-goals -3c0aa03 Align E2E Machine max pods with node config -44b711f Adopt authoritative AKS Machine goals -072fe11 Validate AKS Machine goal responses (#276) -981f1cd Add AKS-owned labels to Flex Nodes (#275) -3c0aa0397c81c66cd190332a2b9dff7ead3f7cea -``` - -Command: - -```bash -az version -``` - -Response, relevant fields: - -```json -{ - "azure-cli": "2.86.0", - "extensions": { - "aks-preview": "22.0.0b1" - } -} -``` - -### Step 2: Verify Subscription and Preview Features - -- Result: **PASS** - -Command: - -```bash -az account show \ - --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --query '{id:id,name:name,state:state,tenantId:tenantId,userType:user.type}' \ - -o json -``` - -Response: - -```json -{ - "id": "8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8", - "name": "Azure Container Service - Test (AKS Standalone)", - "state": "Enabled", - "tenantId": "72f988bf-86f1-41af-91ab-2d7cd011db47", - "userType": "user" -} -``` - -Commands: - -```bash -az feature show --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --namespace Microsoft.ContainerService --name AKSFlexNodePreview \ - --query '{name:name,state:properties.state}' -o json - -az feature show --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --namespace Microsoft.ContainerService --name PutMachinePreview \ - --query '{name:name,state:properties.state}' -o json -``` - -Responses: - -```json -{"name":"Microsoft.ContainerService/AKSFlexNodePreview","state":"Registered"} -{"name":"Microsoft.ContainerService/PutMachinePreview","state":"Registered"} -``` - -### Step 3: Verify Regional Kubernetes and VM Capacity - -- Result: **PASS** - -Command: - -```bash -az aks get-versions \ - --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --location westcentralus \ - --query 'values[*].patchVersions.keys(@)[]' -o json -``` - -Response summary: - -```text -Available versions include 1.35.7 and 1.36.3. -Selected test version: 1.35.7. -``` - -Commands: - -```bash -az vm list-skus --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --location westcentralus --size Standard_D4s_v5 \ - --resource-type virtualMachines -o json - -az vm list-skus --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --location westcentralus --size Standard_D2s_v5 \ - --resource-type virtualMachines -o json - -az vm list-usage --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --location westcentralus -o json -``` - -Response summary: - -```text -Standard_D4s_v5: unrestricted, 4 vCPU, 16 GiB -Standard_D2s_v5: unrestricted, 2 vCPU, 8 GiB -Standard DSv5 quota: 100 vCPU available -Regional total quota: 2300 vCPU available -``` - -### Step 4: Verify Azure CLI FlexNodes Command Surface - -- Result: **PASS** - -Commands: - -```bash -az aks nodepool add -h -az aks machine add -h -az aks nodepool get-bootstrap-data -h -``` - -Response summary: - -```text -az aks nodepool add supports --vm-set-type FlexNodes. -az aks machine add supports machine name, Kubernetes version, max pods, labels, and taints. -az aks nodepool get-bootstrap-data is available from aks-preview. -``` - -### Step 5: Build the PR Agent Artifact - -- Result: **PASS** -- Release publication decision: no public alpha release was needed; the exact PR binary will be copied to the test VM. - -Command: - -```bash -BUILD_DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" -GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build \ - -ldflags "-X github.com/Azure/AKSFlexNode/pkg/cmd/version.Version=v0.1.7-alpha.277 \ - -X github.com/Azure/AKSFlexNode/pkg/cmd/version.GitCommit=3c0aa0397c81c66cd190332a2b9dff7ead3f7cea \ - -X github.com/Azure/AKSFlexNode/pkg/cmd/version.BuildTime=${BUILD_DATE} -w -s" \ - -o /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64 \ - ./cmd/aks-flex-node - -tar -C /tmp/opencode/fn277-wcu-artifacts -czf \ - /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64.tar.gz \ - aks-flex-node-linux-amd64 - -sha256sum /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64.tar.gz -/tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64 version -``` - -Response: - -```text -47909047b39973069cb9ecc5f60cfae83429deb5b8a8df42bf605540ad759bfe aks-flex-node-linux-amd64.tar.gz -AKS Flex Node Agent -Version: v0.1.7-alpha.277 -Git Commit: 3c0aa0397c81c66cd190332a2b9dff7ead3f7cea -Build Time: 2026-08-17T19:05:31Z -``` - -### Step 6: Create the Azure Resource Group and Network - -- Result: **PASS** - -Commands: - -```bash -az account set --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 - -az group create --name fn277-wcu-20260817 --location westcentralus \ - --tags purpose=flexnode-pr277-e2e owner=wenxuan pr=277 \ - agentCommit=3c0aa03 rpRelease=v20260807 - -az network vnet create -g fn277-wcu-20260817 -n fn277-vnet \ - --location westcentralus --address-prefixes 10.247.0.0/16 \ - --subnet-name aks-subnet --subnet-prefixes 10.247.0.0/22 - -az network nsg create -g fn277-wcu-20260817 -n fn277-vm-nsg \ - --location westcentralus - -az network vnet subnet create -g fn277-wcu-20260817 \ - --vnet-name fn277-vnet --name flex-subnet \ - --address-prefixes 10.247.4.0/24 --network-security-group fn277-vm-nsg -``` - -Response summary: - -```text -Resource group provisioning: Succeeded -VNet: 10.247.0.0/16 -AKS subnet: 10.247.0.0/22 -Flex VM subnet: 10.247.4.0/24 -``` - -### Step 7: Create the AKS Cluster - -- Result: **PASS** - -Command: - -```bash -az aks create \ - --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --resource-group fn277-wcu-20260817 \ - --name fn277wcu \ - --location westcentralus \ - --kubernetes-version 1.35.7 \ - --nodepool-name systempool \ - --node-count 1 \ - --node-vm-size Standard_D2s_v5 \ - --network-plugin azure \ - --network-plugin-mode overlay \ - --vnet-subnet-id \ - --service-cidr 10.248.0.0/16 \ - --dns-service-ip 10.248.0.10 \ - --enable-managed-identity \ - --ssh-key-value ~/.ssh/id_rsa.pub -``` - -Response, relevant fields: - -```json -{ - "currentVersion": "1.35.7", - "location": "westcentralus", - "name": "fn277wcu", - "networkPlugin": "azure", - "networkPluginMode": "overlay", - "state": "Succeeded", - "systemPool": { - "count": 1, - "name": "systempool", - "state": "Succeeded", - "vmSize": "Standard_D2s_v5" - } -} -``` - -### Step 8: Create the FlexNodes Pool and Inspect Bootstrap Data - -- Result: **PASS** - -Command: - -```bash -az aks nodepool add \ - --resource-group fn277-wcu-20260817 \ - --cluster-name fn277wcu \ - --name flexpool \ - --vm-set-type FlexNodes \ - --mode User \ - --kubernetes-version 1.35.7 \ - --max-pods 75 \ - --labels pool-source=bootstrap-stale remove-after-bootstrap=true \ - --node-taints pool-source=bootstrap-stale:NoSchedule \ - --max-unavailable 30% -``` - -Response, relevant fields: - -```json -{ - "currentVersion": "1.35.7", - "labels": { - "pool-source": "bootstrap-stale", - "remove-after-bootstrap": "true" - }, - "maxPods": 75, - "mode": "User", - "name": "flexpool", - "state": "Succeeded", - "taints": ["pool-source=bootstrap-stale:NoSchedule"], - "type": "FlexNodes" -} -``` - -Command: - -```bash -az aks nodepool get-bootstrap-data \ - -g fn277-wcu-20260817 --cluster-name fn277wcu -n flexpool \ - --query '{targetPool:azure.targetAgentPoolName,kubernetes:components.kubernetes,maxPods:node.maxPods,labels:node.labels,taints:node.taints,hasToken:length(azure.bootstrapToken.token) > `0`,hasCACert:length(node.kubelet.caCertData) > `0`}' \ - -o json -``` - -Sanitized response: - -```json -{ - "hasCACert": true, - "hasToken": true, - "kubernetes": "1.35.7", - "labels": { - "pool-source": "bootstrap-stale", - "remove-after-bootstrap": "true" - }, - "maxPods": 75, - "taints": ["pool-source=bootstrap-stale:NoSchedule"], - "targetPool": "flexpool" -} -``` - -Observation: pool bootstrap data contains only the configured customer labels. It does not contain the four AKS-owned labels. - -### Step 9: Pre-create the Authoritative RP Machine - -- Result: **PASS WITH RP RESPONSE OBSERVATION** - -Command: - -```bash -az aks machine add \ - -g fn277-wcu-20260817 \ - --cluster-name fn277wcu \ - --nodepool-name flexpool \ - --machine-name fn277vm \ - --kubernetes-version 1.35.7 \ - --max-pods 61 \ - --labels source=machine-authoritative machine-only=true \ - --node-taints source=machine-authoritative:NoSchedule -``` - -Response, relevant fields: - -```json -{ - "name": "fn277vm", - "properties": { - "eTag": "91f22a47-dee8-4ab4-ba67-337abdb82b76", - "kubernetes": { - "currentOrchestratorVersion": "1.35.7", - "maxPods": 61, - "nodeLabels": { - "kubernetes.azure.com/managed": "false", - "machine-only": "true", - "source": "machine-authoritative" - }, - "nodeName": "fn277vm", - "nodeTaints": ["source=machine-authoritative:NoSchedule"], - "orchestratorVersion": "1.35.7" - }, - "provisioningState": "Succeeded" - } -} -``` - -Observation: the CLI request contained only `source` and `machine-only`; the RP response added `kubernetes.azure.com/managed=false`. `machine show` and `machine list` returned the same expanded label map. The remaining three AKS-owned labels were not present in the ARM Machine response. - -The Kubernetes Node did not exist before VM bootstrap: - -```text -Error from server (NotFound): nodes "fn277vm" not found -``` - -### Step 10: Create the Real Azure VM and Assign AKS Access - -- Result: **PASS** - -Commands: - -```bash -az network nsg rule create -g fn277-wcu-20260817 \ - --nsg-name fn277-vm-nsg --name AllowSSHFromOperator \ - --priority 100 --direction Inbound --access Allow --protocol Tcp \ - --source-address-prefixes 67.168.38.253/32 \ - --destination-port-ranges 22 - -az vm create \ - -g fn277-wcu-20260817 -n fn277vm --location westcentralus \ - --image Ubuntu2404 --size Standard_D4s_v5 \ - --admin-username azureuser --ssh-key-values ~/.ssh/id_rsa.pub \ - --vnet-name fn277-vnet --subnet flex-subnet --nsg "" \ - --assign-identity --public-ip-sku Standard --os-disk-size-gb 64 \ - --security-type TrustedLaunch -``` - -Response summary: - -```text -VM private IP: 10.247.4.4 -VM public IP: 20.168.179.165 -VM state: running -System-assigned principal: a7183dd9-4799-4b8b-b09a-10889758d431 -``` - -Command: - -```bash -az role assignment create \ - --assignee-object-id a7183dd9-4799-4b8b-b09a-10889758d431 \ - --assignee-principal-type ServicePrincipal \ - --role "Azure Kubernetes Service Contributor Role" \ - --scope -``` - -Response summary: - -```text -Role assignment ID: d03807f8-20da-4369-a646-8dd218d41122 -Scope: AKS cluster fn277wcu -``` - -### Step 11: Stage and Verify the PR Agent on the VM - -- Result: **PASS** - -Commands: - -```bash -scp -i ~/.ssh/id_rsa \ - /tmp/opencode/fn277-wcu-artifacts/aks-flex-node-linux-amd64.tar.gz \ - azureuser@20.168.179.165:/tmp/aks-flex-node-linux-amd64.tar.gz - -scp -i ~/.ssh/id_rsa scripts/bootstrap.sh \ - azureuser@20.168.179.165:/tmp/bootstrap.sh - -ssh -i ~/.ssh/id_rsa azureuser@20.168.179.165 \ - 'sha256sum /tmp/aks-flex-node-linux-amd64.tar.gz; bash -n /tmp/bootstrap.sh' - -ssh -i ~/.ssh/id_rsa azureuser@20.168.179.165 \ - 'curl -fsS -H Metadata:true "http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fmanagement.azure.com%2F" | jq -r '"'"'if (.access_token | length) > 0 then "token-acquired" else "missing-token" end'"'"'' -``` - -Response: - -```text -47909047b39973069cb9ecc5f60cfae83429deb5b8a8df42bf605540ad759bfe /tmp/aks-flex-node-linux-amd64.tar.gz -token-acquired -``` - -An initial artifact-verification command ran concurrently with the copy and observed the files before transfer completion. The serial retry above passed; no bootstrap mutation had started. - -### Step 12: Bootstrap with Deliberately Conflicting Local Settings - -- Result: **PASS** - -Command, non-secret form: - -```bash -sudo bash /tmp/bootstrap.sh \ - --auth msi \ - --fetch-bootstrap-data \ - --cluster-resource-id \ - --agent-pool-name flexpool \ - --agent-url file:///tmp/aks-flex-node-linux-amd64.tar.gz \ - --agent-sha256 47909047b39973069cb9ecc5f60cfae83429deb5b8a8df42bf605540ad759bfe \ - --config-overrides '{ - "agent":{"nodeName":"fn277vm","logLevel":"debug"}, - "node":{ - "maxPods":47, - "labels":{"pool-source":"bootstrap-overridden","local-only":"true"}, - "taints":["local-source=bootstrap-stale:NoSchedule"], - "kubelet":{ - "nodeIP":"10.247.4.4", - "imageGCHighThreshold":90, - "imageGCLowThreshold":75 - } - } - }' -``` - -Response summary: - -```text -bootstrap: fetching fresh bootstrap data from AKS RP -bootstrap: rendered config at /etc/aks-flex-node/config.json -preflight: all required checks passed -level=INFO msg="machine already registered, adopting remote goal" -level=INFO msg="operation completed successfully" operation=bootstrap -``` - -Installed binary: - -```text -Version: v0.1.7-alpha.277 -Git Commit: 3c0aa0397c81c66cd190332a2b9dff7ead3f7cea -``` - -Service state: - -```text -aks-flex-node-agent.service: active, enabled -nspawn machine: kube1 -kubelet: active -containerd: active -``` - -### Step 13: Validate Initial RP Machine Authority - -- Result: **PASS WITH RP CUSTOM-LABEL ISOLATION FINDING** - -The persisted local config intentionally remains stale: - -```json -{ - "configMaxPods": 47, - "configLabels": { - "local-only": "true", - "pool-source": "bootstrap-overridden", - "remove-after-bootstrap": "true" - }, - "configTaints": ["local-source=bootstrap-stale:NoSchedule"], - "imageGC": {"high": 90, "low": 75}, - "kubernetes": "1.35.7", - "nodeIP": "10.247.4.4" -} -``` - -The daemon state adopted the RP Machine goal: - -```json -{ - "appliedGoal": { - "kubernetesVersion": "1.35.7", - "settingsVersion": "91f22a47-dee8-4ab4-ba67-337abdb82b76", - "maxPods": 61, - "nodeLabels": { - "kubernetes.azure.com/managed": "false", - "machine-only": "true", - "source": "machine-authoritative" - }, - "nodeTaints": ["source=machine-authoritative:NoSchedule"], - "kubeletConfig": { - "imageGCHighThreshold": 90, - "imageGCLowThreshold": 75 - } - }, - "activeMachine": "kube1" -} -``` - -Generated kubelet settings: - -```text -maxPods: 61 -imageGCHighThresholdPercent: 90 -imageGCLowThresholdPercent: 75 ---node-ip=10.247.4.4 ---node-labels=kubernetes.azure.com/agentpool=flexpool,kubernetes.azure.com/managed=false,kubernetes.azure.com/mode=user,kubernetes.azure.com/nodepool-type=FlexNodes,machine-only=true,source=machine-authoritative ---register-with-taints=source=machine-authoritative:NoSchedule -``` - -Kubernetes Node result: - -```json -{ - "uid": "78179c06-2b60-4cb5-aeee-f16dadad3fed", - "maxPods": "61", - "kubeletVersion": "v1.35.7", - "serverOwnedLabels": { - "kubernetes.azure.com/managed": "false", - "kubernetes.azure.com/agentpool": "flexpool", - "kubernetes.azure.com/mode": "user", - "kubernetes.azure.com/nodepool-type": "FlexNodes" - }, - "machineLabels": { - "source": "machine-authoritative", - "machine-only": "true" - }, - "staleLabels": { - "pool-source": null, - "remove-after-bootstrap": null, - "local-only": null - } -} -``` - -The Machine taint was present; stale pool/local taints were absent. The Node registered but remained `Ready=False` because no CNI configuration was installed on this externally provisioned VM. Workload networking was intentionally outside this test scope. - -Conclusions: - -1. The pre-created RP Machine, not pool bootstrap data or local config overrides, controlled Kubernetes version, `maxPods`, labels, and taints. -2. The agent added all four AKS-owned labels to the Node. -3. The original CLI request did not specify any AKS-owned label. However, the RP create/show/list response injected `kubernetes.azure.com/managed=false` into `properties.kubernetes.nodeLabels`, and the agent persisted that returned value in `appliedGoal.nodeLabels`. Therefore strict server-owned/custom-label separation does **not** hold in the current westcentralus RP response contract. -4. The other three AKS-owned labels were absent from ARM Machine labels and added only at kubelet registration. - -### Step 14: Validate Restart Persistence - -- Result: **PASS** - -Commands: - -```bash -sudo systemctl restart systemd-nspawn@kube1.service -sudo systemctl -M kube1 is-active kubelet containerd -sudo systemctl restart aks-flex-node-agent.service -sudo systemctl is-active aks-flex-node-agent.service -``` - -Response: - -```text -kubelet: active -containerd: active -aks-flex-node-agent.service: active -``` - -Daemon state checksum before and after restart: - -```text -de7638b2c4764e3d865261668ef4cae994583e8da7a3174cecebfc21f16aed33 -de7638b2c4764e3d865261668ef4cae994583e8da7a3174cecebfc21f16aed33 -``` - -Post-restart assertions: - -```text -Node UID unchanged: 78179c06-2b60-4cb5-aeee-f16dadad3fed -Active nspawn machine unchanged: kube1 -Machine ETag unchanged: 91f22a47-dee8-4ab4-ba67-337abdb82b76 -Node maxPods unchanged: 61 -Kubelet version unchanged: v1.35.7 -Machine custom labels and taint preserved -All four AKS-owned Node labels preserved -Stale pool/local metadata remained absent -``` - -The daemon repeatedly selected `ReportSucceeded` after restart, with no goal apply or repave. - -### Step 15: Inspect Regional Cluster Integration - -- Result: **INFORMATIONAL** - -Commands: - -```bash -kubectl api-resources --api-group=unbounded-cloud.io -o wide -kubectl get clusterrole,clusterrolebinding -l kubernetes.azure.com/managedby=aks -o name -``` - -Response summary: - -```text -No unbounded-cloud.io MachineOperation API was installed. -No dedicated AKS Flex daemon RBAC was present. -The agent logged: Machina MachineOperation API not found; using noop machine operation reconciler. -``` - -This means the regional test can validate authoritative bootstrap, daemon state, and restart persistence, but not PR #4's future in-place acknowledgement path. - -### Step 16: Update the RP Machine Goal - -- Result: **PASS** - -Command: - -```bash -az aks machine update \ - -g fn277-wcu-20260817 \ - --cluster-name fn277wcu \ - --nodepool-name flexpool \ - --machine-name fn277vm \ - --labels source=machine-updated update-only=true \ - --node-taints source=machine-updated:NoExecute -``` - -Response, relevant fields: - -```json -{ - "name": "fn277vm", - "properties": { - "eTag": "30f40af4-ef1a-4969-a06d-dfe46e80b2d5", - "kubernetes": { - "currentOrchestratorVersion": "1.35.7", - "maxPods": 61, - "nodeLabels": { - "kubernetes.azure.com/managed": "false", - "source": "machine-updated", - "update-only": "true" - }, - "nodeTaints": ["source=machine-updated:NoExecute"], - "orchestratorVersion": "1.35.7" - }, - "provisioningState": "Succeeded" - } -} -``` - -Before deleting the Node: - -```text -Live Node UID remained 78179c06-2b60-4cb5-aeee-f16dadad3fed. -Live Node retained source=machine-authoritative and machine-only=true. -Daemon applied ETag remained 91f22a47-dee8-4ab4-ba67-337abdb82b76. -Agent decision changed to WaitForNodeSignal. -``` - -Conclusion: PR #277 correctly did not acknowledge or apply the changed goal in place. That behavior remains in the separate PR #4 scope. - -### Step 17: Trigger and Validate Blue-Green Repave - -- Result: **PASS WITH ONE TRANSIENT RETRY** - -Command: - -```bash -kubectl delete node fn277vm --wait=false -``` - -Response: - -```text -node "fn277vm" deleted -``` - -Observed transition: - -```text -19:42:36 active=kube1 appliedETag=91f22a47-... node=absent -19:43:21 active=kube2 appliedETag=30f40af4-... nodeUID=a5bbaf76-... source=machine-updated update-only=true maxPods=61 -``` - -Agent log sequence: - -```text -decision=WaitForNodeSignal reason="goal state differs but node deletion trigger is absent" -decision=ApplyGoalState reason="node deletion observed and goal state is not applied" -refresh bootstrap data for repave: ... dial tcp ... i/o timeout -decision=ApplyGoalState reason="node deletion observed and goal state is not applied" -refreshed AKS bootstrap data for repave -starting nspawn machine goal-state apply oldMachine=kube1 newMachine=kube2 settingsVersion=30f40af4-... kubernetesVersion=1.35.7 -completed task=save-daemon-state status=ok -``` - -The first `listBootstrapData` call hit a transient ARM network timeout. Controller-runtime retried immediately and the second call succeeded without manual intervention. - -Final daemon state: - -```json -{ - "appliedGoal": { - "kubernetesVersion": "1.35.7", - "settingsVersion": "30f40af4-ef1a-4969-a06d-dfe46e80b2d5", - "maxPods": 61, - "nodeLabels": { - "kubernetes.azure.com/managed": "false", - "source": "machine-updated", - "update-only": "true" - }, - "nodeTaints": ["source=machine-updated:NoExecute"], - "kubeletConfig": { - "imageGCHighThreshold": 90, - "imageGCLowThreshold": 75 - } - }, - "previousAppliedGoal": { - "kubernetesVersion": "1.35.7", - "settingsVersion": "91f22a47-dee8-4ab4-ba67-337abdb82b76", - "maxPods": 61, - "nodeLabels": { - "kubernetes.azure.com/managed": "false", - "machine-only": "true", - "source": "machine-authoritative" - }, - "nodeTaints": ["source=machine-authoritative:NoSchedule"] - }, - "activeMachine": "kube2" -} -``` - -Final Kubernetes Node: - -```json -{ - "uid": "a5bbaf76-07f7-497c-bbb8-9d0b57c266c8", - "maxPods": "61", - "kubeletVersion": "v1.35.7", - "labels": { - "kubernetes.azure.com/managed": "false", - "kubernetes.azure.com/agentpool": "flexpool", - "kubernetes.azure.com/mode": "user", - "kubernetes.azure.com/nodepool-type": "FlexNodes", - "source": "machine-updated", - "update-only": "true", - "machine-only": null, - "pool-source": null, - "local-only": null - }, - "taint": "source=machine-updated:NoExecute" -} -``` - -The recreated Node remained `Ready=False` only because the externally provisioned VM had no CNI configuration. - -## Test Matrix - -| Test | Result | Evidence | -| --- | --- | --- | -| Azure CLI FlexNodes pool create/show/bootstrap | PASS | `flexpool` Succeeded; bootstrap data returned stale pool goal | -| Azure CLI Machine create/show/list | PASS | synchronous Machine with ETag `91f22a47-...` | -| Real Azure VM + MSI bootstrap | PASS | PR binary installed; AKS role scoped to cluster | -| Existing RP Machine authoritative at first bootstrap | PASS | local maxPods 47 vs applied/Node maxPods 61; Machine metadata won | -| Stale pool/local labels and taints excluded from Node | PASS | all deliberately stale keys absent | -| Four AKS-owned labels present on Node | PASS | managed, agentpool, mode, nodepool-type verified | -| CLI Machine create request used customer labels only | PASS | Request specified only `source` and `machine-only` | -| Server-owned labels absent from Machine custom-label response | **FAIL / RP FINDING** | RP injected `kubernetes.azure.com/managed=false` into Machine `nodeLabels` | -| Complete applied goal persistence | PASS | daemon state stores Machine ETag, maxPods, labels, taints, GC thresholds | -| Legacy scalar projections written | PASS | applied settings/Kubernetes fields match full goal | -| Nspawn and daemon restart persistence | PASS | state checksum and Node UID/settings unchanged | -| Machine update waits for Node deletion | PASS | agent selected `WaitForNodeSignal` | -| Machine goal repave | PASS | kube1→kube2, new ETag, new UID, updated metadata | -| Previous applied goal rotation | PASS | old full goal retained in `previousAppliedGoal` | -| Bootstrap-data refresh on repave | PASS WITH RETRY | first ARM call timed out; automatic retry succeeded | -| Workload networking / Node Ready | NOT IN SCOPE | no CNI installed on external VM | -| In-place label/taint acknowledgement | NOT AVAILABLE | regional cluster lacks MachineOperation CRD; PR #4 is separate | - -## Findings - -1. **RP custom-label isolation:** `az aks machine add` requested only `source` and `machine-only`, but create/show/list/raw ARM responses included `kubernetes.azure.com/managed=false` under `properties.kubernetes.nodeLabels`. The agent therefore persisted that RP-returned label in its complete applied goal. The other three AKS-owned labels remained outside the Machine response and were added only at kubelet registration. -2. **No regional MachineOperation integration:** westcentralus did not install `unbounded-cloud.io` CRDs or dedicated Flex daemon RBAC for this cluster, so PR #4 acknowledgement could not be tested. -3. **External VM CNI:** the Node remains `Ready=False` with `NetworkPluginNotReady`. This was expected because workload networking was outside the requested goal-authority test and the VM is not RP-provisioned. -4. **Transient ARM timeout:** one repave bootstrap-data refresh timed out, then succeeded through the existing reconciliation retry. - -## Overall Result - -PRs #275, #276, and #277 passed the requested real-VM goal-authority validation: - -- the existing RP Machine controlled first bootstrap despite conflicting pool/local config; -- the exact RP `maxPods`, labels, taints, version, and ETag reached kubelet, daemon state, and the Node; -- all four AKS-owned labels were present on the Node; -- full applied state survived restart; -- a real CLI Machine update was applied through blue-green repave and rotated the previous complete goal. - -The only requested assertion that did not hold was strict server-owned/custom-label separation in the ARM Machine response because the westcentralus RP adds `kubernetes.azure.com/managed=false` to `properties.kubernetes.nodeLabels`. - -## Test Resources - -| Resource | Planned value | -| --- | --- | -| Resource group | `fn277-wcu-20260817` | -| AKS cluster | `fn277wcu` | -| System pool | `systempool` | -| FlexNodes pool | `flexpool` | -| Azure VM / Machine / Node | `fn277vm` | -| VNet | `fn277-vnet` | -| AKS subnet | `aks-subnet` | -| Flex VM subnet | `flex-subnet` | -| Kubernetes version | `1.35.7` | -| Pool max pods | `75` | -| Machine max pods | `61` | -| Initial Machine custom labels | `source=machine-authoritative`, `machine-only=true` | -| Initial Machine taint | `source=machine-authoritative:NoSchedule` | -| Deliberately stale local labels | `pool-source=bootstrap-overridden`, `local-only=true` | - -## Cleanup - -- Result: **PASS** - -Commands: - -```bash -az group delete \ - --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --name fn277-wcu-20260817 \ - --yes --no-wait - -az group wait \ - --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --name fn277-wcu-20260817 \ - --deleted --interval 15 --timeout 1800 - -az group exists \ - --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --name fn277-wcu-20260817 - -az resource list \ - --subscription 8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8 \ - --tag purpose=flexnode-pr277-e2e \ - -o json -``` - -Responses: - -```text -Resource group exists: false -Tagged resources remaining: [] -``` From 686965ed51e369dbc790973d0fba375f73c0c635 Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Mon, 17 Aug 2026 16:51:14 -0700 Subject: [PATCH 5/7] Simplify goal state validation --- pkg/aksmachine/client_armapi.go | 2 +- pkg/aksmachine/client_armapi_test.go | 2 +- pkg/aksmachine/types.go | 10 +++------- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/pkg/aksmachine/client_armapi.go b/pkg/aksmachine/client_armapi.go index 428da5b1..eb9805e2 100644 --- a/pkg/aksmachine/client_armapi.go +++ b/pkg/aksmachine/client_armapi.go @@ -47,7 +47,7 @@ func newARMClient(cfg *config.Config, logger *slog.Logger) (MachineClient, error } func (c *armMachineClient) Create(ctx context.Context, desired GoalState) (*Machine, error) { - if err := desired.validate(); err != nil { + if err := desired.Validate(); err != nil { return nil, fmt.Errorf("validate goal state: %w", err) } params := armcontainerservice.Machine{ diff --git a/pkg/aksmachine/client_armapi_test.go b/pkg/aksmachine/client_armapi_test.go index b12720d7..4fd6b4ef 100644 --- a/pkg/aksmachine/client_armapi_test.go +++ b/pkg/aksmachine/client_armapi_test.go @@ -285,7 +285,7 @@ func TestGoalStateValidate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := tt.goal.validate() + err := tt.goal.Validate() if tt.wantErr != "" { if err == nil || !strings.Contains(err.Error(), tt.wantErr) { t.Fatalf("validate() error = %v, want containing %q", err, tt.wantErr) diff --git a/pkg/aksmachine/types.go b/pkg/aksmachine/types.go index 78e65ebf..b7a0c286 100644 --- a/pkg/aksmachine/types.go +++ b/pkg/aksmachine/types.go @@ -27,7 +27,9 @@ type KubeletConfig struct { ImageGCLowThreshold int `json:"imageGCLowThreshold,omitempty"` } -func (g GoalState) validate() error { +// Validate verifies the values present in a goal. SettingsVersion is validated +// by Machine because local bootstrap goals do not have an ETag until persisted. +func (g GoalState) Validate() error { if g.KubernetesVersion == "" { return fmt.Errorf("kubernetes version is empty") } @@ -55,12 +57,6 @@ func (g GoalState) validate() error { return nil } -// Validate verifies the values present in a goal. SettingsVersion is validated -// by Machine because local bootstrap goals do not have an ETag until persisted. -func (g GoalState) Validate() error { - return g.validate() -} - // ValidateEffective verifies that omitted API defaults have been resolved and // the goal has every scalar setting needed to render a node. func (g GoalState) ValidateEffective() error { From 0d93a8ccfd982ba7ba23a36c982ab3cbb057b184 Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Mon, 17 Aug 2026 18:58:56 -0700 Subject: [PATCH 6/7] Simplify effective machine goal handling --- pkg/aksmachine/client_armapi.go | 21 ++-- pkg/aksmachine/client_armapi_test.go | 54 ++++++--- pkg/aksmachine/client_incluster.go | 3 + pkg/aksmachine/client_incluster_test.go | 2 +- pkg/aksmachine/ensure_test.go | 23 ++-- pkg/aksmachine/test_helpers_test.go | 6 +- pkg/aksmachine/types.go | 112 ++++++++++-------- pkg/aksmachine/types_test.go | 52 ++++++-- pkg/daemon/{goalstate.go => goal_state.go} | 31 +++-- .../{goalstate_test.go => goal_state_test.go} | 11 +- pkg/daemon/nodeoperator.go | 12 +- pkg/daemon/nodeoperator_test.go | 19 ++- pkg/daemon/reconcile_test.go | 2 +- pkg/daemon/state.go | 15 +-- pkg/daemon/state_test.go | 4 +- pkg/daemon/test_helpers_test.go | 10 +- 16 files changed, 231 insertions(+), 146 deletions(-) rename pkg/daemon/{goalstate.go => goal_state.go} (67%) rename pkg/daemon/{goalstate_test.go => goal_state_test.go} (89%) diff --git a/pkg/aksmachine/client_armapi.go b/pkg/aksmachine/client_armapi.go index eb9805e2..67a5d8c9 100644 --- a/pkg/aksmachine/client_armapi.go +++ b/pkg/aksmachine/client_armapi.go @@ -214,10 +214,14 @@ func clientCertificateCredentialOptions(clientOpts azcore.ClientOptions) *aziden func buildK8sProfile(goal GoalState) *armcontainerservice.MachineKubernetesProfile { // FlexNode RP accepts the registration surface below; local kubelet defaults // are consumed during node bootstrap and must not be sent as Machine fields. - maxPods := int32(goal.MaxPods) //nolint:gosec // validated non-negative and small + var maxPods *int32 + if goal.MaxPods != nil { + value := int32(*goal.MaxPods) //nolint:gosec // validated non-negative and small + maxPods = &value + } p := &armcontainerservice.MachineKubernetesProfile{ OrchestratorVersion: &goal.KubernetesVersion, - MaxPods: &maxPods, + MaxPods: maxPods, NodeLabels: stringPointerMap(goal.NodeLabels), NodeTaints: stringPointerSlice(goal.NodeTaints), } @@ -283,7 +287,8 @@ func machineFromARM(machine armcontainerservice.Machine, defaultID, defaultName } } if kubernetes.MaxPods != nil { - result.Goal.MaxPods = int(*kubernetes.MaxPods) + value := int(*kubernetes.MaxPods) + result.Goal.MaxPods = &value } if kubernetes.NodeLabels != nil { result.Goal.NodeLabels = stringMapFromPointers(kubernetes.NodeLabels) @@ -293,10 +298,12 @@ func machineFromARM(machine armcontainerservice.Machine, defaultID, defaultName } if kubernetes.KubeletConfig != nil { if kubernetes.KubeletConfig.ImageGcHighThreshold != nil { - result.Goal.KubeletConfig.ImageGCHighThreshold = int(*kubernetes.KubeletConfig.ImageGcHighThreshold) + value := int(*kubernetes.KubeletConfig.ImageGcHighThreshold) + result.Goal.KubeletConfig.ImageGCHighThreshold = &value } if kubernetes.KubeletConfig.ImageGcLowThreshold != nil { - result.Goal.KubeletConfig.ImageGCLowThreshold = int(*kubernetes.KubeletConfig.ImageGcLowThreshold) + value := int(*kubernetes.KubeletConfig.ImageGcLowThreshold) + result.Goal.KubeletConfig.ImageGCLowThreshold = &value } } } @@ -313,9 +320,9 @@ func resolveKubernetesVersionAlias(desired, current string) string { desiredVersion := strings.TrimPrefix(strings.TrimSpace(desired), "v") currentVersion := strings.TrimPrefix(strings.TrimSpace(current), "v") if len(strings.Split(desiredVersion, ".")) == 2 && strings.HasPrefix(currentVersion, desiredVersion+".") { - return current + return currentVersion } - return desired + return desiredVersion } func stringMapFromPointers(values map[string]*string) map[string]string { diff --git a/pkg/aksmachine/client_armapi_test.go b/pkg/aksmachine/client_armapi_test.go index 4fd6b4ef..5e81f7cb 100644 --- a/pkg/aksmachine/client_armapi_test.go +++ b/pkg/aksmachine/client_armapi_test.go @@ -205,12 +205,12 @@ func TestBuildK8sProfile(t *testing.T) { profile := buildK8sProfile(GoalState{ KubernetesVersion: "1.35.1", - MaxPods: 42, + MaxPods: ptr(42), NodeLabels: map[string]string{"workload": "flex"}, NodeTaints: []string{"dedicated=flex:NoSchedule"}, KubeletConfig: KubeletConfig{ - ImageGCHighThreshold: 85, - ImageGCLowThreshold: 80, + ImageGCHighThreshold: ptr(85), + ImageGCLowThreshold: ptr(80), }, }) if profile.OrchestratorVersion == nil || *profile.OrchestratorVersion != "1.35.1" { @@ -240,7 +240,7 @@ func TestGoalStateValidate(t *testing.T) { }{ { name: "valid", - goal: GoalState{KubernetesVersion: "1.35.1"}, + goal: testGoal("1.35.1", ""), }, { name: "missing Kubernetes version", @@ -248,22 +248,42 @@ func TestGoalStateValidate(t *testing.T) { wantErr: "kubernetes version is empty", }, { - name: "negative max pods", - goal: GoalState{KubernetesVersion: "1.35.1", MaxPods: -1}, + name: "missing max pods", + goal: GoalState{KubernetesVersion: "1.35.1"}, + wantErr: "max pods is empty", + }, + { + name: "negative max pods", + goal: GoalState{ + KubernetesVersion: "1.35.1", + MaxPods: ptr(-1), + KubeletConfig: KubeletConfig{ + ImageGCHighThreshold: ptr(85), + ImageGCLowThreshold: ptr(80), + }, + }, wantErr: "max pods must be non-negative", }, { - name: "max pods exceeds int32", - goal: GoalState{KubernetesVersion: "1.35.1", MaxPods: math.MaxInt32 + 1}, + name: "max pods exceeds int32", + goal: GoalState{ + KubernetesVersion: "1.35.1", + MaxPods: ptr(math.MaxInt32 + 1), + KubeletConfig: KubeletConfig{ + ImageGCHighThreshold: ptr(85), + ImageGCLowThreshold: ptr(80), + }, + }, wantErr: "max pods must be less than or equal to", }, { name: "negative image GC high threshold", goal: GoalState{ KubernetesVersion: "1.35.1", - MaxPods: 110, + MaxPods: ptr(110), KubeletConfig: KubeletConfig{ - ImageGCHighThreshold: -1, + ImageGCHighThreshold: ptr(-1), + ImageGCLowThreshold: ptr(80), }, }, wantErr: "image GC high threshold must be non-negative", @@ -272,9 +292,10 @@ func TestGoalStateValidate(t *testing.T) { name: "negative image GC low threshold", goal: GoalState{ KubernetesVersion: "1.35.1", - MaxPods: 110, + MaxPods: ptr(110), KubeletConfig: KubeletConfig{ - ImageGCLowThreshold: -1, + ImageGCHighThreshold: ptr(85), + ImageGCLowThreshold: ptr(-1), }, }, wantErr: "image GC low threshold must be non-negative", @@ -327,13 +348,14 @@ func TestMachineFromARM(t *testing.T) { if machine.Goal.KubernetesVersion != "1.35.1" || machine.Goal.SettingsVersion != "settings-42" { t.Fatalf("goal versions = %#v", machine.Goal) } - if machine.Goal.MaxPods != 42 || machine.Goal.NodeLabels["workload"] != "flex" { + if machine.Goal.MaxPods == nil || *machine.Goal.MaxPods != 42 || machine.Goal.NodeLabels["workload"] != "flex" { t.Fatalf("goal settings = %#v", machine.Goal) } if len(machine.Goal.NodeTaints) != 1 || machine.Goal.NodeTaints[0] != "dedicated=flex:NoSchedule" { t.Fatalf("goal taints = %#v", machine.Goal.NodeTaints) } - if machine.Goal.KubeletConfig.ImageGCHighThreshold != 85 || machine.Goal.KubeletConfig.ImageGCLowThreshold != 80 { + if machine.Goal.KubeletConfig.ImageGCHighThreshold == nil || *machine.Goal.KubeletConfig.ImageGCHighThreshold != 85 || + machine.Goal.KubeletConfig.ImageGCLowThreshold == nil || *machine.Goal.KubeletConfig.ImageGCLowThreshold != 80 { t.Fatalf("kubelet config = %#v", machine.Goal.KubeletConfig) } if machine.Status.ProvisioningState != ProvisioningStateSucceeded { @@ -424,8 +446,8 @@ func TestMachineFromARMResolvesMinorVersionAlias(t *testing.T) { Properties: &armcontainerservice.MachineProperties{ ETag: ptr("42"), Kubernetes: &armcontainerservice.MachineKubernetesProfile{ - OrchestratorVersion: ptr("1.35"), - CurrentOrchestratorVersion: ptr("1.35.2"), + OrchestratorVersion: ptr(" v1.35 "), + CurrentOrchestratorVersion: ptr(" v1.35.2 "), }, }, }, "", "") diff --git a/pkg/aksmachine/client_incluster.go b/pkg/aksmachine/client_incluster.go index 23d6c2fc..42fddbc0 100644 --- a/pkg/aksmachine/client_incluster.go +++ b/pkg/aksmachine/client_incluster.go @@ -105,6 +105,9 @@ func clusterEndpointBaseURL(restCfg *rest.Config, endpointURL string) (*url.URL, } func (c *clusterEndpointClient) Create(ctx context.Context, desired GoalState) (*Machine, error) { + if err := desired.Validate(); err != nil { + return nil, fmt.Errorf("validate goal state: %w", err) + } requestURL := c.machineURL(c.nodeName) payload := armcontainerservice.Machine{ Properties: &armcontainerservice.MachineProperties{ diff --git a/pkg/aksmachine/client_incluster_test.go b/pkg/aksmachine/client_incluster_test.go index a950110b..f1244f53 100644 --- a/pkg/aksmachine/client_incluster_test.go +++ b/pkg/aksmachine/client_incluster_test.go @@ -54,7 +54,7 @@ func TestClusterEndpointClientGet(t *testing.T) { if machine.Goal.KubernetesVersion != "1.34.0" || machine.Goal.SettingsVersion != "42" { t.Fatalf("goal = %#v", machine.Goal) } - if machine.Goal.MaxPods != 42 || machine.Goal.NodeLabels["workload"] != "flex" || len(machine.Goal.NodeTaints) != 1 { + if machine.Goal.MaxPods == nil || *machine.Goal.MaxPods != 42 || machine.Goal.NodeLabels["workload"] != "flex" || len(machine.Goal.NodeTaints) != 1 { t.Fatalf("extended goal = %#v", machine.Goal) } if machine.Status.ProvisioningState != ProvisioningStateSucceeded { diff --git a/pkg/aksmachine/ensure_test.go b/pkg/aksmachine/ensure_test.go index 65b30a7d..a8774ee0 100644 --- a/pkg/aksmachine/ensure_test.go +++ b/pkg/aksmachine/ensure_test.go @@ -86,7 +86,7 @@ func TestEnsureMachineCreatesAndAdoptsSettingsVersion(t *testing.T) { goal := testGoal("1.35.1", "") createdGoal := testGoal("1.35.1", "etag-created") - createdGoal.MaxPods = 42 + createdGoal.MaxPods = ptr(42) client := &ensureMachineClient{createResult: &Machine{Goal: createdGoal}} task := EnsureMachine(client, &goal, true, slog.New(slog.NewTextHandler(io.Discard, nil))) @@ -99,8 +99,8 @@ func TestEnsureMachineCreatesAndAdoptsSettingsVersion(t *testing.T) { if goal.SettingsVersion != "etag-created" { t.Fatalf("SettingsVersion = %q, want etag-created", goal.SettingsVersion) } - if goal.MaxPods != 42 { - t.Fatalf("MaxPods = %d, want server-normalized value 42", goal.MaxPods) + if goal.MaxPods == nil || *goal.MaxPods != 42 { + t.Fatalf("MaxPods = %v, want server-normalized value 42", goal.MaxPods) } } @@ -109,23 +109,23 @@ func TestEnsureMachineAdoptsExistingGoal(t *testing.T) { goal := GoalState{ KubernetesVersion: "1.35.1", - MaxPods: 30, + MaxPods: ptr(30), NodeLabels: map[string]string{"source": "local"}, NodeTaints: []string{"local=true:NoSchedule"}, KubeletConfig: KubeletConfig{ - ImageGCHighThreshold: 85, - ImageGCLowThreshold: 80, + ImageGCHighThreshold: ptr(85), + ImageGCLowThreshold: ptr(80), }, } client := &ensureMachineClient{machine: &Machine{Goal: GoalState{ KubernetesVersion: "1.35.1", SettingsVersion: "etag-42", - MaxPods: 110, + MaxPods: ptr(110), NodeLabels: map[string]string{"source": "remote"}, NodeTaints: []string{"remote=true:NoSchedule"}, KubeletConfig: KubeletConfig{ - ImageGCHighThreshold: 70, - ImageGCLowThreshold: 60, + ImageGCHighThreshold: ptr(70), + ImageGCLowThreshold: ptr(60), }, }}} task := EnsureMachine(client, &goal, true, slog.New(slog.NewTextHandler(io.Discard, nil))) @@ -139,10 +139,11 @@ func TestEnsureMachineAdoptsExistingGoal(t *testing.T) { if goal.SettingsVersion != "etag-42" { t.Fatalf("SettingsVersion = %q, want etag-42", goal.SettingsVersion) } - if goal.MaxPods != 110 || goal.NodeLabels["source"] != "remote" || goal.NodeTaints[0] != "remote=true:NoSchedule" { + if goal.MaxPods == nil || *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 != 70 || goal.KubeletConfig.ImageGCLowThreshold != 60 { + if goal.KubeletConfig.ImageGCHighThreshold == nil || *goal.KubeletConfig.ImageGCHighThreshold != 70 || + goal.KubeletConfig.ImageGCLowThreshold == nil || *goal.KubeletConfig.ImageGCLowThreshold != 60 { t.Fatalf("remote kubelet config was not adopted: %#v", goal.KubeletConfig) } } diff --git a/pkg/aksmachine/test_helpers_test.go b/pkg/aksmachine/test_helpers_test.go index ad0bb254..2a11c168 100644 --- a/pkg/aksmachine/test_helpers_test.go +++ b/pkg/aksmachine/test_helpers_test.go @@ -4,10 +4,10 @@ func testGoal(kubernetesVersion, settingsVersion string) GoalState { return GoalState{ KubernetesVersion: kubernetesVersion, SettingsVersion: settingsVersion, - MaxPods: 110, + MaxPods: ptr(110), KubeletConfig: KubeletConfig{ - ImageGCHighThreshold: 85, - ImageGCLowThreshold: 80, + ImageGCHighThreshold: ptr(85), + ImageGCLowThreshold: ptr(80), }, } } diff --git a/pkg/aksmachine/types.go b/pkg/aksmachine/types.go index b7a0c286..b31e8358 100644 --- a/pkg/aksmachine/types.go +++ b/pkg/aksmachine/types.go @@ -16,105 +16,119 @@ import ( type GoalState struct { KubernetesVersion string `json:"kubernetesVersion,omitempty"` SettingsVersion string `json:"settingsVersion,omitempty"` - MaxPods int `json:"maxPods,omitempty"` + MaxPods *int `json:"maxPods,omitempty"` NodeLabels map[string]string `json:"nodeLabels,omitempty"` NodeTaints []string `json:"nodeTaints,omitempty"` KubeletConfig KubeletConfig `json:"kubeletConfig"` } type KubeletConfig struct { - ImageGCHighThreshold int `json:"imageGCHighThreshold,omitempty"` - ImageGCLowThreshold int `json:"imageGCLowThreshold,omitempty"` + ImageGCHighThreshold *int `json:"imageGCHighThreshold,omitempty"` + ImageGCLowThreshold *int `json:"imageGCLowThreshold,omitempty"` } -// Validate verifies the values present in a goal. SettingsVersion is validated -// by Machine because local bootstrap goals do not have an ETag until persisted. +// Validate verifies that a goal is complete and can be rendered on a node. +// SettingsVersion is validated by Machine because a local bootstrap goal does +// not have an ETag until it is persisted. func (g GoalState) Validate() error { if g.KubernetesVersion == "" { return fmt.Errorf("kubernetes version is empty") } - if g.MaxPods < 0 { + if g.MaxPods == nil || *g.MaxPods == 0 { + return fmt.Errorf("max pods is empty") + } + if *g.MaxPods < 0 { return fmt.Errorf("max pods must be non-negative") } - if g.MaxPods > math.MaxInt32 { + if *g.MaxPods > math.MaxInt32 { return fmt.Errorf("max pods must be less than or equal to %d", math.MaxInt32) } - if g.KubeletConfig.ImageGCHighThreshold < 0 { + if g.KubeletConfig.ImageGCHighThreshold == nil || *g.KubeletConfig.ImageGCHighThreshold == 0 { + return fmt.Errorf("image GC high threshold is empty") + } + if *g.KubeletConfig.ImageGCHighThreshold < 0 { return fmt.Errorf("image GC high threshold must be non-negative") } - if g.KubeletConfig.ImageGCLowThreshold < 0 { + if g.KubeletConfig.ImageGCLowThreshold == nil { + return fmt.Errorf("image GC low threshold is empty") + } + if *g.KubeletConfig.ImageGCLowThreshold < 0 { return fmt.Errorf("image GC low threshold must be non-negative") } - if g.KubeletConfig.ImageGCHighThreshold > 100 { + if *g.KubeletConfig.ImageGCHighThreshold > 100 { return fmt.Errorf("image GC high threshold must be less than or equal to 100") } - if g.KubeletConfig.ImageGCLowThreshold > 100 { + if *g.KubeletConfig.ImageGCLowThreshold > 100 { return fmt.Errorf("image GC low threshold must be less than or equal to 100") } - if g.KubeletConfig.ImageGCHighThreshold != 0 && g.KubeletConfig.ImageGCLowThreshold >= g.KubeletConfig.ImageGCHighThreshold { + if *g.KubeletConfig.ImageGCLowThreshold >= *g.KubeletConfig.ImageGCHighThreshold { return fmt.Errorf("image GC low threshold must be less than image GC high threshold") } return nil } -// ValidateEffective verifies that omitted API defaults have been resolved and -// the goal has every scalar setting needed to render a node. -func (g GoalState) ValidateEffective() error { - if err := g.Validate(); err != nil { - return err - } - if g.MaxPods == 0 { - return fmt.Errorf("max pods is empty") - } - if g.KubeletConfig.ImageGCHighThreshold == 0 { - return fmt.Errorf("image GC high threshold is empty") - } - return nil -} - // GoalStateFromConfig builds and validates the initial AKS machine goal state // from local agent configuration. func GoalStateFromConfig(cfg *config.Config) (GoalState, error) { + maxPods := cfg.Node.MaxPods + imageGCHighThreshold := cfg.Node.Kubelet.ImageGCHighThreshold + imageGCLowThreshold := cfg.Node.Kubelet.ImageGCLowThreshold goal := GoalState{ KubernetesVersion: cfg.Components.Kubernetes, - MaxPods: cfg.Node.MaxPods, + MaxPods: &maxPods, NodeLabels: maps.Clone(cfg.Node.Labels), NodeTaints: slices.Clone(cfg.Node.Taints), KubeletConfig: KubeletConfig{ - ImageGCHighThreshold: cfg.Node.Kubelet.ImageGCHighThreshold, - ImageGCLowThreshold: cfg.Node.Kubelet.ImageGCLowThreshold, + ImageGCHighThreshold: &imageGCHighThreshold, + ImageGCLowThreshold: &imageGCLowThreshold, }, } - if err := goal.ValidateEffective(); err != nil { + if err := goal.Validate(); err != nil { return GoalState{}, err } return goal, nil } -func cloneGoalState(goal GoalState) GoalState { - cloned := goal - cloned.NodeLabels = maps.Clone(goal.NodeLabels) - cloned.NodeTaints = slices.Clone(goal.NodeTaints) - return cloned +// DeepCopy returns a goal whose mutable fields are independent of the source. +func (g GoalState) DeepCopy() *GoalState { + cloned := g + cloned.NodeLabels = maps.Clone(g.NodeLabels) + cloned.NodeTaints = slices.Clone(g.NodeTaints) + if g.MaxPods != nil { + value := *g.MaxPods + cloned.MaxPods = &value + } + if g.KubeletConfig.ImageGCHighThreshold != nil { + value := *g.KubeletConfig.ImageGCHighThreshold + cloned.KubeletConfig.ImageGCHighThreshold = &value + } + if g.KubeletConfig.ImageGCLowThreshold != nil { + value := *g.KubeletConfig.ImageGCLowThreshold + cloned.KubeletConfig.ImageGCLowThreshold = &value + } + return &cloned } // EffectiveGoal overlays a Machine goal on a complete local goal. AKS owns the // desired values; the local goal only fills scalar fields omitted by the API. func EffectiveGoal(machine, local GoalState) (GoalState, error) { - effective := cloneGoalState(machine) - if effective.MaxPods == 0 { - effective.MaxPods = local.MaxPods + effective := machine.DeepCopy() + if effective.MaxPods == nil && local.MaxPods != nil { + value := *local.MaxPods + effective.MaxPods = &value } - if effective.KubeletConfig.ImageGCHighThreshold == 0 { - effective.KubeletConfig.ImageGCHighThreshold = local.KubeletConfig.ImageGCHighThreshold + if effective.KubeletConfig.ImageGCHighThreshold == nil && local.KubeletConfig.ImageGCHighThreshold != nil { + value := *local.KubeletConfig.ImageGCHighThreshold + effective.KubeletConfig.ImageGCHighThreshold = &value } - if effective.KubeletConfig.ImageGCLowThreshold == 0 { - effective.KubeletConfig.ImageGCLowThreshold = local.KubeletConfig.ImageGCLowThreshold + if effective.KubeletConfig.ImageGCLowThreshold == nil && local.KubeletConfig.ImageGCLowThreshold != nil { + value := *local.KubeletConfig.ImageGCLowThreshold + effective.KubeletConfig.ImageGCLowThreshold = &value } - if err := effective.ValidateEffective(); err != nil { + if err := effective.Validate(); err != nil { return GoalState{}, fmt.Errorf("validate effective goal: %w", err) } - return effective, nil + return *effective, nil } type ProvisioningState string @@ -142,14 +156,14 @@ type Machine struct { Status Status `json:"status"` } -// Validate verifies that a Machine returned by AKS contains a goal suitable -// for bootstrap or reconciliation. +// Validate verifies the required fields in a Machine returned by AKS. Optional +// scalar settings are validated after local defaults are applied. func (m *Machine) Validate() error { if m == nil { return fmt.Errorf("machine is nil") } - if err := m.Goal.Validate(); err != nil { - return fmt.Errorf("goal: %w", err) + if m.Goal.KubernetesVersion == "" { + return fmt.Errorf("goal: kubernetes version is empty") } if m.Goal.SettingsVersion == "" { return fmt.Errorf("goal settings version is empty") diff --git a/pkg/aksmachine/types_test.go b/pkg/aksmachine/types_test.go index c8aebdf2..00067f9e 100644 --- a/pkg/aksmachine/types_test.go +++ b/pkg/aksmachine/types_test.go @@ -44,8 +44,8 @@ func TestGoalStateFromConfig(t *testing.T) { if goal.SettingsVersion != "" { t.Fatalf("SettingsVersion = %q, want empty before Machine persistence", goal.SettingsVersion) } - if goal.MaxPods != 42 { - t.Fatalf("MaxPods = %d, want 42", goal.MaxPods) + if goal.MaxPods == nil || *goal.MaxPods != 42 { + t.Fatalf("MaxPods = %v, want 42", goal.MaxPods) } if len(goal.NodeLabels) != 2 { t.Fatalf("NodeLabels length = %d, want 2", len(goal.NodeLabels)) @@ -65,11 +65,11 @@ func TestGoalStateFromConfig(t *testing.T) { if goal.NodeTaints[1] != "edge=true:NoExecute" { t.Fatalf("NodeTaints[1] = %v, want edge=true:NoExecute", goal.NodeTaints[1]) } - if goal.KubeletConfig.ImageGCHighThreshold != 85 { - t.Fatalf("ImageGCHighThreshold = %d, want 85", goal.KubeletConfig.ImageGCHighThreshold) + if goal.KubeletConfig.ImageGCHighThreshold == nil || *goal.KubeletConfig.ImageGCHighThreshold != 85 { + t.Fatalf("ImageGCHighThreshold = %v, want 85", goal.KubeletConfig.ImageGCHighThreshold) } - if goal.KubeletConfig.ImageGCLowThreshold != 80 { - t.Fatalf("ImageGCLowThreshold = %d, want 80", goal.KubeletConfig.ImageGCLowThreshold) + if goal.KubeletConfig.ImageGCLowThreshold == nil || *goal.KubeletConfig.ImageGCLowThreshold != 80 { + t.Fatalf("ImageGCLowThreshold = %v, want 80", goal.KubeletConfig.ImageGCLowThreshold) } } @@ -104,6 +104,9 @@ func TestMachineValidate(t *testing.T) { "complete machine": { machine: &Machine{Goal: testGoal("1.35.1", "42")}, }, + "omitted scalar defaults": { + machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "42"}}, + }, } for name, tt := range tests { @@ -128,11 +131,11 @@ func TestEffectiveGoal(t *testing.T) { t.Parallel() local := testGoal("1.34.0", "") - local.MaxPods = 30 + local.MaxPods = ptr(30) local.NodeLabels = map[string]string{"source": "local"} local.NodeTaints = []string{"local=true:NoSchedule"} - local.KubeletConfig.ImageGCHighThreshold = 90 - local.KubeletConfig.ImageGCLowThreshold = 75 + local.KubeletConfig.ImageGCHighThreshold = ptr(90) + local.KubeletConfig.ImageGCLowThreshold = ptr(75) machine := GoalState{ KubernetesVersion: "1.35.0", SettingsVersion: "42", @@ -144,13 +147,14 @@ func TestEffectiveGoal(t *testing.T) { if err != nil { t.Fatalf("EffectiveGoal() error = %v", err) } - if effective.KubernetesVersion != "1.35.0" || effective.SettingsVersion != "42" || effective.MaxPods != 30 { + if effective.KubernetesVersion != "1.35.0" || effective.SettingsVersion != "42" || effective.MaxPods == nil || *effective.MaxPods != 30 { t.Fatalf("effective versions/maxPods = %#v", effective) } if len(effective.NodeLabels) != 0 || len(effective.NodeTaints) != 0 { t.Fatalf("effective collections = %#v, want authoritative empty collections", effective) } - if effective.KubeletConfig.ImageGCHighThreshold != 90 || effective.KubeletConfig.ImageGCLowThreshold != 75 { + if effective.KubeletConfig.ImageGCHighThreshold == nil || *effective.KubeletConfig.ImageGCHighThreshold != 90 || + effective.KubeletConfig.ImageGCLowThreshold == nil || *effective.KubeletConfig.ImageGCLowThreshold != 75 { t.Fatalf("effective kubelet config = %#v", effective.KubeletConfig) } @@ -158,4 +162,30 @@ func TestEffectiveGoal(t *testing.T) { if _, ok := machine.NodeLabels["source"]; ok { t.Fatal("EffectiveGoal returned Machine-owned label map") } + *effective.MaxPods = 50 + *effective.KubeletConfig.ImageGCHighThreshold = 80 + if *local.MaxPods != 30 || *local.KubeletConfig.ImageGCHighThreshold != 90 { + t.Fatal("EffectiveGoal returned local-owned scalar pointers") + } +} + +func TestEffectiveGoalPreservesExplicitZero(t *testing.T) { + t.Parallel() + + local := testGoal("1.34.0", "") + machine := GoalState{ + KubernetesVersion: "1.35.0", + SettingsVersion: "42", + KubeletConfig: KubeletConfig{ + ImageGCLowThreshold: ptr(0), + }, + } + + effective, err := EffectiveGoal(machine, local) + if err != nil { + t.Fatalf("EffectiveGoal() error = %v", err) + } + if effective.KubeletConfig.ImageGCLowThreshold == nil || *effective.KubeletConfig.ImageGCLowThreshold != 0 { + t.Fatalf("ImageGCLowThreshold = %v, want explicit zero", effective.KubeletConfig.ImageGCLowThreshold) + } } diff --git a/pkg/daemon/goalstate.go b/pkg/daemon/goal_state.go similarity index 67% rename from pkg/daemon/goalstate.go rename to pkg/daemon/goal_state.go index 76e9f5b4..ae49c3bc 100644 --- a/pkg/daemon/goalstate.go +++ b/pkg/daemon/goal_state.go @@ -27,16 +27,29 @@ func ResolveMachineGoalState( if err != nil { return nil, nil, nil, err } + return resolveEffectiveMachineGoalState(ctx, log, cfg, machineName, *effectiveGoal) +} + +func resolveEffectiveMachineGoalState( + ctx context.Context, + log *slog.Logger, + cfg *config.Config, + machineName string, + goal aksmachine.GoalState, +) (*agentconfig.AgentConfig, *goalstates.MachineGoalState, *goalstates.ContainerImageArchiveStaging, error) { + if err := goal.Validate(); err != nil { + return nil, nil, nil, fmt.Errorf("validate machine goal: %w", err) + } resolvedConfig := cfg.DeepCopy() if resolvedConfig == nil { return nil, nil, nil, fmt.Errorf("copy config for machine goal") } - resolvedConfig.Components.Kubernetes = effectiveGoal.KubernetesVersion - resolvedConfig.Node.MaxPods = effectiveGoal.MaxPods - resolvedConfig.Node.Labels = maps.Clone(effectiveGoal.NodeLabels) - resolvedConfig.Node.Taints = slices.Clone(effectiveGoal.NodeTaints) - resolvedConfig.Node.Kubelet.ImageGCHighThreshold = effectiveGoal.KubeletConfig.ImageGCHighThreshold - resolvedConfig.Node.Kubelet.ImageGCLowThreshold = effectiveGoal.KubeletConfig.ImageGCLowThreshold + resolvedConfig.Components.Kubernetes = goal.KubernetesVersion + resolvedConfig.Node.MaxPods = *goal.MaxPods + resolvedConfig.Node.Labels = maps.Clone(goal.NodeLabels) + resolvedConfig.Node.Taints = slices.Clone(goal.NodeTaints) + resolvedConfig.Node.Kubelet.ImageGCHighThreshold = *goal.KubeletConfig.ImageGCHighThreshold + resolvedConfig.Node.Kubelet.ImageGCLowThreshold = *goal.KubeletConfig.ImageGCLowThreshold return config.ResolveMachineGoalState(ctx, log, resolvedConfig, machineName) } @@ -57,8 +70,8 @@ func effectiveMachineGoal(cfg *config.Config, goal *aksmachine.GoalState) (*aksm func goalForRestart(cfg *config.Config, state *State) (*aksmachine.GoalState, error) { if state != nil && state.AppliedGoal != nil { - goal := cloneGoalState(*state.AppliedGoal) - if err := goal.ValidateEffective(); err != nil { + goal := state.AppliedGoal.DeepCopy() + if err := goal.Validate(); err != nil { return nil, fmt.Errorf("validate persisted restart goal: %w", err) } return goal, nil @@ -74,7 +87,7 @@ func goalForRestart(cfg *config.Config, state *State) (*aksmachine.GoalState, er goal.KubernetesVersion = state.AppliedKubernetesVersion } } - if err := goal.ValidateEffective(); err != nil { + if err := goal.Validate(); err != nil { return nil, fmt.Errorf("validate legacy restart goal: %w", err) } return &goal, nil diff --git a/pkg/daemon/goalstate_test.go b/pkg/daemon/goal_state_test.go similarity index 89% rename from pkg/daemon/goalstate_test.go rename to pkg/daemon/goal_state_test.go index c50b4dba..16645a3e 100644 --- a/pkg/daemon/goalstate_test.go +++ b/pkg/daemon/goal_state_test.go @@ -22,11 +22,11 @@ func TestResolveMachineGoalStateUsesCompleteMachineGoal(t *testing.T) { }, } goal := testMachineGoal("1.35.1", "42") - goal.MaxPods = 50 + goal.MaxPods = intPointer(50) goal.NodeLabels = map[string]string{"source": "machine"} goal.NodeTaints = []string{"machine=true:NoExecute"} - goal.KubeletConfig.ImageGCHighThreshold = 70 - goal.KubeletConfig.ImageGCLowThreshold = 60 + goal.KubeletConfig.ImageGCHighThreshold = intPointer(70) + goal.KubeletConfig.ImageGCLowThreshold = intPointer(60) agentCfg, _, _, err := ResolveMachineGoalState(t.Context(), slog.Default(), cfg, "kube1", &goal) if err != nil { @@ -73,13 +73,14 @@ func TestGoalForRestartLegacyStatePreservesConfigSettings(t *testing.T) { if err != nil { t.Fatalf("goalForRestart: %v", err) } - if goal.KubernetesVersion != "1.35.1" || goal.SettingsVersion != "42" || goal.MaxPods != 30 { + if goal.KubernetesVersion != "1.35.1" || goal.SettingsVersion != "42" || goal.MaxPods == nil || *goal.MaxPods != 30 { t.Fatalf("goal versions/maxPods = %#v", goal) } if !maps.Equal(goal.NodeLabels, cfg.Node.Labels) || len(goal.NodeTaints) != 1 || goal.NodeTaints[0] != cfg.Node.Taints[0] { t.Fatalf("legacy restart goal lost config settings: %#v", goal) } - if goal.KubeletConfig.ImageGCHighThreshold != 90 || goal.KubeletConfig.ImageGCLowThreshold != 75 { + if goal.KubeletConfig.ImageGCHighThreshold == nil || *goal.KubeletConfig.ImageGCHighThreshold != 90 || + goal.KubeletConfig.ImageGCLowThreshold == nil || *goal.KubeletConfig.ImageGCLowThreshold != 75 { t.Fatalf("legacy restart kubelet config = %#v", goal.KubeletConfig) } } diff --git a/pkg/daemon/nodeoperator.go b/pkg/daemon/nodeoperator.go index 4230142b..18306f24 100644 --- a/pkg/daemon/nodeoperator.go +++ b/pkg/daemon/nodeoperator.go @@ -42,7 +42,7 @@ func (o *nspawnNodeOperator) RestartNode(ctx context.Context, log *slog.Logger) if err != nil { return err } - _, gs, containerImageArchives, err := ResolveMachineGoalState(ctx, log, o.cfg, active.Name, goal) + _, gs, containerImageArchives, err := resolveEffectiveMachineGoalState(ctx, log, o.cfg, active.Name, *goal) if err != nil { return fmt.Errorf("resolve goal state for node restart: %w", err) } @@ -86,7 +86,7 @@ func (o *nspawnNodeOperator) ApplyGoalState(ctx context.Context, log *slog.Logge if err != nil { return nil, err } - cfg, err := o.configForGoalState(ctx, log, goal) + cfg, err := o.configForRepave(ctx, log) if err != nil { return nil, err } @@ -103,7 +103,7 @@ func (o *nspawnNodeOperator) ApplyGoalState(ctx context.Context, log *slog.Logge "kubernetesVersion", effectiveGoal.KubernetesVersion, ) - _, gs, containerImageArchives, err := ResolveMachineGoalState(ctx, log, cfg, newMachine, effectiveGoal) + _, gs, containerImageArchives, err := resolveEffectiveMachineGoalState(ctx, log, cfg, newMachine, *effectiveGoal) if err != nil { return nil, fmt.Errorf("resolve goal state for repave: %w", err) } @@ -121,7 +121,7 @@ func (o *nspawnNodeOperator) ApplyGoalState(ctx context.Context, log *slog.Logge return newState, nil } -func (o *nspawnNodeOperator) configForGoalState(ctx context.Context, log *slog.Logger, goal aksmachine.GoalState) (*config.Config, error) { +func (o *nspawnNodeOperator) configForRepave(ctx context.Context, log *slog.Logger) (*config.Config, error) { // Keep short-lived bootstrap credentials scoped to this repave. Persisting // them would make a later repave depend on this token's lifetime again. cfg := o.cfg.DeepCopy() @@ -161,11 +161,11 @@ func (o *nspawnNodeOperator) StopDaemon(ctx context.Context, log *slog.Logger) e func nextAppliedState(current *State, goal aksmachine.GoalState, active *activeMachine) *State { next := &State{ - AppliedGoal: cloneGoalState(goal), + AppliedGoal: goal.DeepCopy(), } if current != nil { if current.AppliedGoal != nil { - next.PreviousAppliedGoal = cloneGoalState(*current.AppliedGoal) + next.PreviousAppliedGoal = current.AppliedGoal.DeepCopy() } else if current.AppliedKubernetesVersion != "" { next.PreviousSettingsVersion = current.AppliedSettingsVersion next.PreviousKubernetesVersion = current.AppliedKubernetesVersion diff --git a/pkg/daemon/nodeoperator_test.go b/pkg/daemon/nodeoperator_test.go index f5182138..aba215d7 100644 --- a/pkg/daemon/nodeoperator_test.go +++ b/pkg/daemon/nodeoperator_test.go @@ -9,7 +9,6 @@ import ( "strings" "testing" - "github.com/Azure/AKSFlexNode/pkg/aksmachine" "github.com/Azure/AKSFlexNode/pkg/bootstrapdata" "github.com/Azure/AKSFlexNode/pkg/config" "github.com/Azure/unbounded/pkg/agent/goalstates" @@ -65,7 +64,7 @@ func TestFindActiveMachine(t *testing.T) { } } -func TestConfigForGoalStateRefreshesBootstrapData(t *testing.T) { +func TestConfigForRepaveRefreshesBootstrapData(t *testing.T) { t.Parallel() cfg := &config.Config{ @@ -88,9 +87,9 @@ func TestConfigForGoalStateRefreshesBootstrapData(t *testing.T) { var logs bytes.Buffer log := slog.New(slog.NewTextHandler(&logs, nil)) - got, err := operator.configForGoalState(t.Context(), log, aksmachine.GoalState{KubernetesVersion: "1.36.2"}) + got, err := operator.configForRepave(t.Context(), log) if err != nil { - t.Fatalf("configForGoalState() error = %v", err) + t.Fatalf("configForRepave() error = %v", err) } if refresher.calls != 1 { t.Fatalf("refresh calls = %d, want 1", refresher.calls) @@ -126,7 +125,7 @@ func TestConfigForGoalStateRefreshesBootstrapData(t *testing.T) { } } -func TestConfigForGoalStateSkipsBootstrapDataRefreshWithoutBothAuthTypes(t *testing.T) { +func TestConfigForRepaveSkipsBootstrapDataRefreshWithoutBothAuthTypes(t *testing.T) { t.Parallel() tests := map[string]*config.Config{ @@ -142,8 +141,8 @@ func TestConfigForGoalStateSkipsBootstrapDataRefreshWithoutBothAuthTypes(t *test t.Parallel() refresher := bootstrapDataRefresherForConfig(cfg) operator := &nspawnNodeOperator{cfg: cfg, bootstrapDataRefresher: refresher} - if _, err := operator.configForGoalState(t.Context(), discardLogger(), aksmachine.GoalState{}); err != nil { - t.Fatalf("configForGoalState() error = %v", err) + if _, err := operator.configForRepave(t.Context(), discardLogger()); err != nil { + t.Fatalf("configForRepave() error = %v", err) } if _, ok := refresher.(noopBootstrapDataRefresher); !ok { t.Fatalf("refresher = %T, want noopBootstrapDataRefresher", refresher) @@ -165,7 +164,7 @@ func TestBootstrapDataRefresherForDualAuthConfig(t *testing.T) { } } -func TestConfigForGoalStateBootstrapDataRefreshFailure(t *testing.T) { +func TestConfigForRepaveBootstrapDataRefreshFailure(t *testing.T) { t.Parallel() cfg := &config.Config{Azure: config.AzureConfig{ @@ -176,9 +175,9 @@ func TestConfigForGoalStateBootstrapDataRefreshFailure(t *testing.T) { cfg: cfg, bootstrapDataRefresher: &fakeBootstrapDataRefresher{err: errors.New("ARM unavailable")}, } - _, err := operator.configForGoalState(t.Context(), discardLogger(), aksmachine.GoalState{}) + _, err := operator.configForRepave(t.Context(), discardLogger()) if err == nil || !errors.Is(err, operator.bootstrapDataRefresher.(*fakeBootstrapDataRefresher).err) { - t.Fatalf("configForGoalState() error = %v", err) + t.Fatalf("configForRepave() error = %v", err) } if cfg.Azure.BootstrapToken.Token != "oldtok.0123456789abcdef" { t.Fatal("original config bootstrap token was mutated") diff --git a/pkg/daemon/reconcile_test.go b/pkg/daemon/reconcile_test.go index b5e4d4fe..f59363a8 100644 --- a/pkg/daemon/reconcile_test.go +++ b/pkg/daemon/reconcile_test.go @@ -13,7 +13,7 @@ func TestDecide(t *testing.T) { goal := testMachineGoal("1.34.0", "42") machine := machineSnapshot{machine: &aksmachine.Machine{Goal: goal}} - applied := &State{AppliedGoal: cloneGoalState(goal)} + applied := &State{AppliedGoal: goal.DeepCopy()} staleGoal := testMachineGoal("1.33.0", "41") stale := &State{AppliedGoal: &staleGoal} node := nodeSnapshot{node: &corev1.Node{}} diff --git a/pkg/daemon/state.go b/pkg/daemon/state.go index 28e3cbf0..31ac76dd 100644 --- a/pkg/daemon/state.go +++ b/pkg/daemon/state.go @@ -7,10 +7,8 @@ import ( "encoding/json" "errors" "fmt" - "maps" "os" "path/filepath" - "slices" "strings" "github.com/Azure/AKSFlexNode/pkg/aksmachine" @@ -49,12 +47,12 @@ func (s *State) validate() error { return fmt.Errorf("daemon state applied goal is missing") } if s.AppliedGoal != nil { - if err := s.AppliedGoal.ValidateEffective(); err != nil { + if err := s.AppliedGoal.Validate(); err != nil { return fmt.Errorf("daemon state applied goal: %w", err) } } if s.PreviousAppliedGoal != nil { - if err := s.PreviousAppliedGoal.ValidateEffective(); err != nil { + if err := s.PreviousAppliedGoal.Validate(); err != nil { return fmt.Errorf("daemon state previous applied goal: %w", err) } } @@ -86,18 +84,11 @@ func (t *saveStateTask) Do(ctx context.Context) error { } func SeededState(goal aksmachine.GoalState) *State { - state := &State{AppliedGoal: cloneGoalState(goal), ActiveMachine: goalstates.NSpawnMachineKube1} + state := &State{AppliedGoal: goal.DeepCopy(), ActiveMachine: goalstates.NSpawnMachineKube1} state.populateLegacyFields() return state } -func cloneGoalState(goal aksmachine.GoalState) *aksmachine.GoalState { - cloned := goal - cloned.NodeLabels = maps.Clone(goal.NodeLabels) - cloned.NodeTaints = slices.Clone(goal.NodeTaints) - return &cloned -} - func (s *State) populateLegacyFields() { if s.AppliedGoal != nil { s.AppliedSettingsVersion = s.AppliedGoal.SettingsVersion diff --git a/pkg/daemon/state_test.go b/pkg/daemon/state_test.go index 7259be08..a859d0f5 100644 --- a/pkg/daemon/state_test.go +++ b/pkg/daemon/state_test.go @@ -17,8 +17,8 @@ func TestFileStateStoreSaveLoad(t *testing.T) { t.Fatalf("newFileStateStore: %v", err) } want := &State{ - AppliedGoal: cloneGoalState(testMachineGoal("1.34.0", "42")), - PreviousAppliedGoal: cloneGoalState(testMachineGoal("1.33.0", "41")), + AppliedGoal: testMachineGoal("1.34.0", "42").DeepCopy(), + PreviousAppliedGoal: testMachineGoal("1.33.0", "41").DeepCopy(), ActiveMachine: "kube2", } want.AppliedGoal.NodeLabels = map[string]string{"workload": "flex"} diff --git a/pkg/daemon/test_helpers_test.go b/pkg/daemon/test_helpers_test.go index 8ba5b7fc..783b724f 100644 --- a/pkg/daemon/test_helpers_test.go +++ b/pkg/daemon/test_helpers_test.go @@ -6,10 +6,14 @@ func testMachineGoal(kubernetesVersion, settingsVersion string) aksmachine.GoalS return aksmachine.GoalState{ KubernetesVersion: kubernetesVersion, SettingsVersion: settingsVersion, - MaxPods: 110, + MaxPods: intPointer(110), KubeletConfig: aksmachine.KubeletConfig{ - ImageGCHighThreshold: 85, - ImageGCLowThreshold: 80, + ImageGCHighThreshold: intPointer(85), + ImageGCLowThreshold: intPointer(80), }, } } + +func intPointer(value int) *int { + return &value +} From 143ad93cdbd6983f5dabacef7ea38537d15c64af Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Mon, 17 Aug 2026 21:44:41 -0700 Subject: [PATCH 7/7] Acknowledge RP-applied Machine metadata --- docs/design/agent-and-aks.md | 8 +- docs/design/in-cluster-machine.md | 7 +- pkg/daemon/nodeoperator.go | 13 ++ pkg/daemon/nodeoperator_test.go | 64 +++++++++- pkg/daemon/reconcile_test.go | 182 +++++++++++++++++++++++++++ pkg/daemon/repave_reconciler.go | 151 ++++++++++++++++++++++ pkg/daemon/repave_reconciler_test.go | 158 +++++++++++++++++++++-- 7 files changed, 564 insertions(+), 19 deletions(-) diff --git a/docs/design/agent-and-aks.md b/docs/design/agent-and-aks.md index f1119ff3..1d44c1de 100644 --- a/docs/design/agent-and-aks.md +++ b/docs/design/agent-and-aks.md @@ -90,7 +90,7 @@ ARM machine creation must be idempotent. If AKS RP retries creation and the ARM The Flex Node agent reconciles two external signals. -The ARM machine resource provides desired settings and a version for those settings. The current minimal settings are desired Kubernetes version and settings version. The agent compares the settings version from ARM with its locally applied settings version to detect drift. Future schema extensions can add more settings, but the agent should treat the ARM machine resource as the source of truth for host/nspawn reconciliation. +The ARM machine resource provides desired Kubernetes version, max pods, custom labels and taints, kubelet image-GC settings, and a version for those settings. The agent compares the settings version from ARM with its locally applied settings version to detect drift. Future schema extensions can add more settings, but the agent should treat the ARM machine resource as the source of truth for host/nspawn reconciliation. The ARM machine resource does not own the nspawn side. Selecting `kube1` or `kube2` is an internal host implementation detail used by the agent to apply settings atomically. @@ -130,14 +130,14 @@ flowchart TD ## Current Repave Implementation -AKS Flex Node no longer runs a standalone local drift detector. Desired node settings come from an AKS machine resource. The agent compares the desired machine goal with locally persisted daemon state and repaves the nspawn-backed worker when Kubernetes `Node` deletion indicates AKS has approved replacement. +AKS Flex Node no longer runs a standalone local drift detector. Desired node settings come from an AKS machine resource. The agent compares the desired machine goal with locally persisted daemon state. It acknowledges label- and taint-only updates that AKS RP already reconciled onto the Kubernetes `Node`; other changes repave the nspawn-backed worker after `Node` deletion indicates AKS has approved replacement. The current machine goal comes from the ARM machine model: - `properties.kubernetes` contains the desired Kubernetes version and node settings. - `properties.eTag` is exposed internally as the settings version. -The ETag is the drift key. If it differs from the locally applied ETag, the agent waits for the Kubernetes `Node` object to disappear before mutating host state. Status-only updates must not change the ETag. +The ETag is the drift key. If it differs from the locally applied ETag, the agent first checks whether only labels or taints changed and whether the existing Kubernetes `Node` reflects that complete delta. A matching Node lets the agent persist the new goal without host mutation. Otherwise, it waits for the `Node` object to disappear before mutating host state. Status-only updates must not change the ETag. The daemon uses two inputs: @@ -170,7 +170,7 @@ This keeps scheduling and disruption decisions outside the agent. AKS RP, an ope 9. Start node-problem-detector inside the new side. 10. Clean up the old side's nspawn artifacts. -After successful repave, the daemon patches machine status and persists the applied goal locally. +After successful repave or in-place acknowledgement, the daemon persists the applied goal locally and reports status through the selected machine client. Direct ARM Machine status is currently read-only, so that client skips the status mutation while retaining local convergence. AKS Flex Node uses two local nspawn machine names: diff --git a/docs/design/in-cluster-machine.md b/docs/design/in-cluster-machine.md index f5ce53bb..ca89456c 100644 --- a/docs/design/in-cluster-machine.md +++ b/docs/design/in-cluster-machine.md @@ -48,8 +48,11 @@ After bootstrap, the remote machine is authoritative: 2. `NewMachineClient` receives that REST config and selects the in-cluster backend. 3. The client periodically reads the ARM-compatible machine through the service-proxy endpoint. 4. The daemon compares `properties.eTag` with its locally applied settings version. -5. A changed ETag represents a new goal. The daemon waits for the Kubernetes `Node` deletion signal before applying it. -6. Reconciliation status is sent to the endpoint's `/status` subresource without changing the ETag. +5. A changed ETag represents a new goal. If only labels or taints changed and AKS RP already reconciled them onto the existing Kubernetes `Node`, the daemon acknowledges the observed goal without mutating or repaving the node. +6. Other goal changes wait for the Kubernetes `Node` deletion signal before the daemon applies them through blue-green repave. +7. Reconciliation status is sent to the endpoint's `/status` subresource without changing the ETag. + +Direct ARM Machine status is currently read-only. In that mode, acknowledgement still advances the local applied goal and ETag, while the status mutation is skipped by the ARM client. ## Request path diff --git a/pkg/daemon/nodeoperator.go b/pkg/daemon/nodeoperator.go index 18306f24..96483a76 100644 --- a/pkg/daemon/nodeoperator.go +++ b/pkg/daemon/nodeoperator.go @@ -23,6 +23,7 @@ type activeMachine struct { type nodeOperator interface { LoadState(ctx context.Context) (*State, error) ApplyGoalState(ctx context.Context, log *slog.Logger, goal aksmachine.GoalState) (*State, error) + AcknowledgeGoalState(ctx context.Context, goal aksmachine.GoalState) (*State, error) RestartNode(ctx context.Context, log *slog.Logger) error // ResetNode removes nspawn node runtime and persisted daemon state but must // not stop this daemon process. The controller publishes lifecycle completion @@ -151,6 +152,18 @@ func (o *nspawnNodeOperator) configForRepave(ctx context.Context, log *slog.Logg return cfg, nil } +func (o *nspawnNodeOperator) AcknowledgeGoalState(ctx context.Context, goal aksmachine.GoalState) (*State, error) { + active, err := o.findActiveMachine(ctx) + if err != nil { + return nil, err + } + newState := nextAppliedState(active.State, goal, &activeMachine{Name: active.Name}) + if err := o.state.Save(ctx, newState); err != nil { + return nil, fmt.Errorf("save acknowledged machine goal state: %w", err) + } + return newState, nil +} + func (o *nspawnNodeOperator) ResetNode(ctx context.Context, log *slog.Logger) error { return phases.ExecuteTask(ctx, log, ResetNode(log)) } diff --git a/pkg/daemon/nodeoperator_test.go b/pkg/daemon/nodeoperator_test.go index aba215d7..6d395efe 100644 --- a/pkg/daemon/nodeoperator_test.go +++ b/pkg/daemon/nodeoperator_test.go @@ -295,15 +295,75 @@ func TestNextAppliedStateRotatesCompleteGoals(t *testing.T) { } } +func TestAcknowledgeGoalState(t *testing.T) { + t.Parallel() + + appliedGoal := testMachineGoal("1.34.0", "41") + appliedGoal.NodeLabels = map[string]string{"source": "old"} + appliedGoal.NodeTaints = []string{"source=old:NoSchedule"} + store := &testStateStore{state: &State{AppliedGoal: &appliedGoal, ActiveMachine: goalstates.NSpawnMachineKube1}} + operator := &nspawnNodeOperator{state: store} + desiredGoal := testMachineGoal("1.34.0", "42") + desiredGoal.NodeLabels = map[string]string{"source": "new"} + desiredGoal.NodeTaints = []string{"source=new:NoExecute"} + + got, err := operator.AcknowledgeGoalState(t.Context(), desiredGoal) + if err != nil { + t.Fatalf("AcknowledgeGoalState: %v", err) + } + if store.state != got { + t.Fatal("acknowledged state was not persisted") + } + if got.ActiveMachine != goalstates.NSpawnMachineKube1 || got.AppliedGoal == nil || got.AppliedGoal.SettingsVersion != "42" { + t.Fatalf("state = %#v", got) + } + if got.PreviousAppliedGoal == nil || got.PreviousAppliedGoal.SettingsVersion != "41" { + t.Fatalf("PreviousAppliedGoal = %#v, want settings version 41", got.PreviousAppliedGoal) + } + if got.AppliedSettingsVersion != "42" || got.PreviousSettingsVersion != "41" { + t.Fatalf("legacy projections = %#v", got) + } + + desiredGoal.NodeLabels["source"] = "mutated" + desiredGoal.NodeTaints[0] = "mutated=true:NoSchedule" + appliedGoal.NodeLabels["source"] = "mutated" + if got.AppliedGoal.NodeLabels["source"] != "new" || got.AppliedGoal.NodeTaints[0] != "source=new:NoExecute" || got.PreviousAppliedGoal.NodeLabels["source"] != "old" { + t.Fatal("AcknowledgeGoalState retained caller-owned collections") + } +} + +func TestAcknowledgeGoalStateSaveFailure(t *testing.T) { + t.Parallel() + + saveErr := errors.New("save failed") + appliedGoal := testMachineGoal("1.34.0", "41") + oldState := &State{AppliedGoal: &appliedGoal, ActiveMachine: goalstates.NSpawnMachineKube1} + store := &testStateStore{state: oldState, saveErr: saveErr} + operator := &nspawnNodeOperator{state: store} + + _, err := operator.AcknowledgeGoalState(t.Context(), testMachineGoal("1.34.0", "42")) + if err == nil || !errors.Is(err, saveErr) { + t.Fatalf("AcknowledgeGoalState error = %v, want save failure", err) + } + if store.state != oldState { + t.Fatal("failed acknowledgement replaced persisted state") + } +} + type testStateStore struct { - state *State + state *State + saveErr error } func (s *testStateStore) Load(context.Context) (*State, error) { return s.state, nil } -func (s *testStateStore) Save(context.Context, *State) error { +func (s *testStateStore) Save(_ context.Context, state *State) error { + if s.saveErr != nil { + return s.saveErr + } + s.state = state return nil } diff --git a/pkg/daemon/reconcile_test.go b/pkg/daemon/reconcile_test.go index f59363a8..a47c7b9f 100644 --- a/pkg/daemon/reconcile_test.go +++ b/pkg/daemon/reconcile_test.go @@ -1,9 +1,11 @@ package daemon import ( + "maps" "testing" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/Azure/AKSFlexNode/pkg/aksmachine" ) @@ -19,6 +21,20 @@ func TestDecide(t *testing.T) { node := nodeSnapshot{node: &corev1.Node{}} missingNode := nodeSnapshot{} deleteNode := nodeSnapshot{node: &corev1.Node{Spec: corev1.NodeSpec{Taints: []corev1.Taint{deletionTaint()}}}} + inPlaceAppliedGoal := testMachineGoal("1.34.0", "41") + inPlaceAppliedGoal.NodeLabels = map[string]string{"workload": "old", "removed": "true"} + inPlaceAppliedGoal.NodeTaints = []string{"dedicated=old:NoSchedule", "removed=true:NoExecute"} + inPlaceGoal := testMachineGoal("1.34.0", "42") + inPlaceGoal.NodeLabels = map[string]string{"workload": "new"} + inPlaceGoal.NodeTaints = []string{"dedicated=new:NoSchedule"} + inPlaceMachine := machineSnapshot{machine: &aksmachine.Machine{Goal: inPlaceGoal}} + inPlaceState := &State{AppliedGoal: &inPlaceAppliedGoal} + inPlaceNode := nodeSnapshot{node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"workload": "new"}}, + Spec: corev1.NodeSpec{Taints: []corev1.Taint{ + {Key: "dedicated", Value: "new", Effect: corev1.TaintEffectNoSchedule}, + }}, + }} tests := map[string]struct { machine machineSnapshot @@ -74,6 +90,12 @@ func TestDecide(t *testing.T) { state: nil, want: decisionWaitForNodeSignal, }, + "present node acknowledges RP-applied labels and taints": { + machine: inPlaceMachine, + node: inPlaceNode, + state: inPlaceState, + want: decisionAcknowledgeGoalState, + }, } for name, tt := range tests { @@ -88,6 +110,166 @@ func TestDecide(t *testing.T) { } } +func TestGoalForInPlaceAcknowledgement(t *testing.T) { + t.Parallel() + + newFixture := func() (aksmachine.GoalState, *State, *corev1.Node) { + appliedGoal := testMachineGoal("1.34.0", "41") + appliedGoal.NodeLabels = map[string]string{"workload": "old", "removed": "true"} + appliedGoal.NodeTaints = []string{"dedicated=old:NoSchedule", "removed=true:NoExecute"} + desiredGoal := testMachineGoal("1.34.0", "42") + desiredGoal.NodeLabels = map[string]string{"workload": "new", "empty": ""} + desiredGoal.NodeTaints = []string{"dedicated=new:NoSchedule", "empty:NoSchedule"} + now := metav1.Now() + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "workload": "new", + "empty": "", + "kubernetes.io/hostname": "node1", + "kubernetes.azure.com/managed": "false", + "kubernetes.azure.com/agentpool": "flexpool", + "kubernetes.azure.com/mode": "user", + "kubernetes.azure.com/nodepool-type": "FlexNodes", + }}, + Spec: corev1.NodeSpec{Taints: []corev1.Taint{ + {Key: "unrelated.example/taint", Effect: corev1.TaintEffectNoExecute}, + {Key: "empty", Effect: corev1.TaintEffectNoSchedule, TimeAdded: &now}, + {Key: "dedicated", Value: "new", Effect: corev1.TaintEffectNoSchedule}, + }}, + } + return desiredGoal, &State{AppliedGoal: &appliedGoal}, node + } + + tests := map[string]struct { + mutate func(*aksmachine.GoalState, *State, *corev1.Node) + want bool + wantMaxPods int + }{ + "reflected label and taint delta": {want: true, wantMaxPods: 110}, + "omitted scalar defaults are preserved": { + mutate: func(goal *aksmachine.GoalState, _ *State, _ *corev1.Node) { + goal.MaxPods = nil + goal.KubeletConfig = aksmachine.KubeletConfig{} + }, + want: true, + wantMaxPods: 110, + }, + "nil labels and taints clear applied values": { + mutate: func(goal *aksmachine.GoalState, _ *State, node *corev1.Node) { + goal.NodeLabels = nil + goal.NodeTaints = nil + delete(node.Labels, "workload") + delete(node.Labels, "empty") + node.Spec.Taints = node.Spec.Taints[:1] + }, + want: true, + wantMaxPods: 110, + }, + "empty labels and taints clear applied values": { + mutate: func(goal *aksmachine.GoalState, _ *State, node *corev1.Node) { + goal.NodeLabels = map[string]string{} + goal.NodeTaints = []string{} + delete(node.Labels, "workload") + delete(node.Labels, "empty") + node.Spec.Taints = node.Spec.Taints[:1] + }, + want: true, + wantMaxPods: 110, + }, + "Kubernetes version changed": { + mutate: func(goal *aksmachine.GoalState, _ *State, _ *corev1.Node) { goal.KubernetesVersion = "1.35.0" }, + }, + "max pods changed": { + mutate: func(goal *aksmachine.GoalState, _ *State, _ *corev1.Node) { goal.MaxPods = intPointer(50) }, + }, + "image GC high threshold changed": { + mutate: func(goal *aksmachine.GoalState, _ *State, _ *corev1.Node) { + goal.KubeletConfig.ImageGCHighThreshold = intPointer(90) + }, + }, + "image GC low threshold changed": { + mutate: func(goal *aksmachine.GoalState, _ *State, _ *corev1.Node) { + goal.KubeletConfig.ImageGCLowThreshold = intPointer(75) + }, + }, + "applied goal unavailable": { + mutate: func(_ *aksmachine.GoalState, state *State, _ *corev1.Node) { state.AppliedGoal = nil }, + }, + "desired label missing": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { delete(node.Labels, "workload") }, + }, + "desired label has wrong value": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { node.Labels["workload"] = "old" }, + }, + "desired empty label missing": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { delete(node.Labels, "empty") }, + }, + "removed label remains with another value": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { node.Labels["removed"] = "changed" }, + }, + "desired taint missing": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { + node.Spec.Taints = node.Spec.Taints[:2] + }, + }, + "desired taint has wrong value": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { + node.Spec.Taints[2].Value = "old" + }, + }, + "updated taint key retains another effect": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { + node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{Key: "dedicated", Value: "stale", Effect: corev1.TaintEffectNoExecute}) + }, + }, + "removed taint identity remains": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { + node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{Key: "removed", Value: "changed", Effect: corev1.TaintEffectNoExecute}) + }, + }, + "removed taint key remains with another effect": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { + node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{Key: "removed", Value: "changed", Effect: corev1.TaintEffectNoSchedule}) + }, + }, + "malformed desired taint": { + mutate: func(goal *aksmachine.GoalState, _ *State, _ *corev1.Node) { + goal.NodeTaints = append(goal.NodeTaints, "malformed") + }, + }, + "duplicate desired taint identity": { + mutate: func(goal *aksmachine.GoalState, _ *State, _ *corev1.Node) { + goal.NodeTaints = append(goal.NodeTaints, "dedicated=duplicate:NoSchedule") + }, + }, + "node is deleting": { + mutate: func(_ *aksmachine.GoalState, _ *State, node *corev1.Node) { + now := metav1.Now() + node.DeletionTimestamp = &now + }, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + goal, state, node := newFixture() + if tt.mutate != nil { + tt.mutate(&goal, state, node) + } + got, ok := goalForInPlaceAcknowledgement(goal, state, node) + if ok != tt.want { + t.Fatalf("goalForInPlaceAcknowledgement() ok = %v, want %v", ok, tt.want) + } + if ok { + if got.MaxPods == nil || *got.MaxPods != tt.wantMaxPods || !maps.Equal(got.NodeLabels, goal.NodeLabels) { + t.Fatalf("acknowledged goal = %#v", got) + } + } + }) + } +} + func TestHasDeletionSignal(t *testing.T) { t.Parallel() diff --git a/pkg/daemon/repave_reconciler.go b/pkg/daemon/repave_reconciler.go index bf9cbffe..280eae2e 100644 --- a/pkg/daemon/repave_reconciler.go +++ b/pkg/daemon/repave_reconciler.go @@ -7,11 +7,13 @@ import ( "fmt" "log/slog" "math/big" + "reflect" "strings" "time" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -42,6 +44,7 @@ type decisionKind string const ( decisionNoop decisionKind = "Noop" decisionApplyGoalState decisionKind = "ApplyGoalState" + decisionAcknowledgeGoalState decisionKind = "AcknowledgeGoalState" decisionResetDelete decisionKind = "ResetDelete" decisionWaitForMachineDelete decisionKind = "WaitForMachineDelete" decisionWaitForNodeSignal decisionKind = "WaitForNodeSignal" @@ -182,6 +185,8 @@ func (r *repaveReconciler) reconcileOnce(ctx context.Context) error { return r.patchStatus(ctx, aksmachine.ProvisioningStateSucceeded, decision.Goal.SettingsVersion, decision.Reason) case decisionApplyGoalState: return r.applyGoalState(ctx, state, decision.Goal) + case decisionAcknowledgeGoalState: + return r.acknowledgeGoalState(ctx, state, decision.Goal) case decisionResetDelete: return r.resetDelete(ctx) default: @@ -226,6 +231,15 @@ func (r *repaveReconciler) applyGoalState(ctx context.Context, state *State, goa return r.patchStatus(ctx, aksmachine.ProvisioningStateSucceeded, stateObservedVersion(newState), "machine goal state applied") } +func (r *repaveReconciler) acknowledgeGoalState(ctx context.Context, state *State, goal aksmachine.GoalState) error { + newState, err := r.operator.AcknowledgeGoalState(ctx, goal) + if err != nil { + _ = r.patchStatus(ctx, aksmachine.ProvisioningStateFailed, stateObservedVersion(state), err.Error()) + return err + } + return r.patchStatus(ctx, aksmachine.ProvisioningStateSucceeded, stateObservedVersion(newState), "in-place machine goal state observed") +} + func (r *repaveReconciler) resetDelete(ctx context.Context) error { // Stage 1 clears local runtime/settings while keeping this daemon alive. if err := r.operator.ResetNode(ctx, r.log); err != nil { @@ -273,6 +287,9 @@ func decide(machine machineSnapshot, node nodeSnapshot, state *State) decision { if goalApplied(goal, state) { return decision{Kind: decisionReportSucceeded, Goal: goal, Reason: "goal state is applied"} } + if acknowledgedGoal, ok := goalForInPlaceAcknowledgement(goal, state, node.node); ok { + return decision{Kind: decisionAcknowledgeGoalState, Goal: acknowledgedGoal, Reason: "goal update is already reflected on the node"} + } return decision{Kind: decisionWaitForNodeSignal, Goal: goal, Reason: "goal state differs but node deletion trigger is absent"} } @@ -281,6 +298,140 @@ func goalApplied(goal aksmachine.GoalState, state *State) bool { return goal.SettingsVersion != "" && stateObservedVersion(state) == goal.SettingsVersion } +// goalForInPlaceAcknowledgement recognizes label and taint updates already +// reconciled onto the Node by AKS RP. Non-observable goal changes fail closed +// and continue through the existing node-deletion repave flow. +func goalForInPlaceAcknowledgement(goal aksmachine.GoalState, state *State, node *corev1.Node) (aksmachine.GoalState, bool) { + if goal.SettingsVersion == "" || state == nil || state.AppliedGoal == nil || node == nil || node.DeletionTimestamp != nil { + return aksmachine.GoalState{}, false + } + + appliedGoal := *state.AppliedGoal + if err := appliedGoal.Validate(); err != nil { + return aksmachine.GoalState{}, false + } + + // ARM can omit scalar defaults that were resolved before the applied goal + // was persisted. Complete the new goal with those values before comparing it. + acknowledgedGoal, err := aksmachine.EffectiveGoal(goal, appliedGoal) + if err != nil { + return aksmachine.GoalState{}, false + } + if !goalMatchesOutsideNodeState(acknowledgedGoal, appliedGoal) { + return aksmachine.GoalState{}, false + } + if !nodeLabelsMatchGoal(node.Labels, appliedGoal.NodeLabels, acknowledgedGoal.NodeLabels) { + return aksmachine.GoalState{}, false + } + if !nodeTaintsMatchGoal(node.Spec.Taints, appliedGoal.NodeTaints, acknowledgedGoal.NodeTaints) { + return aksmachine.GoalState{}, false + } + return acknowledgedGoal, true +} + +func goalMatchesOutsideNodeState(goal, applied aksmachine.GoalState) bool { + // Normalize the version and Node-observable fields verified below. Comparing + // the remaining complete value makes future GoalState fields fail closed. + applied.SettingsVersion = goal.SettingsVersion + applied.NodeLabels = goal.NodeLabels + applied.NodeTaints = goal.NodeTaints + return reflect.DeepEqual(applied, goal) +} + +func nodeLabelsMatchGoal(nodeLabels, appliedLabels, desiredLabels map[string]string) bool { + for key, desiredValue := range desiredLabels { + if currentValue, present := nodeLabels[key]; !present || currentValue != desiredValue { + return false + } + } + for key := range appliedLabels { + if _, stillDesired := desiredLabels[key]; stillDesired { + continue + } + if _, stillPresent := nodeLabels[key]; stillPresent { + return false + } + } + return true +} + +type taintIdentity struct { + key string + effect corev1.TaintEffect +} + +func nodeTaintsMatchGoal(nodeTaints []corev1.Taint, appliedTaints, desiredTaints []string) bool { + appliedByIdentity, ok := parseGoalTaints(appliedTaints) + if !ok { + return false + } + desiredByIdentity, ok := parseGoalTaints(desiredTaints) + if !ok { + return false + } + + reflectedDesired := make(map[taintIdentity]struct{}, len(desiredByIdentity)) + reconciledKeys := make(map[string]struct{}, len(appliedByIdentity)) + for identity, applied := range appliedByIdentity { + desired, stillDesired := desiredByIdentity[identity] + if !stillDesired || desired.Value != applied.Value { + reconciledKeys[identity.key] = struct{}{} + } + } + for _, current := range nodeTaints { + identity := taintIdentity{key: current.Key, effect: current.Effect} + if desired, wanted := desiredByIdentity[identity]; wanted { + if _, duplicate := reflectedDesired[identity]; duplicate || current.Value != desired.Value { + return false + } + reflectedDesired[identity] = struct{}{} + continue + } + if _, reconciled := reconciledKeys[current.Key]; reconciled { + return false + } + } + return len(reflectedDesired) == len(desiredByIdentity) +} + +func parseGoalTaints(taints []string) (map[taintIdentity]corev1.Taint, bool) { + indexed := make(map[taintIdentity]corev1.Taint, len(taints)) + for _, value := range taints { + taint, ok := parseGoalTaint(value) + if !ok { + return nil, false + } + identity := taintIdentity{key: taint.Key, effect: taint.Effect} + if _, duplicate := indexed[identity]; duplicate { + return nil, false + } + indexed[identity] = taint + } + return indexed, true +} + +func parseGoalTaint(value string) (corev1.Taint, bool) { + keyValue, effectValue, found := strings.Cut(value, ":") + if !found || strings.Contains(effectValue, ":") { + return corev1.Taint{}, false + } + effect := corev1.TaintEffect(effectValue) + switch effect { + case corev1.TaintEffectNoSchedule, corev1.TaintEffectPreferNoSchedule, corev1.TaintEffectNoExecute: + default: + return corev1.Taint{}, false + } + + key, taintValue, hasValue := strings.Cut(keyValue, "=") + if strings.Contains(taintValue, "=") || len(validation.IsQualifiedName(key)) != 0 { + return corev1.Taint{}, false + } + if hasValue && len(validation.IsValidLabelValue(taintValue)) != 0 { + return corev1.Taint{}, false + } + return corev1.Taint{Key: key, Value: taintValue, Effect: effect}, true +} + func hasDeletionSignal(taints []corev1.Taint) bool { for _, taint := range taints { if taint.Key == DeletionTaintKey && taint.Effect == DeletionTaintEffect && strings.EqualFold(strings.TrimSpace(taint.Value), DeletionTaintValue) { diff --git a/pkg/daemon/repave_reconciler_test.go b/pkg/daemon/repave_reconciler_test.go index 88676c10..c0db6a2b 100644 --- a/pkg/daemon/repave_reconciler_test.go +++ b/pkg/daemon/repave_reconciler_test.go @@ -45,6 +45,126 @@ func TestRepaveReconcilerApplyGoalState(t *testing.T) { } } +func TestRepaveReconcilerAcknowledgesInPlaceGoalState(t *testing.T) { + t.Parallel() + + appliedGoal := testMachineGoal("1.34.0", "41") + appliedGoal.NodeLabels = map[string]string{"workload": "old"} + appliedGoal.NodeTaints = []string{"dedicated=old:NoSchedule"} + desiredGoal := testMachineGoal("1.34.0", "42") + desiredGoal.NodeLabels = map[string]string{"workload": "new"} + desiredGoal.NodeTaints = []string{"dedicated=new:NoSchedule"} + machines := &fakeMachineClient{machine: &aksmachine.Machine{Goal: desiredGoal}} + operator := &fakeNodeOperator{state: &State{AppliedGoal: &appliedGoal, ActiveMachine: "kube1"}} + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: map[string]string{"workload": "new"}}, + Spec: corev1.NodeSpec{Taints: []corev1.Taint{ + {Key: "dedicated", Value: "new", Effect: corev1.TaintEffectNoSchedule}, + }}, + } + repaves := newTestRepaveReconciler(t, machines, fakeClient(node), operator) + + if err := repaves.reconcileOnce(t.Context()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if !operator.acknowledged { + t.Fatal("AcknowledgeGoalState was not called") + } + if operator.applied || operator.restarted || operator.reset || operator.stopped { + t.Fatalf("host mutation called: %#v", operator) + } + if stateObservedVersion(operator.state) != "42" || operator.state.ActiveMachine != "kube1" { + t.Fatalf("state = %#v", operator.state) + } + if operator.state.PreviousAppliedGoal == nil || operator.state.PreviousAppliedGoal.SettingsVersion != "41" { + t.Fatalf("PreviousAppliedGoal = %#v, want settings version 41", operator.state.PreviousAppliedGoal) + } + if got := machines.status.ObservedSettingsVersion; got != "42" { + t.Fatalf("observed settings version = %q, want 42", got) + } + if got := machines.status.ProvisioningState; got != aksmachine.ProvisioningStateSucceeded { + t.Fatalf("status = %s", got) + } + + operator.acknowledged = false + if err := repaves.reconcileOnce(t.Context()); err != nil { + t.Fatalf("second Reconcile: %v", err) + } + if operator.acknowledged { + t.Fatal("AcknowledgeGoalState was called again for an applied ETag") + } + if operator.state.PreviousAppliedGoal == nil || operator.state.PreviousAppliedGoal.SettingsVersion != "41" { + t.Fatalf("second reconcile rotated PreviousAppliedGoal: %#v", operator.state.PreviousAppliedGoal) + } +} + +func TestRepaveReconcilerRetriesStatusAfterAcknowledgement(t *testing.T) { + t.Parallel() + + appliedGoal := testMachineGoal("1.34.0", "41") + appliedGoal.NodeLabels = map[string]string{"workload": "old"} + desiredGoal := testMachineGoal("1.34.0", "42") + desiredGoal.NodeLabels = map[string]string{"workload": "new"} + statusErr := errors.New("patch status") + machines := &fakeMachineClient{ + machine: &aksmachine.Machine{Goal: desiredGoal}, + patchErr: statusErr, + } + operator := &fakeNodeOperator{state: &State{AppliedGoal: &appliedGoal, ActiveMachine: "kube1"}} + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: map[string]string{"workload": "new"}}} + repaves := newTestRepaveReconciler(t, machines, fakeClient(node), operator) + + if err := repaves.reconcileOnce(t.Context()); !errors.Is(err, statusErr) { + t.Fatalf("first Reconcile error = %v, want status failure", err) + } + if !operator.acknowledged || stateObservedVersion(operator.state) != "42" { + t.Fatalf("acknowledged state = %#v", operator.state) + } + + operator.acknowledged = false + machines.patchErr = nil + if err := repaves.reconcileOnce(t.Context()); err != nil { + t.Fatalf("second Reconcile: %v", err) + } + if operator.acknowledged { + t.Fatal("second reconcile acknowledged the same ETag again") + } + if operator.state.PreviousAppliedGoal == nil || operator.state.PreviousAppliedGoal.SettingsVersion != "41" { + t.Fatalf("second reconcile rotated PreviousAppliedGoal: %#v", operator.state.PreviousAppliedGoal) + } + if got := machines.status.ObservedSettingsVersion; got != "42" { + t.Fatalf("observed settings version = %q, want 42", got) + } +} + +func TestRepaveReconcilerAcknowledgementFailurePatchesFailed(t *testing.T) { + t.Parallel() + + appliedGoal := testMachineGoal("1.34.0", "41") + appliedGoal.NodeLabels = map[string]string{"workload": "old"} + desiredGoal := testMachineGoal("1.34.0", "42") + desiredGoal.NodeLabels = map[string]string{"workload": "new"} + machines := &fakeMachineClient{machine: &aksmachine.Machine{Goal: desiredGoal}} + ackErr := errors.New("save acknowledgement") + operator := &fakeNodeOperator{ + state: &State{AppliedGoal: &appliedGoal, ActiveMachine: "kube1"}, + ackErr: ackErr, + } + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: map[string]string{"workload": "new"}}} + repaves := newTestRepaveReconciler(t, machines, fakeClient(node), operator) + + err := repaves.reconcileOnce(t.Context()) + if !errors.Is(err, ackErr) { + t.Fatalf("Reconcile error = %v, want acknowledgement failure", err) + } + if got := machines.status.ProvisioningState; got != aksmachine.ProvisioningStateFailed { + t.Fatalf("status = %s, want Failed", got) + } + if got := machines.status.ObservedSettingsVersion; got != "41" { + t.Fatalf("observed settings version = %q, want 41", got) + } +} + func TestRepaveReconcilerResetDelete(t *testing.T) { t.Parallel() @@ -121,6 +241,7 @@ func fakeClient(objects ...client.Object) client.Client { type fakeMachineClient struct { machine *aksmachine.Machine status aksmachine.Status + patchErr error notFound bool } @@ -137,20 +258,22 @@ func (f *fakeMachineClient) Get(context.Context) (*aksmachine.Machine, error) { func (f *fakeMachineClient) PatchStatus(_ context.Context, status aksmachine.Status) error { f.status = status - return nil + return f.patchErr } type fakeNodeOperator struct { - state *State - newState *State - err error - restartErr error - resetErr error - stopErr error - applied bool - restarted bool - reset bool - stopped bool + state *State + newState *State + err error + ackErr error + restartErr error + resetErr error + stopErr error + applied bool + acknowledged bool + restarted bool + reset bool + stopped bool } func (f *fakeNodeOperator) LoadState(context.Context) (*State, error) { @@ -166,6 +289,19 @@ func (f *fakeNodeOperator) ApplyGoalState(context.Context, *slog.Logger, aksmach return f.state, nil } +func (f *fakeNodeOperator) AcknowledgeGoalState(_ context.Context, goal aksmachine.GoalState) (*State, error) { + f.acknowledged = true + if f.ackErr != nil { + return nil, f.ackErr + } + activeMachineName := "" + if f.state != nil { + activeMachineName = f.state.ActiveMachine + } + f.state = nextAppliedState(f.state, goal, &activeMachine{Name: activeMachineName}) + return f.state, nil +} + func (f *fakeNodeOperator) RestartNode(context.Context, *slog.Logger) error { f.restarted = true return f.restartErr