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

Policy host transport node profile resource #954

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

ksamoray
Copy link
Collaborator

No description provided.

@ksamoray ksamoray force-pushed the p_xport_node_profile branch 3 times, most recently from 4f0f9d7 to 33eb5dd Compare August 27, 2023 11:03
"tag": getTagsSchema(),
// host_switch_spec
"standard_host_switch": getStandardHostSwitchSchema(),
"preconfigured_host_switch": getPreconfiguredHostSwitchSchema(),
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 this attr is ignored later on?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getHostSwitchSpecFromSchema() and setHostSwitchSpecInSchema() handle these two attributes (it's the same schema which is used by transport_node).

@annakhm
Copy link
Collaborator

annakhm commented Aug 29, 2023

Did I understand correctly that in transport node resource user can either provide equivalent config, or define this profile as separate resource and specify it in transport node?

@ksamoray
Copy link
Collaborator Author

Did I understand correctly that in transport node resource user can either provide equivalent config, or define this profile as separate resource and specify it in transport node?

The simpler way to configure the host nodes (e.g ESXs) is to associate a transport_node_profile with the compute manager, and from there NSX will configure the nodes automatically.

@ksamoray ksamoray force-pushed the p_xport_node_profile branch 4 times, most recently from 0e89461 to a8c07b1 Compare September 5, 2023 13:46
@ksamoray ksamoray force-pushed the p_xport_node_profile branch 3 times, most recently from d509992 to 44f37b5 Compare September 14, 2023 07:34
@ksamoray ksamoray force-pushed the p_xport_node_profile branch from 44f37b5 to 195a4f1 Compare September 18, 2023 08:58
* `mac` - (Required) MAC address.
* `static_ip_pool_id` - (Optional) IP assignment specification for Static IP Pool.
* `is_migrate_pnics` - (Optional) Migrate any pnics which are in use.
* `pnics` - (Optional) Physical NICs connected to the host switch.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be pnic

}
host_switch_profile_id = [nsxt_uplink_host_switch_profile.hsw_profile1.id]
is_migrate_pnics = false
pnics {
Copy link
Collaborator

Choose a reason for hiding this comment

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

pnic

* `host_switch_profile_id` - (Optional) Identifiers of host switch profiles to be associated with this host switch.
* `host_switch_type` - (Optional) Type of HostSwitch. Accepted values - 'NVDS' or 'VDS'. The default value is 'NVDS'.
* `ip_assignment` - (Required) - Specification for IPs to be used with host switch virtual tunnel endpoints. Should contain exatly one of the below:
* `assigned_by_dhcp` - (Optional) Enables DHCP assignment. Should be set to true.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't get the Should be set to true part :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IpAssignmentSpec is a polymorphic schema with StaticIpListSpec, StaticIpPoolSpec containing members. Instead AssignedByDhcp has no members - it's a struct which has only resource_type. As we can't have a schema without any attributes in TF, I added a boolean. I'd appreciate a better suggestion really as it does look weird.

Copy link
Collaborator

Choose a reason for hiding this comment

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

From terraform perspective polymorphic structure is hidden, but the flag should be only set to true if user wants dhcp feature. I would suggest to add false default for this flag, and explain in docs, that static_ip and static_ip_mac can only be specified if dhcp is set to false.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only one of assigned_by_dhcp/static_ip/static_mac/static_ip_pool can be used. It's enforced here as I couldn't use TF ExactlyOneOf capability. That's also documented.
So if I set a false default value - it'll always add assigned_by_dhcp which will create a mess IMO.
I think that I'll leave it as is - if it is set to false the transaction fails (as some IP assignment should be chosen and if assigned_by_dhcp is false, then there's no other assignment enabled).
And remove "should be set to true" from doc.

@ksamoray ksamoray force-pushed the p_xport_node_profile branch from 195a4f1 to 68b8659 Compare September 19, 2023 09:44
* `host_switch_mode` - (Optional) Operational mode of a HostSwitch. Accepted values - 'STANDARD', 'ENS', 'ENS_INTERRUPT' or 'LEGACY'. The default value is 'STANDARD'.
* `host_switch_profile_id` - (Optional) Identifiers of host switch profiles to be associated with this host switch.
* `host_switch_type` - (Optional) Type of HostSwitch. Accepted values - 'NVDS' or 'VDS'. The default value is 'NVDS'.
* `ip_assignment` - (Required) - Specification for IPs to be used with host switch virtual tunnel endpoints. Should contain exatly one of the below:
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo in exatly

@ksamoray
Copy link
Collaborator Author

/test-all

@ksamoray ksamoray force-pushed the p_xport_node_profile branch from 68b8659 to 58c871a Compare September 21, 2023 07:13
@ksamoray ksamoray merged commit 140c5cc into vmware:master Sep 21, 2023
2 checks passed
@ksamoray ksamoray deleted the p_xport_node_profile branch September 21, 2023 09:14
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