multus: add multus manifests

[zshi-redhat]

• add manifests for multus-cni, sriov-cni, sriov-device-plugin
• add Namespace and Annotations fields in AdditionalNetworkDefinition structure to enable creation of namespace isolated and annotated Custom Resource (annotated CR is used to pass SR-IOV resource annotation to multus).
• add multusNodeConfig function to generate content of Multus CNI configMap: 70-multus.conf

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zshi-redhat
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: squeed

If they are not already assigned, you can assign the PR to them by writing /assign @squeed in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[zshi-redhat]

/assign @squeed Hi Casey, this is a follow up PR to add multus related manifests, please help to review, Thanks!

@s1061123 @dougbtv please help to review if multus manifests are correct, I copied them from downstream repo and did a little modification to fit with openshift.
This is to add manifests for multus-cni, sriov-cni, sriov-device-plugin. I think we can add more (supported/unsupported containernteworking cni, admission controllers ) if related manifests are ready to consume.

With current PR, I occasionally saw a problem when the operator applys CR of net-attach-def Kind:

could not apply (k8s.cni.cncf.io/v1, Kind=NetworkAttachmentDefinition) multus/sriov-net: could not retrieve existing (k8s.cni.cncf.io/v1, Kind=NetworkAttachmentDefinition) multus/sriov-net: no matches for kind "NetworkAttachmentDefinition" in version "k8s.cni.cncf.io/v1"

And this is the yaml file I used to created NetworkConfig custom resource:

apiVersion: "networkoperator.openshift.io/v1"
kind: "NetworkConfig"
metadata:
  name: "default"
spec:
  serviceNetwork: "172.30.0.0/16"
  clusterNetworks:
    - cidr: "10.128.0.0/14"
      hostSubnetLength: 9
  defaultNetwork:
    type: OpenShiftSDN
    openshiftSDNConfig:
      mode: NetworkPolicy
  additionalNetworks:
    - type: Raw
      name: sriov-net
      namespace: multus
      annotations:
        k8s.v1.cni.cncf.io/resourceName: intel.com/sriov
      rawCNIConfig: '{
        "type": "sriov",
        "name": "sriov-network",
        "namespace": multus,
        "ipam": {
                "type": "host-local",
                "subnet": "10.56.217.0/24",
                "routes": [{
                        "dst": "0.0.0.0/0"
                }],
                "gateway": "10.56.217.1"
        }
}'

do you see what might be the problem?

[zshi-redhat]

updated code gen and verified code style.

[zshi-redhat]

The CI test failure doesn't seem to be related with network operator, re-run the tests:
/retest

[openshift-ci-robot]

@zshi-redhat: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	ae8945e	link	/test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[zshi-redhat]

/assign @squeed Hi Casey, this is a follow up PR to add multus related manifests, please help to review, Thanks!

@s1061123 @dougbtv please help to review if multus manifests are correct, I copied them from downstream repo and did a little modification to fit with openshift.
This is to add manifests for multus-cni, sriov-cni, sriov-device-plugin. I think we can add more (supported/unsupported containernteworking cni, admission controllers ) if related manifests are ready to consume.

With current PR, I occasionally saw a problem when the operator applys CR of net-attach-def Kind:

could not apply (k8s.cni.cncf.io/v1, Kind=NetworkAttachmentDefinition) multus/sriov-net: could not retrieve existing (k8s.cni.cncf.io/v1, Kind=NetworkAttachmentDefinition) multus/sriov-net: no matches for kind "NetworkAttachmentDefinition" in version "k8s.cni.cncf.io/v1"

And this is the yaml file I used to created NetworkConfig custom resource:

apiVersion: "networkoperator.openshift.io/v1"
kind: "NetworkConfig"
metadata:
  name: "default"
spec:
  serviceNetwork: "172.30.0.0/16"
  clusterNetworks:
    - cidr: "10.128.0.0/14"
      hostSubnetLength: 9
  defaultNetwork:
    type: OpenShiftSDN
    openshiftSDNConfig:
      mode: NetworkPolicy
  additionalNetworks:
    - type: Raw
      name: sriov-net
      namespace: multus
      annotations:
        k8s.v1.cni.cncf.io/resourceName: intel.com/sriov
      rawCNIConfig: '{
        "type": "sriov",
        "name": "sriov-network",
        "namespace": multus,
        "ipam": {
                "type": "host-local",
                "subnet": "10.56.217.0/24",
                "routes": [{
                        "dst": "0.0.0.0/0"
                }],
                "gateway": "10.56.217.1"
        }
}'

do you see what might be the problem?

It looks like that CRD and associated CR cannot be created in a single run, if I restart the operator, the net-attach-def CR can be successfully created (since the net-attach-def CRD is already created before restarting operator). It might imply that we need to apply CRD and CR separately in different context. It doesn't seem that operator supports it now.
@squeed @dcbw @dougbtv @s1061123 do you have any insight on this?

[dcbw]

@squeed @zshi-redhat so the branch I'm working on for Multus support takes a lighter weight approach, see here: #54 . I think that we should keep the Multus stuff separate and not logically combine it with AdditionalNetworks or the SRIOV plugin.

SRIOV can be a separate PR that does actually inspect AdditionalNetworks for SRIOV-enabled ones and then renders the DevicePlugin and associated RBAC/Namespace. But that's logically separate from Multus configuration, and the Multus config can be very simple (eg we don't need to specify Delegates since everything except the default network should be handled by AdditionalNetworks).

Thoughts?

[zshi-redhat]

@squeed @zshi-redhat so the branch I'm working on for Multus support takes a lighter weight approach, see here: #54 . I think that we should keep the Multus stuff separate and not logically combine it with AdditionalNetworks or the SRIOV plugin.

+1 to keep Multus config separate from SR-IOV and additionalNetworks.
This will also be useful when later we want to enable migration between different network solutions.

SRIOV can be a separate PR that does actually inspect AdditionalNetworks for SRIOV-enabled ones and then renders the DevicePlugin and associated RBAC/Namespace. But that's logically separate from Multus configuration, and the Multus config can be very simple (eg we don't need to specify Delegates since everything except the default network should be handled by AdditionalNetworks).

I remember Multus now supports config default network with net-attach-def CR, in which case it may re-use the additional Network CR template to render the default network during Multus configuration, this may requires additional network be configured to provide default network CR.

Thoughts?

[openshift-ci-robot]

@zshi-redhat: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[dcbw]

Should we close this one and do separate follow-ups for any parts that aren't already there?

[zshi-redhat]

Should we close this one and do separate follow-ups for any parts that aren't already there?

Yes, this should be closed. Thanks for the reminding!