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/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 1c8a413d..d845c314 100644 --- a/hack/e2e/lib/node-join-token.sh +++ b/hack/e2e/lib/node-join-token.sh @@ -95,7 +95,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" diff --git a/pkg/aksmachine/client_armapi.go b/pkg/aksmachine/client_armapi.go index f6c2327b..2eb39a0c 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{ @@ -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) @@ -290,7 +296,8 @@ func machineFromARM(machine armcontainerservice.Machine, defaultID, defaultName result.Goal.KubeletConfig.ImageGCHighThreshold = int(*kubernetes.KubeletConfig.ImageGcHighThreshold) } if kubernetes.KubeletConfig.ImageGcLowThreshold != nil { - result.Goal.KubeletConfig.ImageGCLowThreshold = int(*kubernetes.KubeletConfig.ImageGcLowThreshold) + lowThreshold := int(*kubernetes.KubeletConfig.ImageGcLowThreshold) + result.Goal.KubeletConfig.ImageGCLowThreshold = &lowThreshold } } } @@ -303,6 +310,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 currentVersion + } + return desiredVersion +} + 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..a4813c0d 100644 --- a/pkg/aksmachine/client_armapi_test.go +++ b/pkg/aksmachine/client_armapi_test.go @@ -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,31 +248,54 @@ func TestGoalStateValidate(t *testing.T) { wantErr: "kubernetes version is empty", }, { - name: "negative max pods", - goal: GoalState{KubernetesVersion: "1.35.1", MaxPods: -1}, - wantErr: "max pods must be non-negative", + name: "missing max pods", + goal: GoalState{KubernetesVersion: "1.35.1"}, + wantErr: "max pods must be positive", }, { - name: "max pods exceeds int32", - goal: GoalState{KubernetesVersion: "1.35.1", MaxPods: math.MaxInt32 + 1}, + name: "negative max pods", + goal: GoalState{ + KubernetesVersion: "1.35.1", + MaxPods: -1, + KubeletConfig: KubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: 80, + }, + }, + wantErr: "max pods must be positive", + }, + { + name: "max pods exceeds int32", + goal: GoalState{ + KubernetesVersion: "1.35.1", + MaxPods: math.MaxInt32 + 1, + KubeletConfig: KubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: 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, KubeletConfig: KubeletConfig{ ImageGCHighThreshold: -1, + ImageGCLowThreshold: 80, }, }, - wantErr: "image GC high threshold must be non-negative", + wantErr: "image GC high threshold must be positive", }, { name: "negative image GC low threshold", goal: GoalState{ KubernetesVersion: "1.35.1", + MaxPods: 110, KubeletConfig: KubeletConfig{ - ImageGCLowThreshold: -1, + ImageGCHighThreshold: 85, + ImageGCLowThreshold: -1, }, }, wantErr: "image GC low threshold must be non-negative", @@ -283,7 +306,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) @@ -331,7 +354,8 @@ func TestMachineFromARM(t *testing.T) { 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 != 85 || + machine.Goal.KubeletConfig.ImageGCLowThreshold == nil || *machine.Goal.KubeletConfig.ImageGCLowThreshold != 80 { t.Fatalf("kubelet config = %#v", machine.Goal.KubeletConfig) } if machine.Status.ProvisioningState != ProvisioningStateSucceeded { @@ -415,6 +439,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(" v1.35 "), + CurrentOrchestratorVersion: ptr(" v1.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..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{ @@ -130,7 +133,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 +143,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 +155,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..9e8c4fbb 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 := testMachineGoal("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{ @@ -115,15 +117,15 @@ func TestEnsureMachineAdoptsExistingSettingsVersionWithoutReplacingLocalGoal(t * ImageGCLowThreshold: 80, }, } - client := &ensureMachineClient{machine: &Machine{Goal: GoalState{ + client := &ensureMachineClient{machine: &Machine{Goal: MachineGoal{ KubernetesVersion: "1.35.1", SettingsVersion: "etag-42", MaxPods: 110, NodeLabels: map[string]string{"source": "remote"}, NodeTaints: []string{"remote=true:NoSchedule"}, - KubeletConfig: KubeletConfig{ + KubeletConfig: MachineKubeletConfig{ ImageGCHighThreshold: 70, - ImageGCLowThreshold: 60, + ImageGCLowThreshold: ptr(60), }, }}} task := EnsureMachine(client, &goal, true, slog.New(slog.NewTextHandler(io.Discard, nil))) @@ -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 := testMachineGoal("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: testMachineGoal("1.35.1", "")}, }, "required rejects missing settings version": { - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}}, + machine: &Machine{Goal: testMachineGoal("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))) @@ -272,7 +245,17 @@ func (c *ensureMachineClient) Create(_ context.Context, goal GoalState) (*Machin if c.createResult != nil { return c.createResult, nil } - return &Machine{Goal: goal}, nil + return &Machine{Goal: MachineGoal{ + KubernetesVersion: goal.KubernetesVersion, + SettingsVersion: goal.SettingsVersion, + MaxPods: goal.MaxPods, + NodeLabels: goal.NodeLabels, + NodeTaints: goal.NodeTaints, + KubeletConfig: MachineKubeletConfig{ + ImageGCHighThreshold: goal.KubeletConfig.ImageGCHighThreshold, + ImageGCLowThreshold: ptr(goal.KubeletConfig.ImageGCLowThreshold), + }, + }}, nil } func (c *ensureMachineClient) PatchStatus(context.Context, Status) error { diff --git a/pkg/aksmachine/test_helpers_test.go b/pkg/aksmachine/test_helpers_test.go new file mode 100644 index 00000000..e8a799c4 --- /dev/null +++ b/pkg/aksmachine/test_helpers_test.go @@ -0,0 +1,25 @@ +package aksmachine + +func testGoal(kubernetesVersion, settingsVersion string) GoalState { + return GoalState{ + KubernetesVersion: kubernetesVersion, + SettingsVersion: settingsVersion, + MaxPods: 110, + KubeletConfig: KubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: 80, + }, + } +} + +func testMachineGoal(kubernetesVersion, settingsVersion string) MachineGoal { + return MachineGoal{ + KubernetesVersion: kubernetesVersion, + SettingsVersion: settingsVersion, + MaxPods: 110, + KubeletConfig: MachineKubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: ptr(80), + }, + } +} diff --git a/pkg/aksmachine/types.go b/pkg/aksmachine/types.go index 35b1f4b8..74d9e2a6 100644 --- a/pkg/aksmachine/types.go +++ b/pkg/aksmachine/types.go @@ -10,9 +10,57 @@ import ( "github.com/Azure/AKSFlexNode/pkg/config" ) -// GoalState is the local agent representation of ARM machine desired settings. -// Keep this type independent from the public Azure SDK shape; adapt the SDK -// payload to this model when the ARM contract is finalized. +// MachineGoal preserves optional settings returned by the ARM Machine API. +// ImageGCLowThreshold is a pointer because zero is both valid and distinct from +// an omitted value. +type MachineGoal struct { + KubernetesVersion string `json:"kubernetesVersion,omitempty"` + SettingsVersion string `json:"settingsVersion,omitempty"` + MaxPods int `json:"maxPods,omitempty"` + NodeLabels map[string]string `json:"nodeLabels,omitempty"` + NodeTaints []string `json:"nodeTaints,omitempty"` + KubeletConfig MachineKubeletConfig `json:"kubeletConfig"` +} + +type MachineKubeletConfig struct { + ImageGCHighThreshold int `json:"imageGCHighThreshold,omitempty"` + ImageGCLowThreshold *int `json:"imageGCLowThreshold,omitempty"` +} + +// Validate verifies every scalar present in a Machine goal while allowing the +// API to omit values that the agent will fill from local configuration. +func (g MachineGoal) Validate() error { + if g.KubernetesVersion == "" { + return fmt.Errorf("kubernetes version is empty") + } + if g.MaxPods < 0 { + return fmt.Errorf("max pods must be positive") + } + 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 { + return fmt.Errorf("image GC high threshold must be positive") + } + if g.KubeletConfig.ImageGCHighThreshold > 100 { + return fmt.Errorf("image GC high threshold must be less than or equal to 100") + } + if g.KubeletConfig.ImageGCLowThreshold != nil { + if *g.KubeletConfig.ImageGCLowThreshold < 0 { + return fmt.Errorf("image GC low threshold must be non-negative") + } + 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 != nil && + *g.KubeletConfig.ImageGCLowThreshold >= g.KubeletConfig.ImageGCHighThreshold { + return fmt.Errorf("image GC low threshold must be less than image GC high threshold") + } + return nil +} + +// GoalState contains the complete effective settings used to render a node. type GoalState struct { KubernetesVersion string `json:"kubernetesVersion,omitempty"` SettingsVersion string `json:"settingsVersion,omitempty"` @@ -27,21 +75,25 @@ type KubeletConfig struct { ImageGCLowThreshold int `json:"imageGCLowThreshold,omitempty"` } -func (g GoalState) validate() error { - if g.KubernetesVersion == "" { - return fmt.Errorf("kubernetes version is empty") - } - if g.MaxPods < 0 { - return fmt.Errorf("max pods must be non-negative") - } - if g.MaxPods > math.MaxInt32 { - return fmt.Errorf("max pods must be less than or equal to %d", math.MaxInt32) +// 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 err := (MachineGoal{ + KubernetesVersion: g.KubernetesVersion, + MaxPods: g.MaxPods, + KubeletConfig: MachineKubeletConfig{ + ImageGCHighThreshold: g.KubeletConfig.ImageGCHighThreshold, + ImageGCLowThreshold: &g.KubeletConfig.ImageGCLowThreshold, + }, + }).Validate(); err != nil { + return err } - if g.KubeletConfig.ImageGCHighThreshold < 0 { - return fmt.Errorf("image GC high threshold must be non-negative") + if g.MaxPods == 0 { + return fmt.Errorf("max pods must be positive") } - if g.KubeletConfig.ImageGCLowThreshold < 0 { - return fmt.Errorf("image GC low threshold must be non-negative") + if g.KubeletConfig.ImageGCHighThreshold == 0 { + return fmt.Errorf("image GC high threshold must be positive") } return nil } @@ -59,12 +111,46 @@ func GoalStateFromConfig(cfg *config.Config) (GoalState, error) { ImageGCLowThreshold: cfg.Node.Kubelet.ImageGCLowThreshold, }, } - if err := goal.validate(); err != nil { + if err := goal.Validate(); err != nil { return GoalState{}, err } return goal, nil } +// 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) + 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 MachineGoal, local GoalState) (GoalState, error) { + effective := GoalState{ + KubernetesVersion: machine.KubernetesVersion, + SettingsVersion: machine.SettingsVersion, + MaxPods: local.MaxPods, + NodeLabels: maps.Clone(machine.NodeLabels), + NodeTaints: slices.Clone(machine.NodeTaints), + KubeletConfig: local.KubeletConfig, + } + if machine.MaxPods != 0 { + effective.MaxPods = machine.MaxPods + } + if machine.KubeletConfig.ImageGCHighThreshold != 0 { + effective.KubeletConfig.ImageGCHighThreshold = machine.KubeletConfig.ImageGCHighThreshold + } + if machine.KubeletConfig.ImageGCLowThreshold != nil { + effective.KubeletConfig.ImageGCLowThreshold = *machine.KubeletConfig.ImageGCLowThreshold + } + if err := effective.Validate(); err != nil { + return GoalState{}, fmt.Errorf("validate effective goal: %w", err) + } + return effective, nil +} + type ProvisioningState string const ( @@ -84,19 +170,19 @@ type Status struct { // Machine is the local agent representation of the AKS RP machine resource. type Machine struct { - ID string `json:"id,omitempty"` - Name string `json:"name,omitempty"` - Goal GoalState `json:"goal"` - Status Status `json:"status"` + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + Goal MachineGoal `json:"goal"` + 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 and every scalar present in a Machine +// returned by AKS. Omitted scalars are validated after local defaults apply. 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..bc397471 100644 --- a/pkg/aksmachine/types_test.go +++ b/pkg/aksmachine/types_test.go @@ -94,15 +94,33 @@ func TestMachineValidate(t *testing.T) { wantErr: "machine is nil", }, "missing Kubernetes version": { - machine: &Machine{Goal: GoalState{SettingsVersion: "42"}}, + machine: &Machine{Goal: MachineGoal{SettingsVersion: "42"}}, wantErr: "kubernetes version is empty", }, "missing settings version": { - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}}, + machine: &Machine{Goal: testMachineGoal("1.35.1", "")}, wantErr: "goal settings version is empty", }, "complete machine": { - machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "42"}}, + machine: &Machine{Goal: testMachineGoal("1.35.1", "42")}, + }, + "omitted scalar defaults": { + machine: &Machine{Goal: MachineGoal{KubernetesVersion: "1.35.1", SettingsVersion: "42"}}, + }, + "invalid present max pods": { + machine: &Machine{Goal: MachineGoal{KubernetesVersion: "1.35.1", SettingsVersion: "42", MaxPods: -1}}, + wantErr: "max pods must be positive", + }, + "invalid present image GC thresholds": { + machine: &Machine{Goal: MachineGoal{ + KubernetesVersion: "1.35.1", + SettingsVersion: "42", + KubeletConfig: MachineKubeletConfig{ + ImageGCHighThreshold: 70, + ImageGCLowThreshold: ptr(80), + }, + }}, + wantErr: "image GC low threshold must be less than image GC high threshold", }, } @@ -123,3 +141,60 @@ 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 := MachineGoal{ + 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") + } +} + +func TestEffectiveGoalPreservesExplicitZero(t *testing.T) { + t.Parallel() + + local := testGoal("1.34.0", "") + machine := MachineGoal{ + KubernetesVersion: "1.35.0", + SettingsVersion: "42", + KubeletConfig: MachineKubeletConfig{ + ImageGCLowThreshold: ptr(0), + }, + } + + effective, err := EffectiveGoal(machine, local) + if err != nil { + t.Fatalf("EffectiveGoal() error = %v", err) + } + if effective.KubeletConfig.ImageGCLowThreshold != 0 { + t.Fatalf("ImageGCLowThreshold = %d, want explicit zero", effective.KubeletConfig.ImageGCLowThreshold) + } +} diff --git a/pkg/cmd/start/start.go b/pkg/cmd/start/start.go index 23e112c5..de1d3b37 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/daemon.go b/pkg/daemon/daemon.go index 74798ae6..57b5057f 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -177,7 +177,6 @@ func daemonRESTConfig(ctx context.Context, cfg *config.Config) (*rest.Config, fu } return credentials.RESTConfig(), stop, nil } - func daemonRESTConfigProvider(ctx context.Context, cfg *config.Config, base *rest.Config) (*daemoncred.RESTConfigProvider, func(), error) { credentialDir := filepath.Join(config.ConfigDir, daemonCredentialDir) if err := os.MkdirAll(credentialDir, 0o700); err != nil { diff --git a/pkg/daemon/goal_state.go b/pkg/daemon/goal_state.go new file mode 100644 index 00000000..aebbfaff --- /dev/null +++ b/pkg/daemon/goal_state.go @@ -0,0 +1,65 @@ +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 a complete effective goal 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) { + 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 = 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) +} + +func goalForRestart(cfg *config.Config, state *State) (*aksmachine.GoalState, error) { + if state != nil && state.AppliedGoal != nil { + goal := state.AppliedGoal.DeepCopy() + if err := goal.Validate(); 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.Validate(); err != nil { + return nil, fmt.Errorf("validate legacy restart goal: %w", err) + } + return &goal, nil +} diff --git a/pkg/daemon/goal_state_test.go b/pkg/daemon/goal_state_test.go new file mode 100644 index 00000000..0bb2684c --- /dev/null +++ b/pkg/daemon/goal_state_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 := testGoalState("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 := testGoalState("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..224cc799 100644 --- a/pkg/daemon/nodeoperator.go +++ b/pkg/daemon/nodeoperator.go @@ -22,7 +22,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) + ApplyGoalState(ctx context.Context, log *slog.Logger, goal aksmachine.MachineGoal) (*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 @@ -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) } @@ -81,12 +81,20 @@ func (o *nspawnNodeOperator) findActiveMachine(ctx context.Context) (*activeMach return activeMachineFromStore(ctx, o.state) } -func (o *nspawnNodeOperator) ApplyGoalState(ctx context.Context, log *slog.Logger, goal aksmachine.GoalState) (*State, error) { +func (o *nspawnNodeOperator) ApplyGoalState(ctx context.Context, log *slog.Logger, goal aksmachine.MachineGoal) (*State, error) { active, err := o.findActiveMachine(ctx) 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 + } + localGoal, err := aksmachine.GoalStateFromConfig(cfg) + if err != nil { + return nil, fmt.Errorf("build local machine goal: %w", err) + } + effectiveGoal, err := aksmachine.EffectiveGoal(goal, localGoal) if err != nil { return nil, err } @@ -96,14 +104,14 @@ func (o *nspawnNodeOperator) ApplyGoalState(ctx context.Context, log *slog.Logge "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), @@ -117,7 +125,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() @@ -144,15 +152,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 +165,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: goal.DeepCopy(), } if current != nil { - next.PreviousSettingsVersion = current.AppliedSettingsVersion - next.PreviousKubernetesVersion = current.AppliedKubernetesVersion + if current.AppliedGoal != nil { + next.PreviousAppliedGoal = current.AppliedGoal.DeepCopy() + } 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..1063d548 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" @@ -24,11 +23,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": { @@ -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) @@ -104,8 +103,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 +113,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()) @@ -129,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{ @@ -145,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) @@ -168,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{ @@ -179,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") @@ -272,6 +268,33 @@ func discardLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) } +func TestNextAppliedStateRotatesCompleteGoals(t *testing.T) { + t.Parallel() + + currentGoal := testGoalState("1.34.0", "41") + currentGoal.NodeLabels = map[string]string{"source": "old"} + current := &State{AppliedGoal: ¤tGoal, ActiveMachine: goalstates.NSpawnMachineKube1} + nextGoal := testGoalState("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..6ac82b08 100644 --- a/pkg/daemon/reconcile_test.go +++ b/pkg/daemon/reconcile_test.go @@ -11,10 +11,12 @@ import ( func TestDecide(t *testing.T) { t.Parallel() - goal := aksmachine.GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "42"} - machine := machineSnapshot{machine: &aksmachine.Machine{Goal: goal}} - applied := &State{AppliedSettingsVersion: "42", AppliedKubernetesVersion: "1.34.0"} - stale := &State{AppliedSettingsVersion: "41", AppliedKubernetesVersion: "1.33.0"} + machineGoal := testMachineGoal("1.34.0", "42") + machine := machineSnapshot{machine: &aksmachine.Machine{Goal: machineGoal}} + appliedGoal := testGoalState("1.34.0", "42") + applied := &State{AppliedGoal: appliedGoal.DeepCopy()} + staleGoal := testGoalState("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..425779b9 100644 --- a/pkg/daemon/repave_reconciler.go +++ b/pkg/daemon/repave_reconciler.go @@ -59,7 +59,7 @@ type nodeSnapshot struct { type decision struct { Kind decisionKind - Goal aksmachine.GoalState + Goal aksmachine.MachineGoal Reason string } @@ -214,7 +214,7 @@ func (r *repaveReconciler) nodeSnapshot(ctx context.Context) (nodeSnapshot, erro return nodeSnapshot{node: &node}, nil } -func (r *repaveReconciler) applyGoalState(ctx context.Context, state *State, goal aksmachine.GoalState) error { +func (r *repaveReconciler) applyGoalState(ctx context.Context, state *State, goal aksmachine.MachineGoal) error { if err := r.patchStatus(ctx, aksmachine.ProvisioningStateReconciling, stateObservedVersion(state), "applying machine goal state"); err != nil { return err } @@ -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 { @@ -277,11 +277,11 @@ func decide(machine machineSnapshot, node nodeSnapshot, state *State) decision { return decision{Kind: decisionWaitForNodeSignal, Goal: goal, Reason: "goal state differs but node deletion trigger is absent"} } -func goalApplied(goal aksmachine.GoalState, state *State) bool { - if state == nil { - return false - } - return goal.SettingsVersion != "" && state.AppliedSettingsVersion == goal.SettingsVersion +func goalApplied(goal aksmachine.MachineGoal, state *State) bool { + return goal.SettingsVersion != "" && + state != nil && + state.AppliedGoal != nil && + state.AppliedGoal.SettingsVersion == goal.SettingsVersion } func hasDeletionSignal(taints []corev1.Taint) bool { @@ -297,6 +297,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..9d279193 100644 --- a/pkg/daemon/repave_reconciler_test.go +++ b/pkg/daemon/repave_reconciler_test.go @@ -18,8 +18,18 @@ 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"}} + machineGoal := testMachineGoal("1.34.0", "42") + goal := testGoalState("1.34.0", "42") + previousGoal := testGoalState("1.33.0", "41") + machines := &fakeMachineClient{machine: &aksmachine.Machine{Goal: machineGoal}} + 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 +38,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 { @@ -77,7 +87,7 @@ func TestRepaveReconcilerStateLoadFailurePatchesFailed(t *testing.T) { func TestRepaveReconcilerRejectsInvalidMachineGoal(t *testing.T) { t.Parallel() - machines := &fakeMachineClient{machine: &aksmachine.Machine{Goal: aksmachine.GoalState{KubernetesVersion: "1.34.0"}}} + machines := &fakeMachineClient{machine: &aksmachine.Machine{Goal: aksmachine.MachineGoal{KubernetesVersion: "1.34.0"}}} operator := &fakeNodeOperator{state: &State{AppliedSettingsVersion: "41", AppliedKubernetesVersion: "1.33.0", ActiveMachine: "kube1"}} repaves := newTestRepaveReconciler(t, machines, fakeClient(), operator) @@ -148,7 +158,7 @@ func (f *fakeNodeOperator) LoadState(context.Context) (*State, error) { return f.state, f.err } -func (f *fakeNodeOperator) ApplyGoalState(context.Context, *slog.Logger, aksmachine.GoalState) (*State, error) { +func (f *fakeNodeOperator) ApplyGoalState(context.Context, *slog.Logger, aksmachine.MachineGoal) (*State, error) { f.applied = true if f.newState != nil { f.state = f.newState diff --git a/pkg/daemon/state.go b/pkg/daemon/state.go index 3c16a4fc..31ac76dd 100644 --- a/pkg/daemon/state.go +++ b/pkg/daemon/state.go @@ -23,14 +23,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.Validate(); err != nil { + return fmt.Errorf("daemon state applied goal: %w", err) + } + } + if s.PreviousAppliedGoal != nil { + if err := s.PreviousAppliedGoal.Validate(); err != nil { + return fmt.Errorf("daemon state previous applied goal: %w", err) + } + } + return nil } type saveStateTask struct { @@ -58,10 +84,19 @@ 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: goal.DeepCopy(), ActiveMachine: goalstates.NSpawnMachineKube1} + state.populateLegacyFields() + return state +} + +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 +112,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 +163,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..ecff11c8 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: testGoalState("1.34.0", "42").DeepCopy(), + PreviousAppliedGoal: testGoalState("1.33.0", "41").DeepCopy(), + 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 := testGoalState("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..2f18d92e --- /dev/null +++ b/pkg/daemon/test_helpers_test.go @@ -0,0 +1,31 @@ +package daemon + +import "github.com/Azure/AKSFlexNode/pkg/aksmachine" + +func testGoalState(kubernetesVersion, settingsVersion string) aksmachine.GoalState { + return aksmachine.GoalState{ + KubernetesVersion: kubernetesVersion, + SettingsVersion: settingsVersion, + MaxPods: 110, + KubeletConfig: aksmachine.KubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: 80, + }, + } +} + +func testMachineGoal(kubernetesVersion, settingsVersion string) aksmachine.MachineGoal { + return aksmachine.MachineGoal{ + KubernetesVersion: kubernetesVersion, + SettingsVersion: settingsVersion, + MaxPods: 110, + KubeletConfig: aksmachine.MachineKubeletConfig{ + ImageGCHighThreshold: 85, + ImageGCLowThreshold: intPointer(80), + }, + } +} + +func intPointer(value int) *int { + return &value +}