Look up instance by providerID and node name consistently.

Currently, some instance operations look up instances by providerID and then
fall back to node name, and other operations just use providerID. This causes
problems using the cluster api, since nodes aren't provisioned with a
providerID initially. This patch looks up instances by both providerID and node
name consistently, and lightly refactors lookups to avoid repeat queries.

Author: jmcarp

internal/provider/instances_v2.go

@@ -6,8 +6,8 @@ package provider
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/oxidecomputer/oxide.go/oxide"
@@ -36,6 +36,8 @@ func (i *InstancesV2) InstanceExists(ctx context.Context, node *v1.Node) (bool,
 	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
 	defer cancel()
 
+	// Look up instance by the node's providerID. By this point, the providerID will have been set
+	// via InstanceMetadata.
 	instanceID, err := InstanceIDFromProviderID(node.Spec.ProviderID)
 	if err != nil {
 		return false, fmt.Errorf("failed retrieving instance id from provider id: %w", err)
@@ -44,7 +46,8 @@ func (i *InstancesV2) InstanceExists(ctx context.Context, node *v1.Node) (bool,
 	if _, err := i.client.InstanceView(ctx, oxide.InstanceViewParams{
 		Instance: oxide.NameOrId(instanceID),
 	}); err != nil {
-		if strings.Contains(err.Error(), "NotFound") {
+		var httpErr *oxide.HTTPError
+		if errors.As(err, &httpErr) && httpErr.ErrorResponse.ErrorCode == "ObjectNotFound" {
 			return false, nil
 		}
 
@@ -63,32 +66,23 @@ func (i *InstancesV2) InstanceMetadata(
 	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
 	defer cancel()
 
-	// Get the instance ID, either from the provider ID or by looking up by name.
-	instanceID, err := i.getInstanceID(ctx, node)
+	instance, err := i.getInstance(ctx, node)
 	if err != nil {
-		return nil, err
-	}
-
-	// Retrieve the instance details.
-	instance, err := i.client.InstanceView(ctx, oxide.InstanceViewParams{
-		Instance: oxide.NameOrId(instanceID),
-	})
-	if err != nil {
-		return nil, fmt.Errorf("failed viewing oxide instance: %v", err)
+		return nil, fmt.Errorf("failed retrieving instance: %w", err)
 	}
 
 	nics, err := i.client.InstanceNetworkInterfaceList(
 		ctx,
 		oxide.InstanceNetworkInterfaceListParams{
-			Instance: oxide.NameOrId(instanceID),
+			Instance: oxide.NameOrId(instance.Id),
 		},
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed listing instance network interfaces: %v", err)
 	}
 
 	externalIPs, err := i.client.InstanceExternalIpList(ctx, oxide.InstanceExternalIpListParams{
-		Instance: oxide.NameOrId(instanceID),
+		Instance: oxide.NameOrId(instance.Id),
 	})
 	if err != nil {
 		return nil, fmt.Errorf("failed listing instance external ips: %v", err)
@@ -148,36 +142,43 @@ func (i *InstancesV2) InstanceMetadata(
 	}
 
 	return &cloudprovider.InstanceMetadata{
-		ProviderID:    NewProviderID(instanceID),
+		ProviderID:    NewProviderID(instance.Id),
 		InstanceType:  fmt.Sprintf("%d-%d", instance.Ncpus, instance.Memory/gibibyte),
 		NodeAddresses: nodeAddresses,
 	}, nil
 }
 
-// getInstanceID retrieves the instance ID either from the node's provider ID
+// getInstance retrieves the instance, either from the node's provider ID
 // or by looking up the instance by name.
-func (i *InstancesV2) getInstanceID(ctx context.Context, node *v1.Node) (string, error) {
+func (i *InstancesV2) getInstance(ctx context.Context, node *v1.Node) (*oxide.Instance, error) {
+	var params oxide.InstanceViewParams
+
 	if node.Spec.ProviderID != "" {
-		return InstanceIDFromProviderID(node.Spec.ProviderID)
+		instanceID, err := InstanceIDFromProviderID(node.Spec.ProviderID)
+		if err != nil {
+			return nil, fmt.Errorf("failed parsing provider id: %w", err)
+		}
+		params.Instance = oxide.NameOrId(instanceID)
+	} else {
+		params.Project = oxide.NameOrId(i.project)
+		params.Instance = oxide.NameOrId(node.GetName())
 	}
 
-	// If no provider ID is set, look up the instance by name.
-	instance, err := i.client.InstanceView(ctx, oxide.InstanceViewParams{
-		Project:  oxide.NameOrId(i.project),
-		Instance: oxide.NameOrId(node.GetName()),
-	})
+	instance, err := i.client.InstanceView(ctx, params)
 	if err != nil {
-		return "", fmt.Errorf("failed viewing oxide instance by name: %v", err)
+		return nil, fmt.Errorf("failed looking up oxide instance: %w", err)
 	}
 
-	return instance.Id, nil
+	return instance, nil
 }
 
 // InstanceShutdown checks whether the provided node is shut down in Oxide.
 func (i *InstancesV2) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) {
 	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
 	defer cancel()
 
+	// Look up instance by the node's providerID. By this point, the providerID will have been set
+	// via InstanceMetadata.
 	instanceID, err := InstanceIDFromProviderID(node.Spec.ProviderID)
 	if err != nil {
 		return false, fmt.Errorf("failed retrieving instance id from provider id: %w", err)