Skip to content

Fix Azure node-policy apply failures, resync API surface, align v1/v2 fields, add CRUD test harness - #28

Merged
Tzvonimir merged 9 commits into
mainfrom
fix/node-policy-decode-and-field-alignment
Aug 21, 2026
Merged

Fix Azure node-policy apply failures, resync API surface, align v1/v2 fields, add CRUD test harness#28
Tzvonimir merged 9 commits into
mainfrom
fix/node-policy-decode-and-field-alignment

Conversation

@Tzvonimir

Copy link
Copy Markdown
Contributor

What?

Fixes the customer-reported v0.1.6 failure where devzero_node_policy with azure {} and/or taints errored during apply ("Struct defines fields not found in object: kubelet" / "can't unmarshal into *provider.Taint, needs FromTerraform5Value method"), then brings the whole provider back in line with the current DAKR API: proto resync, removal of three HPA fields the backend deleted, NotFound handling in every Read/Delete, a real node-policy delete, and full field alignment for workload policies (v1), workload rules (v2), targets, and node policies. Adds a three-layer test harness so this class of bug fails in CI instead of at a customer's terraform apply.

Why?

Customers were blocked creating Azure node policies — plan succeeded but apply failed inside the provider before any API call. Beyond that one bug, the provider had drifted ~2 months behind the backend: it still sent hpa_rule.target_utilization/primary_metric/target_memory_utilization (deleted server-side in July, causing "Provider produced inconsistent result after apply"), destroy left node policies and enabled targets running, out-of-band deletions bricked terraform plan, and a long list of new backend fields (JVM knobs, floor/ceiling bounds, RSS sizing, per-rule lookback, disabled, zonal shift, startup taints…) weren't reachable from Terraform.

