Skip to content

feat: add vm-migration-network setting#811

Merged
innobead merged 17 commits into
harvester:mainfrom
FrankYang0529:HARV-5848
Aug 8, 2025
Merged

feat: add vm-migration-network setting#811
innobead merged 17 commits into
harvester:mainfrom
FrankYang0529:HARV-5848

Conversation

@FrankYang0529

Copy link
Copy Markdown
Contributor

Problem:

Solution:

Related Issue(s):

harvester/harvester#5848

Test plan:

Additional documentation or context

@github-actions

github-actions Bot commented Jun 30, 2025

Copy link
Copy Markdown
Name Link
🔨 Latest commit 4dc3f88
😎 Deploy Preview https://689476ab87317246c3e866f3--harvester-preview.netlify.app

@innobead innobead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, LGTM. Just a few feedback.

Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated

@jillian-maroket jillian-maroket left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review done. Note that the structural changes that I suggested here will be applied to the "Storage Network" page as well. I will create the PR after we merge this one.

Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/settings.md Outdated
@innobead innobead requested a review from Vicente-Cheng August 7, 2025 07:57

@innobead innobead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor feedback.

Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated
Comment thread docs/advanced/vm-migration-network.md Outdated

- The `ippools.whereabouts.cni.cncf.io` CRD exists. You can check this using the command `kubectl get crd ippools.whereabouts.cni.cncf.io`. In certain [upgrade scenarios](https://github.com/harvester/harvester/issues/3168), the Whereabouts CNI is not installed correctly.

- The IP range of the VM migration network is in the IPv4 CIDR format and must neither conflict nor overlap with Kubernetes cluster networks. You must exclude IP addresses that KubeVirt pods and the VM migration network must not use. The following addresses are reserved: `10.42.0.0/16`, `10.43.0.0/16`, `10.52.0.0/16` and `10.53.0.0/16`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You already explained the IP range for the VM migration network, but why is it necessary to mention "the VM migration network must not use"? It’s a little confusing here.

@jillian-maroket jillian-maroket Aug 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is legacy text that was copied from the storage network doc and reorganized a bit. I recently updated the storage network doc. LMK if you want to reuse the wording that what Ivan and I agreed on:

Screenshot 2025-08-07 at 5 09 41 PM

https://docs.harvesterhci.io/v1.6/advanced/storagenetwork#prerequisites

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove You must exclude IP addresses that KubeVirt pods and the VM migration network must not use..

Comment thread docs/advanced/settings.md

:::info important

Specify an IP range in the IPv4 CIDR format. The number of IP addresses must be larger than or equal to the number of your cluster nodes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The storage network is required for Longhorn pods for data plane (IM and share manager pods), while the VM migration network is needed for each VM/virt-launcher pod. I suggest updating the explanation of the IP range as below. WDYT?

cc @Vicente-Cheng @WebberHuang1118

Suggested change
Specify an IP range in the IPv4 CIDR format. The number of IP addresses must be larger than or equal to the number of your cluster nodes.
Specify an IP range in the IPv4 CIDR format. The number of IP addresses must be larger than or equal to the number of VMs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my testing, only virt-handler get live-migration IP like:

    k8s.v1.cni.cncf.io/network-status: |-
      [{
          "name": "k8s-pod-network",
          "ips": [
              "10.52.0.93"
          ],
          "default": true,
          "dns": {}
      },{
          "name": "harvester-system/vm-migration-network-g2cr7",
          "interface": "migration0",
          "ips": [
              "10.1.2.2"
          ],
          "mac": "ae:20:fb:20:67:94",
          "dns": {}
      }]

Other kubevirt pods including virt-lanucher, they don't have live-migration IP:

    k8s.v1.cni.cncf.io/network-status: |-
      [{
          "name": "k8s-pod-network",
          "ips": [
              "10.52.1.32"
          ],
          "default": true,
          "dns": {}
      }]

Each node has one virt-handler, I think we can just mention "cluster nodes" here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's fair, the same as storage network.

FrankYang0529 and others added 15 commits August 7, 2025 16:45
Signed-off-by: PoAn Yang <poan.yang@suse.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
Signed-off-by: PoAn Yang <poan.yang@suse.com>
Comment thread docs/advanced/vm-migration-network.md Outdated
Signed-off-by: PoAn Yang <poan.yang@suse.com>
Co-authored-by: Jillian Maroket <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: PoAn Yang <yangpoan@gmail.com>
@innobead innobead merged commit ca8f528 into harvester:main Aug 8, 2025
4 checks passed
@FrankYang0529 FrankYang0529 deleted the HARV-5848 branch August 8, 2025 05:24
@jillian-maroket jillian-maroket added the product-doc/added Content added to SUSE Virtualization doc label Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

product-doc/added Content added to SUSE Virtualization doc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants