Graduate the DynamicPodInterfaceNaming FG #13243

orelmisan · 2024-11-13T09:04:02Z

What this PR does

The DynamicPodInterfaceNaming feature-gate was introduced in v1.4 by PR #13078.

The feature works well for at least two projects.
No negative feedbacks were received from the community on the implementation other than the fact that is does
not support migration between two different primary interface names (in this case the migration will fail).

This scenario is not supposed to be supported for the foreseeable future.

Fixes #

Why we need it and why it was done in this way

The following tradeoffs were made:

The following alternatives were considered:

Links to places where the discussion took place:

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.

Design: A design document was considered and is present (link) or not required
PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
Community: Announcement to kubevirt-dev was considered

Release note

Dynamic pod interface naming is declared GA

orelmisan · 2024-11-13T09:07:39Z

/uncc @RamLavi @0xFelix
/cc @nirdothan

ormergi

LGTM

kubevirt-bot · 2024-11-21T16:04:03Z

Pull requests that are marked with lgtm should receive a review
from an approver within 1 week.

After that period the bot marks them with the label needs-approver-review.

/label needs-approver-review

EdDev · 2024-11-21T19:00:11Z

@vladikr , per your suggestion, here is the added option on the original design proposal to consider a scenario in which the pod iface name may change: kubevirt/community#356

We will do our best to reject support for such an edge scenario, but if we fail, we do have a way to mitigate it.

Hope this makes the graduation of the feature safer.

EdDev

Thank you!

/approve

vladikr · 2024-11-24T21:49:20Z

@vladikr , per your suggestion, here is the added option on the original design proposal to consider a scenario in which the pod iface name may change: kubevirt/community#356

We will do our best to reject support for such an edge scenario, but if we fail, we do have a way to mitigate it.

Hope this makes the graduation of the feature safer.

@EdDev It doesn't look like what I've suggested can fully mitigate the failure to live migrate VMs that are using this feature in case the target pod interface name changes.
As @orelmisan pointed out I've missed that you are deriving the interface name from the tap device name.
We need a mitigation before we can graduate this, otherwise, I fear that users will face severe consequences if the interface name changes.

Following the graduation of the `DynamicPodInterfaceNaming` feature gate in [1], enable it by default. [1] kubevirt/kubevirt#13243 Signed-off-by: Orel Misan <omisan@redhat.com>

Following the graduation of KubeVirt's `DynamicPodInterfaceNaming` feature gate[1], enable it by default. [1] kubevirt/kubevirt#13243 Signed-off-by: Orel Misan <omisan@redhat.com>

orelmisan · 2024-11-28T14:19:16Z

Change: Rebase.

This is done in preparation for the feature-gate's graduation to GA. This reverts commit af28187. Signed-off-by: Orel Misan <omisan@redhat.com>

The DynamicPodInterfaceNaming feature-gate was introduced in v1.4 by PR kubevirt#13078. The feature works well for at least two projects. No negative feedbacks were received from the community on the implementation other than the fact that is does not support migration between two different primary interface names (in this case the migration will fail). This scenario is not supposed to be supported for the foreseeable future. Signed-off-by: Orel Misan <omisan@redhat.com>

orelmisan · 2024-11-28T14:33:56Z

Change: Added a generated file that was required following the rebase.

ormergi · 2024-11-28T16:59:35Z

/lgtm

orelmisan · 2024-11-29T14:42:25Z

@vladikr Following the merge of PR #13374, could we move forward with this one? Please let me know if there's anything else needed on my end to proceed.

vladikr · 2024-11-29T22:07:53Z

@vladikr Following the merge of PR #13374, could we move forward with this one? Please let me know if there's anything else needed on my end to proceed.

yes, thanks @orelmisan . I was waiting for the backport pr to get merged.

orelmisan · 2024-11-29T22:10:40Z

@vladikr Following the merge of PR #13374, could we move forward with this one? Please let me know if there's anything else needed on my end to proceed.

yes, thanks @orelmisan . I was waiting for the backport pr to get merged.

Thank you @vladikr, would it be possible for you to approve and hold?

vladikr · 2024-11-29T22:31:51Z

/approve
/hold

kubevirt-bot · 2024-11-29T22:31:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EdDev, vladikr

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~~ [vladikr]

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

kubevirt-commenter-bot · 2024-11-29T22:32:02Z

Required labels detected, running phase 2 presubmits:
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-e2e-kind-1.30-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-k8s-1.30-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.29-sig-network
/test pull-kubevirt-e2e-k8s-1.29-sig-storage
/test pull-kubevirt-e2e-k8s-1.29-sig-compute
/test pull-kubevirt-e2e-k8s-1.29-sig-operator
/test pull-kubevirt-e2e-k8s-1.30-sig-network
/test pull-kubevirt-e2e-k8s-1.30-sig-storage
/test pull-kubevirt-e2e-k8s-1.30-sig-compute
/test pull-kubevirt-e2e-k8s-1.30-sig-operator

orelmisan · 2024-11-30T05:07:17Z

#13387 was merged.
/unhold

orelmisan · 2024-11-30T05:08:14Z

/retest-required

orelmisan · 2024-11-30T10:05:47Z

/test pull-kubevirt-unit-test-s390x

kubevirt-bot · 2024-11-30T12:32:16Z

