Support for custom CNI file name #4382
Conversation
This PR allows users to change the filename via installation manfiest.
example:
```
apiVersion: operator.tigera.io/v1
kind: Installation
metadata:
name: default
spec:
cni:
type: Calico
ipam:
type: Calico
confName: "00-calico.conflist"
```
| // please ensure that this field matches the same name as specified in the container runtime settings. | ||
| // Default: "10-calico.conflist" | ||
| // +optional | ||
| // +kubebuilder:validation:Type=string |
There was a problem hiding this comment.
We should add a kubebuilder default here as well
| // +kubebuilder:validation:Type=string | |
| // +kubebuilder:validation:Type=string | |
| // +kubebuilder:default=10-calico.conflist |
|
|
||
| // 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. |
There was a problem hiding this comment.
I don't think this field needs to match anything in the container runtime settings (looks like copy/paste from the ConfDir param)
| defaultCNIConfName := "10-calico.conflist" | ||
| instance.Spec.CNI.ConfName = &defaultCNIConfName |
There was a problem hiding this comment.
| defaultCNIConfName := "10-calico.conflist" | |
| instance.Spec.CNI.ConfName = &defaultCNIConfName | |
| instance.Spec.CNI.ConfName = ptr.To("10-calico.conflist") |
| } | ||
|
|
||
| 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 { |
There was a problem hiding this comment.
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.
|
|
||
| envVars := []corev1.EnvVar{ | ||
| {Name: "CNI_CONF_NAME", Value: "10-calico.conflist"}, | ||
| {Name: "CNI_CONF_NAME", Value: *c.cfg.Installation.CNI.ConfName}, |
There was a problem hiding this comment.
Would like to see a unit test for this field
| // Default: "10-calico.conflist" | ||
| // +optional | ||
| // +kubebuilder:validation:Type=string | ||
| ConfName *string `json:"confName,omitempty"` |
There was a problem hiding this comment.
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!
3 participants
Description
This PR allows users to change the filename via installation manfiest.
example:
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.