Skip to content

[PRODENG-3642] Validate pod CIDR against the live Swarm overlay address pool - #657

Open
james-nesbitt wants to merge 4 commits into
mainfrom
PRODENG-3642
Open

[PRODENG-3642] Validate pod CIDR against the live Swarm overlay address pool#657
james-nesbitt wants to merge 4 commits into
mainfrom
PRODENG-3642

Conversation

@james-nesbitt

@james-nesbitt james-nesbitt commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

What

Validate --pod-cidr against an existing swarm's live overlay pool instead of spec.mcr.swarmInstallFlags.

Why

The pool is fixed at swarm init and InitSwarm discards those flags, so setting --default-addr-pool — what the overlap error recommends — silenced the check while the live overlap remained.

How

  • swarm.DefaultAddrPool reads pool and swarm state in one docker info
  • GatherFacts stores it; ValidateFacts prefers it over config
  • Live overlap warns; new-install overlap still fails
  • Divergent configured pool reported with both values
  • Docs: immutability, reading the pool, change procedure

Testing

  • make unit-test green; 10 pre-existing pod CIDR tests unchanged
  • Immutability verified on MCR 29.1.3 — swarm update rejects the flag
  • Parser tests use output captured from MCR 29.1.3
  • Smoke coverage for the install and upgrade paths is being added on this branch; the upgrade harness applies twice against one cluster, so the second apply does run against a pre-existing swarm. The deliberate-overlap case stays opt-in, since an overlap can break host networking by design.

Links

Checklist

  • Tests added or updated
  • Docs updated if user-visible behaviour changed
  • No debug output or dead code left in

Written by AI: claude-opus-5

A swarm's default address pool is fixed when the swarm is created:
--default-addr-pool is a field of docker's swarm InitRequest but not of the
Spec that "docker swarm update" mutates, so no command changes it on a
running cluster. The InitSwarm phase already discards
spec.mcr.swarmInstallFlags when a swarm exists.

ValidateFacts nevertheless read the pool from configuration, so setting
--default-addr-pool - which is what the overlap error message recommends -
made the check pass while the live overlap remained. The guard against the
daemon restarting into a broken network state during MKE bootstrap was
silenced by the workaround it suggested, leaving the cluster exposed to the
failure it exists to prevent.

GatherFacts now reads the pool from the swarm leader, in the same call that
reports swarm state so that "no swarm" stays distinguishable from "swarm
with no explicit pool". ValidateFacts validates against that pool when a
swarm exists and against the configured pool before one does.

A live overlap warns rather than fails: the pool cannot be changed, so
failing would only block upgrades of clusters already running this way. The
pre-existing hard failure for new installations is unchanged, where the
configured pool is still what swarm init applies.

A configured pool that differs from the running swarm is now reported
explicitly, and the ignored-flags warning names the flags it drops.

Refs PRODENG-3642

Written by AI: claude-opus-5
Covers the behaviour added for PRODENG-3642: the live pool taking
precedence over the configured one, every live pool being checked, a live
overlap that the configuration hides being reported without failing the
run, and nothing being reported when the live pool does not overlap.

Divergence reporting is covered in both directions, including the case that
decides whether the warning is usable at all: a cluster that never
configured a pool must stay silent rather than compare an empty setting
against the fallback and warn on every apply.

Both functions are also covered with absent MCR metadata, which is
populated from yaml and can be missing when a config is built in code.

Parsing of the docker info output is split out of DefaultAddrPool so it can
be tested without a host. Its cases use output captured verbatim from MCR
29.1.3, so a change in docker's rendering fails a test rather than silently
mis-parsing against a live cluster. Swarm states other than active return
no pool, since reporting the fallback would assert a pool never observed.

Refs PRODENG-3642

Written by AI: claude-opus-5
…nd upgrade

Two properties of the PRODENG-3642 change cannot be shown without a real
cluster: that swarm init applies a configured pool, and that a pool
discovered from a running swarm is the one validation uses.

Existing-swarm behaviour is asserted in the upgrade test, which is the only
place CI applies twice against one cluster. The upgrade YAML now declares
the customer's configuration -- a pool the running swarm does not use and a
pod CIDR overlapping the pool it does -- and the test requires that the
apply still succeeds, that both conditions are reported, and that the
configured pool was genuinely not applied. Both settings are inert on an
existing cluster: InitSwarm discards swarmInstallFlags once a swarm exists
and UpgradeMKE passes upgradeFlags rather than installFlags, so this
declares the conflict without creating the network state the check prevents.

The install side is a separate label-gated test on its own two-node stack:
the overlap is refused while no swarm exists, configuring a pool resolves
it, and a second apply confirms the pool is in force. It omits the push
trigger and the broad smoke-test label, since it spends a stack on
validation logic that changes rarely.

Provisioning is duplicated rather than extracted from runSmokeTest.
Reshaping that function's deferred Destroy would put the teardown of four
existing smoke tests at risk of leaking VPCs (PRODENG-3631) to save about
forty lines.

Not run: both paths need AWS and have not been executed.

Refs PRODENG-3642

Written by AI: claude-opus-5
@james-nesbitt james-nesbitt added smoke-upgrade Run smoke-upgrade CI job smoke-swarm-pool Run swarm overlay address pool smoke test labels Aug 19, 2026
…lure

The install-side smoke test re-applies purely so that Gather Facts observes
the swarm created by the previous step. Requiring that whole apply to
succeed made the result depend on every later phase, and the first CI run
failed exactly that way: Validate Facts passed and the pool was discovered
correctly as 10.99.0.0/16, then Prepare hosts died installing base packages
from a public mirror with apt exit status 100.

Gather Facts runs before any of that, so the assertions are already decided
by the time those phases run. A failure to complete is now logged and the
pool assertion stands on its own; failing to discover the pool still fails
the test, on the assertion that checks it.

Refs PRODENG-3642

Written by AI: claude-opus-5
@james-nesbitt james-nesbitt removed the smoke-upgrade Run smoke-upgrade CI job label Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

smoke-swarm-pool Run swarm overlay address pool smoke test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant