Merge pull request #287 from detiber/backportMultipleReservations

✨  Add support for comma separated reservationIDs

Author: k8s-ci-robot

.github/workflows/pr-post.yml

@@ -217,7 +217,7 @@ cd: confirm branchname
 # needed kubebuilder for tests
 kubebuilder: $(KUBEBUILDER)
 $(KUBEBUILDER):
-	curl -sL https://go.kubebuilder.io/dl/$(KUBEBUILDER_VERSION)/$(BUILDOS)/$(BUILDARCH) | tar -xz -C /tmp/
+	curl -sL https://github.com/kubernetes-sigs/kubebuilder/releases/download/v$(KUBEBUILDER_VERSION)/kubebuilder_$(KUBEBUILDER_VERSION)_$(BUILDOS)_$(BUILDARCH).tar.gz | tar -xz -C /tmp/
 	# move to a long-term location and put it on your path
 	# (you'll need to set the KUBEBUILDER_ASSETS env var if you put it somewhere else)
 	mv /tmp/kubebuilder_$(KUBEBUILDER_VERSION)_$(BUILDOS)_$(BUILDARCH) $(KUBEBUILDER_DIR)

Makefile

@@ -1 +1 @@
-v0.3.9
+v0.3.10

VERSION

@@ -48,7 +48,8 @@ type PacketMachineSpec struct {
 	// +optional
 	IPXEUrl string `json:"ipxeURL,omitempty"`
 
-	// HardwareReservationID is the unique device hardware reservation ID or `next-available` to
+	// HardwareReservationID is the unique device hardware reservation ID, a comma separated list of
+	// hardware reservation IDs, or `next-available` to
 	// automatically let the Packet api determine one.
 	// +optional
 	HardwareReservationID string `json:"hardwareReservationID,omitempty"`

api/v1alpha3/packetmachine_types.go

@@ -63,7 +63,7 @@ spec:
                 description: Facility represents the Packet facility for this cluster. Override from the PacketCluster spec.
                 type: string
               hardwareReservationID:
-                description: HardwareReservationID is the unique device hardware reservation ID or `next-available` to automatically let the Packet api determine one.
+                description: HardwareReservationID is the unique device hardware reservation ID, a comma separated list of hardware reservation IDs, or `next-available` to automatically let the Packet api determine one.
                 type: string
               ipxeURL:
                 description: IPXEUrl can be used to set the pxe boot url when using custom OSes with this provider. Note that OS should also be set to "custom_ipxe" if using this value.

config/crd/bases/infrastructure.cluster.x-k8s.io_packetmachines.yaml

@@ -48,7 +48,7 @@ spec:
                         description: Facility represents the Packet facility for this cluster. Override from the PacketCluster spec.
                         type: string
                       hardwareReservationID:
-                        description: HardwareReservationID is the unique device hardware reservation ID or `next-available` to automatically let the Packet api determine one.
+                        description: HardwareReservationID is the unique device hardware reservation ID, a comma separated list of hardware reservation IDs, or `next-available` to automatically let the Packet api determine one.
                         type: string
                       ipxeURL:
                         description: IPXEUrl can be used to set the pxe boot url when using custom OSes with this provider. Note that OS should also be set to "custom_ipxe" if using this value.

config/crd/bases/infrastructure.cluster.x-k8s.io_packetmachinetemplates.yaml

@@ -13,5 +13,5 @@ bases:
 - default
 images:
 - name: docker.io/packethost/cluster-api-provider-packet:e2e # images with this name
-  newTag: v0.3.9 # {"type":"string","x-kustomize":{"setter":{"name":"image-tag","value":"v0.3.9"}}}
+  newTag: v0.3.10 # {"type":"string","x-kustomize":{"setter":{"name":"image-tag","value":"v0.3.10"}}}
   newName: docker.io/packethost/cluster-api-provider-packet # and this name

config/kustomization.yaml

@@ -245,6 +245,10 @@ func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *s
 		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
 			return ctrl.Result{}, fmt.Errorf("failed to create machine %s: %w", machineScope.Name(), err)
+		case err != nil && strings.Contains(err.Error(), "Server is not provisionable"):
+			// Do not treat an error indicating that reserved hardware is not provisionable as fatal
+			// This occurs when reserved hardware is in the process of being deprovisioned
+			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)
 			machineScope.SetErrorReason(capierrors.CreateMachineError)

