From b1214702e87f544ea4ad06a5f2c1c387ef86c75f Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Tue, 19 Sep 2023 01:19:40 +0000 Subject: [PATCH] Fix error handling for MP SDK resources 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 --- nsxt/policy_errors.go | 21 ++++++++++--------- nsxt/resource_nsxt_compute_manager.go | 21 +++++++------------ nsxt/resource_nsxt_edge_cluster.go | 15 +++++--------- nsxt/resource_nsxt_failure_domain.go | 15 ++++++-------- nsxt/resource_nsxt_manager_cluster.go | 18 +++++++++------- nsxt/resource_nsxt_transport_node.go | 30 ++++++++++----------------- 6 files changed, 50 insertions(+), 70 deletions(-) diff --git a/nsxt/policy_errors.go b/nsxt/policy_errors.go index 9186f9ca8..6d310b20c 100644 --- a/nsxt/policy_errors.go +++ b/nsxt/policy_errors.go @@ -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) @@ -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)) @@ -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) } @@ -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) diff --git a/nsxt/resource_nsxt_compute_manager.go b/nsxt/resource_nsxt_compute_manager.go index 477160014..d5fdebc83 100644 --- a/nsxt/resource_nsxt_compute_manager.go +++ b/nsxt/resource_nsxt_compute_manager.go @@ -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{ @@ -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) } @@ -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) @@ -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{ @@ -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) @@ -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 } diff --git a/nsxt/resource_nsxt_edge_cluster.go b/nsxt/resource_nsxt_edge_cluster.go index 6fc9299d0..369a9ce6a 100644 --- a/nsxt/resource_nsxt_edge_cluster.go +++ b/nsxt/resource_nsxt_edge_cluster.go @@ -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) } @@ -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) @@ -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) @@ -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 } diff --git a/nsxt/resource_nsxt_failure_domain.go b/nsxt/resource_nsxt_failure_domain.go index 1c6380817..fcadae5da 100644 --- a/nsxt/resource_nsxt_failure_domain.go +++ b/nsxt/resource_nsxt_failure_domain.go @@ -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) @@ -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" @@ -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) @@ -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) diff --git a/nsxt/resource_nsxt_manager_cluster.go b/nsxt/resource_nsxt_manager_cluster.go index 25e83c31f..ecece9c39 100644 --- a/nsxt/resource_nsxt_manager_cluster.go +++ b/nsxt/resource_nsxt_manager_cluster.go @@ -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) @@ -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) @@ -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) @@ -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) } } } @@ -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) } } } @@ -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 diff --git a/nsxt/resource_nsxt_transport_node.go b/nsxt/resource_nsxt_transport_node.go index f38b7a6c7..516619b31 100644 --- a/nsxt/resource_nsxt_transport_node.go +++ b/nsxt/resource_nsxt_transport_node.go @@ -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) } @@ -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 { @@ -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) @@ -1560,11 +1552,11 @@ 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 { @@ -1572,7 +1564,7 @@ func resourceNsxtTransportNodeRead(d *schema.ResourceData, m interface{}) error 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 @@ -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) @@ -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 }