Support CNI RuntimeConfig for portmap/bandwidth/ip/mac

[s1061123]

This changes introduce CNI RuntimeConfig for portmap, bandwidth, ip and mac for latest net-attach-def specification. IP and Mac is previously applied through CNI_ARGS, but it is changed to RuntimeConfig for now.

[coveralls]

Pull Request Test Coverage Report for Build 910

• 88 of 120 (73.33%) changed or added relevant lines in 3 files are covered.
• 5 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.07%) to 74.745%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
types/conf.go	34	41	82.93%
multus/multus.go	46	55	83.64%
k8sclient/k8sclient.go	8	24	33.33%

Files with Coverage Reduction	New Missed Lines	%
k8sclient/k8sclient.go	1	75.82%
types/conf.go	1	82.44%
multus/multus.go	3	70.86%

Totals
Change from base Build 909:	0.07%
Covered Lines:	1024
Relevant Lines:	1370

---

💛 - Coveralls

[booxter]

Doesn't it break backwards compatibility with CNI plugins that implemented the previous specification where e.g. mac is passed through CNI_ARGS? Won't these plugins break when this is merged? Shouldn't Multus provide a compatibility layer for these plugins to continue working until they adopt the newer way of passing the information?

I am specifically concerned about SR-IOV CNI plugin that implements e.g. mac attribute as was so far defined in the WG standard document.

[s1061123]

@booxter , Thank you for your comment. I'm planning old CNI_ARGS support by 1-2 minor version (i.e. 6-8month) as K8s does (update version and change a lot).
Then planning to will remove the CNI_ARGS code. In NPWG spec, we just mention that "mac" filed is available, but NPWG spec does not mention how we implemented.
Hence, CNI_ARGS for mac, staticip are multus original feature, apart from NPWG standards.

[ahalimx86]

@booxter , Thank you for your comment. I'm planning old CNI_ARGS support by 1-2 minor version (i.e. 6-8month) as K8s does (update version and change a lot).
Then planning to will remove the CNI_ARGS code. In NPWG spec, we just mention that "mac" filed is available, but NPWG spec does not mention how we implemented.

Hi @s1061123, what is the mechanism of Multus to reflect its version info into delegate plugins? How the delegate plugin will know through which channel the required parameters are available? Obviously, this will require the delegate plugin to be aware of Multus versions.

[booxter]

@s1061123 thanks for reply. It's nice to hear that you plan for compatibility. I've reviewed the change proposed against and I couldn't find where the compatibility layer at least for MAC address is. I probably should try it out myself, but in the meantime, while I get there, could you please point out which particular lines implement the compatibility layer for CNI_ARGS? Thank you.

[dougbtv]

Hey Tomo I went to do a functional test of this, and I might have something wrong, but, here's what I used...

Here's the net-attach-def I used...

[centos@kube-singlehost-master multus-cni]$ cat nad.yaml 
apiVersion: "k8s.cni.cncf.io/v1"
kind: NetworkAttachmentDefinition
metadata:
  name: centos-runtimeconfig-def
spec:
  config: '{
      "cniVersion": "0.3.1",
      "plugins": [{
          "type": "macvlan",
          "capabilites": { "ips": true },
          "master": "eth0",
          "mode": "bridge",
          "ipam": {
            "type": "static"
          }
        },
        {
          "capabilities": { "mac": true },
          "type": "tuning"
        }]
    }'

And I created a pod like so...

[centos@kube-singlehost-master multus-cni]$ cat pod.yaml 
apiVersion: v1
kind: Pod
metadata:
  name: samplepod
  annotations:
    k8s.v1.cni.cncf.io/networks: '[
      {
        "name": "centos-runtimeconfig-def",
        "ips": [ "192.168.2.205/24" ],
        "mac": "CA:FE:C0:FF:EE:00"
      }
    ]'
spec:
  containers:
  - name: samplepod
    command: ["/bin/bash", "-c", "trap : TERM INT; sleep infinity & wait"]
    image: dougbtv/centos-network

You can see Multus is built with the latest commit from this PR...

