✨Add NSX-VPC Network Provider Support #2848

silvery1622 · 2024-03-25T00:41:10Z

This commit introduces NSX-T Virtual Private Cloud (nsx-vpc) as a new network provider for the vSphere Supervisor Cluster in anticipation of the upcoming vSphere 9 release. Key Changes:
nsx-vpc implemented as a new NetworkProvider using nsx-operator libs.
Skipped VM Readiness check as nsx-vpc offers private network access.
Added unit tests for nsx-vpc network provider.

What this PR does / why we need it:
The vSphere Supervisor Cluster currently accommodates two network types: vSphere Distributed Switch (VDS) and NSX-T network. With the impending release of vSphere 9, a third network type will be introduced: NSX-T Virtual Private Cloud (nsx-vpc).
To adapt this change, CAPV need to add support for nsx-vpc as a new network provider.

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 #2847

linux-foundation-easycla · 2024-03-25T00:41:14Z

The committers listed above are authorized under a signed CLA.

✅ login: silvery1622 (df04b11)

k8s-ci-robot · 2024-03-25T00:41:19Z

Welcome @silvery1622!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-vsphere 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-vsphere has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-03-25T00:41:20Z

Hi @silvery1622. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

zhanggbj · 2024-03-25T02:30:23Z

/ok-to-test

codecov · 2024-03-25T02:36:51Z

Codecov Report

Attention: Patch coverage is 67.25664% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 64.05%. Comparing base (de24dc5) to head (4e74865).
Report is 26 commits behind head on main.

❗ Current head 4e74865 differs from pull request most recent head df04b11. Consider uploading reports for the commit df04b11 to get more accurate results

Files	Patch %	Lines
pkg/services/network/nsxt_vpc_provider.go	70.29%	25 Missing and 5 partials ⚠️
pkg/manager/network.go	0.00%	3 Missing ⚠️
pkg/services/network/netop_provider.go	0.00%	2 Missing ⚠️
pkg/services/network/nsxt_provider.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2848      +/-   ##
==========================================
+ Coverage   63.97%   64.05%   +0.07%     
==========================================
  Files         160      161       +1     
  Lines        9367     9486     +119     
==========================================
+ Hits         5993     6076      +83     
- Misses       2914     2947      +33     
- Partials      460      463       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

silvery1622 · 2024-03-25T02:40:26Z

/retest

pkg/services/interfaces.go

pkg/manager/manager.go

pkg/services/network/nsxvpc_provider.go

zhanggbj

Thanks for the PR! Just left some nits :-)

I think it would be great to add some test output from your local nsx-vpc setup in the PR comments for reference, as upstream doesn't have such infra to test it in e2e or integration test.

pkg/services/network/nsxvpc_provider.go

pkg/services/network/network_test.go

silvery1622 · 2024-03-25T06:42:09Z

Thanks for @zhanggbj 's comments.
I have tested the changes in a VCF env with nsx-vpc enabled. Attached the output of cluster/vspherecluster/vspheremachine/virtualmachine and capv log.
capv.log
cluster.log
vspherecluster.log
vspheremachine.log
virtualmachine.log
subnetset.log
subnetport.log

zhanggbj

Generally it looks good :-) Just added some other nits in the second round review.

pkg/services/network/nsxvpc_provider.go

pkg/services/network/network_test.go

pkg/services/network/constants.go

pkg/services/network/network_test.go

pkg/services/interfaces.go

pkg/services/network/constants.go

controllers/vspheremachine_controller.go

pkg/services/vmoperator/vmopmachine.go

pkg/services/network/nsxvpc_provider.go

pkg/services/network/constants.go

pkg/services/network/nsxvpc_provider.go

pkg/services/vmoperator/vmopmachine.go

pkg/services/network/nsxvpc_provider.go

sbueringer · 2024-04-04T11:36:10Z

@silvery1622 ^^

silvery1622 · 2024-04-08T08:50:40Z

A big thanks to @sbueringer and @chrischdi for the awesome reviews and input. I've managed to deal with most of what you pointed out. The one thing I'm still sorting out is the annotation, but I'm about to kick off some tests without it. Could our goal still be to include this in the v1.10.0 release?

sbueringer · 2024-04-08T12:21:48Z

The one thing I'm still sorting out is the annotation, but I'm about to kick off some tests without it. Could our goal still be to include this in the v1.10.0 release?

Yup, no worries.

