Skip to content

instancetype: UpdateInstanceTypes drops known SKUs on a partial resourceSKUs response, causing spurious NodeClaim drift #1838

Description

Description

DefaultProvider.UpdateInstanceTypes (pkg/providers/instancetype/instancetypes.go) refreshes the in-memory instance-type catalog on a timer (InstanceTypesRefreshInterval = 12h) by doing a live Azure fetch and then unconditionally replacing the catalog with the result. The only safeguard is a check for a totally empty result:

skus := cache.List(ctx, skewer.ResourceTypeFilter("virtualMachines"))
for i := range skus {
    ...
    if !skus[i].HasLocationRestriction(p.region) && p.isSupported(&skus[i], vmsize) {
        instanceTypes[skus[i].GetName()] = &skus[i]
    }
}
if len(instanceTypes) == 0 {          // ~L463: only guard
    return fmt.Errorf("no instance types found")
}
...
p.instanceTypesInfo = instanceTypes    // ~L474: full replace, no regression guard

The Azure resourceSKUs API (and the per-SKU HasLocationRestriction / capability metadata) is eventually consistent — a single refresh can return an HTTP 200 with a valid but incomplete SKU set, or transiently flag a family/zone as restricted. When that happens, the affected SKUs silently disappear from instanceTypesInfo. List() and Get() then stop offering them, so every NodeClaim on those SKUs is marked Drifted with InstanceTypeNotFound, even though the SKU is compiled-in, generally available, and unrestricted. Because the refresh is on a 12h timer, the catalog stays wrong — and those NodeClaims keep drifting/churning — until the next complete refresh, up to 12h later. It then self-heals with no restart.

This is distinct from the UnavailableOfferings path: that correctly marks an offering unavailable for a TTL. Here the instance type is dropped from the catalog entirely, which is what triggers drift.

Reproduction

  1. Let the provider populate its catalog from a complete fetch (contains, say, Standard_E32pds_v6).
  2. On the next UpdateInstanceTypes, have the resource client return a response that omits that SKU (simulating an eventually-consistent/partial resourceSKUs response, or a transient HasLocationRestriction on that family).
  3. Get("Standard_E32pds_v6") now returns not-found and List() no longer offers it → existing NodeClaims of that type are flagged Drifted: InstanceTypeNotFound.
  4. The catalog stays in this state until a subsequent complete refresh (up to InstanceTypesRefreshInterval later).

Impact

  • Spurious Drifted/InstanceTypeNotFound on healthy NodeClaims → unnecessary node replacement / churn.
  • Long recovery window (up to 12h) driven by the refresh interval, not by how long Azure's response was actually incomplete.
  • The AWS provider does not have this class of issue because its DescribeInstanceTypes pagination returns an error on a failed page (return fmt.Errorf(...)), so a failure preserves the previous catalog. Azure's failure mode here is a successful-but-incomplete response, which produces no error to key off — so the empty-only guard doesn't catch it.

Proposed fix

Make the catalog update regression-resistant instead of trusting any non-empty fetch:

  1. Gross-truncation guard — reject a fetch that returns far fewer SKUs than the previous catalog (e.g. < prev/2); keep last-known-good and return an error so the controller retries on backoff rather than pinning a truncated catalog for the full refresh interval.
  2. Per-SKU absence hysteresis — do not drop a previously-known SKU on a single refresh. Track consecutive absence per SKU and only remove it after it has been absent for N consecutive refreshes (e.g. 2). A transient miss is retained (no drift); a genuinely retired SKU still ages out.

This keeps additions immediate (fast-positive) while making removals require confirmation (slow-negative), which is the appropriate reconciliation model for an eventually-consistent source. It has no steady-state behavior change when fetches are complete.

Happy to open a PR with this + unit and integration tests (the integration test drives UpdateInstanceTypes against a fake resource client that omits a SKU on one call and asserts it is retained, then removed only after two consecutive omissions). Let me know if the maintainers prefer a different shape (e.g. threshold as an option, or routing transient unavailability through UnavailableOfferings instead of dropping from the catalog).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions