Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for NSX-T Alb General Settings #403

Merged
merged 5 commits into from
Nov 4, 2021

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented Oct 7, 2021

This PR starts adding NSX-T ALB support for Tenant view. It is in tenant view, but still requires System user (just like creating an edge gateway)
This is the first resource in series to allow controlling ALB on NSX-T Edge Gateway. It still requires system user to modify, just as creating an edge gateway, but is presented in Tenants view
image

From technical standpoint it adds type NsxtAlbConfig and functions NsxtEdgeGateway.UpdateAlbGeneralSettings, NsxtEdgeGateway.GetAlbGeneralSettings, NsxtEdgeGateway.DisableAlb as well as tests.
It also adds another testing helper function spawnAlbControllerCloudServiceEngineGroup which does all Provider side configuration of NSX-T ALB and will be required for all tests in Tenant view.

Tests on tag alb have passed on 10.1 (ALB is not supported there), 10.2 and 10.3

Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius self-assigned this Oct 7, 2021
@Didainius Didainius marked this pull request as ready for review October 8, 2021 09:44
@@ -0,0 +1,2 @@
* Added type `NsxtAlbConfig` and functions `NsxtEdgeGateway.UpdateAlbSettings`, `NsxtEdgeGateway.GetAlbSettings`,
`NsxtEdgeGateway.DisableAlb` [GH-403]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe worth mentioning the helper function too?

Copy link
Contributor

Choose a reason for hiding this comment

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

The helper function is a private one made for testing that should not be mentioned in the changes log.

Copy link
Collaborator Author

@Didainius Didainius Oct 18, 2021

Choose a reason for hiding this comment

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

Yep, I am on this one with dataclouder - it is an internal function used for testing only.

@@ -78,3 +78,34 @@ func (vcd *TestVCD) Test_GetAllAlbServiceEngineGroups(check *C) {
err = controller.Delete()
check.Assert(err, IsNil)
}

// spawnAlbControllerCloudServiceEngineGroup is a helper function to spawn NSX-T ALB Controller,ALB Cloud, and ALB
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// spawnAlbControllerCloudServiceEngineGroup is a helper function to spawn NSX-T ALB Controller,ALB Cloud, and ALB
// spawnAlbControllerCloudServiceEngineGroup is a helper function to spawn NSX-T ALB Controller, ALB Cloud and ALB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1,2 @@
* Added type `NsxtAlbConfig` and functions `NsxtEdgeGateway.UpdateAlbSettings`, `NsxtEdgeGateway.GetAlbSettings`,
`NsxtEdgeGateway.DisableAlb` [GH-403]
Copy link
Contributor

Choose a reason for hiding this comment

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

The helper function is a private one made for testing that should not be mentioned in the changes log.

Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

LGTM!

@Didainius Didainius merged commit 693e518 into vmware:master Nov 4, 2021
@Didainius Didainius deleted the alb_general_settings-pr branch November 4, 2021 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants