Use get instead of list to retrieve servers

Under certain circumstances, when they are non-responsive cells, nova
may return a partial result instead of an error.

(this is controlled by list_records_by_skipping_down_cells parameter
that is true by default - no error will be thrown)

When it faces this issue, capo controller will try to create a new
server for the machine, resulting in deleting ports of existing one.

In order ovoid this issue, use GET instead of LIST with server uuid stored
in machine spec when it is created, and store server ID in machine spec
immediately

Author: zioc

controllers/openstackmachine_controller.go

@@ -246,7 +246,10 @@ func (r *OpenStackMachineReconciler) reconcileDelete(scope scope.Scope, cluster
 		}
 	}
 
-	instanceStatus, err := computeService.GetInstanceStatusByName(openStackMachine, openStackMachine.Name)
+	var instanceStatus *compute.InstanceStatus
+	if openStackMachine.Spec.InstanceID != nil {
+		instanceStatus, err = computeService.GetInstanceStatusByName(openStackMachine, openStackMachine.Name)
+	}
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -431,18 +434,27 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 }
 
 func (r *OpenStackMachineReconciler) getOrCreate(logger logr.Logger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, computeService *compute.Service, userData string) (*compute.InstanceStatus, error) {
-	instanceStatus, err := computeService.GetInstanceStatusByName(openStackMachine, openStackMachine.Name)
-	if err != nil {
-		logger.Info("Unable to get OpenStack instance", "name", openStackMachine.Name)
-		conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.OpenStackErrorReason, clusterv1.ConditionSeverityError, err.Error())
-		return nil, err
+
+	var instanceStatus *compute.InstanceStatus
+	var err error
+	if openStackMachine.Spec.InstanceID != nil {
+		instanceStatus, err = computeService.GetInstanceStatus(*openStackMachine.Spec.InstanceID)
+		if err != nil {
+			logger.Info("Unable to get OpenStack instance", "name", openStackMachine.Name)
+			conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.OpenStackErrorReason, clusterv1.ConditionSeverityError, err.Error())
+			return nil, err
+		}
 	}
 
 	if instanceStatus == nil {
 		instanceSpec := machineToInstanceSpec(openStackCluster, machine, openStackMachine, userData)
 		logger.Info("Machine does not exist, creating Machine", "name", openStackMachine.Name)
 		instanceStatus, err = computeService.CreateInstance(openStackMachine, openStackCluster, instanceSpec, cluster.Name)
 		if err != nil {
+			if instanceStatus != nil {
+				// Instance was created but in error, store instanceID for subsequent calls to this function
+				openStackMachine.Spec.InstanceID = pointer.String(instanceStatus.ID())
+			}
 			conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceCreateFailedReason, clusterv1.ConditionSeverityError, err.Error())
 			return nil, fmt.Errorf("create OpenStack instance: %w", err)
 		}

controllers/suite_test.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
+	"k8s.io/utils/pointer"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/test/framework"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -148,9 +149,13 @@ var _ = Describe("When calling getOrCreate", func() {
 		cluster := &clusterv1.Cluster{}
 		openStackCluster := &infrav1.OpenStackCluster{}
 		machine := &clusterv1.Machine{}
-		openStackMachine := &infrav1.OpenStackMachine{}
+		openStackMachine := &infrav1.OpenStackMachine{
+			Spec: infrav1.OpenStackMachineSpec{
+				InstanceID: pointer.String("machine-uuid"),
+			},
+		}
 
-		mockScopeFactory.ComputeClient.EXPECT().ListServers(gomock.Any()).Return(nil, errors.New("Test error when listing servers"))
+		mockScopeFactory.ComputeClient.EXPECT().GetServer(gomock.Any()).Return(nil, errors.New("Test error when getting server"))
 		instanceStatus, err := reconsiler.getOrCreate(logger, cluster, openStackCluster, machine, openStackMachine, computeService, "")
 		Expect(err).To(HaveOccurred())
 		Expect(instanceStatus).To(BeNil())

pkg/cloud/services/compute/instance.go

@@ -349,7 +349,7 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste
 	})
 	if err != nil {
 		record.Warnf(eventObject, "FailedCreateServer", "Failed to create server %s: %v", createdInstance.Name(), err)
-		return nil, err
+		return createdInstance, err
 	}
 
 	record.Eventf(eventObject, "SuccessfulCreateServer", "Created server %s with id %s", createdInstance.Name(), createdInstance.ID())