diff --git a/.circleci/config.yml b/.circleci/config.yml index 35d86cb13c..3196ac2e50 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -46,8 +46,8 @@ commands: chmod +x ./kubectl sudo mv ./kubectl /usr/local/bin/kubectl - wget https://get.helm.sh/helm-v3.9.4-linux-amd64.tar.gz - tar -zxvf helm-v3.9.4-linux-amd64.tar.gz + wget https://get.helm.sh/helm-v3.7.0-linux-amd64.tar.gz + tar -zxvf helm-v3.7.0-linux-amd64.tar.gz sudo mv linux-amd64/helm /usr/local/bin/helm create-kind-clusters: parameters: diff --git a/charts/consul/templates/cni-clusterrole.yaml b/charts/consul/templates/cni-clusterrole.yaml index 84551205c0..39dc5ead50 100644 --- a/charts/consul/templates/cni-clusterrole.yaml +++ b/charts/consul/templates/cni-clusterrole.yaml @@ -27,4 +27,12 @@ rules: - {{ template "consul.fullname" . }}-cni verbs: - use +{{- if .Values.global.openshift.enabled}} +- apiGroups: ["security.openshift.io"] + resources: ["securitycontextconstraints"] + resourceNames: + - {{ template "consul.fullname" . }}-cni + verbs: + - use +{{- end }} {{- end }} diff --git a/charts/consul/templates/cni-daemonset.yaml b/charts/consul/templates/cni-daemonset.yaml index a74693da57..7b9f90d939 100644 --- a/charts/consul/templates/cni-daemonset.yaml +++ b/charts/consul/templates/cni-daemonset.yaml @@ -63,6 +63,7 @@ spec: - -log-level={{ default .Values.global.logLevel .Values.connectInject.cni.logLevel }} - -cni-bin-dir={{ .Values.connectInject.cni.cniBinDir }} - -cni-net-dir={{ .Values.connectInject.cni.cniNetDir }} + - -multus={{ .Values.connectInject.cni.multus }} {{- with .Values.connectInject.cni.resources }} resources: {{- toYaml . | nindent 12 }} diff --git a/charts/consul/templates/cni-networkattachmentdefinition.yaml b/charts/consul/templates/cni-networkattachmentdefinition.yaml new file mode 100644 index 0000000000..d0feaf5cb1 --- /dev/null +++ b/charts/consul/templates/cni-networkattachmentdefinition.yaml @@ -0,0 +1,25 @@ +{{- if (and (.Values.connectInject.cni.enabled) (.Values.connectInject.cni.multus)) }} +apiVersion: "k8s.cni.cncf.io/v1" +kind: NetworkAttachmentDefinition +metadata: + name: {{ template "consul.fullname" . }}-cni + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: cni +spec: + config: '{ + "cniVersion": "0.3.1", + "type": "consul-cni", + "cni_bin_dir": "{{ .Values.connectInject.cni.cniBinDir }}", + "cni_net_dir": "{{ .Values.connectInject.cni.cniNetDir }}", + "kubeconfig": "ZZZ-consul-cni-kubeconfig", + "log_level": "{{ default .Values.global.logLevel .Values.connectInject.cni.logLevel }}", + "multus": true, + "name": "consul-cni", + "type": "consul-cni" + }' +{{- end }} diff --git a/charts/consul/templates/cni-securitycontextconstraints.yaml b/charts/consul/templates/cni-securitycontextconstraints.yaml new file mode 100644 index 0000000000..6e76aefd84 --- /dev/null +++ b/charts/consul/templates/cni-securitycontextconstraints.yaml @@ -0,0 +1,50 @@ +{{- if (and (.Values.connectInject.cni.enabled) (.Values.global.openshift.enabled)) }} +apiVersion: security.openshift.io/v1 +kind: SecurityContextConstraints +metadata: + name: {{ template "consul.fullname" . }}-cni + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: cni + annotations: + kubernetes.io/description: {{ template "consul.fullname" . }}-cni are the security context constraints required + to run consul-cni. +allowHostDirVolumePlugin: true +allowHostIPC: false +allowHostNetwork: true +allowHostPID: false +allowHostPorts: true +allowPrivilegeEscalation: true +allowPrivilegedContainer: true +allowedCapabilities: null +defaultAddCapabilities: null +fsGroup: + type: MustRunAs +groups: [] +priority: null +readOnlyRootFilesystem: false +requiredDropCapabilities: +- KILL +- MKNOD +- SETUID +- SETGID +runAsUser: + type: MustRunAsRange +seLinuxContext: + type: MustRunAs +supplementalGroups: + type: MustRunAs +users: [] +volumes: +- configMap +- downwardAPI +- emptyDir +- persistentVolumeClaim +- projected +- secret +- hostPath +{{- end }} diff --git a/charts/consul/test/unit/cni-daemonset.bats b/charts/consul/test/unit/cni-daemonset.bats index e826642a00..0c423abfec 100644 --- a/charts/consul/test/unit/cni-daemonset.bats +++ b/charts/consul/test/unit/cni-daemonset.bats @@ -63,6 +63,7 @@ load _helpers --set 'connectInject.cni.logLevel=bar' \ --set 'connectInject.cni.cniBinDir=baz' \ --set 'connectInject.cni.cniNetDir=foo' \ + --set 'connectInject.cni.multus=false' \ bar \ . | tee /dev/stderr | yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) @@ -86,6 +87,10 @@ load _helpers local actual=$(echo "$cmd" | yq 'any(contains("cni-net-dir=foo"))' | tee /dev/stderr) [ "${actual}" = "true" ] + + local actual=$(echo "$cmd" | + yq 'any(contains("multus=false"))' | tee /dev/stderr) + [ "${actual}" = "true" ] } #-------------------------------------------------------------------- diff --git a/charts/consul/test/unit/cni-networkattachmentdefinition.bats b/charts/consul/test/unit/cni-networkattachmentdefinition.bats new file mode 100644 index 0000000000..a7f0d1da03 --- /dev/null +++ b/charts/consul/test/unit/cni-networkattachmentdefinition.bats @@ -0,0 +1,61 @@ +#!/usr/bin/env bats + +load _helpers + +@test "cni/NetworkAttachmentDefinition: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/cni-networkattachmentdefinition.yaml \ + . +} + +@test "cni/NetworkAttachmentDefinition: disabled when cni enabled and multus disabled" { + cd `chart_dir` + assert_empty helm template \ + -s templates/cni-securitycontextconstraints.yaml \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.cni.enabled=true' \ + --set 'connectInject.cni.multus=false' \ + . +} + +@test "cni/NetworkAttachmentDefinition: enabled when cni enabled and multus enabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/cni-networkattachmentdefinition.yaml \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.cni.enabled=true' \ + --set 'connectInject.cni.multus=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "cni/NetworkAttachmentDefinition: config is set" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/cni-networkattachmentdefinition.yaml \ + --set 'connectInject.cni.enabled=true' \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.cni.multus=true' \ + --set 'connectInject.cni.logLevel=bar' \ + --set 'connectInject.cni.cniBinDir=baz' \ + --set 'connectInject.cni.cniNetDir=foo' \ + bar \ + . | tee /dev/stderr | + yq -rc '.spec.config' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq '.log_level' | tee /dev/stderr) + [ "${actual}" = '"bar"' ] + + local actual=$(echo "$cmd" | + yq '.cni_bin_dir' | tee /dev/stderr) + [ "${actual}" = '"baz"' ] + + local actual=$(echo "$cmd" | + yq '.cni_net_dir' | tee /dev/stderr) + [ "${actual}" = '"foo"' ] + +} + diff --git a/charts/consul/test/unit/cni-securitycontextcontstraints.bats b/charts/consul/test/unit/cni-securitycontextcontstraints.bats new file mode 100644 index 0000000000..759979aee2 --- /dev/null +++ b/charts/consul/test/unit/cni-securitycontextcontstraints.bats @@ -0,0 +1,33 @@ +#!/usr/bin/env bats + +load _helpers + +@test "cni/SecurityContextConstraints: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/cni-securitycontextconstraints.yaml \ + . +} + +@test "cni/SecurityContextConstraints: disabled when cni disabled and global.openshift.enabled=true" { + cd `chart_dir` + assert_empty helm template \ + -s templates/cni-securitycontextconstraints.yaml \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.cni.enabled=false' \ + --set 'global.openshift.enabled=true' \ + . +} + +@test "cni/SecurityContextConstraints: enabled when cni enabled and global.openshift.enabled=true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/cni-securitycontextconstraints.yaml \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.cni.enabled=true' \ + --set 'global.openshift.enabled=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index cfe87e1205..deae0ad6a4 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -1960,7 +1960,20 @@ connectInject: # Location on the kubernetes node of all CNI configuration. Should be the absolute path and start with a '/' # @type: string cniNetDir: "/etc/cni/net.d" - + + # If multus CNI plugin is enabled with consul-cni. When enabled, consul-cni will not be installed as a chained + # CNI plugin. Instead, a NetworkAttachementDefinition CustomResourceDefinition (CRD) will be created in the helm + # release namespace. Following multus plugin standards, an annotation is required in order for the consul-cni plugin + # to be executed and for your service to be added to the Consul Service Mesh. + # + # Add the annotation `'k8s.v1.cni.cncf.io/networks': '[{ "name":"consul-cni","namespace": "consul" }]'` to your pod + # to use the default installed NetworkAttachementDefinition CRD. + # + # Please refer to the [Multus Quickstart Guide](https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/quickstart.md) + # for more information about using multus. + # @type: string + multus: false + # The resource settings for CNI installer daemonset. # @recurse: false # @type: map diff --git a/control-plane/cni/main.go b/control-plane/cni/main.go index c0685efb52..24c473a853 100644 --- a/control-plane/cni/main.go +++ b/control-plane/cni/main.go @@ -137,26 +137,28 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error { Level: hclog.LevelFromString(cfg.LogLevel), }) - // Check to see if the plugin is a chained plugin. - if cfg.PrevResult == nil { - return fmt.Errorf("must be called as final chained plugin") - } - logger.Debug("consul-cni plugin config", "config", cfg) - // Convert the PrevResult to a concrete Result type that can be modified. The CNI standard says - // that the previous result needs to be passed onto the next plugin. - prevResult, err := current.GetResult(cfg.PrevResult) - if err != nil { - return fmt.Errorf("failed to convert prevResult: %w", err) - } - if len(prevResult.IPs) == 0 { - return fmt.Errorf("got no container IPs") - } + // Only chained plugins have a previous result. + var result *current.Result - // Pass the prevResult through this plugin to the next one. - result := prevResult - logger.Debug("consul-cni previous result", "result", result) + // Check to see if the plugin is a chained plugin. + if cfg.PrevResult == nil { + // The plugin is not chained (ie multus) so create a fake result + result = ¤t.Result{ + CNIVersion: "0.3.1", + } + } else { + prevResult, err := current.GetResult(cfg.PrevResult) + if err != nil { + return fmt.Errorf("failed to convert prevResult: %w", err) + } + if len(prevResult.IPs) == 0 { + return fmt.Errorf("got no container IPs") + } + // Pass the prevResult through this plugin to the next one. + result = prevResult + } ctx := context.Background() if c.client == nil { @@ -186,7 +188,7 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error { // We do not throw an error here because kubernetes will often throw a benign error where the pod has been // updated in between the get and update of the annotation. Eventually kubernetes will update the annotation - ok := c.updateTransparentProxyStatusAnnotation(pod, podNamespace, waiting) + ok := c.updateTransparentProxyStatusAnnotation(podName, podNamespace, waiting) if !ok { logger.Info("unable to update %s pod annotation to waiting", keyTransparentProxyStatus) } @@ -213,7 +215,7 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error { // We do not throw an error here because kubernetes will often throw a benign error where the pod has been // updated in between the get and update of the annotation. Eventually kubernetes will update the annotation - ok = c.updateTransparentProxyStatusAnnotation(pod, podNamespace, complete) + ok = c.updateTransparentProxyStatusAnnotation(podName, podNamespace, complete) if !ok { logger.Info("unable to update %s pod annotation to complete", keyTransparentProxyStatus) } @@ -270,8 +272,13 @@ func parseAnnotation(pod corev1.Pod, annotation string) (iptables.Config, error) // updateTransparentProxyStatusAnnotation updates the transparent-proxy-status annotation. We use it as a simple inicator of // CNI status on the pod. Failing is not fatal. -func (c *Command) updateTransparentProxyStatusAnnotation(pod *corev1.Pod, namespace, status string) bool { +func (c *Command) updateTransparentProxyStatusAnnotation(podName, namespace, status string) bool { + // Refresh the pod so that we can update it without problems + pod, err := c.client.CoreV1().Pods(namespace).Get(context.Background(), podName, metav1.GetOptions{}) + if err != nil { + return false + } pod.Annotations[keyTransparentProxyStatus] = status - _, err := c.client.CoreV1().Pods(namespace).Update(context.Background(), pod, metav1.UpdateOptions{}) + _, err = c.client.CoreV1().Pods(namespace).Update(context.Background(), pod, metav1.UpdateOptions{}) return err == nil } diff --git a/control-plane/subcommand/install-cni/cniconfig.go b/control-plane/subcommand/install-cni/cniconfig.go index 2c40bcda36..e4d2078ad7 100644 --- a/control-plane/subcommand/install-cni/cniconfig.go +++ b/control-plane/subcommand/install-cni/cniconfig.go @@ -16,7 +16,7 @@ import ( // defaultCNIConfigFile gets the the correct config file from the cni net dir. // Adapted from kubelet: https://github.com/kubernetes/kubernetes/blob/954996e231074dc7429f7be1256a579bedd8344c/pkg/kubelet/dockershim/network/cni/cni.go#L134. func defaultCNIConfigFile(dir string) (string, error) { - files, err := libcni.ConfFiles(dir, []string{".conf", ".conflist", ".json"}) + files, err := libcni.ConfFiles(dir, []string{".conf", ".conflist"}) if err != nil { return "", fmt.Errorf("error while trying to find files in %s: %w", dir, err) } @@ -58,13 +58,57 @@ func defaultCNIConfigFile(dir string) (string, error) { // CNI config list has no networks, skipping". continue } - return confFile, nil } // There were files but none of them were valid return "", fmt.Errorf("no valid config files found in %s", dir) } +// confListFileFromConfFile converts a .conf file into a .conflist file. Chained plugins use .conflist files. +func confListFileFromConfFile(cfgFile string) (string, error) { + if !strings.HasSuffix(cfgFile, ".conf") { + return "", fmt.Errorf("invalid conf file: %s", cfgFile) + } + + // Convert the .conf file into a map so that we can remove pieces of it. + cfgMap, err := configFileToMap(cfgFile) + if err != nil { + return "", fmt.Errorf("could not convert .conf file to map: %w", err) + } + + // Remove the cniVersion header from the conf map. + delete(cfgMap, "cniVersion") + + // Create the new plugins: [] section and add the contents from cfgMap to it. + plugins := make([]map[string]interface{}, 1) + plugins[0] = cfgMap + + listMap := map[string]interface{}{ + "name": "k8s-pod-network", + "cniVersion": "0.3.1", + "plugins": plugins, + } + + listFile := fmt.Sprintf("%s%s", cfgFile, "list") + + // Marshal into a new json object. + listJSON, err := json.MarshalIndent(listMap, "", " ") + if err != nil { + return "", fmt.Errorf("error marshalling conflist: %w", err) + } + + // Libcni nuance/bug. If the newline is missing, the cni plugin will throw errors saying that it cannot get parse the config. + listJSON = append(listJSON, "\n"...) + + // Write the .conflist file out. + err = os.WriteFile(listFile, listJSON, os.FileMode(0o644)) + if err != nil { + return "", fmt.Errorf("error writing conflist file %s: %w", listFile, err) + } + + return listFile, nil +} + // The format of the main cni config file is unstructured json consisting of a header and list of plugins // // { diff --git a/control-plane/subcommand/install-cni/cniconfig_test.go b/control-plane/subcommand/install-cni/cniconfig_test.go index 961d59cba6..640b9d93cb 100644 --- a/control-plane/subcommand/install-cni/cniconfig_test.go +++ b/control-plane/subcommand/install-cni/cniconfig_test.go @@ -26,10 +26,11 @@ func TestDefaultCNIConfigFile_NoFiles(t *testing.T) { // TestDefaultCNIConfigFile tests finding the correct config file in the cniNetDir directory. func TestDefaultCNIConfigFile(t *testing.T) { cases := []struct { - name string - cfgFile string - dir func(string) string - expectedErr error + name string + cfgFile string + dir func(string) string + expectedFile string + expectedErr error }{ { name: "valid .conflist file found", @@ -42,7 +43,8 @@ func TestDefaultCNIConfigFile(t *testing.T) { } return tempDir }, - expectedErr: nil, + expectedFile: "10-kindnet.conflist", + expectedErr: nil, }, { name: "several files, should choose .conflist file", @@ -60,7 +62,8 @@ func TestDefaultCNIConfigFile(t *testing.T) { return tempDir }, - expectedErr: nil, + expectedFile: "10-kindnet.conflist", + expectedErr: nil, }, } for _, c := range cases { @@ -68,14 +71,38 @@ func TestDefaultCNIConfigFile(t *testing.T) { tempDir := c.dir(c.cfgFile) actual, err := defaultCNIConfigFile(tempDir) - filename := filepath.Base(c.cfgFile) - filepath := filepath.Join(tempDir, filename) + filepath := filepath.Join(tempDir, c.expectedFile) require.Equal(t, filepath, actual) require.Equal(t, c.expectedErr, err) }) } } +func TestConfListFromConfFile(t *testing.T) { + + cfgFile := "testdata/00-single-plugin.conf" + expectedCfgFile := "testdata/00-chained-plugins.conflist" + + tempDir := t.TempDir() + err := copyFile(cfgFile, tempDir) + require.NoError(t, err) + + filename := filepath.Base(cfgFile) + tempCfgFile := filepath.Join(tempDir, filename) + + actualFile, err := confListFileFromConfFile(tempCfgFile) + require.NoError(t, err) + + actual, err := ioutil.ReadFile(actualFile) + require.NoError(t, err) + + expected, err := ioutil.ReadFile(expectedCfgFile) + require.NoError(t, err) + + require.Equal(t, string(expected), string(actual)) + +} + // TestCreateCNIConfigFile tests the writing of the config file. func TestAppendCNIConfig(t *testing.T) { cases := []struct { @@ -112,6 +139,20 @@ func TestAppendCNIConfig(t *testing.T) { cfgFile: "testdata/10-calico.conflist", goldenFile: "testdata/10-calico.conflist.golden", }, + { + name: "chained plugin file", + consulConfig: &config.CNIConfig{ + Name: config.DefaultPluginName, + Type: config.DefaultPluginType, + CNIBinDir: "/var/lib/cni/bin", + CNINetDir: "/etc/kubernetes/cni/net.d", + Kubeconfig: config.DefaultKubeconfig, + LogLevel: config.DefaultLogLevel, + Multus: config.DefaultMultus, + }, + cfgFile: "testdata/00-chained-plugins.conflist", + goldenFile: "testdata/00-chained-plugins.conflist.golden", + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/control-plane/subcommand/install-cni/command.go b/control-plane/subcommand/install-cni/command.go index 22c1acdf84..7c481a0800 100644 --- a/control-plane/subcommand/install-cni/command.go +++ b/control-plane/subcommand/install-cni/command.go @@ -7,6 +7,7 @@ import ( "os" "os/signal" "path/filepath" + "strings" "sync" "syscall" @@ -117,7 +118,7 @@ func (c *Command) Run(args []string) int { defer cancel() // Generate the kubeconfig file that will be used by the plugin to communicate with the kubernetes api. - c.logger.Debug("Creating kubeconfig", "file", cfg.Kubeconfig) + c.logger.Info("Creating kubeconfig", "file", cfg.Kubeconfig) err := createKubeConfig(cfg.CNINetDir, cfg.Kubeconfig) if err != nil { c.logger.Error("could not create kube config", "error", err) @@ -125,7 +126,7 @@ func (c *Command) Run(args []string) int { } // Copy the consul-cni binary from the installer container to the host. - c.logger.Debug("Copying consul-cni binary", "destination", cfg.CNIBinDir) + c.logger.Info("Copying consul-cni binary", "destination", cfg.CNIBinDir) srcFile := filepath.Join(c.flagCNIBinSourceDir, consulCNIName) err = copyFile(srcFile, cfg.CNIBinDir) if err != nil { @@ -134,32 +135,50 @@ func (c *Command) Run(args []string) int { } // Get the config file that is on the host. - c.logger.Debug("Getting default config file from", "destination", cfg.CNINetDir) + c.logger.Info("Getting default config file from", "destination", cfg.CNINetDir) cfgFile, err := defaultCNIConfigFile(cfg.CNINetDir) if err != nil { c.logger.Error("could not get default CNI config file", "error", err) return 1 } - // The config file does not exist and it probably means that the consul-cni plugin was installed or scheduled - // before other cni plugins on the node. We will add a directory watcher and wait for another plugin to - // be installed. - if cfgFile == "" { - c.logger.Info("CNI config file not found. Consul-cni is a chained plugin and another plugin must be installed first. Waiting...", "directory", cfg.CNINetDir) - } else { - // Check if there is valid config in the config file. It is invalid if no consul-cni config exists, - // the consul-cni config is not the last in the plugin chain or the consul-cni config is different from - // what is passed into helm (it could happen in a helm upgrade). - err := validConfig(cfg, cfgFile) - if err != nil { - // The invalid config is not critical and we can recover from it. - c.logger.Info("Installing plugin", "reason", err) - err = appendCNIConfig(cfg, cfgFile) + // Install as a chained plugin. + if !cfg.Multus { + if strings.HasSuffix(cfgFile, ".conf") { + c.logger.Info("Converting .conf file to .conflist file", "file", cfgFile) + cfgFile, err = confListFileFromConfFile(cfgFile) if err != nil { - c.logger.Error("could not append configuration to config file", "error", err) + c.logger.Error("could convert .conf file to .conflist file", "error", err) return 1 } } + + // The config file does not exist and it probably means that the consul-cni plugin was installed or scheduled + // before other cni plugins on the node. We will add a directory watcher and wait for another plugin to + // be installed. + if cfgFile == "" { + c.logger.Info("CNI config file not found. Consul-cni is a chained plugin and another plugin must be installed first. Waiting...", "directory", cfg.CNINetDir) + } else { + // Check if there is valid config in the config file. It is invalid if no consul-cni config exists, + // the consul-cni config is not the last in the plugin chain or the consul-cni config is different from + // what is passed into helm (it could happen in a helm upgrade). + c.logger.Info("Using config file", "file", cfgFile) + err := validConfig(cfg, cfgFile) + if err != nil { + // The invalid config is not critical and we can recover from it. + c.logger.Info("Installing plugin", "reason", err) + err = appendCNIConfig(cfg, cfgFile) + if err != nil { + c.logger.Error("could not append configuration to config file", "error", err) + return 1 + } + } + } + } else { + // When multus is enabled, the plugin configuration is set in a NetworkAttachementDefinition CRD and multus + // handles the configuration and running of the consul-cni plugin. Also, we add a `k8s.v1.cni.cncf.io/networks: consul-cni` + // annotation during connect inject so that multus knows to run the consul-cni plugin. + c.logger.Info("Multus enabled, using multus NetworkAttachementDefinition for configuration") } // Watch for changes in the cniNetDir directory and fix/install the config file if need be. @@ -220,32 +239,44 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d // config file is available, do nothing. This can happen if the consul-cni daemonset was // created before other CNI plugins were installed. if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 { - c.logger.Info("Modified event", "event", event) - // Always get the config file that is on the host as we do not know if it was deleted - // or not. - cfgFile, err = defaultCNIConfigFile(dir) - if err != nil { - c.logger.Error("Unable get default config file", "error", err) - break - } - if cfgFile != "" { - c.logger.Info("Using config file", "file", cfgFile) - - err = validConfig(cfg, cfgFile) + // Only repair things if this is a non-multus setup. Multus config is handled differently + // than chained plugins + if !cfg.Multus { + c.logger.Info("Modified event", "event", event) + // Always get the config file that is on the host as we do not know if it was deleted + // or not. + cfgFile, err = defaultCNIConfigFile(dir) if err != nil { - // The invalid config is not critical and we can recover from it. - c.logger.Info("Installing plugin", "reason", err) - err = appendCNIConfig(cfg, cfgFile) + c.logger.Error("Unable get default config file", "error", err) + break + } + + if strings.HasSuffix(cfgFile, ".conf") { + cfgFile, err = confListFileFromConfFile(cfgFile) if err != nil { - c.logger.Error("Unable to install consul-cni config", "error", err) - return err + c.logger.Error("could convert .conf file to .conflist file", "error", err) + break } - } else { - c.logger.Info("Valid config file detected, nothing to do") - break } - } + if cfgFile != "" { + c.logger.Info("Using config file", "file", cfgFile) + + err = validConfig(cfg, cfgFile) + if err != nil { + // The invalid config is not critical and we can recover from it. + c.logger.Info("Installing plugin", "reason", err) + err = appendCNIConfig(cfg, cfgFile) + if err != nil { + c.logger.Error("Unable to install consul-cni config", "error", err) + return err + } + } else { + c.logger.Info("Valid config file detected, nothing to do") + break + } + } + } } case err, ok := <-watcher.Errors: if !ok { diff --git a/control-plane/subcommand/install-cni/testdata/00-chained-plugins.conflist b/control-plane/subcommand/install-cni/testdata/00-chained-plugins.conflist new file mode 100644 index 0000000000..7f9e222ede --- /dev/null +++ b/control-plane/subcommand/install-cni/testdata/00-chained-plugins.conflist @@ -0,0 +1,23 @@ +{ + "cniVersion": "0.3.1", + "name": "k8s-pod-network", + "plugins": [ + { + "binDir": "/opt/multus/bin", + "delegates": [ + { + "cniVersion": "0.3.1", + "name": "openshift-sdn", + "type": "openshift-sdn" + } + ], + "globalNamespaces": "default,openshift-multus,openshift-sriov-network-operator", + "kubeconfig": "/etc/kubernetes/cni/net.d/multus.d/multus.kubeconfig", + "logLevel": "verbose", + "name": "multus-cni-network", + "namespaceIsolation": true, + "readinessindicatorfile": "/var/run/multus/cni/net.d/80-openshift-network.conf", + "type": "multus" + } + ] +} diff --git a/control-plane/subcommand/install-cni/testdata/00-chained-plugins.conflist.golden b/control-plane/subcommand/install-cni/testdata/00-chained-plugins.conflist.golden new file mode 100644 index 0000000000..5a7ed53311 --- /dev/null +++ b/control-plane/subcommand/install-cni/testdata/00-chained-plugins.conflist.golden @@ -0,0 +1,32 @@ +{ + "cniVersion": "0.3.1", + "name": "k8s-pod-network", + "plugins": [ + { + "binDir": "/opt/multus/bin", + "delegates": [ + { + "cniVersion": "0.3.1", + "name": "openshift-sdn", + "type": "openshift-sdn" + } + ], + "globalNamespaces": "default,openshift-multus,openshift-sriov-network-operator", + "kubeconfig": "/etc/kubernetes/cni/net.d/multus.d/multus.kubeconfig", + "logLevel": "verbose", + "name": "multus-cni-network", + "namespaceIsolation": true, + "readinessindicatorfile": "/var/run/multus/cni/net.d/80-openshift-network.conf", + "type": "multus" + }, + { + "cni_bin_dir": "/var/lib/cni/bin", + "cni_net_dir": "/etc/kubernetes/cni/net.d", + "kubeconfig": "ZZZ-consul-cni-kubeconfig", + "log_level": "info", + "multus": false, + "name": "consul-cni", + "type": "consul-cni" + } + ] +} diff --git a/control-plane/subcommand/install-cni/testdata/00-single-plugin.conf b/control-plane/subcommand/install-cni/testdata/00-single-plugin.conf new file mode 100644 index 0000000000..a111fc091a --- /dev/null +++ b/control-plane/subcommand/install-cni/testdata/00-single-plugin.conf @@ -0,0 +1,18 @@ +{ + "cniVersion": "0.3.1", + "name": "multus-cni-network", + "type": "multus", + "namespaceIsolation": true, + "globalNamespaces": "default,openshift-multus,openshift-sriov-network-operator", + "logLevel": "verbose", + "binDir": "/opt/multus/bin", + "readinessindicatorfile": "/var/run/multus/cni/net.d/80-openshift-network.conf", + "kubeconfig": "/etc/kubernetes/cni/net.d/multus.d/multus.kubeconfig", + "delegates": [ + { + "cniVersion": "0.3.1", + "name": "openshift-sdn", + "type": "openshift-sdn" + } + ] +}