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

Implement multi-tenancy support for relevant resources #895

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

ksamoray
Copy link
Collaborator

Some of the resource have been added support to multi-tenancy.
Support is implemented using a generated API wrapper, which abstracts the calls to the various NSX APIs (local manager/global manager/multi tenancy).

nsxt/utils.go Outdated
@@ -662,3 +662,25 @@ func handlePagination(lister func(*paginationInfo) error) (int64, error) {

return total, nil
}

func getContextSchema(forceNew bool) *schema.Schema {
Copy link
Collaborator

Choose a reason for hiding this comment

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

When do we expect forceNew to be false?
Since context triggers object path change, seems to me ForceNew should always be defined

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. Thanks :)

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'm guessing that the whole context - no matter what we'll add to it in the future, should be ForceNew as well.

@ksamoray ksamoray force-pushed the api-wrapper branch 7 times, most recently from 1d1fa0f to 561c60b Compare May 22, 2023 10:48
@ksamoray
Copy link
Collaborator Author

/test-all

@ksamoray ksamoray force-pushed the api-wrapper branch 6 times, most recently from 26e5ce6 to 45c086f Compare May 25, 2023 07:03
@ksamoray
Copy link
Collaborator Author

/test-all

@ksamoray ksamoray force-pushed the api-wrapper branch 14 times, most recently from 90c9528 to 24f7a22 Compare May 29, 2023 16:02
d.Set("display_name", obj.DisplayName)
d.Set("description", obj.Description)
d.Set("path", obj.Path)
err := resourceNsxtPolicyTier1GatewayReadEdgeCluster(d, connector)
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like edge cluster would not be set for local manager?

var cursor *string
total := 0

// Make sure global objects are not found (path needs to start with infra)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this comment is not relevant here

if gwID == "" {
// infra segment
client := segments.NewDhcpStaticBindingConfigsClient(connector)
if context.ClientType == utl.Global || gwID == "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

fixed segments might be supported on GM now. I suggest not to change the logic in this PR but lets check this later (it would simplify the code to only look at GW present in this condition)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK. So should we open an issue to track this?

@ksamoray ksamoray force-pushed the api-wrapper branch 4 times, most recently from 11a99b6 to 3481f4b Compare June 5, 2023 07:46
@ksamoray
Copy link
Collaborator Author

ksamoray commented Jun 5, 2023

/test-all

@ksamoray ksamoray force-pushed the api-wrapper branch 2 times, most recently from 335e5fe to 1106dfc Compare June 5, 2023 07:57
type: Multitenancy
model_name: PolicyContextProfile
obj_name: ContextProfile
var_name: policyContextProfileParam
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please explain the purpose of var_name? Can it be unified across all resources?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Schema is explained at the top of the file. Specifically, var_name explained here

return nil, err[0]
}

gmObj, err := converter.ConvertToGolang(dataValue, destType)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not specific to gm (global manager). Perhaps convertedObj?

@annakhm
Copy link
Collaborator

annakhm commented Jun 6, 2023

I wonder if enforcement points relevant for /orgs? We have functions like getPolicyEnforcementPointPath that hard-code infra and global-infra

@annakhm
Copy link
Collaborator

annakhm commented Jun 6, 2023

nsxtSegmentResourceImporter also has infra and global-infra hard-coded. But sinceimport needs multitenancy support for all resources, likely using policy path, I expect this issue to be fixed together with the global fix.

@ksamoray ksamoray force-pushed the api-wrapper branch 4 times, most recently from 7934a6f to 2295333 Compare June 8, 2023 15:30
@annakhm
Copy link
Collaborator

annakhm commented Jun 8, 2023

We would also need to craft a documentation guide for multitenancy use case, but this can be added in follow up

@ksamoray ksamoray force-pushed the api-wrapper branch 3 times, most recently from 28e0c6d to 16d0498 Compare June 15, 2023 06:29
@ksamoray
Copy link
Collaborator Author

/test-all

@ksamoray
Copy link
Collaborator Author

/test-all

@ksamoray ksamoray merged commit bca7b09 into vmware:master Jun 21, 2023
@ksamoray ksamoray deleted the api-wrapper branch June 21, 2023 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants