[WIP] configure vlan sub interfaces #151
Conversation
This commit attempts to create vlan sub interfaces on the pod container for the given selective vlan trunks. It also configures IP based on the IPAM config given for each VLAN ID. TODO: figure out what else needed by ipam module apart from just overwriting netconf with trunk's ipam data. does it also need more changes into netconf and cni_args ? Signed-off-by: Periyasamy Palanisamy <[email protected]>
Signed-off-by: Periyasamy Palanisamy <[email protected]>
Signed-off-by: Periyasamy Palanisamy <[email protected]>
This reverts commit ccf4430.
Signed-off-by: Periyasamy Palanisamy <[email protected]>
Signed-off-by: Periyasamy Palanisamy <[email protected]>
kubevirt-bot
added
the
do-not-merge/work-in-progress
|
@pperiyasamy: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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. |
kubevirt-bot
added
the
dco-signoff: no
|
@pperiyasamy: GitHub didn't allow me to request PR reviews from the following users: JanScheurich. Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
kubevirt-bot
added
the
do-not-merge/release-note-label-needed
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pperiyasamy The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@pperiyasamy can we accomplish the same result by using multiple NetworkAttachmentDefinitions? |
@phoracek: We introduced the support for VLAN-trunking in ovs-cni for pods that require access to up to hundreds of VLANs in parallel. Using one NAD per VLAN would imply hundreds of NADs and hundreds of veth pairs connecting the pod to OVS. This was considered impractical. Furthermore, the set of trunked VLANs changes over time. With trunks it is possible to adapt by updating the trunk NAD and restart the pods. Using individual access NADs would require updating the pod specs every time. We have realized now that just exposing the raw VLANs to pods on a trunk interface is sometimes not enough. Applications may want to use additional CNI services like interface creation, IPAM or other interface configuration also for the trunked networks. I appreciate that creating hundreds of VLAN sub-interfaces would, from the pod perspective, look not much different than hundreds of veth endpoints, even though it does make a difference for OVS. But some of our target applications use raw packet sockets to handle the VLAN tags internally and would still like to rely on IPAM to provide an IP address on each and every trunked VLAN, either in-band through dhcp IPAM or out-of-band through network status annotations. |
|
I see, makes sense. How would you like to handle this? Shall we first review the proposed API and approach before we get to implementation of tests and docs? |
|
I also thought the end-to-end proposal including supporting changes in net-attach-def-client and multus should first be discussed in the NPWG before we go ahead with implementation everywhere. |
|
Thanks @phoracek and @JanScheurich for your input , Let me propose the changes in multus-cni and net-attach-def-client via pull requests and get their feedback. |
Signed-off-by: Periyasamy Palanisamy <[email protected]>
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. 📝 Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
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. |
kubevirt-bot
added
the
needs-rebase
|
@pperiyasamy: 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. |
Currently ovs-cni supports selective vlan trunking in the networking configuration and creates ovs port on host ovs bridge with given selective trunk vlan IDs. But the vlan handling on the pod container is upto the applications like creating vlan sub interfaces, ip assignment etc.
This PR attempts to solve this by enhancing ovs-cni to accept IPAM configuration for every VLAN and let ovs-cni creates vlan sub interface and invokes relevant IPAM plugin for every vlan ID for the ip assignment. Of course we could also provide an option in net-attach-def not to create vlan sub interface for every vlan ID in trunks parameter. This is just an initial PR and would make changes accordingly based on your feedback.
The multus-cni and network-attachment-definition-clients need code changes to report all interfaces with ip information in the nw status, will raise PRs on those repos and get their opinon.
/cc @JanScheurich