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.

Sign up for GitHub

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

Jump to bottom

Disable MP Resources from NSX 9.0.0 onwards #1384

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Disable MP Resources from NSX 9.0.0 onwards #1384

wants to merge 1 commit into from
+274 −72

Conversation

salv-orlando
Copy link
Member

This change removes support for NSX MP API client when the NSX manager version is higher or equal to 9.0.0

For MP resources, with NSX >= 9.0.0:

  • terraform apply, destroy, and import operations will fail with the following error: "This resource is not supported with given provider settings. Management Plane resources are not supported since NSX 9.0.0;
  • terraform plan operations for resources already in the Terraform state will fail with the same error;
  • all terraform operations involving MP data sources will fail with the same error;
  • terraform plan operations for resource not yet in the Terraform state will succeed, but will then fail at the apply stage. This because Terraform won't perform a read from the backend if the resource is not present at all in the state.

@salv-orlando
Copy link
Member Author

/test-all

annakhm
annakhm reviewed Sep 25, 2024
View reviewed changes
nsxt/provider.go Outdated
@@ -1090,6 +1090,12 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
return nil, err
}

initNSXVersion(getPolicyConnector(clients))
if util.NsxVersionHigherOrEqual("9.0.0") {
// Disable MP client from 9.0.0 onwards
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 fine for now, but ideally we should not invoke configureNsxtClient when NSX is 9.0.0 and above.
This would require us to rewrite the session create API which is currently handle fired from inside configureNsxtClient. We should do this regardless, because it paves the way of removing the old SDK when the time comes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've removed this code. I will handle the change in session create API with a new commit in this PR or a new PR

@annakhm
Copy link
Collaborator

annakhm commented Sep 25, 2024 •
edited
Loading

Food for thought: imagine a user has MP resources in their state, and they upgrade to 9.0.0. They understand that those are no longer supported and try to delete them from the config and apply. With current PR, apply will fail on delete.
Moreover, even refresh will fail.
We might want to offer noop for the delete and read operations in this case (which will require a change in every MP resource).
Another option is to add a note in documentation explaining that resources need to be manually deleted from the state as well as config.
What do you think?

@salv-orlando salv-orlando force-pushed the disable_mp_client branch 2 times, most recently from 9311b93 to c33dba4 Compare September 30, 2024 17:41
@salv-orlando
Copy link
Member Author

Food for thought: imagine a user has MP resources in their state, and they upgrade to 9.0.0. They understand that those are no longer supported and try to delete them from the config and apply. With current PR, apply will fail on delete. Moreover, even refresh will fail. We might want to offer noop for the delete and read operations in this case (which will require a change in every MP resource). Another option is to add a note in documentation explaining that resources need to be manually deleted from the state as well as config. What do you think?

All MP resources should now fail on create, delete or import, and do a no-op on read and update.
For MP data sources, they will fail on read.

@salv-orlando
Disable MP Resources from NSX 9.0.0 onwards
607d90c 
This change disables deperecate NSX MP resources and data
sources when  NSX manager version is higher or equal to 9.0.0

For MP resources, with NSX >= 9.0.0:
- terraform apply and import operations will fail
  with an error like the following one:
  "MP resource <name> was deprecated and has been removed in
   NSX 9.0.0";
- terraform plan and delete operations for MP resources already
  in the state will not be impacted as read and delete operations
  are no-op when the NSX version is 9.0.0 or higher. This will
  allow users to easily remove these resources from the terraform
  state;
- all terraform operations involving MP data sources will fail
  with the error above;
- terraform plan operations for resource not yet in the Terraform
  state will succeed, but will then fail at the apply stage. This
  because Terraform won't perform a read from the backend if the
  resource is not present at all in the state.

Signed-off-by: Salvatore Orlando <salv.orlando@gmail.com>
@salv-orlando
Copy link
Member Author

/test-all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annakhm annakhm annakhm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@salv-orlando @annakhm