Treat an unset clusterRoutingMode as the mode Calico defaults to - #5201
Merged
nelljerram merged 1 commit intoAug 16, 2026
Merged
Conversation
clusterRoutingMode returned "" when the field was unset, so felixProgramsIPIPClusterRoutes returned false and validation concluded that BIRD owned the IPIP cluster routes. Since Calico v3.33 that is wrong: Calico's own defaults give Felix the routes for IPIP IP Pools and leave the unencapsulated ones with BIRD. The visible effect is that a cluster which has simply taken Calico's default -- the common case, since the operator writes neither programClusterRoutes field when clusterRoutingMode is unset -- cannot disable BGP even when it has only IPIP pools. The Installation is rejected with "with BIRD cluster routing mode, IPIP encapsulation requires that BGP is enabled", and the operator degrades without touching the DaemonSet. Return FelixIPIPOnly for an unset mode instead, which is exactly the split those defaults produce, and both predicates then come out right: Felix owns IPIP, BIRD keeps no-encap. This does tie the operator to the Calico version it ships with, which is why the unset case previously returned "". The trade is deliberate: every caller has to reach some conclusion about who owns the routes, and answering "assume BIRD" is not neutral -- it rejects a configuration that works. Revisit when the no-encap default moves. Deciding whether to *write* the programClusterRoutes fields is unaffected: setClusterRoutingOnFelixConfiguration and setClusterRoutingOnBGPConfiguration test the field for nil directly, so an unset mode still writes neither, and still means "whatever Calico's defaults are" rather than pinning today's defaults into the datastore. The three validation tests named for BIRD cluster routing mode were relying on unset meaning BIRD; they now set that mode explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Installation validation behavior when spec.calicoNetwork.clusterRoutingMode is unset by treating it as Calico’s effective default behavior (since Calico v3.33): FelixIPIPOnly (Felix owns IPIP cluster routes; BIRD owns no-encap). This prevents a common defaulted cluster (unset mode + only IPIP pools) from being incorrectly blocked when disabling BGP.
Changes:
- Update
clusterRoutingMode()to returnFelixIPIPOnlywhen the field is unset, so downstream predicates reflect Calico’s effective defaults. - Fix three validation tests that were implicitly relying on “unset means BIRD” by explicitly setting
ClusterRoutingModeBIRD. - Add two new validation tests to pin the new/desired behavior for unset mode with IPIP vs no-encap pools when BGP is disabled.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/controller/installation/core_controller.go | Treat unset clusterRoutingMode as effective FelixIPIPOnly to match Calico v3.33 defaults and drive correct route-ownership predicates. |
| pkg/controller/installation/validation_test.go | Make BIRD-mode tests explicit and add coverage for the updated “unset mode” validation behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rene-dekker
approved these changes
Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type: bug fix. Note: master is currently frozen, so this is raised for review rather than
immediate merge.
clusterRoutingMode()returned""for an unsetclusterRoutingMode, sofelixProgramsIPIPClusterRoutes()returned false and the Installation validation concluded thatBIRD owned the IPIP cluster routes. Since Calico v3.33 that is wrong: Calico's own defaults give
Felix the cluster routes for IPIP IP Pools, and leave the unencapsulated ones with BIRD.
The visible effect is that a cluster which has simply taken Calico's default — the common case,
since the operator writes neither
programClusterRoutesfield whenclusterRoutingModeis unset —cannot disable BGP even when it only has IPIP pools. The Installation is accepted by the API
server but the operator degrades without touching the DaemonSet:
This returns
FelixIPIPOnlyfor an unset mode instead — exactly the split Calico's defaultsproduce — so both predicates come out right: Felix owns IPIP, BIRD keeps no-encap.
The trade-off
Returning the effective mode ties the operator to the Calico version it ships with, which is why
the unset case previously returned
""(see the comment being replaced, from #5150). The trade isdeliberate: every caller has to reach some conclusion about who owns the routes, and answering
"nobody knows, so assume BIRD" is not a neutral default — it rejects a configuration that works.
The operator is released against a known Calico version, so encoding that version's defaults is
well defined. It will need revisiting when the no-encap default moves.
Deciding whether to write the
programClusterRoutesfields is unaffected:setClusterRoutingOnFelixConfigurationandsetClusterRoutingOnBGPConfigurationtest the field fornil directly rather than going through this helper, so an unset mode still writes neither field, and
still means "whatever Calico's defaults are" rather than pinning today's defaults into the
datastore.
Testing
they now set that mode explicitly, so they test what their names claim.
go test ./pkg/controller/installation/: 306 passed, same 14 pre-existing failures as on cleanmaster (
Installation CRD CEL validation, which needs envtest binaries not present locally —318/304/14 before, 320/306/14 after).
which needs to disable BGP on an IPIP cluster and was blocked by this.
Related issues/PRs
Follows #5150, which added
FelixIPIPOnly. Related to projectcalico/calico#13470.Release Note