-
Notifications
You must be signed in to change notification settings - Fork 87
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 policy edge resource #1544
base: master
Are you sure you want to change the base?
Conversation
c065805
to
e91adb2
Compare
"static_ipv6_pool": { | ||
Type: schema.TypeString, | ||
Description: "IP assignment specification for Static IPv6 Pool. Input can be MP ip pool UUID or policy path of IP pool", | ||
Optional: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we enforce policy path here? No reason to use MP pool id (same for ipv4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense - the MP pool id capability is provided by NSX (the description is the API's)
MinItems: 1, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"overlay_transport_zone_path": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets use getPolicyPathSchema
? Or validatePolicyPath
Optional: true, | ||
Elem: &schema.Schema{ | ||
Type: schema.TypeString, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we validatePolicyPath
here as well?
Required: true, | ||
Description: "Storage/datastore identifier in the specified vcenter server", | ||
}, | ||
"vc_id": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this compute manager id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed - that's how they named this in API. Maybe it's worthwhile to rename this to compute_manager_id
for consistency?
return nil | ||
} | ||
creds := make(map[string]interface{}) | ||
creds["audit_password"] = obj.AuditPassword |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are passwords present in response? This smells like permadiff potential
* `failure_domain_path` - (Optional) Path of the failure domain. | ||
* `form_factor` - (Optional) Appliance form factor. Valid values - 'SMALL', 'MEDIUM', 'LARGE', 'XLARGE'. The default value is 'MEDIUM'. | ||
* `hostname` - (Required) Host name or FQDN for edge node. | ||
* `management_interface` - (Optional) Applicable For LCM managed Node and contains the management interface info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
judging from the code, this supports two assignments (I'm guessing v4 and v6) - lets mention this here
}, | ||
}, | ||
}, | ||
"uplink_host_switch_profile_path": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validatePolicyPath
here and below?
Also those might be auto-assigned to default profile, in which case Computed
is needed
4b50ece
to
6c62f88
Compare
Signed-off-by: Kobi Samoray <[email protected]>
6c62f88
to
ae6c9b8
Compare
For NSX v9.0, transport node API is deprecated and replaced with policy API which provides extra functionality.