search

vmware / terraform-provider-nsxt

Terraform VMware NSX-T provider
https://www.terraform.io/docs/providers/nsxt/
Other
128 stars 83 forks source link

Add custom groups support to upgrade run resource #1191

Closed ksamoray closed 3 months ago

ksamoray commented 4 months ago

Add the logic to create and update custom host groups while running the upgrade process.

ksamoray commented 4 months ago

/test-all

ksamoray commented 4 months ago

/test-all

ksamoray commented 4 months ago

/test-all

ksamoray commented 4 months ago

/test-all

ksamoray commented 4 months ago

/test-all

annakhm commented 3 months ago

I didn't see any update in setUpgradeRunOutput function (sorry if I missed it), but looks like custom group needs to be updated with its new attributes

ksamoray commented 3 months ago

I didn't see any update in setUpgradeRunOutput function (sorry if I missed it), but looks like custom group needs to be updated with its new attributes

Yeah I missed this. Although as we do not support the state existence, that's a bit meaningless for now.

ksamoray commented 3 months ago

I didn't see any update in setUpgradeRunOutput function (sorry if I missed it), but looks like custom group needs to be updated with its new attributes

But this is only the status type - which doesn't have the extra group attributes. So I'm not sure that a change is required here.

annakhm commented 3 months ago

I didn't see any update in setUpgradeRunOutput function (sorry if I missed it), but looks like custom group needs to be updated with its new attributes

Yeah I missed this. Although as we do not support the state existence, that's a bit meaningless for now.

I think that although we ask to remove state post-upgrade, terraform plan should not produce non-empty diff after upgrade. is this true today?

annakhm commented 3 months ago

LGTM, except for two doubts:

ksamoray commented 3 months ago

Terraform doesn't show an empty diff, also without these changes as far as I understand (there are dependencies on computed groups within the upgrade process - I think that this is the reason?). As for group deletion: it's possible to check the groups for empty groups (with no upgrade units) which do not match the "predefined group pattern" (either have the id of a compute collection, or that hardcoded name which NSX uses for non-clustered hosts). These can be deleted I guess. Is that what you had in mind?

annakhm commented 3 months ago

Terraform doesn't show an empty diff, also without these changes as far as I understand (there are dependencies on computed groups within the upgrade process - I think that this is the reason?). As for group deletion: it's possible to check the groups for empty groups (with no upgrade units) which do not match the "predefined group pattern" (either have the id of a compute collection, or that hardcoded name which NSX uses for non-clustered hosts). These can be deleted I guess. Is that what you had in mind?

I would suggest to rely on GetChange routing to check if groups are explicitly deleted + deleting groups that we know are recreated as a result of update.

ksamoray commented 3 months ago

/test-all