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 Application Port Profiles #378

Merged
merged 2 commits into from
Jun 11, 2021

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented May 18, 2021

This PR adds types NsxtAppPortProfile and types.NsxtAppPortProfile together with org.CreateNsxtAppPortProfile, org.GetAllNsxtAppPortProfiles, org.GetNsxtAppPortProfileByName, NsxtAppPortProfile.Update, NsxtAppPortProfile.Delete and adds tests on top of it.

NsxtAppPortProfile uses OpenAPI endpoint to operate NSX-T Application Port Profiles
It can have 3 types of scopes:

  • SYSTEM - Read-only (The ones that are provided by SYSTEM). Constant types.ApplicationPortProfileScopeSystem
  • PROVIDER - Created by Provider on a particular network provider (NSX-T manager). Constant types.ApplicationPortProfileScopeProvider
  • TENANT (Created by Tenant at Org VDC level). Constant types.ApplicationPortProfileScopeTenant

Additionally:

  • Add a workaround to OpenApiGetAllItems internal function openApiGetAllPages to work around a VCD bug where some endpoints do not return nextPage header to follow and show pageCount as 0 while there are multiple pages available.
  • Fix LDAP related tests to map correct port in latest versions of LDAP container
  • Add optional test config variable Misc.LdapContainer to override default docker container location. (can be used to overcome throttling issues)
  • Prevent nil pointer dereference errors in some tests when vApp is not available
  • Test_GetDiskByHref supplied fake UUID of invalid length. Some API versions fail to validate it. Adjusted to include fake ID of a valid length
  • Test_VdcUpdateStorageProfile used pvdcStorageProfile name in VDC storage profile. This is not always correct and was adjusted to map correctly.
  • Adds two new types.VAppStatuses - PARTIALLY_POWERED_OFF and PARTIALLY_SUSPENDED

Note. Test suites passed on 10, 10.1, 10.2

@Didainius Didainius self-assigned this May 18, 2021
@Didainius Didainius force-pushed the app-profiles-pr branch 9 times, most recently from 12727d7 to 0e51c79 Compare May 19, 2021 19:38
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius marked this pull request as ready for review May 20, 2021 05:14
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.

Just a small comment.

org, err := vcd.client.GetOrgByName(vcd.config.VCD.Org)
check.Assert(err, IsNil)

// For PROVIDER scope application port profile must have ContextEntityId set as NSX-T Managers URN and no Org
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just checking is the comment correct? The function seems to be for tenant.
Few lines below Description is also referring to Provider.

BTW, where does the all-capitalized form of "PROVIDER" and "TENANT" come from? Looks to be "yelling" - not sure if that's really needed here :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I probably have overused capitalization :). These are scopevalues being set in API.

Basically there are 3 scopes system, provider and tenant

  • system - read-only (built-in application profiles that are provided by system scope)
  • provider - can be created and filtered by System user on a particular network provider (NSX-T manager). This means that whatever is set - should be available to all tenants using the same NSX-T manager
  • tenant can be created at tenant level (org VDC)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make a decision on this together with @dataclouder

Copy link
Contributor

Choose a reason for hiding this comment

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

If the API demands uppercase, I prefer keeping the original value in the comments and docs

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.

I have left that one conversation open for when @dataclouder is reviewing this PR. LGTM, otherwise!

org, err := vcd.client.GetOrgByName(vcd.config.VCD.Org)
check.Assert(err, IsNil)

// For PROVIDER scope application port profile must have ContextEntityId set as NSX-T Managers URN and no Org
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make a decision on this together with @dataclouder

This was referenced Jun 4, 2021
@Didainius Didainius merged commit 61bd437 into vmware:master Jun 11, 2021
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.

4 participants