Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/v1/installation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,14 @@ type CNISpec struct {
// +optional
// +kubebuilder:validation:Type=string
ConfDir *string `json:"confDir,omitempty"`

// ConfName is the name of the CNI config file.
// If you have changed the name of the CNI configuration file in the container runtime configuration,
// please ensure that this field matches the same name as specified in the container runtime settings.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this field needs to match anything in the container runtime settings (looks like copy/paste from the ConfDir param)

// Default: "10-calico.conflist"
// +optional
// +kubebuilder:validation:Type=string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a kubebuilder default here as well

Suggested change
// +kubebuilder:validation:Type=string
// +kubebuilder:validation:Type=string
// +kubebuilder:default=10-calico.conflist

ConfName *string `json:"confName,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing we are going to have to solve is what to do if this value is changed to something with less priority.

e.g., if you switch from 20-calico.conflist to 30-calico.conflist, the new configuration file will be ignored by the container runtime because the old configuration sorts higher.

We have some very rudimentary support for removing old config files here: https://github.com/projectcalico/calico/blob/master/cni-plugin/pkg/install/install.go#L413-L420

But it requires knowing the name of the file you want to remove!

}

// InstallationStatus defines the observed state of the Calico or Calico Enterprise installation.
Expand Down
5 changes: 5 additions & 0 deletions api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions pkg/controller/installation/core_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,12 @@ func fillDefaults(instance *operatorv1.Installation, currentPools *crdv1.IPPoolL
instance.Spec.CNI.BinDir = &defaultCNIBinDir
}

// For compatability reasons, default the CNI config name if not specified.
if instance.Spec.CNI.ConfName == nil {
defaultCNIConfName := "10-calico.conflist"
instance.Spec.CNI.ConfName = &defaultCNIConfName
Comment on lines +641 to +642
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
defaultCNIConfName := "10-calico.conflist"
instance.Spec.CNI.ConfName = &defaultCNIConfName
instance.Spec.CNI.ConfName = ptr.To("10-calico.conflist")

}

// While a number of the fields in this section are relevant to all CNI plugins,
// there are some settings which are currently only applicable if using Calico CNI.
// Handle those here.
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/migration/convert/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func handleCore(c *components, install *operatorv1.Installation) error {
}
}

if err := c.node.assertEnv(ctx, c.client, containerInstallCNI, "CNI_CONF_NAME", "10-calico.conflist"); err != nil {
if err := c.node.assertEnv(ctx, c.client, containerInstallCNI, "CNI_CONF_NAME", *install.Spec.CNI.ConfName); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of asserting the env var matches, we should get the CNI_CONF_NAME env var and update the install.Spec.CNI.ConfName field to match.

This means we can now upgrade manifest -> operator even if a custom CNI config name has been used.

return err
}
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/crds/operator/operator.tigera.io_installations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5410,6 +5410,13 @@ spec:
* For KubernetesProvider OpenShift, this field defaults to "/var/run/multus/cni/net.d".
* Otherwise, this field defaults to "/etc/cni/net.d".
type: string
confName:
description: |-
ConfName is the name of the CNI config file.
If you have changed the name of the CNI configuration file in the container runtime configuration,
please ensure that this field matches the same name as specified in the container runtime settings.
Default: "10-calico.conflist"
type: string
ipam:
description: |-
IPAM specifies the pod IP address management that will be used in the Calico or
Expand Down Expand Up @@ -14288,6 +14295,13 @@ spec:
* For KubernetesProvider OpenShift, this field defaults to "/var/run/multus/cni/net.d".
* Otherwise, this field defaults to "/etc/cni/net.d".
type: string
confName:
description: |-
ConfName is the name of the CNI config file.
If you have changed the name of the CNI configuration file in the container runtime configuration,
please ensure that this field matches the same name as specified in the container runtime settings.
Default: "10-calico.conflist"
type: string
ipam:
description: |-
IPAM specifies the pod IP address management that will be used in the Calico or
Expand Down
2 changes: 1 addition & 1 deletion pkg/render/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,7 @@ func (c *nodeComponent) cniEnvvars() []corev1.EnvVar {
}

envVars := []corev1.EnvVar{
{Name: "CNI_CONF_NAME", Value: "10-calico.conflist"},
{Name: "CNI_CONF_NAME", Value: *c.cfg.Installation.CNI.ConfName},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to see a unit test for this field

{Name: "SLEEP", Value: "false"},
{Name: "CNI_NET_DIR", Value: *c.cfg.Installation.CNI.ConfDir},
{
Expand Down
4 changes: 2 additions & 2 deletions pkg/render/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (c *windowsComponent) cniEnvVars() []corev1.EnvVar {
{Name: "SLEEP", Value: "false"},
{Name: "CNI_PLUGIN_TYPE", Value: string(c.cfg.Installation.CNI.Type)},
{Name: "CNI_BIN_DIR", Value: "/host/opt/cni/bin"},
{Name: "CNI_CONF_NAME", Value: "10-calico.conflist"},
{Name: "CNI_CONF_NAME", Value: *c.cfg.Installation.CNI.ConfName},
{Name: "CNI_NET_DIR", Value: cniNetDir},
{Name: "KUBERNETES_DNS_SERVERS", Value: strings.Join(c.cfg.K8sDNSServers, ",")},
{Name: "KUBERNETES_SERVICE_CIDRS", Value: strings.Join(c.cfg.Installation.ServiceCIDRs, ",")},
Expand Down Expand Up @@ -415,7 +415,7 @@ func (c *windowsComponent) uninstallEnvVars() []corev1.EnvVar {
{Name: "SLEEP", Value: "false"},
{Name: "CNI_PLUGIN_TYPE", Value: string(c.cfg.Installation.CNI.Type)},
{Name: "CNI_BIN_DIR", Value: "/host/opt/cni/bin"},
{Name: "CNI_CONF_NAME", Value: "10-calico.conflist"},
{Name: "CNI_CONF_NAME", Value: *c.cfg.Installation.CNI.ConfName},
{Name: "CNI_NET_DIR", Value: "/host/etc/cni/net.d"},
}

Expand Down
Loading