Skip to content

Commit

Permalink
Fix error handling for MP SDK resources
Browse files Browse the repository at this point in the history
Make sure all new fabric resources use error handling helpers,
and those helpers work for error spec defined in all SDK services

Signed-off-by: Anna Khmelnitsky <[email protected]>
  • Loading branch information
annakhm committed Sep 19, 2023
1 parent edf823b commit b121470
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 70 deletions.
21 changes: 11 additions & 10 deletions nsxt/policy_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ import (
"github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model"
)

func isEmptyAPIError(apiError model.ApiError) bool {
return (apiError.ErrorCode == nil && apiError.ErrorMessage == nil)
}

func printAPIError(apiError model.ApiError) string {
if apiError.ErrorMessage != nil && apiError.ErrorCode != nil {
return fmt.Sprintf("%s (code %v)", *apiError.ErrorMessage, *apiError.ErrorCode)
Expand Down Expand Up @@ -81,18 +77,23 @@ func logVapiErrorData(message string, vapiMessages []std.LocalizableMessage, vap
return fmt.Errorf("%s (no additional details provided)", message)
}

// ApiError type is identical in all three relevant SDKs - policy, MP and GM
var typeConverter = bindings.NewTypeConverter()
data, err := typeConverter.ConvertToGolang(apiErrorDataValue, model.ApiErrorBindingType())

// As of today, we cannot trust converter to return error in case target type doesn't
// match the actual error. This issue is being looked into on VAPI level.
// For now, we check both conversion error and actual contents of converted struct
if err != nil || isEmptyAPIError(data.(model.ApiError)) {
if err != nil {
// This is likely not an error coming from NSX
return logRawVapiErrorData(message, vapiType, apiErrorDataValue)
}

apiError := data.(model.ApiError)
apiError, ok := data.(model.ApiError)
if !ok {
// This is likely not an error coming from NSX
return logRawVapiErrorData(message, vapiType, apiErrorDataValue)
}

details := fmt.Sprintf(" %s: %s", message, printAPIError(apiError))

Expand Down Expand Up @@ -138,8 +139,8 @@ func isNotFoundError(err error) bool {
return false
}

func handleCreateError(resourceType string, resourceID string, err error) error {
msg := fmt.Sprintf("Failed to create %s %s", resourceType, resourceID)
func handleCreateError(resourceType string, resource string, err error) error {
msg := fmt.Sprintf("Failed to create %s %s", resourceType, resource)
return logAPIError(msg, err)
}

Expand All @@ -153,8 +154,8 @@ func handleListError(resourceType string, err error) error {
return logAPIError(msg, err)
}

func handleReadError(d *schema.ResourceData, resourceType string, resourceID string, err error) error {
msg := fmt.Sprintf("Failed to read %s %s", resourceType, resourceID)
func handleReadError(d *schema.ResourceData, resourceType string, resource string, err error) error {
msg := fmt.Sprintf("Failed to read %s %s", resourceType, resource)
if isNotFoundError(err) {
d.SetId("")
log.Print(msg)
Expand Down
21 changes: 7 additions & 14 deletions nsxt/resource_nsxt_compute_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,7 @@ func resourceNsxtComputeManagerCreate(d *schema.ResourceData, m interface{}) err
setAsOidcProvider := d.Get("set_as_oidc_provider").(bool)
credential, err := getCredentialValues(d)
if err != nil {
// id isn't known yet
handleCreateError("ComputeManager", "", err)
return handleCreateError("ComputeManager", displayName, err)
}

obj := model.ComputeManager{
Expand All @@ -259,17 +258,12 @@ func resourceNsxtComputeManagerCreate(d *schema.ResourceData, m interface{}) err
SetAsOidcProvider: &setAsOidcProvider,
}

log.Printf("[INFO] Creating Compute Manager %s", displayName)
obj, err = client.Create(obj)
if err != nil {
id := ""
if obj.Id != nil {
id = *obj.Id
}
return handleCreateError("Compute Manager", id, err)
return handleCreateError("Compute Manager", displayName, err)
}

log.Printf("[INFO] Creating Compute Manager with ID %s", *obj.Id)

d.SetId(*obj.Id)
return resourceNsxtComputeManagerRead(d, m)
}
Expand Down Expand Up @@ -381,7 +375,7 @@ func resourceNsxtComputeManagerRead(d *schema.ResourceData, m interface{}) error

obj, err := client.Get(id)
if err != nil {
return fmt.Errorf("error during Compute Manager read: %v", err)
return handleReadError(d, "ComputeManager", id, err)
}

d.Set("revision", obj.Revision)
Expand Down Expand Up @@ -488,8 +482,7 @@ func resourceNsxtComputeManagerUpdate(d *schema.ResourceData, m interface{}) err
setAsOidcProvider := d.Get("set_as_oidc_provider").(bool)
credential, err := getCredentialValues(d)
if err != nil {
// id isn't known yet
handleCreateError("ComputeManager", "", err)
return handleUpdateError("ComputeManager", id, err)
}

obj := model.ComputeManager{
Expand All @@ -509,7 +502,7 @@ func resourceNsxtComputeManagerUpdate(d *schema.ResourceData, m interface{}) err

_, err = client.Update(id, obj)
if err != nil {
return fmt.Errorf("error during Compute Manager %s update: %v", id, err)
return handleUpdateError("ComputeManager", id, err)
}

return resourceNsxtComputeManagerRead(d, m)
Expand All @@ -527,7 +520,7 @@ func resourceNsxtComputeManagerDelete(d *schema.ResourceData, m interface{}) err

err := client.Delete(id)
if err != nil {
return fmt.Errorf("error during Compute Manager delete: %v", err)
return handleDeleteError("ComputeManager", id, err)
}
return nil
}
15 changes: 5 additions & 10 deletions nsxt/resource_nsxt_edge_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,12 @@ func resourceNsxtEdgeClusterCreate(d *schema.ResourceData, m interface{}) error
Members: members,
}

log.Printf("[INFO] Creating Edge Cluster with name %s", displayName)
obj, err := client.Create(obj)
if err != nil {
id := ""
if obj.Id != nil {
id = *obj.Id
}
return handleCreateError("Edge Cluster", id, err)
return handleCreateError("Edge Cluster", displayName, err)
}

log.Printf("[INFO] Creating Edge Cluster with ID %s", *obj.Id)

d.SetId(*obj.Id)
return resourceNsxtEdgeClusterRead(d, m)
}
Expand Down Expand Up @@ -175,7 +170,7 @@ func resourceNsxtEdgeClusterRead(d *schema.ResourceData, m interface{}) error {
client := nsx.NewEdgeClustersClient(connector)
obj, err := client.Get(id)
if err != nil {
return fmt.Errorf("error during Edge Cluster read: %v", err)
return handleReadError(d, "EdgeCluster", id, err)
}

d.Set("revision", obj.Revision)
Expand Down Expand Up @@ -253,7 +248,7 @@ func resourceNsxtEdgeClusterUpdate(d *schema.ResourceData, m interface{}) error

_, err := client.Update(id, obj)
if err != nil {
return fmt.Errorf("error during Edge Cluster %s update: %v", id, err)
return handleUpdateError("EdgeCluster", id, err)
}

return resourceNsxtEdgeClusterRead(d, m)
Expand All @@ -271,7 +266,7 @@ func resourceNsxtEdgeClusterDelete(d *schema.ResourceData, m interface{}) error

err := client.Delete(id)
if err != nil {
return fmt.Errorf("error during Edge Cluster delete: %v", err)
return handleDeleteError("EdgeCluster", id, err)
}
return nil
}
15 changes: 6 additions & 9 deletions nsxt/resource_nsxt_failure_domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func resourceNsxtFailureDomainRead(d *schema.ResourceData, m interface{}) error
client := nsx.NewFailureDomainsClient(connector)
obj, err := client.Get(id)
if err != nil {
return fmt.Errorf("error during FailureDomain read: %v", err)
return handleReadError(d, "FailureDomain", id, err)
}

d.Set("display_name", obj.DisplayName)
Expand All @@ -67,7 +67,7 @@ func resourceNsxtFailureDomainRead(d *schema.ResourceData, m interface{}) error
preferPtr := obj.PreferredActiveEdgeServices
preferStr := "no_preference"
if preferPtr != nil {
if *preferPtr == true {
if *preferPtr {
preferStr = "active"
} else {
preferStr = "standby"
Expand Down Expand Up @@ -106,15 +106,12 @@ func resourceNsxtFailureDomainCreate(d *schema.ResourceData, m interface{}) erro
client := nsx.NewFailureDomainsClient(connector)

failureDomain := failureDomainSchemaToModel(d)
displayName := d.Get("display_name").(string)
log.Printf("[INFO] Creating Failure Domain %s", displayName)
obj, err := client.Create(failureDomain)
if err != nil {
id := ""
if obj.Id != nil {
id = *obj.Id
}
return handleCreateError("Failure Domain", id, err)
return handleCreateError("Failure Domain", displayName, err)
}
log.Printf("[INFO] FailureDomain with ID %s created", *obj.Id)
d.SetId(*obj.Id)

return resourceNsxtFailureDomainRead(d, m)
Expand All @@ -135,7 +132,7 @@ func resourceNsxtFailureDomainUpdate(d *schema.ResourceData, m interface{}) erro

_, err := client.Update(id, failureDomain)
if err != nil {
return handleCreateError("FailureDomain", id, err)
return handleUpdateError("FailureDomain", id, err)
}

return resourceNsxtFailureDomainRead(d, m)
Expand Down
18 changes: 10 additions & 8 deletions nsxt/resource_nsxt_manager_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,13 @@ func resourceNsxtManagerClusterCreate(d *schema.ResourceData, m interface{}) err
}
clusterID, certSha256Thumbprint, hostIP, err := getClusterInfoFromHostNode(d, m)
if err != nil {
return err
return handleCreateError("ManagerCluster", "", err)
}

for _, guestNode := range nodes {
err := joinNodeToCluster(clusterID, certSha256Thumbprint, guestNode, hostIP, d, m)
if err != nil {
return fmt.Errorf("Failed to join node %s: %s", guestNode.ID, err)
return handleCreateError("ManagerCluster", hostIP, fmt.Errorf("failed to join node %s: %s", guestNode.ID, err))
}
}
d.SetId(clusterID)
Expand Down Expand Up @@ -315,18 +315,19 @@ func getHostCredential(m interface{}) (string, string) {
}

func resourceNsxtManagerClusterRead(d *schema.ResourceData, m interface{}) error {
id := d.Id()
connector := getPolicyConnector(m)
client := nsx.NewClusterClient(connector)
clusterConfig, err := client.Get()
if err != nil {
return fmt.Errorf("Failed to read Nsxt Manager Cluster: %s", err)
return handleReadError(d, "ManagerCluster", id, err)
}
nodeInfo := clusterConfig.Nodes
var nodes []NsxClusterNode
hostIP, err := getHostIPFromClusterConfig(m, clusterConfig)
isIPv4 := (net.ParseIP(hostIP)).To4() != nil
if err != nil {
return err
return handleReadError(d, "ManagerCluster", id, err)
}
for _, node := range nodeInfo {
ip := getIPFromNodeInfo(node, isIPv4)
Expand Down Expand Up @@ -368,12 +369,13 @@ func getIPFromNodeInfo(node nsxModel.ClusterNodeInfo, isIPv4 bool) string {
}

func resourceNsxtManagerClusterUpdate(d *schema.ResourceData, m interface{}) error {
id := d.Id()
connector := getPolicyConnector(m)
client := nsx.NewClusterClient(connector)

clusterID, certSha256Thumbprint, hostIP, err := getClusterInfoFromHostNode(d, m)
if err != nil {
return err
return handleUpdateError("ManagerCluster", id, err)
}
oldNodes, newNodes := d.GetChange("node")
oldNodesIPs := getClusterNodesIPs(oldNodes)
Expand All @@ -388,7 +390,7 @@ func resourceNsxtManagerClusterUpdate(d *schema.ResourceData, m interface{}) err
ignoreRepositoryIPCheckParam := "false"
_, err := client.Removenode(id, &force, &gracefulShutdown, &ignoreRepositoryIPCheckParam)
if err != nil {
return err
return handleUpdateError("ManagerCluster", id, err)
}
}
}
Expand All @@ -406,7 +408,7 @@ func resourceNsxtManagerClusterUpdate(d *schema.ResourceData, m interface{}) err
}
err = joinNodeToCluster(clusterID, certSha256Thumbprint, nodeObj, hostIP, d, m)
if err != nil {
return err
return handleUpdateError("ManagerCluster", id, err)
}
}
}
Expand Down Expand Up @@ -434,7 +436,7 @@ func resourceNsxtManagerClusterDelete(d *schema.ResourceData, m interface{}) err
guestNodeID := node.ID
_, err := client.Removenode(guestNodeID, &force, &gracefulShutdown, &ignoreRepositoryIPCheckParam)
if err != nil {
return fmt.Errorf("Failed to remove node %s from cluster: %s", guestNodeID, err)
return handleDeleteError("ManagerCluster", guestNodeID, err)
}
}
return nil
Expand Down
30 changes: 11 additions & 19 deletions nsxt/resource_nsxt_transport_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,13 +909,14 @@ func resourceNsxtTransportNodeCreate(d *schema.ResourceData, m interface{}) erro
if err != nil {
return err
}

log.Printf("[INFO] Creating Transport Node with name %s", *obj.DisplayName)

obj1, err := client.Create(*obj)
if err != nil {
return fmt.Errorf("failed to create Transport Node: %v", err)
return handleCreateError("TransportNode", *obj.DisplayName, err)
}

log.Printf("[INFO] Creating Transport Node with ID %s", *obj1.Id)

d.SetId(*obj1.Id)
return resourceNsxtTransportNodeRead(d, m)
}
Expand Down Expand Up @@ -1481,15 +1482,6 @@ func getTransportZoneEndpointsFromSchema(endpointList []interface{}) []model.Tra
return tzEPList
}

func getHostSwitchData(data map[string]interface{}) (string, map[string]interface{}) {
for _, hswType := range []string{"standard_host_switch", "preconfigured_host_switch"} {
if data[hswType] != nil && len(data[hswType].([]interface{})) > 0 {
return hswType, data[hswType].([]interface{})[0].(map[string]interface{})
}
}
return "", nil
}

func getUplinksFromSchema(uplinksList []interface{}) []model.VdsUplink {
var uplinks []model.VdsUplink
for _, ul := range uplinksList {
Expand Down Expand Up @@ -1550,7 +1542,7 @@ func resourceNsxtTransportNodeRead(d *schema.ResourceData, m interface{}) error
client := nsx.NewTransportNodesClient(connector)
obj, err := client.Get(id)
if err != nil {
return fmt.Errorf("error during Transport Node read: %v", err)
return handleReadError(d, "TransportNode", id, err)
}

d.Set("revision", obj.Revision)
Expand All @@ -1560,19 +1552,19 @@ func resourceNsxtTransportNodeRead(d *schema.ResourceData, m interface{}) error
d.Set("failure_domain", obj.FailureDomainId)
err = setHostSwitchSpecInSchema(d, obj.HostSwitchSpec)
if err != nil {
return err
return handleReadError(d, "TransportNode", id, err)
}
err = setNodeDeploymentInfoInSchema(d, obj.NodeDeploymentInfo)
if err != nil {
return err
return handleReadError(d, "TransportNode", id, err)
}

if obj.RemoteTunnelEndpoint != nil {
rtep := make(map[string]interface{})
rtep["host_switch_name"] = obj.RemoteTunnelEndpoint.HostSwitchName
rtep["ip_assignment"], err = setIPAssignmentInSchema(obj.RemoteTunnelEndpoint.IpAssignmentSpec)
if err != nil {
return err
return handleReadError(d, "TransportNode", id, err)
}
rtep["named_teaming_policy"] = obj.RemoteTunnelEndpoint.NamedTeamingPolicy
rtep["rtep_vlan"] = obj.RemoteTunnelEndpoint.RtepVlan
Expand Down Expand Up @@ -1959,14 +1951,14 @@ func resourceNsxtTransportNodeUpdate(d *schema.ResourceData, m interface{}) erro

obj, err := getTransportNodeFromSchema(d)
if err != nil {
return err
return handleUpdateError("TransportNode", id, err)
}
revision := int64(d.Get("revision").(int))
*obj.Revision = revision

_, err = client.Update(id, *obj, nil, nil, nil, nil, nil, nil, nil)
if err != nil {
return fmt.Errorf("error during Tranport Node %s update: %v", id, err)
return handleUpdateError("TransportNode", id, err)
}

return resourceNsxtTransportNodeRead(d, m)
Expand All @@ -1984,7 +1976,7 @@ func resourceNsxtTransportNodeDelete(d *schema.ResourceData, m interface{}) erro

err := client.Delete(id, nil, nil)
if err != nil {
return fmt.Errorf("error during Transport Node delete: %v", err)
return handleDeleteError("TransportNode", id, err)
}
return nil
}

0 comments on commit b121470

Please sign in to comment.