We'll release CAPV after CAPI v1.7 (16h April) and I wouldn't block it because of the annotation (see our Slack thread, I think we're also talking about different annotations)

sbueringer

I think some of the comments or changes got lost (or not yet pushed). PTAL at the open conversations above

(Please note the "4 hidden conversations" (by GitHub))

pkg/services/network/nsxt_vpc_provider.go

silvery1622 · 2024-04-09T05:30:13Z

I've made some changes based on feedback from the review:

Removed NSXTVPCSelectorKey and let GetVMServiceAnnotations returns a blank map. Also updated the UT for it.
Sorted out a situation where there's no 'ready' condition in subnetset.Status when we're running verifyNsxtVpcSubnetSetStatus. I've also added a UT case for it.
Used klog for logging in ProvisionClusterNetwork and included the network namespace and name in log.
Created a mock up of 'ProvisionClusterNetwork' to sidestep having to check the subnetset ready condition. There are separate cases to deal with when the subnet ready condition hasn't been set or when it's false.
Got rid of the double-checking for errors and switched the name from 'localerr' to just 'err'.

chrischdi

one more readability nit

pkg/services/network/nsxt_vpc_provider.go

This commit introduces NSX-T Virtual Private Cloud (NSX-VPC) as a new network provider for the vSphere Supervisor Cluster in anticipation of the upcoming vSphere 9 release. Key Changes: nsx-vpc implemented as a new NetworkProvider using nsx-operator libs. Skipped VM Readiness check as NSX-VPC offers private network access. Added unit tests for NSX-VPC network provider."

k8s-ci-robot · 2024-04-09T08:27:41Z

@silvery1622: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-vsphere-apidiff-main	`df04b11`	link	false	`/test pull-cluster-api-provider-vsphere-apidiff-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

sbueringer · 2024-04-09T09:24:08Z

/test ?

k8s-ci-robot · 2024-04-09T09:24:11Z

@sbueringer: The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-vsphere-e2e-govmomi-blocking-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-upgrade-1-29-1-30-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-blocking-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-conformance-ci-latest-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-conformance-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-upgrade-1-29-1-30-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main
/test pull-cluster-api-provider-vsphere-test-integration-main
/test pull-cluster-api-provider-vsphere-test-main
/test pull-cluster-api-provider-vsphere-verify-main

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-vsphere-apidiff-main

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-vsphere-apidiff-main
pull-cluster-api-provider-vsphere-e2e-govmomi-blocking-main
pull-cluster-api-provider-vsphere-e2e-supervisor-blocking-main
pull-cluster-api-provider-vsphere-test-integration-main
pull-cluster-api-provider-vsphere-test-main
pull-cluster-api-provider-vsphere-verify-main

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sbueringer · 2024-04-09T09:24:49Z

Thank you very much! Nice work!

/lgtm
/approve

Merge pending CI (want to make sure we don't break CI that close to the release)

/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-upgrade-1-29-1-30-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-conformance-ci-latest-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-conformance-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-upgrade-1-29-1-30-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main

k8s-ci-robot · 2024-04-09T09:24:55Z

LGTM label has been added.

Git tree hash: fdf46b4542c06a6ab564f8d24f9a7a5561129b46

k8s-ci-robot · 2024-04-09T09:24:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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

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

silvery1622 · 2024-04-09T09:43:32Z

Updated downstream test log with latest code.
capv-controller-manager-1.log
capv-controller-manager-2.log
cluster.log
virtualmachine.log
virtualmachineservice.log
vspherecluster.log

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 25, 2024

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 25, 2024

k8s-ci-robot requested review from killianmuldoon and srm09 March 25, 2024 00:41

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 25, 2024

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from 57df559 to d2fcf6e Compare March 25, 2024 02:14

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 25, 2024

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from d2fcf6e to e90acb8 Compare March 25, 2024 02:40

laozc suggested changes Mar 25, 2024

View reviewed changes

pkg/services/interfaces.go Outdated Show resolved Hide resolved

pkg/manager/manager.go Outdated Show resolved Hide resolved

pkg/services/network/nsxvpc_provider.go Outdated Show resolved Hide resolved

pkg/services/network/nsxvpc_provider.go Outdated Show resolved Hide resolved

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from e90acb8 to c5c2846 Compare March 25, 2024 03:15

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 25, 2024

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from c5c2846 to 454e015 Compare March 25, 2024 05:06

zhanggbj reviewed Mar 25, 2024

View reviewed changes

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from 454e015 to 3d1b0c2 Compare March 25, 2024 06:23

zhanggbj suggested changes Mar 25, 2024

View reviewed changes

This comment was marked as abuse.

Sign in to view

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from 3d1b0c2 to 3c47661 Compare March 25, 2024 08:59

chrischdi reviewed Mar 25, 2024

View reviewed changes

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from 3c47661 to 057de59 Compare March 25, 2024 17:08

chrischdi reviewed Mar 26, 2024

View reviewed changes

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from 057de59 to 4e74865 Compare March 26, 2024 12:51

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from 4e74865 to a380565 Compare April 8, 2024 08:41

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from a380565 to 486263f Compare April 8, 2024 09:25

sbueringer reviewed Apr 8, 2024

View reviewed changes

pkg/services/network/nsxt_vpc_provider.go Outdated Show resolved Hide resolved

pkg/services/network/nsxt_vpc_provider.go Show resolved Hide resolved

silvery1622 force-pushed the add-networkprovider-nsxvpc branch 7 times, most recently from 99e67d4 to 590d6d5 Compare April 9, 2024 05:15

chrischdi reviewed Apr 9, 2024

View reviewed changes

pkg/services/network/nsxt_vpc_provider.go Outdated Show resolved Hide resolved

silvery1622 force-pushed the add-networkprovider-nsxvpc branch from 590d6d5 to df04b11 Compare April 9, 2024 08:21

k8s-ci-robot assigned sbueringer Apr 9, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 9, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 9, 2024

k8s-ci-robot merged commit a9a9a39 into kubernetes-sigs:main Apr 9, 2024
25 of 26 checks passed

k8s-ci-robot added this to the v1.10 milestone Apr 9, 2024

silvery1622 mentioned this pull request Nov 1, 2024

REQUEST: New membership for silvery1622 kubernetes/org#5239

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨Add NSX-VPC Network Provider Support #2848

✨Add NSX-VPC Network Provider Support #2848

silvery1622 commented Mar 25, 2024

linux-foundation-easycla bot commented Mar 25, 2024 •

edited

Loading

k8s-ci-robot commented Mar 25, 2024

k8s-ci-robot commented Mar 25, 2024

zhanggbj commented Mar 25, 2024

codecov bot commented Mar 25, 2024 •

edited

Loading

silvery1622 commented Mar 25, 2024

zhanggbj left a comment •

edited

Loading

silvery1622 commented Mar 25, 2024

zhanggbj left a comment

This comment was marked as abuse.

sbueringer commented Apr 4, 2024

silvery1622 commented Apr 8, 2024

sbueringer commented Apr 8, 2024 •

edited

Loading

sbueringer left a comment •

edited

Loading

silvery1622 commented Apr 9, 2024

chrischdi left a comment

k8s-ci-robot commented Apr 9, 2024

sbueringer commented Apr 9, 2024

k8s-ci-robot commented Apr 9, 2024

sbueringer commented Apr 9, 2024 •

edited

Loading

k8s-ci-robot commented Apr 9, 2024

k8s-ci-robot commented Apr 9, 2024

silvery1622 commented Apr 9, 2024

✨Add NSX-VPC Network Provider Support #2848

✨Add NSX-VPC Network Provider Support #2848

Conversation

silvery1622 commented Mar 25, 2024

linux-foundation-easycla bot commented Mar 25, 2024 • edited Loading

k8s-ci-robot commented Mar 25, 2024

k8s-ci-robot commented Mar 25, 2024

zhanggbj commented Mar 25, 2024

codecov bot commented Mar 25, 2024 • edited Loading

Codecov Report

silvery1622 commented Mar 25, 2024

zhanggbj left a comment • edited Loading

Choose a reason for hiding this comment

silvery1622 commented Mar 25, 2024

zhanggbj left a comment

Choose a reason for hiding this comment

This comment was marked as abuse.

sbueringer commented Apr 4, 2024

silvery1622 commented Apr 8, 2024

sbueringer commented Apr 8, 2024 • edited Loading

sbueringer left a comment • edited Loading

Choose a reason for hiding this comment

silvery1622 commented Apr 9, 2024

chrischdi left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 9, 2024

sbueringer commented Apr 9, 2024

k8s-ci-robot commented Apr 9, 2024

sbueringer commented Apr 9, 2024 • edited Loading

k8s-ci-robot commented Apr 9, 2024

k8s-ci-robot commented Apr 9, 2024

silvery1622 commented Apr 9, 2024

linux-foundation-easycla bot commented Mar 25, 2024 •

edited

Loading

codecov bot commented Mar 25, 2024 •

edited

Loading

zhanggbj left a comment •

edited

Loading

sbueringer commented Apr 8, 2024 •

edited

Loading

sbueringer left a comment •

edited

Loading

sbueringer commented Apr 9, 2024 •

edited

Loading