-
Notifications
You must be signed in to change notification settings - Fork 85
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
[WIP] Add support for Edge transport nodes which were created externally #1473
base: master
Are you sure you want to change the base?
Conversation
aef02f1
to
8bf7432
Compare
e20b1dc
to
d9d4d11
Compare
Optional: true, | ||
Computed: true, | ||
Description: "Unique Id of the fabric node", | ||
}, |
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.
This is probably a bad idea, but I'll suggest it anyway.
As this resource is not generated, we do not have to fully respect the NSX API schema.
Perhaps we can move node_id in deployment_config and specify that it conflictsWith vm_deployment_config?
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.
deployment_config
block contains stuff which is related to the Edge VM deployment: also attributes which are outside the vm_deployment_config
block is related, e.g form_factor
. Also node_user_settings
is related.
And as these are required for a non-pre-deployed, we'll have to change those to optional for pre-deployed edges.
In fact I think that it's a good idea to mark deployment_config
and node_id
as conflicting.
fail. | ||
So the provider will ignore node_deployment_info properties when node_id has a value - which would mean that | ||
this edge appliance was created externally. | ||
*/ |
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.
It might be helpful to also add a log statement here for debugging purposes.
e.g: "nodeID not specified, will deploy edge using values in deploymentConfig"
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.
Done
var nodeDeploymentInfo *data.StructValue | ||
if nodeID == "" { | ||
/* | ||
node_id attribute conflicts with node_deployment_info. As node_deployment_info is a complex object which has |
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.
Could you please clarify - even when specifying node_id
, user still needs to populate node_deployment_info
? What properties are needed?
I don't think node_deployment_info
is Computed
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.
The resource has no node_deployment_info
attribute as it's not implementing the entire API, as host nodes are created with policy API. Relevant node_deployment_info
attributes are placed under deployment_config
, and directly under the resource root.
I've set these attributes as conflicting with node_id
.
d9d4d11
to
039889c
Compare
@@ -725,6 +745,16 @@ func resourceNsxtEdgeTransportNodeCreate(d *schema.ResourceData, m interface{}) | |||
connector := getPolicyConnector(m) | |||
client := nsx.NewTransportNodesClient(connector) | |||
|
|||
nodeID := d.Get("node_id").(string) | |||
if nodeID != "" { | |||
_, err := client.Get(nodeID) |
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.
We don't need to set revision/other computed properties here?
Keeping this PR in 3.8.0 milestone as we're almost done with the review process. |
Normally users create Edge Transport Nodes within NSX, which deploys them into the regsitered compute manager. However, users have the option to do the same by deploying the Edge appliance anywhere, outside NSX, and registering it with NSX using the Edge CLI or OVA parameters. Later, this Edge appliance can be converted into a transport node using NSX API - this change attempts to utilize this capability. Fixes: vmware#1459 Signed-off-by: Kobi Samoray <[email protected]>
039889c
to
d1545a1
Compare
Normally users create Edge Transport Nodes within NSX, which deploys them into the regsitered compute manager. However, users have the option to do the same by deploying the Edge appliance anywhere, outside NSX, and registering it with NSX using the Edge CLI or OVA parameters. Later, this Edge appliance can be converted into a transport node using NSX API - this change attempts to utilize this capability.
Fixes: #1459