fix: reconcile external IPAM pool on config drift (mulga-siv-24) (#167)

initPool's first guard inverted: !errors.Is(err, ErrKeyNotFound) was
true when err==nil, so initPool returned immediately when a record
existed and never reached the drift-reconcile or already-initialized
branches. As a result, range_start/range_end/source changes in
spinifex.toml never propagated and nextAvailableExternalIP failed
with "invalid IP range:  - " on dhcp -> static transitions.

Restructure to: bail on hard errors, reconcile or no-op when record
exists, fall through to create when ErrKeyNotFound.

Verified on dev cluster: dhcp -> static toml change now reconciles
KV bounds on restart and RunInstances allocates from the new range.

Stale Allocated entries and stale gateway_ip on source change remain
out of scope for this hotfix; tracked under mulga-siv-24 follow-ups.
This commit is contained in:
Josh Siviour
2026-04-28 10:20:51 +10:00
committed by GitHub
parent a7f965526b
commit dd30790d51
+29 -28
View File
@@ -117,37 +117,38 @@ func (m *ExternalIPAM) initPools() error {
func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error {
chk, revision, err := m.getRecord(pool.Name)
// Catch all other errors
if !errors.Is(err, nats.ErrKeyNotFound) {
if err != nil && !errors.Is(err, nats.ErrKeyNotFound) {
return err
} else if errors.Is(err, nats.ErrKeyNotFound) {
// If key not found, proceed to creation
slog.Info("external IPAM pool not found, creating", "pool", pool.Name)
} else if chk.RangeStart != pool.RangeStart || chk.RangeEnd != pool.RangeEnd || chk.Source != pool.Source {
// Validate our pool has not changed, e.g editing spinifex.toml and specifying a new start/end/source
slog.Info("external IPAM pool start/end range changed, updating KV", "pool", pool.Name, "RangeStart", pool.RangeStart, "RangeEnd", pool.RangeEnd)
// Update record with the new start/end
chk.RangeStart = pool.RangeStart
chk.RangeEnd = pool.RangeEnd
chk.Source = pool.Source
data, err := json.Marshal(chk)
if err != nil {
return fmt.Errorf("marshal external IPAM record: %w", err)
}
if _, err := m.kv.Update(pool.Name, data, revision); err != nil {
slog.Warn("external IPAM update failed", "pool", pool.Name)
return err
}
return nil
} else if err == nil {
slog.Info("external IPAM pool already initialized", "pool", pool.Name)
return nil // Already exists
}
if err == nil {
if chk.RangeStart != pool.RangeStart || chk.RangeEnd != pool.RangeEnd || chk.Source != pool.Source {
slog.Info("external IPAM pool config drift, reconciling KV",
"pool", pool.Name,
"old_range", chk.RangeStart+"-"+chk.RangeEnd, "new_range", pool.RangeStart+"-"+pool.RangeEnd,
"old_source", chk.Source, "new_source", pool.Source)
chk.RangeStart = pool.RangeStart
chk.RangeEnd = pool.RangeEnd
chk.Source = pool.Source
data, err := json.Marshal(chk)
if err != nil {
return fmt.Errorf("marshal external IPAM record: %w", err)
}
if _, err := m.kv.Update(pool.Name, data, revision); err != nil {
slog.Warn("external IPAM update failed", "pool", pool.Name, "err", err)
return err
}
return nil
}
slog.Debug("external IPAM pool already initialized", "pool", pool.Name)
return nil
}
slog.Info("external IPAM pool not found, creating", "pool", pool.Name)
// TODO: Confirm gateway IP defined, should error, given we route traffic to the upstream DHCP gateway
gwIP := pool.GatewayIP
if gwIP == "" && !pool.IsDHCP() {