How?

  • Decode bug: getElementList/getElementMap decoded list/map elements via low-level tftypes.Value.As, which cannot populate tfsdk-tagged structs — every taints/raw block failed. They now use tfsdk.ValueAs (the framework's own reflection). The aws/azure schemas were also missing attributes their Go models declare (kubelet, capacity_reservation_selector_terms, context), which made Plan.Get fail on any config using those blocks; the attributes are now declared.
  • Breaking: hpa_rule.target_utilization, target_memory_utilization and primary_metric are removed — the backend reserved those proto fields and dropped the columns, so they were already silent no-ops. hpa_rule.metrics is the replacement.
  • disabled on workload rules is create-only in the upsert API; updates route through the ToggleWorkloadRuleDisabled RPC instead.
  • Node policy target destroy disables the target (no delete RPC exists server-side) rather than silently leaving it enabled; cluster_ids validates to exactly one entry because the server rejects more (docs/example updated to a for_each per-cluster pattern).
  • Read paths remove the resource from state on CodeNotFound instead of erroring; node-policy reads also skip the read-only source == "cluster" virtual policies that ListNodePolicies mixes in.
  • fromProto pointer-receiver mutators were no-ops (m = &X{} on a local copy), which broke terraform import for nested blocks; rewritten as constructors.

Testing?

Three layers, all in go test, no network:

  • schema_model_consistency_test.go — decodes a fully-populated (and an all-null) value built from each resource's real schema through Plan.GettoProtofromProtoState.Set. With the fix reverted it reproduces the customer's exact errors; it also caught two mistakes during this PR's own development.
  • node_policy_decode_test.go — targeted regressions for the reported azure/taints failures, including the minimal vnet_subnet_id + one-taint config from the report.
  • fake_server_test.go — an in-memory connect server mimicking dakr semantics (upsert keys, NotFound codes, target invariants, fields.disabled rejection on update, virtual cluster policies) driving real Create/Read/Update/Delete lifecycles, including refresh-after-out-of-band-delete and delete idempotency.

go build ./..., go vet ./..., full unit suite, and make generate (docs) are green.

Anything Else?

Deliberately deferred (large nested specs, follow-up PR): gcp/oci node classes, keda_scaled_object, jvm_heap_rule. Backend asks filed separately: a DeleteNodePolicyTarget RPC, a GetNodePolicy RPC, and protecting terraform_* workload-rule sources from v2-policy takeover.

… time

- getElementList/getElementMap decoded list/map elements via tftypes.Value.As,
  which cannot populate tfsdk-tagged structs; every taints/raw block failed
  with 'needs FromTerraform5Value method'. Decode via tfsdk.ValueAs instead.
- The aws/azure schemas were missing attributes the Go model declares
  (aws: kubelet, capacity_reservation_selector_terms, context; azure: kubelet),
  so any config with aws {} / azure {} failed Plan.Get with
  'Struct defines fields not found in object'. Add the missing attributes;
  the to/fromProto code for them already existed.
- Add plan-decode regression tests for the customer-reported failures and a
  schema/model/proto consistency harness covering all six resources.
…s; allow pod_evict

- make proto now also syncs hpa_connector.proto/pb.go (recommendation.proto
  imports it upstream).
- Remove workload_rule hpa_rule.target_utilization / target_memory_utilization /
  primary_metric: dakr reserved HPARuleConfig fields 4/5/12 on 2026-07-07 and
  dropped the columns, so the server ignored and never echoed them, producing
  'Provider produced inconsistent result after apply'. Use hpa_rule.metrics
  (CPU/Memory/external triggers) instead. BREAKING for configs setting them.
- detection_triggers now accepts pod_evict on workload_rule and workload_policy
  (validators previously rejected the documented value; workload_rule was also
  missing the enum mapping).
- Update the workload_rule example accordingly.
…ks, validators

- Every Read now removes the resource from state on CodeNotFound (or
  not-found-in-list) instead of erroring, so out-of-band deletions no longer
  brick 'terraform plan'; every Delete tolerates CodeNotFound.
- node_policy Delete now calls DeleteNodePolicy (the RPC exists and cascades
  targets); it was a state-only no-op that left Karpenter NodePools running
  after destroy. node_policy Read also skips read-only source=="cluster"
  virtual policies that ListNodePolicies mixes in.
- node_policy_target Delete disables the target via UpdateNodePolicyTarget
  (no delete RPC exists server-side) instead of silently leaving it enabled;
  cluster_ids now validates SizeBetween(1,1) — the server rejects >1.
- workload_policy/workload_policy_target fromProto pointer-receiver
  reassignments were no-ops, so 'terraform import' never populated nested
  blocks and drift in unconfigured blocks was invisible; rewritten as
  constructors assigned by the caller.
- Read-backs null-guard optional attributes (emergency_response multipliers,
  hpa fallback/scaling-rules fields, regex pattern flags, match_expressions
  values) so omitted attrs stay null instead of producing 'inconsistent
  result after apply'; hpa metric metadata strips the serverAddress/query
  keys the backend folds in.
- workload_policy Create/Update abort when toProto errored (previously sent
  Policy: nil); cluster Update uses the same CustomName->Name fallback as
  Read; primary_metric accepts network_ingress/network_egress;
  scheduler_plugins no longer rejects an explicit empty list.
… policies with the current API

workload_policy (v1) new attributes:
- enable_in_place_vertical_scaling, allow_in_place_memory_limit_decrease
  (validated: the latter requires the former — the server silently forces it
  off otherwise), pdb_enabled
- cpu/memory (limit) floor/ceiling percents with 1-100 / 1-1000 validators
- JVM heap knobs (jvm_heap_optimization_enabled, target percentile, headroom,
  non-heap overhead %, non-heap overhead bytes, min/max heap bytes,
  prefer_container_support, jvm_cpu_startup_floor_millicores)
- vertical scaling request_use_rss / limit_use_rss (memory only)
- horizontal scaling network_target_throughput_bytes_per_sec,
  target_memory_utilization, composite_formula, scale_down_cooldown_seconds

workload_policy_target new attributes:
- workload_names_not_in, kind_filter_not_in, annotation_selector;
  node_group_names marked deprecated (unused upstream)

workload_rule (v2) new attributes:
- disabled (create-only via upsert; updates go through the
  ToggleWorkloadRuleDisabled RPC since the upsert rejects it)
- lookback_period_seconds (3600..2592000 validator)
- cpu/memory/gpu rule: initial_request, floor/ceiling_percent, initial_limit,
  limit_floor/ceiling_percent, request_use_rss, limit_use_rss (also on
  per-container rules)
- hpa_rule.metrics connector_id (HPAConnector reference)

node_policy new attributes:
- startup_taints, zonal_shift (AWS only), instance_local_nvme (AWS only),
  cloud_provider_id, azure.image_version (operator >= 1.8.4)
Adds an in-memory connect server (httptest + the generated handlers with
Unimplemented* embeds) that mimics the dakr semantics that matter to CRUD
correctness: upsert keys, NotFound codes, the node-policy-target invariants
(empty target_id on create, at most one cluster), fields.disabled rejection
on workload-rule update, and the virtual source=="cluster" policies mixed
into ListNodePolicies.

Lifecycle tests drive each resource's real Create/Read/Update/Delete methods
with schema-typed plans (hydrated via the framework's own reflection), and
assert:
- create round-trips (ids assigned, nested blocks intact, regex flags null)
- refresh after out-of-band deletion removes the resource from state
- node policy delete actually deletes (and cascades targets server-side);
  deleting again is a no-op
- node policy target destroy disables the target (no delete RPC exists)
- flipping workload_rule.disabled on update goes through
  ToggleWorkloadRuleDisabled (the upsert rejects it)
- allow_in_place_memory_limit_decrease without in-place scaling is rejected
  at plan time
…ter example

The API rejects targets naming more than one cluster; the example now shows
a for_each per-cluster pattern instead of the failing multi-cluster shape.
- Checked type assertions in the new test files (forcetypeassert), drop
  ineffectual assignments (ineffassign), omit an inferable type (ST1023),
  remove redundant conversions (unconvert).
- nolint:staticcheck on the intentional NodeGroupNames round-trip sites —
  the field is deprecated upstream but still supported for compatibility.
- Remove pulumi-devzero-provider-epic.md: a Pulumi planning document
  committed here by accident.
- Drop stringOrNull (duplicate of the existing stringValue) and reuse
  labelSelectorOperatorToString instead of an inline enum switch.
- Extract taintAttrTypes + taintListFromProto: the taint object shape was
  spelled out four times across taints/startup_taints read-backs.
- Fix copy-pasted error messages that said 'to Terraform value' on
  conversions that go from Terraform config to the API.
…haped read, proto round-trip stability

- Fake-server lifecycle now covers devzero_cluster end to end (create, rename
  with token preservation, drop-from-state after out-of-band deletion,
  tolerant delete), Update for node_policy and workload_policy_target
  (asserting the change actually lands backend-side), and an import-shaped
  Read (state carrying only the id) proving nested blocks populate on
  'terraform import'.
- The consistency harness additionally asserts proto round-trip stability
  (toProto(fromProto(p)) == p) for node policy, node policy target and
  workload policy, so a field silently dropped inside toProto — invisible to
  the previous model-level round-trip — now fails the suite.
@Tzvonimir
Tzvonimir merged commit 8fd1c73 into main Aug 21, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants