Add support for Edge transport nodes which were created externally #1473
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ksamoray
force-pushed
the
edge_ova_create
branch
from
aef02f1 to
8bf7432
Compare
ksamoray
force-pushed
the
edge_ova_create
branch
2 times, most recently
from
e20b1dc to
d9d4d11
Compare
annakhm
reviewed
Nov 30, 2024
ksamoray
force-pushed
the
edge_ova_create
branch
from
d9d4d11 to
039889c
Compare
annakhm
reviewed
Dec 2, 2024
| @@ -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.
We don't need to set revision/other computed properties here?
annakhm
changed the title
|
Keeping this PR in 3.8.0 milestone as we're almost done with the review process. |
ksamoray
force-pushed
the
edge_ova_create
branch
from
039889c to
d1545a1
Compare
ksamoray
changed the title
annakhm
reviewed
Dec 18, 2024
| converter := bindings.NewTypeConverter() | ||
| base, errs := converter.ConvertToGolang(obj.NodeDeploymentInfo, mpmodel.EdgeNodeBindingType()) | ||
| if errs != nil { | ||
| return handleReadError(d, "TransportNode", nodeID, errs[0]) |
There was a problem hiding this comment.
I think this should still be handleCreateError, since we're in create flow
ksamoray
force-pushed
the
edge_ova_create
branch
from
d1545a1 to
c5c64ca
Compare
annakhm
reviewed
Dec 18, 2024
ksamoray
force-pushed
the
edge_ova_create
branch
from
c5c64ca to
400861e
Compare
annakhm
reviewed
Dec 18, 2024
| } | ||
|
|
||
| d.SetId(nodeID) | ||
| return resourceNsxtEdgeTransportNodeUpdate(d, m) |
There was a problem hiding this comment.
In case update fails, there might be a state problem here. Since we already performed d.SetId(nodeId), the resource will be already present in state, if I'm not mistaken.
If would be safer to catch error here, and in case Update fails, nullify the Id.
sdk reference:
https://github.com/hashicorp/terraform-plugin-sdk/blob/00fc746ade8f31a96a9030b867bedcff3365afa5/helper/schema/resource.go#L134
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 <kobi.samoray@broadcom.com>
ksamoray
force-pushed
the
edge_ova_create
branch
from
400861e to
738a846
Compare
salv-orlando
approved these changes
Dec 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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