Allow the default-route to be empty

[venuiyer]

In
https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ,
section 4.1.2.1.9,

"
4.1.2.1.9 “default-route” Default route selection for a particular attachment

This optional key with value of type string-array is used to explicitly select
which attachment will receive the default route. The value of items in the
“default-route” array are intended to be gateways, e.g. an IP address to which
packets that do not match any other routes are sent. This key must only be set
on one item in the Network Attachment Selection Annotation. This list may be empty.
"

However en empty list will fail currently; this change accommodates an
empty "default-route" by retaining the default route added by the
delegate.

Signed-off-by: venugopal iyer venugopali@nvidia.com

[coveralls]

Pull Request Test Coverage Report for Build 245363538

• 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.08%) to 71.073%

---

Totals
Change from base Build 238189460:	0.08%
Covered Lines:	1086
Relevant Lines:	1528

---

💛 - Coveralls

[s1061123]

Thank you for your PR, @venuiyer !
It looks good to me. So wait a few days for additional comment and then merged.

[venuiyer]

thanks for reviewing, @s1061123!

[dougbtv]

Hey @venuiyer -- thanks for the discerning eye against the spec, I appreciate it a lot!

I think I'm OK to move forward with accepting the PR, it looks good to me. I'm going to run a manual test against it locally. However, I can't currently recreate the problem under the current code base.

Do you mind providing the annotation which caused the problem?
I'm asking partially so that I can add a unit test so we don't cause this to regress.

Here's a test I ran against Multus (one that's a couple months old honestly)

# This is the net-attach-def from the quickstart guide.
[centos@kube-singlehost-master ~]$ kubectl describe net-attach-def macvlan-conf
Name:         macvlan-conf
Namespace:    default
Labels:       <none>
Annotations:  API Version:  k8s.cni.cncf.io/v1
Kind:         NetworkAttachmentDefinition
Metadata:
  Creation Timestamp:  2020-05-27T18:16:35Z
  Generation:          4
  Managed Fields:
    API Version:  k8s.cni.cncf.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:kubectl.kubernetes.io/last-applied-configuration:
      f:spec:
        .:
        f:config:
    Manager:         kubectl
    Operation:       Update
    Time:            2020-07-17T13:58:45Z
  Resource Version:  4672973
  Self Link:         /apis/k8s.cni.cncf.io/v1/namespaces/default/network-attachment-definitions/macvlan-conf
  UID:               e39645ba-be06-455c-b423-b13ddc6cc717
Spec:
  Config:  { "cniVersion":"0.3.0", "type":"macvlan", "master":"eth0", "mode":"bridge", "ipam":{ "type":"whereabouts", "datastore":"kubernetes", "kubernetes":{ "kubeconfig":"/etc/cni/net.d/whereabouts.d/whereabouts.kubeconfig" }, "range":"192.168.2.225/28", "log_file":"/tmp/whereabouts.log", "log_level":"debug", "gateway":"192.168.2.1" } }
Events:    <none>
[centos@kube-singlehost-master ~]$ cat empty-gw.pod.yaml 
apiVersion: v1
kind: Pod
metadata:
  name: testempty
  annotations:
    k8s.v1.cni.cncf.io/networks: '[{"name": "macvlan-conf", "default-route": []}]'
spec:
  containers:
  - name: testempty
    command: ["/bin/ash", "-c", "trap : TERM INT; sleep infinity & wait"]
    image: alpine
[centos@kube-singlehost-master ~]$ kubectl create -f empty-gw.pod.yaml 
pod/testempty created
[centos@kube-singlehost-master ~]$ kubectl get pods --all-namespaces | grep -P "NAME|testempty"
NAMESPACE     NAME                                              READY   STATUS    RESTARTS   AGE
default       testempty                                         1/1     Running   0          26s

[centos@kube-singlehost-master ~]$ kubectl exec -it testempty -- ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
3: eth0@if273: <BROADCAST,MULTICAST,UP,LOWER_UP,M-DOWN> mtu 1450 qdisc noqueue state UP 
    link/ether f6:29:86:ba:9b:8f brd ff:ff:ff:ff:ff:ff
    inet 10.244.0.51/24 scope global eth0
       valid_lft forever preferred_lft forever
4: net1@if2: <BROADCAST,MULTICAST,UP,LOWER_UP,M-DOWN> mtu 1500 qdisc noqueue state UNKNOWN 
    link/ether ca:76:d4:8f:ec:68 brd ff:ff:ff:ff:ff:ff
    inet 192.168.2.225/28 scope global net1
       valid_lft forever preferred_lft forever

[centos@kube-singlehost-master ~]$ kubectl exec -it testempty -- ip route
default via 10.244.0.1 dev eth0 
10.244.0.0/24 dev eth0 scope link  src 10.244.0.51 
10.244.0.0/16 via 10.244.0.1 dev eth0 
192.168.2.224/28 dev net1 scope link  src 192.168.2.225 

[centos@kube-singlehost-master ~]$ /opt/cni/bin/multus --version
multus-cni version:v3.4.1, commit:bfaf22964b51b93b08dcb1f10cbd8f8cb9a3603a, date:2020-03-16T13:02:12+0000

[venuiyer]

Thanks for checking this; I'll get back on this, please hold off on the merge till then. I think I am missing one change from what I tested with, so let me verify and get back. Thanks!

[venuiyer]

Sorry @dougbtv for the delayed response, was caught up on something else. Hopefully, the following is useful info. for this - let me know if that helps and/or I am missing anything..

1. Scenario; have 3 interfaces to the pod and the conf files in each have routes
   • default OVN PV interface + 2 SR-IOV interfaces
2. When creating the pod, administratively, we want to say which in gets to retain the default route

So, the sriov-conf in my case looks like:

cat /etc/cni/net.d/10-sriov_net.conf /etc/cni/net.d/10-sriov_net1.conf
{
"cniVersion": "0.3.1",
"type": "sriov",
"name": "sriov-stream-net",
"ipam": {
"type": "host-local",
"subnet": "10.1.0.0/19",
"rangeStart": "10.1.12.141",
"rangeEnd": "10.1.12.160",
"routes": [{
"dst": "0.0.0.0/0"
}],
"gateway": "10.1.0.1"
}
}

{
"cniVersion": "0.3.1",
"type": "sriov",
"name": "sriov-stream-net-1",
"ipam": {
"type": "host-local",
"subnet": "10.2.0.0/19",
"rangeStart": "10.2.12.141",
"rangeEnd": "10.2.12.160",
"routes": [{
"dst": "0.0.0.0/0"
}],
"gateway": "10.2.0.1"
}
}

The CRDs are:

kubectl describe network-attachment-definitions sriov-stream-net
Name: sriov-stream-net
Namespace: default
Labels:
Annotations: k8s.v1.cni.cncf.io/resourceName: intel.com/sriov_stream
API Version: k8s.cni.cncf.io/v1
Kind: NetworkAttachmentDefinition
Metadata:
Creation Timestamp: 2020-09-16T22:55:48Z
Generation: 1
Resource Version: 57999
Self Link: /apis/k8s.cni.cncf.io/v1/namespaces/default/network-attachment-definitions/sriov-stream-net
UID: 5b10a7b7-9ccf-4d28-aef2-91a75ff203a4
Events:

kubectl describe network-attachment-definitions sriov-stream-net-1
Name: sriov-stream-net-1
Namespace: default
Labels:
Annotations: k8s.v1.cni.cncf.io/resourceName: intel.com/sriov_stream1
API Version: k8s.cni.cncf.io/v1
Kind: NetworkAttachmentDefinition
Metadata:
Creation Timestamp: 2020-09-16T22:55:38Z
Generation: 1
Resource Version: 57954
Self Link: /apis/k8s.cni.cncf.io/v1/namespaces/default/network-attachment-definitions/sriov-stream-net-1
UID: 1dd21574-878b-40a8-b0d2-bc3f227f2f51
Events:

And, my pod spec looks like:

apiVersion: v1
kind: Pod
metadata:
name: testpod
annotations:
k8s.v1.cni.cncf.io/networks: |
[
{ "name":"sriov-stream-net",
"default-route": [""]
},
{ "name":"sriov-stream-net-1"
}
]
spec:
containers:

• name: appcntr1
  image: shaharklein/ub-gen:latest
  imagePullPolicy: IfNotPresent
  command: [ "/bin/bash", "-c", "--" ]
  args: [ "while true; do sleep 300000; done;" ]
  resources:
  requests:
  intel.com/sriov_stream: '1'
  intel.com/sriov_stream1: '1'
  limits:
  intel.com/sriov_stream: '1'
  intel.com/sriov_stream1: '1'

I want to set the "default-route" to either sriov-stream-net or sriov-stream-net-1 based on the annotation.

With the above, I see:

kubectl describe pod testpod
....
Type Reason Age From Message

---

Warning FailedCreatePodSandBox 8m9s kubelet, k8s-node-08 Failed to create pod sandbox: rpc error: code = Unknown desc = [failed to set up sandbox container "02239ef434a480e913d8d7666fbea5415a7b73ae18d2bd71a787bbbf125e0066" network for pod "testpod": networkPlugin cni failed to set up pod "testpod_default" network: Multus: [default/testpod]: error setting default gateway: one of Dst.IP, Src, or Gw must not be nil, failed to clean up sandbox container "02239ef434a480e913d8d7666fbea5415a7b73ae18d2bd71a787bbbf125e0066" network for pod "testpod": networkPlugin cni failed to teardown pod "testpod_default" network: delegateDel: error invoking DelegateDel - "sriov": error in getting result from DelNetwork: error reading cached NetConf in /var/lib/cni/sriov with name 02239ef434a480e913d8d7666fbea5415a7b73ae18d2bd71a787bbbf125e0066-net2]

This comes from:

                    tmpResult, err = netutils.SetDefaultGW(args, ifName, delegate.GatewayRequest, &tmpResult)
                    if err != nil {
                            return nil, cmdErr(k8sArgs, "error setting default gateway: %v", err)
                    }

With the proposed changed in this PR and the same pod spec:

kubectl exec -it testpod -- ip ro
default via 10.1.0.1 dev net1
10.1.0.0/19 dev net1 proto kernel scope link src 10.1.12.141
10.2.0.0/19 dev net2 proto kernel scope link src 10.2.12.148
192.168.1.0/24 dev eth0 proto kernel scope link src 192.168.1.10

with the pod spec changed to :

...

annotations:
k8s.v1.cni.cncf.io/networks: |
[
{ "name":"sriov-stream-net"
},
{ "name":"sriov-stream-net-1",
"default-route": [""]
}
]

...

kubectl exec -it testpod -- ip ro
default via 10.2.0.1 dev net2
10.1.0.0/19 dev net1 proto kernel scope link src 10.1.12.142
10.2.0.0/19 dev net2 proto kernel scope link src 10.2.12.149
192.168.1.0/24 dev eth0 proto kernel scope link src 192.168.1.11

[dougbtv]

I owe you a replication @venuiyer ! My apologies this got left behind, we had missed a maintainers meeting so I didn't get a review of open PRs in. I haven't forgotten -- thank you for the scenario for me to use! Appreciate it.

[venuiyer]

Thanks @dougbtv

[dougbtv]

LGTM! (sorry for the delay)

[venuiyer]

Thanks @dougbtv !!