Make PacketMachine controller re-entrant when crashes happen during creation process

Author: detiber

controllers/packetmachine_controller.go

@@ -23,16 +23,16 @@ import (
 	"strings"
 	"time"
 
-	corev1 "k8s.io/api/core/v1"
-
 	"github.com/go-logr/logr"
-	"github.com/google/uuid"
 	"github.com/packethost/packngo"
 	"github.com/pkg/errors"
-
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
+	infrastructurev1alpha3 "sigs.k8s.io/cluster-api-provider-packet/api/v1alpha3"
+	packet "sigs.k8s.io/cluster-api-provider-packet/pkg/cloud/packet"
+	"sigs.k8s.io/cluster-api-provider-packet/pkg/cloud/packet/scope"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	capierrors "sigs.k8s.io/cluster-api/errors"
 	"sigs.k8s.io/cluster-api/util"
@@ -42,11 +42,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
-
-	packet "sigs.k8s.io/cluster-api-provider-packet/pkg/cloud/packet"
-	"sigs.k8s.io/cluster-api-provider-packet/pkg/cloud/packet/scope"
-
-	infrastructurev1alpha3 "sigs.k8s.io/cluster-api-provider-packet/api/v1alpha3"
 )
 
 const (
@@ -173,6 +168,7 @@ func (r *PacketMachineReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *scope.MachineScope, clusterScope *scope.ClusterScope, logger logr.Logger) (ctrl.Result, error) {
 	logger.Info("Reconciling PacketMachine")
+
 	packetmachine := machineScope.PacketMachine
 	// If the PacketMachine is in an error state, return early.
 	if packetmachine.Status.ErrorReason != nil || packetmachine.Status.ErrorMessage != nil {
@@ -195,28 +191,50 @@ func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *s
 	}
 
 	providerID := machineScope.GetInstanceID()
+
+	defaultTags := []string{
+		packet.GenerateClusterTag(clusterScope.Name()),
+		packet.GenerateMachineNameTag(machineScope.Name()),
+		packet.GenerateNamespaceTag(machineScope.Namespace()),
+	}
+
 	var (
 		dev                  *packngo.Device
 		addrs                []corev1.NodeAddress
 		err                  error
 		controlPlaneEndpoint packngo.IPAddressReservation
 	)
-	// if we have no provider ID, then we are creating
+
 	if providerID != "" {
+		// If we already have a providerID, then retrieve the device using the
+		// providerID. This means that the Machine has already been created
+		// and bootstrapped as a Node into the cluster.
 		dev, err = r.PacketClient.GetDevice(providerID)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
 	}
+
 	if dev == nil {
+		// We don't yet have a providerID, check to see if we've already
+		// created a device by using the tags that we assign to devices
+		// on creation.
+		dev, err = r.PacketClient.GetDeviceByTags(
+			machineScope.PacketCluster.Spec.ProjectID,
+			defaultTags,
+		)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
+	if dev == nil {
+		// We weren't able to find a device by either providerID or by tags,
+		// so we need to create a new device.
 		createDeviceReq := packet.CreateDeviceRequest{
+			ExtraTags:    defaultTags,
 			MachineScope: machineScope,
 		}
-		mUID := uuid.New().String()
-		tags := []string{
-			packet.GenerateMachineTag(mUID),
-			packet.GenerateClusterTag(clusterScope.Name()),
-		}
 
 		// when the node is a control plan we should check if the elastic ip
 		// for this cluster is not assigned. If it is free we can prepare the
@@ -236,14 +254,14 @@ func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *s
 			createDeviceReq.ControlPlaneEndpoint = controlPlaneEndpoint.Address
 		}
 
-		createDeviceReq.ExtraTags = tags
-
 		dev, err = r.PacketClient.NewDevice(createDeviceReq)
 
 		switch {
 		// TODO: find a better way than parsing the error messages for this.
 		case err != nil && strings.Contains(err.Error(), " no available hardware reservations "):
-			// Do not treat an error indicating there are no hardware reservations available as fatal
+			// Do not treat an error indicating there are no hardware
+			// reservations available as fatal, we should continue to retry
+			// device creation until a reservation is available
 			return ctrl.Result{}, fmt.Errorf("failed to create machine %s: %w", machineScope.Name(), err)
 		case err != nil:
 			errs := fmt.Errorf("failed to create machine %s: %w", machineScope.Name(), err)

pkg/cloud/packet/util.go

@@ -21,16 +21,26 @@ import (
 )
 
 const (
-	MachineUIDTag = "cluster-api-provider-packet:machine-uid"
-	clusterIDTag  = "cluster-api-provider-packet:cluster-id"
-	AnnotationUID = "cluster.k8s.io/machine-uid"
+	MachineUIDTag  = "cluster-api-provider-packet:machine-uid"
+	machineNameTag = "cluster-api-provider-packet:machine-name"
+	clusterIDTag   = "cluster-api-provider-packet:cluster-id"
+	namespaceTag   = "cluster-api-provider-packet:namespace"
 )
 
-func GenerateMachineTag(ID string) string {
-	return fmt.Sprintf("%s:%s", MachineUIDTag, ID)
+func GenerateMachineTag(id string) string {
+	return fmt.Sprintf("%s:%s", MachineUIDTag, id)
 }
-func GenerateClusterTag(ID string) string {
-	return fmt.Sprintf("%s:%s", clusterIDTag, ID)
+
+func GenerateMachineNameTag(name string) string {
+	return fmt.Sprintf("%s:%s", machineNameTag, name)
+}
+
+func GenerateClusterTag(clusterName string) string {
+	return fmt.Sprintf("%s:%s", clusterIDTag, clusterName)
+}
+
+func GenerateNamespaceTag(namespace string) string {
+	return fmt.Sprintf("%s:%s", namespaceTag, namespace)
 }
 
 // ItemsInList checks if all items are in the list