controllers/packetmachine_controller.go

@@ -38,7 +38,10 @@ const (
 	ipxeOS          = "custom_ipxe"
 )
 
-var ErrControlPlanEndpointNotFound = errors.New("control plane not found")
+var (
+	ErrControlPlanEndpointNotFound = errors.New("control plane not found")
+	ErrInvalidRequest              = errors.New("invalid request")
+)
 
 type PacketClient struct {
 	*packngo.Client
@@ -75,6 +78,13 @@ type CreateDeviceRequest struct {
 }
 
 func (p *PacketClient) NewDevice(req CreateDeviceRequest) (*packngo.Device, error) {
+	if req.MachineScope.PacketMachine.Spec.IPXEUrl != "" {
+		// Error if pxe url and OS conflict
+		if req.MachineScope.PacketMachine.Spec.OS != ipxeOS {
+			return nil, fmt.Errorf("os should be set to custom_pxe when using pxe urls: %w", ErrInvalidRequest)
+		}
+	}
+
 	userDataRaw, err := req.MachineScope.GetRawBootstrapData()
 	if err != nil {
 		return nil, errors.Wrap(err, "impossible to retrieve bootstrap data from secret")
@@ -119,28 +129,42 @@ func (p *PacketClient) NewDevice(req CreateDeviceRequest) (*packngo.Device, erro
 	}
 
 	serverCreateOpts := &packngo.DeviceCreateRequest{
-		Hostname:              req.MachineScope.Name(),
-		ProjectID:             req.MachineScope.PacketCluster.Spec.ProjectID,
-		Facility:              []string{facility},
-		BillingCycle:          req.MachineScope.PacketMachine.Spec.BillingCycle,
-		HardwareReservationID: req.MachineScope.PacketMachine.Spec.HardwareReservationID,
-		Plan:                  req.MachineScope.PacketMachine.Spec.MachineType,
-		OS:                    req.MachineScope.PacketMachine.Spec.OS,
-		Tags:                  tags,
-		UserData:              userData,
-	}
-
-	// Update server options to pass pxe url if specified
-	if req.MachineScope.PacketMachine.Spec.IPXEUrl != "" {
-		// Error if pxe url and OS conflict
-		if req.MachineScope.PacketMachine.Spec.OS != ipxeOS {
-			return nil, fmt.Errorf("os should be set to custom_pxe when using pxe urls")
+		Hostname:      req.MachineScope.Name(),
+		ProjectID:     req.MachineScope.PacketCluster.Spec.ProjectID,
+		Facility:      []string{facility},
+		BillingCycle:  req.MachineScope.PacketMachine.Spec.BillingCycle,
+		Plan:          req.MachineScope.PacketMachine.Spec.MachineType,
+		OS:            req.MachineScope.PacketMachine.Spec.OS,
+		IPXEScriptURL: req.MachineScope.PacketMachine.Spec.IPXEUrl,
+		Tags:          tags,
+		UserData:      userData,
+	}
+
+	reservationIDs := strings.Split(req.MachineScope.PacketMachine.Spec.HardwareReservationID, ",")
+
+	// If there are no reservationIDs to process, go ahead and return early
+	if len(reservationIDs) == 0 {
+		dev, _, err := p.Client.Devices.Create(serverCreateOpts)
+		return dev, err
+	}
+
+	// Do a naive loop through the list of reservationIDs, continuing if we hit any error
+	// TODO: if we can determine how to differentiate a failure based on the reservation
+	// being in use vs other errors, then we can make this a bit smarter in the future.
+	var lastErr error
+
+	for _, resID := range reservationIDs {
+		serverCreateOpts.HardwareReservationID = resID
+		dev, _, err := p.Client.Devices.Create(serverCreateOpts)
+		if err != nil {
+			lastErr = err
+			continue
 		}
-		serverCreateOpts.IPXEScriptURL = req.MachineScope.PacketMachine.Spec.IPXEUrl
+
+		return dev, nil
 	}
 
-	dev, _, err := p.Client.Devices.Create(serverCreateOpts)
-	return dev, err
+	return nil, lastErr
 }
 
 func (p *PacketClient) GetDeviceAddresses(device *packngo.Device) ([]corev1.NodeAddress, error) {