WIP - move ports out of instance

Author: EmilienM

api/v1alpha5/conversion.go

@@ -437,3 +437,14 @@ func Convert_v1alpha5_OpenStackClusterStatus_To_v1alpha7_OpenStackClusterStatus(
 func Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in *infrav1.OpenStackMachineSpec, out *OpenStackMachineSpec, s conversion.Scope) error {
 	return autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in, out, s)
 }
+
+func Convert_v1alpha7_OpenStackMachineStatus_To_v1alpha5_OpenStackMachineStatus(in *infrav1.OpenStackMachineStatus, out *OpenStackMachineStatus, s conversion.Scope) error {
+	err := autoConvert_v1alpha7_OpenStackMachineStatus_To_v1alpha5_OpenStackMachineStatus(in, out, s)
+	if err != nil {
+		return err
+	}
+
+	in.Ports = nil
+
+	return nil
+}

api/v1alpha5/zz_generated.conversion.go

@@ -659,3 +659,14 @@ func Convert_v1alpha6_OpenStackClusterStatus_To_v1alpha7_OpenStackClusterStatus(
 func Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *infrav1.OpenStackMachineSpec, out *OpenStackMachineSpec, s apiconversion.Scope) error {
 	return autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in, out, s)
 }
+
+func Convert_v1alpha7_OpenStackMachineStatus_To_v1alpha6_OpenStackMachineStatus(in *infrav1.OpenStackMachineStatus, out *OpenStackMachineStatus, s apiconversion.Scope) error {
+	err := autoConvert_v1alpha7_OpenStackMachineStatus_To_v1alpha6_OpenStackMachineStatus(in, out, s)
+	if err != nil {
+		return err
+	}
+
+	in.Ports = nil
+
+	return nil
+}

api/v1alpha6/conversion.go

