network: Add link state management section #864

orelmisan · 2025-01-21T07:55:19Z

What this PR does / why we need it:
Add documentation for the link state management feature.

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.

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:

NONE

orelmisan · 2025-01-21T07:56:29Z

Lets wait for:
kubevirt/enhancements#2
kubevirt/kubevirt#13744
kubevirt/kubevirt#13708

/hold

orelmisan · 2025-01-21T07:56:55Z

/uncc @RamLavi
/cc @ormergi @nirdothan

orelmisan · 2025-01-22T13:37:57Z

Change: Added a disclaimer about setting the primary interface's link state to down when using readiness / liveness probes.

nirdothan

Thanks @orelmisan. Please see my comments below.

nirdothan · 2025-01-23T11:02:06Z

docs/network/interfaces_and_networks.md

+## Link State Management
+
+KubeVirt enables setting the desired interface's link state using the interface's `state` field.
+The allowed values are: `up`, `down`. Unspecified value is considered as `up`.


what about absent

Added a note about absent.

I think this is a good observation about a problematic separation.
We have now one field named interface state that through it a user can implement hotplug/unplug and link-state.

We could have organized it under a common "Interface state management" section.
Lets consider this in a follow up.

nirdothan · 2025-01-23T11:09:07Z

docs/network/interfaces_and_networks.md

+KubeVirt enables setting the desired interface's link state using the interface's `state` field.
+The allowed values are: `up`, `down`. Unspecified value is considered as `up`.
+
+When the desired link state is set to `down`, it is equivalent to having the network cable disconnected from the machine.


nit: And I'm not even sure that I agree with myself here :) but it might technically be more correct, since the NIC is emulated.

Suggested change

When the desired link state is set to `down`, it is equivalent to having the network cable disconnected from the machine.

When the desired link state is set to `down`, it emulates the effect of disconnecting the network cable from the machine

Changed the wording a bit.

nirdothan · 2025-01-23T11:09:56Z

docs/network/interfaces_and_networks.md

