udn: set persistentIPs for UDN ifaces

[maiqueb]

What this PR does and why is it needed

This PR looks for a special annotation on pods, where KubeVirt (or an agent on behalf of KubeVirt) will indicate what is the name of the IPAMClaim used for the primary UDN interface.

An alternative would be to use an hard-coded value, but then we wouldn't be able to distinguish between a pod and a VM, and we do not want to abuse reading the KubeVirt label.

Which issue(s) this PR fixes

Fixes #

Special notes for reviewers

Pending adding the CI requirements, ensuring kubevirt e2e tests are run.

How to verify it

Details to documentation updates

Description for the changelog

Configure persistent IPs on UDN networks when requested

Does this PR introduce a user-facing change?

Configure persistent IPs on UDN networks when requested

[jcaamano]

Will we have integration docs for this new annotation?

[maiqueb]

Will we have integration docs for this new annotation?

Yes. But since we have no documentation about primary UDN, it feels very weird to add it.

Once that exists, I will explain the annotation.

[jcaamano]

One other question:

Would it have been theoretically possible to use the ipam claim reference in a network selection element for the default network (instead of this new annotation) leveraging v1.multus-cni.io/default-network annotation. I know that is out of spec but we are out of spec here anyway and we wouldn't need to define new things.

[maiqueb]

One other question:

Would it have been theoretically possible to use the ipam claim reference in a network selection element for the default network (instead of this new annotation) leveraging v1.multus-cni.io/default-network annotation. I know that is out of spec but we are out of spec here anyway and we wouldn't need to define new things.

Not sure ... how would that play along for the rest of the synthetic network selection element we are using to attach to UDN ? For UDN pods, we'd use the synthetic network-selection-element, for UDN VMs we'd use this explicitly defined network-selection-element ?... We'd merge the synthetic NSE w/ the existing one ?...

Maybe it's me, but it sounds more complex.

[jcaamano]

One other question:
Would it have been theoretically possible to use the ipam claim reference in a network selection element for the default network (instead of this new annotation) leveraging v1.multus-cni.io/default-network annotation. I know that is out of spec but we are out of spec here anyway and we wouldn't need to define new things.

Not sure ... how would that play along for the rest of the synthetic network selection element we are using to attach to UDN ? For UDN pods, we'd use the synthetic network-selection-element, for UDN VMs we'd use this explicitly defined network-selection-element ?... We'd merge the synthetic NSE w/ the existing one ?...

Maybe it's me, but it sounds more complex.

I am not sure what's the synthetic network selection element used for, but that looks like an implementation detail.

I would value more having a consistent API for our users.

[maiqueb]

One other question:
Would it have been theoretically possible to use the ipam claim reference in a network selection element for the default network (instead of this new annotation) leveraging v1.multus-cni.io/default-network annotation. I know that is out of spec but we are out of spec here anyway and we wouldn't need to define new things.

Not sure ... how would that play along for the rest of the synthetic network selection element we are using to attach to UDN ? For UDN pods, we'd use the synthetic network-selection-element, for UDN VMs we'd use this explicitly defined network-selection-element ?... We'd merge the synthetic NSE w/ the existing one ?...
Maybe it's me, but it sounds more complex.

I am not sure what's the synthetic network selection element used for, but that looks like an implementation detail.

I would value more having a consistent API for our users.

It's how we're requesting the UDN interface attachment without persistent IPs.

Meaning, the API is not consistent now. I don't think that can converge in this PR.

[jcaamano]

It's how we're requesting the UDN interface attachment without persistent IPs.

But you mean completely internal to OVN-K, we are not using it as an API to interface with any other external component. Or what does synthetic refer to in this context?

Meaning, the API is not consistent now. I don't think that can converge in this PR.

The IPAM claim reference API could be consistent because it would be the same for secondary and primary networks.

Not for this PR, but maybe worth exploring before the final integration.

[maiqueb]

It's how we're requesting the UDN interface attachment without persistent IPs.

But you mean completely internal to OVN-K, we are not using it as an API to interface with any other external component. Or what does synthetic refer to in this context?

Meaning, the API is not consistent now. I don't think that can converge in this PR.

The IPAM claim reference API could be consistent because it would be the same for secondary and primary networks.

Yes, but, I think that by making that API consistent, you are then making the UDN API inconsistent - if persistent IPs are used, we require a network selection element; if not, we'd just annotate the namespace.

Not for this PR, but maybe worth exploring before the final integration.

This would maybe be an interesting idea to explore in the enhancement proposal.

[jcaamano]

Yes, but, I think that by making that API consistent, you are then making the UDN API inconsistent - if persistent IPs are used, we require a network selection element; if not, we'd just annotate the namespace.

The network selection element would not replace the namespace annotation, that is just a shortcut to default a different network in your namespace (so more akin to a shortcut for v1.multus-cni.io/default-network). But if you want to change the details of that network on a pod-per-pod basis, then you would have to annotate the pod with the NSE for the default network as you do today, for example, to use sr-iov on the default network with a specific pod.

[maiqueb]

The network selection element would not replace the namespace annotation, that is just a shortcut to default a different network in your namespace (so more akin to a shortcut for v1.multus-cni.io/default-network). But if you want to change the details of that network on a pod-per-pod basis, then you would have to annotate the pod with the NSE for the default network as you do today, for example, to use sr-iov on the default network with a specific pod.

Right now, it is impossible to refer to a primary UDN via network-selection-element. We disallow that explicitly in the code. This will need to be discussed, but, I agree it could be a more generic way to extend the primary UDN in a per pod way. SR-IOV and persistent IPs would not be that different - I agree with you.

I think I start to see your point. I will consider this option in the enhancement. Thanks for raising it.

[maiqueb]

ovn-ci / e2e (kv-live-migration, noHA, shared, dualstack, SnatGW, 1br, ic-single-node-zones, 3

#4709

[maiqueb]

The upgrade test is flaking on #4481; updated that issue.

[qinqon]

/lgtm

The kubevirt issues were related to #4625

[maiqueb]

Fixed the errors in the kubevirt lane, introduced after rebasing #4625 (since we were testing a dual stack UDN in an IPv4 cluster) in this forced push.

[maiqueb]

/lgtm

The kubevirt issues were related to #4625

Thanks for the help !!! 🚀

[maiqueb]

lane e2e (external-gateway, noHA, shared, ipv4, noSnatGW, 2br, ic-single-node-zones) failed on a known flake: #4432

lane e2e (control-plane, noHA, shared, ipv6, noSnatGW, 2br, ic-single-node-zones, enable-dns-name-reso failed on known flake #4144

@jcaamano