worked on PR comments: change in logic validation

Signed-off-by: Bharath Nallapeta <[email protected]>

Author: bnallapeta

controllers/openstackcluster_controller.go

@@ -497,7 +497,10 @@ func (r *OpenStackClusterReconciler) reconcileBastionServer(ctx context.Context,
 	}
 
 	// If the bastion is found but the spec has changed, we need to delete it and reconcile.
-	bastionServerSpec := bastionToOpenStackServerSpec(openStackCluster)
+	bastionServerSpec, err := bastionToOpenStackServerSpec(openStackCluster)
+	if err != nil {
+		return nil, true, err
+	}
 	if !bastionNotFound && server != nil && !apiequality.Semantic.DeepEqual(bastionServerSpec, &server.Spec) {
 		scope.Logger().Info("Bastion spec has changed, re-creating the OpenStackServer object")
 		if err := r.deleteBastion(ctx, scope, cluster, openStackCluster); err != nil {
@@ -543,7 +546,10 @@ func (r *OpenStackClusterReconciler) getBastionServer(ctx context.Context, openS
 // createBastionServer creates the OpenStackServer object for the bastion server.
 // It returns the OpenStackServer object and an error if any.
 func (r *OpenStackClusterReconciler) createBastionServer(ctx context.Context, openStackCluster *infrav1.OpenStackCluster, cluster *clusterv1.Cluster) (*infrav1alpha1.OpenStackServer, error) {
-	bastionServerSpec := bastionToOpenStackServerSpec(openStackCluster)
+	bastionServerSpec, err := bastionToOpenStackServerSpec(openStackCluster)
+	if err != nil {
+		return nil, err
+	}
 	bastionServer := &infrav1alpha1.OpenStackServer{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{
@@ -571,7 +577,7 @@ func (r *OpenStackClusterReconciler) createBastionServer(ctx context.Context, op
 
 // bastionToOpenStackServerSpec converts the OpenStackMachineSpec for the bastion to an OpenStackServerSpec.
 // It returns the OpenStackServerSpec and an error if any.
-func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) *infrav1alpha1.OpenStackServerSpec {
+func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) (*infrav1alpha1.OpenStackServerSpec, error) {
 	bastion := openStackCluster.Spec.Bastion
 	if bastion == nil {
 		bastion = &infrav1.Bastion{}
@@ -586,9 +592,12 @@ func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) *i
 	if bastion.AvailabilityZone != nil {
 		az = *bastion.AvailabilityZone
 	}
-	openStackServerSpec := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network.ID)
+	openStackServerSpec, err := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network)
+	if err != nil {
+		return nil, err
+	}
 
-	return openStackServerSpec
+	return openStackServerSpec, nil
 }
 
 func bastionName(clusterResourceName string) string {

controllers/openstackmachine_controller.go

@@ -479,7 +479,18 @@ func (r *OpenStackMachineReconciler) getMachineServer(ctx context.Context, openS
 
 // openStackMachineSpecToOpenStackServerSpec converts an OpenStackMachineSpec to an OpenStackServerSpec.
 // It returns the OpenStackServerSpec object and an error if there is any.
-func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, defaultNetworkID string) *infrav1alpha1.OpenStackServerSpec {
+func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, clusterNetwork *infrav1.NetworkStatusWithSubnets) (*infrav1alpha1.OpenStackServerSpec, error) {
+	// Determine default network ID if the cluster status exposes one.
+	var defaultNetworkID string
+	if clusterNetwork != nil {
+		defaultNetworkID = clusterNetwork.ID
+	}
+
+	// If no cluster network is available AND the machine spec did not define any ports with a network, we cannot choose a network.
+	if defaultNetworkID == "" && len(openStackMachineSpec.Ports) == 0 {
+		return nil, capoerrors.Terminal(infrav1.InvalidMachineSpecReason, "no network configured: cluster network is missing and machine spec does not define ports with a network")
+	}
+
 	openStackServerSpec := &infrav1alpha1.OpenStackServerSpec{
 		AdditionalBlockDevices:            openStackMachineSpec.AdditionalBlockDevices,
 		ConfigDrive:                       openStackMachineSpec.ConfigDrive,
@@ -535,7 +546,7 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
 	}
 	openStackServerSpec.Ports = serverPorts
 
-	return openStackServerSpec
+	return openStackServerSpec, nil
 }
 
 // reconcileMachineServer reconciles the OpenStackServer object for the OpenStackMachine.
@@ -584,18 +595,10 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
 			}
 			return openStackCluster.Spec.IdentityRef
 		}()
-			// Determine default network ID if the cluster status exposes one.
-		var defaultNetworkID string
-		if openStackCluster.Status.Network != nil {
-			defaultNetworkID = openStackCluster.Status.Network.ID
-		}
-
-		// If no cluster network is available AND the machine spec did not define any ports with a network, we cannot choose a network.
-		if defaultNetworkID == "" && len(openStackMachine.Spec.Ports) == 0 {
-			return nil, capoerrors.Terminal(infrav1.InvalidMachineSpecReason, "no network configured: cluster network is missing and machine spec does not define ports with a network")
+		machineServerSpec, err := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network)
+		if err != nil {
+			return nil, err
 		}
-
-		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), defaultNetworkID)
 		machineServer = &infrav1alpha1.OpenStackServer{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{