+          pod: { }
+```
+
+The desired link state specification could be specified:


Suggested change

The desired link state specification could be specified:

The desired link state specification can be specified:

nirdothan · 2025-01-23T11:10:28Z

docs/network/interfaces_and_networks.md

+4. When a new network interface is hot-plugged.
+
+> [!NOTE]  
+> The desired link state could be specified for network interfaces using all bindings other than `sriov`, as the virtualization


nirdothan · 2025-01-23T11:12:07Z

docs/network/interfaces_and_networks.md

+> stack used by KubeVirt does not allow setting it for SR-IOV devices.
+
+> [!WARNING]  
+> When HTTP / TCP readiness and/or liveness probes are specified on the VM, setting the primary interface's link state to `down` will


please consider a link to the probes documentation

Not sure how to do it in a way that will not easily break.
@aburdenthehand could you please guide me?

nirdothan · 2025-01-23T11:14:10Z

docs/network/interfaces_and_networks.md

@@ -509,6 +509,75 @@ spec:
 > **Note:** Placement on dedicated CPUs can only be achieved if the Kubernetes CPU manager is running on the SR-IOV capable workers.
 > For further details please refer to the [dedicated cpu resources documentation](../compute/dedicated-cpu_resources.md/).

+## Link State Management


Suggested change

## Link State Management

### Link State Management

As Interfaces are at level ##

Good catch, done.

orelmisan · 2025-01-23T12:40:03Z

Change: Addressed @nirdothan's comments.

EdDev

Thank you!

I am looking to re-organize the network documentation to avoid big doc files.
It should not effect this addition, but it better trigger it.

One request: Please mention the version from which this feature is available.

/approve

I am unsure if we can merge this before the v1.5 release, we need to consult over this. (in addition to waiting for the implementation to complete)
/hold

EdDev · 2025-01-23T13:25:29Z

docs/network/interfaces_and_networks.md

@@ -509,6 +509,78 @@ spec:
 > **Note:** Placement on dedicated CPUs can only be achieved if the Kubernetes CPU manager is running on the SR-IOV capable workers.
 > For further details please refer to the [dedicated cpu resources documentation](../compute/dedicated-cpu_resources.md/).

+### Link State Management


Continuing adding stuff in this file is problematic.

I want to re-organize the documentation we have and start splitting it into smaller pieces.
For now, I do not know what is the correct separation.
So I am good to place it here for now and suggest a better location later.

EdDev · 2025-01-23T13:41:28Z

docs/network/interfaces_and_networks.md

+## Link State Management
+
+KubeVirt enables setting the desired interface's link state using the interface's `state` field.
+The allowed values are: `up`, `down`. Unspecified value is considered as `up`.


I think this is a good observation about a problematic separation.
We have now one field named interface state that through it a user can implement hotplug/unplug and link-state.

We could have organized it under a common "Interface state management" section.
Lets consider this in a follow up.

kubevirt-bot · 2025-01-23T13:52:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EdDev

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:

~~docs/network/OWNERS~~ [EdDev]

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

Add documentation for the link state management feature. Signed-off-by: Orel Misan <[email protected]>

kubevirt-bot · 2025-01-23T13:55:35Z

New changes are detected. LGTM label has been removed.

orelmisan · 2025-01-23T13:56:53Z

Change: Added version number.

kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jan 21, 2025

kubevirt-bot requested review from EdDev and RamLavi January 21, 2025 07:55

kubevirt-bot added the size/M label Jan 21, 2025

kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2025

kubevirt-bot requested review from nirdothan and ormergi and removed request for RamLavi January 21, 2025 07:56

orelmisan force-pushed the link-state branch from 2e1127b to 4215dc7 Compare January 22, 2025 13:36

nirdothan reviewed Jan 23, 2025

View reviewed changes

orelmisan force-pushed the link-state branch from 4215dc7 to caf6965 Compare January 23, 2025 12:38

nirdothan approved these changes Jan 23, 2025

View reviewed changes

kubevirt-bot assigned nirdothan Jan 23, 2025

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2025

EdDev reviewed Jan 23, 2025

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 23, 2025

network: Add link state management section

7132641

Add documentation for the link state management feature. Signed-off-by: Orel Misan <[email protected]>

orelmisan force-pushed the link-state branch from caf6965 to 7132641 Compare January 23, 2025 13:55

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

network: Add link state management section #864

network: Add link state management section #864

orelmisan commented Jan 21, 2025

orelmisan commented Jan 21, 2025

orelmisan commented Jan 21, 2025

orelmisan commented Jan 22, 2025

nirdothan left a comment

nirdothan Jan 23, 2025

orelmisan Jan 23, 2025

EdDev Jan 23, 2025

nirdothan Jan 23, 2025

orelmisan Jan 23, 2025

nirdothan Jan 23, 2025

orelmisan Jan 23, 2025

nirdothan Jan 23, 2025

orelmisan Jan 23, 2025

nirdothan Jan 23, 2025

orelmisan Jan 23, 2025

nirdothan Jan 23, 2025

orelmisan Jan 23, 2025

orelmisan commented Jan 23, 2025

EdDev left a comment

EdDev Jan 23, 2025

EdDev Jan 23, 2025

kubevirt-bot commented Jan 23, 2025

kubevirt-bot commented Jan 23, 2025

orelmisan commented Jan 23, 2025

	When the desired link state is set to `down`, it is equivalent to having the network cable disconnected from the machine.
	When the desired link state is set to `down`, it emulates the effect of disconnecting the network cable from the machine

	The desired link state specification could be specified:
	The desired link state specification can be specified:

network: Add link state management section #864

Are you sure you want to change the base?

network: Add link state management section #864

Conversation

orelmisan commented Jan 21, 2025

orelmisan commented Jan 21, 2025

orelmisan commented Jan 21, 2025

orelmisan commented Jan 22, 2025

nirdothan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orelmisan commented Jan 23, 2025

EdDev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kubevirt-bot commented Jan 23, 2025

kubevirt-bot commented Jan 23, 2025

orelmisan commented Jan 23, 2025