Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Master CNI file flag option isn't working for thin plugin #1226

Closed
poojapatil010677 opened this issue Feb 7, 2024 · 3 comments · Fixed by #1270
Closed

Master CNI file flag option isn't working for thin plugin #1226

poojapatil010677 opened this issue Feb 7, 2024 · 3 comments · Fixed by #1270

Comments

@poojapatil010677
Copy link

What happend:
I am trying to use use the --multus-master-cni-file-name flag with the thin-plugin. It is expected that it should use the cni file name passed here everytime as default master cni and not the alphabetically first file in the cni-bin directory. But this feature does not seem to work and it always picks up the first file alphabetically instead of what is passed in that flag.

I checked the code and I see that the file name is taken and is not being used to configure anything in this function:

func (o *Options) createMultusConfig() (string, error) {

Tested this for thick-plugin and works as expected.

What you expected to happen:
It was expected that the master cni is always the plugin file that is passed.

How to reproduce it (as minimally and precisely as possible):
Install the helm chart on a k8s cluster. Additionally, include the flag --multus-master-cni-file-name under args in the daemonset yaml.

Anything else we need to know?:

Environment:

  • Multus version : lates
    image path and image ID (from 'docker images')
  • Kubernetes version (use kubectl version): 1.25
  • Primary CNI for Kubernetes cluster: calico
  • OS (e.g. from /etc/os-release):
  • File of '/etc/cni/net.d/'
  • File of '/etc/cni/multus/net.d'
  • NetworkAttachment info (use kubectl get net-attach-def -o yaml)
  • Target pod yaml info (with annotation, use kubectl get pod <podname> -o yaml)
  • Other log outputs (if you use multus logging)
@jhoblitt
Copy link

I ran into this as well upgrading from (ancient) 3.4 -> 4.0.2. With

     - name: kube-multus
        image: ghcr.io/k8snetworkplumbingwg/multus-cni:v4.0.2
        command: ["/thin_entrypoint"]
        args:
        - "--multus-conf-file=auto"
        - "--multus-autoconfig-dir=/host/etc/cni/net.d"
        - "--cni-conf-dir=/host/etc/cni/net.d"
        - "--multus-master-cni-file-name=10-canal.conflist"

It keeps sucking in 00-multus.conf on every restart:

pretty printed

{
  "cniVersion": "0.3.1",
  "name": "multus-cni-network",
  "type": "multus",
  "kubeconfig": "/etc/cni/net.d/multus.d/multus.kubeconfig",
  "delegates": [
    {
      "cniVersion": "0.3.1",
      "delegates": [
        {
          "cniVersion": "0.3.1",
          "delegates": [
            {
              "cniVersion": "0.3.1",
              "delegates": [
                {
                  "cniVersion": "0.3.1",
                  "delegates": [
                    {
                      "cniVersion": "0.3.1",
                      "name": "k8s-pod-network",
                      "plugins": [
                        {
                          "datastore_type": "kubernetes",
                          "ipam": {
                            "subnet": "usePodCidr",
                            "type": "host-local"
                          },
                          "kubernetes": {
                            "kubeconfig": "/etc/cni/net.d/calico-kubeconfig"
                          },
                          "log_file_path": "/var/log/calico/cni/cni.log",
                          "log_level": "info",
                          "mtu": 1450,
                          "nodename": "kueyen01",
                          "policy": {
                            "type": "k8s"
                          },
                          "type": "calico"
                        },
                        {
                          "capabilities": {
                            "portMappings": true
                          },
                          "snat": true,
                          "type": "portmap"
                        },
                        {
                          "capabilities": {
                            "bandwidth": true
                          },
                          "type": "bandwidth"
                        }
                      ]
                    }
                  ],
                  "kubeconfig": "/etc/cni/net.d/multus.d/multus.kubeconfig",
                  "name": "multus-cni-network",
                  "type": "multus"
                }
              ],
              "kubeconfig": "/etc/cni/net.d/multus.d/multus.kubeconfig",
              "name": "multus-cni-network",
              "type": "multus"
            }
          ],
          "kubeconfig": "/etc/cni/net.d/multus.d/multus.kubeconfig",
          "name": "multus-cni-network",
          "type": "multus"
        }
      ],
      "kubeconfig": "/etc/cni/net.d/multus.d/multus.kubeconfig",
      "name": "multus-cni-network",
      "type": "multus"
    }
  ]
}

It seems like the default output file should be filtered out of the input cni config file list.

@dougbtv
Copy link
Member

dougbtv commented Feb 29, 2024

Thanks for the heads up. When we deprecated the entrypoint.sh it looks like this parameter might've been missed.

@ordovicia
Copy link
Contributor

HI, I will work on this issue next a few weeks.

ordovicia added a commit to ordovicia/multus-cni that referenced this issue Apr 25, 2024
Multus v3.9.3 has `--multus-master-cni-file-name` flag to specify the
name of a primary CNI config file.
https://github.com/k8snetworkplumbingwg/multus-cni/blob/v3.9.3/images/entrypoint.sh#L22

In Multus v4.0.2, the thin plugin has the flag defined, but it is not
read and so does not have effect.

This pull request fixes the problem by making the thin plugin correctly
handles `--multus-master-cni-file-name` flag.

Fixes k8snetworkplumbingwg#1226

Signed-off-by: Hidehito Yabuuchi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants