Default multiInterfaceMode to Multus on new installations - #5239
Open
fasaxc wants to merge 1 commit into
Open
Conversation
multiInterfaceMode selects which WorkloadEndpointConverter the Calico Enterprise components use. None, the previous default, gives the single-interface converter, which cannot express anything a modern cluster needs; Multus gives the converter that reads the k8s.v1.cni.cncf.io annotations and produces a WorkloadEndpoint per pod interface. New installations now get Multus. Existing clusters keep the mode they were installed with. The core controller writes its computed defaults back to the Installation spec precisely so that changing a default cannot disturb a running cluster, so every already-reconciled cluster carries multiInterfaceMode: None and the defaulting code never sees a nil. The default stays in fillDefaults rather than becoming a +kubebuilder:default: it only applies when the CNI is Calico, and validation rejects the field outright for any other CNI, so a schema default would fail every non-Calico-CNI Installation.
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: enhancement.
spec.calicoNetwork.multiInterfaceModeselects whichWorkloadEndpointConvertertheCalico Enterprise components use.
None, the previous default, gives thesingle-interface converter, which cannot express anything a modern cluster needs.
Multusgives the converter that reads thek8s.v1.cni.cncf.io/networksand.../network-statuspod annotations and produces a WorkloadEndpoint per pod interface.New installations now get
Multus.Existing clusters keep the mode they were installed with. The core controller writes
its computed defaults back to the Installation spec precisely so that changing a default
cannot disturb a running cluster ("ensures that we don't surprise anyone by changing
defaults in a future version of the operator"), so every already-reconciled cluster
carries
multiInterfaceMode: Noneand the defaulting code never sees a nil. #5203 keepsthis property by a different route — it records defaults in
status.Defaultsand seedsdefaulting from them first — so this change is correct both before and after that lands,
and the two don't need ordering.
The default stays in
fillDefaultsrather than becoming a+kubebuilder:default, eventhough
docs/api_design.mdprefers markers: it only applies when the CNI is Calico, andvalidation rejects the field outright for any other CNI, so a schema-level default would
fail every non-Calico-CNI Installation. It also has to stay distinguishable from a
user-set value for the
status.Defaultsbookkeeping in #5203.Defaulting stays unconditional across variants, as it is today. Only the Enterprise
render paths emit
MULTI_INTERFACE_MODE, so Calico OSS gets an inert value in its spec —already true of
None— and keeping it unconditional means an OSS cluster laterconverted to Enterprise keeps what was recorded at install time instead of picking up a
new default mid-life.
Two behaviour changes worth a reviewer's attention
pkg/controller/migration/converthas no persisted value, so it picks up
Multusat migration. This is benign for podswith no Multus annotations — the Multus converter's
defaultInterfaceForPodreproduces the default converter's WorkloadEndpoint name (
<node>-k8s-<pod>-eth0) andhost-side veth name (
cali+ 11 hex chars ofsha1("<ns>.<pod>")) exactly. But on acluster that already runs Multus with Calico in
nonemode, Calico will startcreating WorkloadEndpoints for the secondary interfaces.
projectcalico.org/network-attachment,projectcalico.org/network-attachment-namespaceandprojectcalico.org/network-interfaceto every WorkloadEndpoint.Testing
make ut— 101 suites, all pass. No render-fixture fallout: the render tests thatexercise this field construct the Installation with an explicit mode rather than going
through
fillDefaults.pkg/controller/installation/defaults_test.go— the two empty-instance tests(Calico and CalicoEnterprise) now assert the
Multusdefault. The existing "should notoverride custom configuration" cases already pin an explicit
Noneand pass unchanged,which is the regression guard for user-set values.
make gen-filesproduces exactly the two regenerated field descriptions (underspecand under
status.computed) and nothing else;make format-checkclean.Affected components:
pkg/controller/installation(defaulting),api/v1(doc comment), and the regenerated Installation CRD. Behaviour reaches
calico-node(install-cniandcalico-nodecontainers),calico-typha,calico-kube-controllersand the apiserver, all of which renderMULTI_INTERFACE_MODEfrom this field.Related: #5203.
Release Note
For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.