/remove-label needs-approver-review

Following the graduation of KubeVirt's `DynamicPodInterfaceNaming` feature gate[1], enable it by default. [1] kubevirt/kubevirt#13243 Signed-off-by: Orel Misan <omisan@redhat.com>

The dynamic pod interface naming feature was declared GA in KubeVirt v1.5 [1]. There is no longer a need to set this FG. [1] kubevirt/kubevirt#13243 Signed-off-by: Orel Misan <omisan@redhat.com>

* KubeVirt default FG: Remove NetworkBindingPlugins The network binding plugins feature was declared GA in KubeVirt v1.5 [1]. There is no longer a need to set this FG. [1] kubevirt/kubevirt#13314 Signed-off-by: Orel Misan <omisan@redhat.com> * KubeVirt default FG: Remove DynamicPodInterfaceNaming The dynamic pod interface naming feature was declared GA in KubeVirt v1.5 [1]. There is no longer a need to set this FG. [1] kubevirt/kubevirt#13243 Signed-off-by: Orel Misan <omisan@redhat.com> --------- Signed-off-by: Orel Misan <omisan@redhat.com>

kubevirt-bot added release-note-none dco-signoff: no size/L area/controller sig/compute sig/network labels Nov 13, 2024

kubevirt-bot requested review from 0xFelix, EdDev, fossedihelm, ormergi and RamLavi November 13, 2024 09:04

dosubot bot added the kind/api-change label Nov 13, 2024

orelmisan force-pushed the ga-dynamic-pod-iface-naming branch from fed5e8c to 2f6b423 Compare November 13, 2024 09:05

kubevirt-bot added dco-signoff: yes and removed dco-signoff: no labels Nov 13, 2024

kubevirt-bot requested review from nirdothan and removed request for 0xFelix and RamLavi November 13, 2024 09:07

ormergi approved these changes Nov 14, 2024

View reviewed changes

kubevirt-bot assigned ormergi Nov 14, 2024

kubevirt-bot added the lgtm label Nov 14, 2024

kubevirt-bot added the needs-approver-review label Nov 21, 2024

EdDev approved these changes Nov 22, 2024

View reviewed changes

kubevirt-bot assigned EdDev Nov 22, 2024

orelmisan added 2 commits November 28, 2024 16:32

net, e2e: Do not enable DynamicPodInterfaceNaming FG

9eb5d97

This is done in preparation for the feature-gate's graduation to GA. This reverts commit af28187. Signed-off-by: Orel Misan <omisan@redhat.com>

orelmisan force-pushed the ga-dynamic-pod-iface-naming branch from ece89a3 to 11cf0e3 Compare November 28, 2024 14:33

kubevirt-bot added the lgtm label Nov 28, 2024

kubevirt-bot added the do-not-merge/hold label Nov 29, 2024

kubevirt-bot added the approved label Nov 29, 2024

kubevirt-bot removed the do-not-merge/hold label Nov 30, 2024

kubevirt-bot merged commit 4dbd00b into kubevirt:main Nov 30, 2024
39 checks passed

orelmisan deleted the ga-dynamic-pod-iface-naming branch November 30, 2024 10:23

kubevirt-bot removed the needs-approver-review label Nov 30, 2024

orelmisan mentioned this pull request Jan 13, 2025

virt-config: Feature gate management unification #11989

Merged

8 tasks

orelmisan mentioned this pull request Mar 4, 2025

KubeVirt default FG: Remove graduated network FGs kubevirt/hyperconverged-cluster-operator#3327

Merged

11 tasks

kubevirt-bot added release-note and removed release-note-none labels Mar 9, 2025

Graduate the DynamicPodInterfaceNaming FG #13243

Graduate the DynamicPodInterfaceNaming FG #13243

Conversation

orelmisan commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Why we need it and why it was done in this way

Special notes for your reviewer

Checklist

Release note

Uh oh!

orelmisan commented Nov 13, 2024

Uh oh!

ormergi left a comment

Choose a reason for hiding this comment

Uh oh!

kubevirt-bot commented Nov 21, 2024

Uh oh!

EdDev commented Nov 21, 2024

Uh oh!

EdDev left a comment

Choose a reason for hiding this comment

Uh oh!

vladikr commented Nov 24, 2024

Uh oh!

orelmisan commented Nov 28, 2024

Uh oh!

Uh oh!

orelmisan commented Nov 28, 2024

Uh oh!

ormergi commented Nov 28, 2024

Uh oh!

orelmisan commented Nov 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vladikr commented Nov 29, 2024

Uh oh!

orelmisan commented Nov 29, 2024

Uh oh!

vladikr commented Nov 29, 2024

Uh oh!

kubevirt-bot commented Nov 29, 2024

Uh oh!

kubevirt-commenter-bot commented Nov 29, 2024

Uh oh!

orelmisan commented Nov 30, 2024

Uh oh!

orelmisan commented Nov 30, 2024

Uh oh!

orelmisan commented Nov 30, 2024

Uh oh!

Uh oh!

kubevirt-bot commented Nov 30, 2024

Uh oh!

Uh oh!

Uh oh!

orelmisan commented Nov 13, 2024 •

edited

Loading

orelmisan commented Nov 29, 2024 •

edited

Loading