[centos@kube-singlehost-master multus-cni]$ /opt/cni/bin/multus --version
multus-cni version:master, commit:62ba0ff0e31b54146a08ffdc5962229819eefe85, date:2019-10-09T18:40:09+0000

However, the pod fails to spin up with error output reading AddNetworkList: IPAM plugin returned missing IP config:

[centos@kube-singlehost-master multus-cni]$ kubectl describe pod samplepod | grep -A4 -P "^Events"
Events:
  Type     Reason                  Age                   From                             Message
  ----     ------                  ----                  ----                             -------
  Normal   Scheduled               <unknown>             default-scheduler                Successfully assigned default/samplepod to kube-singlehost-master
  Warning  FailedCreatePodSandBox  2m12s                 kubelet, kube-singlehost-master  Failed create pod sandbox: rpc error: code = Unknown desc = failed to set up sandbox container "276efd10842ca9d50cba4154cae3ac035887281bdad9ce34c2bed05d0fa0b841" network for pod "samplepod": networkPlugin cni failed to set up pod "samplepod_default" network: Multus: Err adding pod to network "centos-runtimeconfig-def": Multus: error in invoke Conflist add - "centos-runtimeconfig-def": error in getting result from AddNetworkList: IPAM plugin returned missing IP config

[s1061123]

@booxter nice catch! I may removed at debugging.... Now I've reverted.

[s1061123]

@dougbtv your yaml contains typo (s/"capabilites"/"capabilities"). well, I also took an hour to debug....

          "capabilites": { "ips": true },

[dougbtv]

Tomo thanks for taking the time to debug my typo!!! Appreciate it.

Works perfectly with that typo fixed.

net-attach-def used:

apiVersion: "k8s.cni.cncf.io/v1"
kind: NetworkAttachmentDefinition
metadata:
  name: centos-runtimeconfig-def
spec:
  config: '{
      "cniVersion": "0.3.1",
      "plugins": [{
          "type": "macvlan",
          "capabilities": { "ips": true },
          "master": "eth0",
          "mode": "bridge",
          "ipam": {
            "type": "static"
          }
        }, {
          "capabilities": { "mac": true },
          "type": "tuning"
        }]
    }'

pod spec used:

apiVersion: v1
kind: Pod
metadata:
  name: samplepod
  annotations:
    k8s.v1.cni.cncf.io/networks: '[
      {
        "name": "centos-runtimeconfig-def",
        "ips": [ "192.168.2.205/24" ],
        "mac": "CA:FE:C0:FF:EE:00"
      }
    ]'
spec:
  containers:
  - name: samplepod
    command: ["/bin/bash", "-c", "trap : TERM INT; sleep infinity & wait"]
    image: dougbtv/centos-network

[zshi-redhat]

@s1061123 thanks for reply. It's nice to hear that you plan for compatibility. I've reviewed the change proposed against and I couldn't find where the compatibility layer at least for MAC address is. I probably should try it out myself, but in the meantime, while I get there, could you please point out which particular lines implement the compatibility layer for CNI_ARGS? Thank you.

@s1061123 I tested this PR with SR-IOV CNI change, RuntimeConfig for mac configuration works fine, but it seems old CNI_ARGs (via envArgs) doesn't work (MAC specified in Pod annotation is not applied on VF).
If I use latest Multus master, CNI_ARGs works.

[Elegant996]

Applied this same test (with the correction) on the latest-amd64 tag but received the following:

Failed create pod sandbox: rpc error: code = Unknown desc = failed to create pod network sandbox k8s_samplepod_default_c786662f-e2ea-48a8-9f57-0cf85fc81f5e_0(89349eed2d87dfa597f3c623aa6a632f3755a19d1273712ef1f582ada24cad99): Multus: error adding pod to network "centos-runtimeconfig-def": delegateAdd: error invoking conflistAdd - "centos-runtimeconfig-def": conflistAdd: error in getting result from AddNetworkList: failed to add IP addr {Version:4 Interface:0xc0000fa530 Address:{IP:192.168.2.205 Mask:ffffff00} Gateway:<nil>} to "net1": file exists

[s1061123]

@Elegant996 could you please show me your yaml file and ip -d a show command output?

[s1061123]

This may be causes due to containernetworking/plugins#400
With this PR, verified the issue.