@@ -111,6 +111,9 @@ type OpenStackMachineStatus struct {
 	// +optional
 	InstanceState *InstanceState `json:"instanceState,omitempty"`
 
+	// Ports contains the list of OpenStack ports and their status associated with the machine.
+	Ports []PortStatus `json:"ports,omitempty"`
+
 	FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`
 
 	// FailureMessage will be set in the event that there is a terminal problem

api/v1alpha6/zz_generated.conversion.go

@@ -128,6 +128,11 @@ type PortOpts struct {
 	ValueSpecs []ValueSpec `json:"valueSpecs,omitempty"`
 }
 
+type PortStatus struct {
+	// ID is the unique identifier of the port.
+	ID string `json:"id,omitempty"`
+}
+
 type BindingProfile struct {
 	// OVSHWOffload enables or disables the OVS hardware offload feature.
 	OVSHWOffload bool `json:"ovsHWOffload,omitempty"`

api/v1alpha7/openstackmachine_types.go

@@ -1603,6 +1603,16 @@ spec:
                 description: InstanceState is the state of the OpenStack instance
                   for this machine.
                 type: string
+              ports:
+                description: Ports contains the list of OpenStack ports and their
+                  status associated with the machine.
+                items:
+                  properties:
+                    id:
+                      description: ID is the unique identifier of the port.
+                      type: string
+                  type: object
+                type: array
               ready:
                 description: Ready is true when the provider resource is ready.
                 type: boolean

api/v1alpha7/types.go

@@ -320,6 +320,11 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
 		return err
 	}
 
+	networkingService, err := networking.NewService(scope)
+	if err != nil {
+		return err
+	}
+
 	instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name)
 	bastionHash, err := compute.HashInstanceSpec(instanceSpec)
 	if err != nil {
@@ -349,15 +354,27 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
 		}
 	}
 
-	instanceStatus, err = computeService.CreateInstance(openStackCluster, openStackCluster, instanceSpec, cluster.Name)
+	// TODO
+	machine := &clusterv1.Machine{}
+	bastionMachineSpec := openStackCluster.Spec.Bastion.Instance
+	openStackMachine := &infrav1.OpenStackMachine{
+		Spec: bastionMachineSpec,
+	}
+
+	portStatus, err := networkingService.CreatePortsForInstance(machine, openStackMachine, openStackCluster, true)
 	if err != nil {
-		return fmt.Errorf("failed to reconcile bastion: %w", err)
+		return fmt.Errorf("failed to create ports for bastion: %w", err)
+	}
+	var portIDs []string
+	for _, port := range portStatus {
+		portIDs = append(portIDs, port.ID)
 	}
 
-	networkingService, err := networking.NewService(scope)
+	instanceStatus, err = computeService.CreateInstance(openStackCluster, openStackCluster, instanceSpec, cluster.Name, portIDs)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to reconcile bastion: %w", err)
 	}
+
 	clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)
 	fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, openStackCluster.Spec.Bastion.Instance.FloatingIP)
 	if err != nil {

api/v1alpha7/zz_generated.deepcopy.go

@@ -328,7 +328,11 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 		return ctrl.Result{}, err
 	}
 
-	instanceStatus, err := r.getOrCreate(scope.Logger(), cluster, openStackCluster, machine, openStackMachine, computeService, userData)
+	portStatus, err := r.getOrCreatePorts(scope.Logger(), cluster, openStackCluster, machine, openStackMachine, networkingService)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	instanceStatus, err := r.getOrCreateInstance(scope.Logger(), cluster, openStackCluster, machine, openStackMachine, computeService, networkingService, userData, portStatus)
 	if err != nil {
 		// Conditions set in getOrCreate
 		return ctrl.Result{}, err
@@ -430,7 +434,11 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 	return ctrl.Result{}, nil
 }
 
-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) {
+func (r *OpenStackMachineReconciler) getOrCreateInstance(logger logr.Logger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, computeService *compute.Service, networkingService *networking.Service, userData string, portStatus []infrav1.PortStatus) (*compute.InstanceStatus, error) {
+	var portIDs []string
+	for _, port := range portStatus {
+		portIDs = append(portIDs, port.ID)
+	}
 	instanceStatus, err := computeService.GetInstanceStatusByName(openStackMachine, openStackMachine.Name)
 	if err != nil {
 		logger.Info("Unable to get OpenStack instance", "name", openStackMachine.Name)
@@ -441,16 +449,32 @@ func (r *OpenStackMachineReconciler) getOrCreate(logger logr.Logger, cluster *cl
 	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)
+		instanceStatus, err = computeService.CreateInstance(openStackMachine, openStackCluster, instanceSpec, cluster.Name, portIDs)
 		if err != nil {
 			conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceCreateFailedReason, clusterv1.ConditionSeverityError, err.Error())
+
+			err := networkingService.DeletePortsForInstance(machine, openStackMachine)
+			if err != nil {
+				return nil, fmt.Errorf("delete ports: %w", err)
+			}
 			return nil, fmt.Errorf("create OpenStack instance: %w", err)
 		}
 	}
 
 	return instanceStatus, nil
 }
 
+func (r *OpenStackMachineReconciler) getOrCreatePorts(logger logr.Logger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, networkingService *networking.Service) ([]infrav1.PortStatus, error) {
+	logger.Info("Reconciling ports for machine", "name", openStackMachine.Name)
+	portStatus, err := networkingService.CreatePortsForInstance(machine, openStackMachine, openStackCluster, false)
+	if err != nil {
+		return nil, fmt.Errorf("create ports: %w", err)
+	}
+	logger.Info("Reconciled ports for machine", "name", openStackMachine.Name)
+
+	return portStatus, nil
+}
+
 func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, userData string) *compute.InstanceSpec {
 	instanceSpec := compute.InstanceSpec{
 		Name:                   openStackMachine.Name,

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

@@ -39,6 +39,7 @@ import (
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7"
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute"
+	"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking"
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
 	"sigs.k8s.io/cluster-api-provider-openstack/test/helpers/external"
 )
@@ -126,12 +127,13 @@ var _ = Describe("EnvTest sanity check", func() {
 
 var _ = Describe("When calling getOrCreate", func() {
 	var (
-		logger           logr.Logger
-		reconsiler       OpenStackMachineReconciler
-		mockCtrl         *gomock.Controller
-		mockScopeFactory *scope.MockScopeFactory
-		computeService   *compute.Service
-		err              error
+		logger            logr.Logger
+		reconsiler        OpenStackMachineReconciler
+		mockCtrl          *gomock.Controller
+		mockScopeFactory  *scope.MockScopeFactory
+		computeService    *compute.Service
+		networkingService *networking.Service
+		err               error
 	)
 
 	BeforeEach(func() {
@@ -142,16 +144,19 @@ var _ = Describe("When calling getOrCreate", func() {
 		mockScopeFactory = scope.NewMockScopeFactory(mockCtrl, "1234", logger)
 		computeService, err = compute.NewService(mockScopeFactory)
 		Expect(err).NotTo(HaveOccurred())
+		networkingService, err = networking.NewService(mockScopeFactory)
+		Expect(err).NotTo(HaveOccurred())
 	})
 
 	It("should return an error if unable to get instance", func() {
 		cluster := &clusterv1.Cluster{}
 		openStackCluster := &infrav1.OpenStackCluster{}
 		machine := &clusterv1.Machine{}
 		openStackMachine := &infrav1.OpenStackMachine{}
+		portStatus := []infrav1.PortStatus{}
 
 		mockScopeFactory.ComputeClient.EXPECT().ListServers(gomock.Any()).Return(nil, errors.New("Test error when listing servers"))
-		instanceStatus, err := reconsiler.getOrCreate(logger, cluster, openStackCluster, machine, openStackMachine, computeService, "")
+		instanceStatus, err := reconsiler.getOrCreateInstance(logger, cluster, openStackCluster, machine, openStackMachine, computeService, networkingService, "", portStatus)
 		Expect(err).To(HaveOccurred())
 		Expect(instanceStatus).To(BeNil())
 		conditions := openStackMachine.GetConditions()