design-proposals: Extend the pod nic detection to support name changes #356
Conversation
The extension is added as an option that may be considered in the future. Signed-off-by: Edward Haas <[email protected]>
kubevirt-bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
/uncc cwilkers |
kubevirt-bot
requested review from
orelmisan and
vladikr
and removed request for
cwilkers
| A new field may be added to the migration status, indicating the target pod interface name. | ||
| The Migration controller can update the field based on the target pod network-status annotation. | ||
|
|
||
| On the source, the virt-handler and virt-launcher components will continue |
There was a problem hiding this comment.
The source virt-launcher will probably need to edit the domain xml prior to sending it to the target, as is done in other similar features.
There was a problem hiding this comment.
The iface name is not specified on the domain xml, except for rare cases. And that problem is not related to this specific feature.
There was a problem hiding this comment.
For domain attachments of type tap we fill the domain's interface's target.Device field [1] with the name of the tap. We derive the tap's name from the pod interface name [2][3].
Since the target pod could have a pod interface name that is different from the source - the tap name in the target will be different as well.
Other features, such as dedicated CPUs[4] modify the domain in the source, with info collected from the target prior to sending it.
[1] https://libvirt.org/formatdomain.html#generic-ethernet-connection
[2] https://github.com/kubevirt/kubevirt/blob/f95b08230d92ea5ec0603164b943c27ff0f90899/pkg/network/setup/netpod/netpod.go#L384
[3] https://github.com/kubevirt/kubevirt/blob/917ea8e368e6e9816e656be7883abaf3e24ad27e/pkg/network/domainspec/generators.go#L102
[4] https://github.com/kubevirt/kubevirt/blob/22b1872d965c2c39885ddee2542a42ce12b4da49/pkg/virt-launcher/virtwrap/live-migration-source.go#L219
There was a problem hiding this comment.
The information itself is still available on the vmi status for the target and source, so I do not see a problem to make it work.
I have intentionally avoided to give all the details as this requires more work with hopefully not a big chance to occur in the future.
I am unsure how opening this up to low level details contributes to getting this in and with it to GA the feature in general.
There was a problem hiding this comment.
I missed the fact that the interface name is derived from the tap and is set in the domain XML...
There was a problem hiding this comment.
I think we must find a way to mitigate this issue. Perhaps we need to implement a way to communicate and use the target tapdevice to the source via the vmi.status.migrationState
What this PR does / why we need it:
The extension is added as an option that may be considered in the future.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.
Release note: