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 PolicyTransportZone resource #940

Merged
merged 6 commits into from
Aug 25, 2023
Merged

Add PolicyTransportZone resource #940

merged 6 commits into from
Aug 25, 2023

Conversation

wsquan171
Copy link
Contributor

@wsquan171 wsquan171 commented Aug 10, 2023

This change adds PolicyTransportZone as resource.

Compared to existing data source, the following changes are made:

  • Added site_path and enforcement_point_id in case a TZ is to be created under non-default site / ep. When left out, the resource will still be CRUD'ed from default / default, or default / nsxt.enforcement_point in case of VMC
  • site_path, enforcement_point_id and transport_type are made ForceNew as they are immutable.
  • Added uplink_teaming_policy_names which is only supported on VLAN_BACKED transport zones.
  • When importing, only policy path is accepted.

Fields not added due to missing from current SDK:

  • forwarding_mode: Choice of IPV4_ONLY, IPV6_ONLY, IPV4_AND_IPV6
  • Global manager does not support C/U/D of TZs. Thus only infra client is used.

Fields not added due to missing data source and unclear use case:

  • transport_zone_profile_paths: List of /infra/transport-zone-profiles. This option is not even showing up on current UI.

}
siteID = d.Get("site_id").(string)
if siteID == "" {
siteID = defaultSite
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason not to specify this default in the schema instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Will make it default in schema.

Copy link
Contributor Author

@wsquan171 wsquan171 Aug 10, 2023

Choose a reason for hiding this comment

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

@annakhm Actually we can't use default with computed.

Default must be nil if computed

For our use case this must be computed I assume, so we have to set the fallback value in CRUD logic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this need to be computed, in case default is set whenever the user does not specify the value? Does platform ever override this value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I got what you're saying.

Also want to confirm that for enforcement_point_id this has to be done with current approach, correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I don't think enforcement point has a default

@annakhm annakhm linked an issue Aug 10, 2023 that may be closed by this pull request
@ksamoray
Copy link
Collaborator

Code LGTM, no doc or test?

@wsquan171 wsquan171 force-pushed the res-tz branch 2 times, most recently from 1bb18f6 to a995a2d Compare August 17, 2023 00:57
@wsquan171
Copy link
Contributor Author

/test-all

2 similar comments
@wsquan171
Copy link
Contributor Author

/test-all

@annakhm
Copy link
Collaborator

annakhm commented Aug 21, 2023

/test-all

Default: defaultInfraSitePath,
ValidateFunc: validatePolicyPath(),
},
"enforcement_point_id": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest to rename this to enforcement_point to be consistent with provider attribute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@annakhm
Copy link
Collaborator

annakhm commented Aug 22, 2023

/test-all


# nsxt_policy_transport_zone

This resource provides a method for the management of Policy based Transport Zones (TZ). A Transport Zone defines the scope to which a network can extend in NSX. For example an overlay based Transport Zone is associated with both hypervisors and logical switches and defines which hypervisors will be able to serve the defined logical switch. Virtual machines on the hypervisor associated with a Transport Zone can be attached to logical switches in that same Transport Zone.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should use policy terminology here (segment rather than logical switch)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@wsquan171
Copy link
Contributor Author

/test-all

@annakhm
Copy link
Collaborator

annakhm commented Aug 24, 2023

/test-all

@annakhm
Copy link
Collaborator

annakhm commented Aug 24, 2023

/test-all

@wsquan171 wsquan171 merged commit 3f7ab1e into vmware:master Aug 25, 2023
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.

Add support for creating transport zones
3 participants