Merge pull request #1856 from shiftstack/subnet-spec

✨ Move subnet options to SubnetSpec

Author: k8s-ci-robot

api/v1alpha5/conversion.go

@@ -196,6 +196,11 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in *
 		out.ExternalNetworkID = in.ExternalNetwork.ID
 	}
 
+	if len(in.ManagedSubnets) > 0 {
+		out.NodeCIDR = in.ManagedSubnets[0].CIDR
+		out.DNSNameservers = in.ManagedSubnets[0].DNSNameservers
+	}
+
 	if in.Subnets != nil {
 		if len(in.Subnets) >= 1 {
 			if err := Convert_v1alpha8_SubnetFilter_To_v1alpha5_SubnetFilter(&in.Subnets[0], &out.Subnet, s); err != nil {
@@ -228,6 +233,16 @@ func Convert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in *
 		out.Subnets = []infrav1.SubnetFilter{subnet}
 	}
 
+	if len(in.NodeCIDR) > 0 {
+		out.ManagedSubnets = []infrav1.SubnetSpec{
+			{
+				CIDR:           in.NodeCIDR,
+				DNSNameservers: in.DNSNameservers,
+			},
+		}
+	}
+	// We're dropping DNSNameservers even if these were set as without NodeCIDR it doesn't make sense.
+
 	return nil
 }
 

api/v1alpha5/zz_generated.conversion.go

@@ -127,6 +127,11 @@ func restorev1alpha6ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackCl
 		}
 	}
 
+	// We only restore DNSNameservers when these were lossly converted when NodeCIDR is empty.
+	if len(previous.DNSNameservers) > 0 && dst.NodeCIDR == "" {
+		dst.DNSNameservers = previous.DNSNameservers
+	}
+
 	prevBastion := previous.Bastion
 	dstBastion := dst.Bastion
 	if prevBastion != nil && dstBastion != nil {
@@ -545,6 +550,11 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *
 		}
 	}
 
+	if len(in.ManagedSubnets) > 0 {
+		out.NodeCIDR = in.ManagedSubnets[0].CIDR
+		out.DNSNameservers = in.ManagedSubnets[0].DNSNameservers
+	}
+
 	return nil
 }
 
@@ -569,6 +579,16 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in *
 		out.Subnets = []infrav1.SubnetFilter{subnet}
 	}
 
+	// DNSNameservers without NodeCIDR doesn't make sense, so we drop that.
+	if len(in.NodeCIDR) > 0 {
+		out.ManagedSubnets = []infrav1.SubnetSpec{
+			{
+				CIDR:           in.NodeCIDR,
+				DNSNameservers: in.DNSNameservers,
+			},
+		}
+	}
+
 	return nil
 }
 

api/v1alpha6/conversion.go

@@ -132,6 +132,16 @@ func TestFuzzyConversion(t *testing.T) {
 					}
 				}
 			},
+
+			func(spec *infrav1.SubnetSpec, c fuzz.Continue) {
+				c.FuzzNoCustom(spec)
+
+				// CIDR is required and API validates that it's present, so
+				// we force it to always be set.
+				for spec.CIDR == "" {
+					spec.CIDR = c.RandString()
+				}
+			},
 		}
 	}
 
@@ -156,6 +166,7 @@ func TestFuzzyConversion(t *testing.T) {
 		Hub:              &infrav1.OpenStackClusterTemplate{},
 		Spoke:            &OpenStackClusterTemplate{},
 		HubAfterMutation: ignoreDataAnnotation,
+		FuzzerFuncs:      []fuzzer.FuzzerFuncs{fuzzerFuncs},
 	})))
 
 	t.Run("for OpenStackClusterTemplate with mutate", runParallel(testhelpers.FuzzMutateTestFunc(testhelpers.FuzzMutateTestFuncInput{

api/v1alpha6/conversion_test.go

@@ -39,6 +39,24 @@ var v1alpha7OpenStackClusterRestorer = conversion.RestorerFor[*OpenStackCluster]
 		},
 		restorev1alpha7Bastion,
 	),
+	"spec": conversion.HashedFieldRestorer(
+		func(c *OpenStackCluster) *OpenStackClusterSpec {
+			return &c.Spec
+		},
+		restorev1alpha7ClusterSpec,
+
+		// Filter out Bastion, which is restored separately
+		conversion.HashedFilterField[*OpenStackCluster, OpenStackClusterSpec](
+			func(s *OpenStackClusterSpec) *OpenStackClusterSpec {
+				if s.Bastion != nil {
+					f := *s
+					f.Bastion = nil
+					return &f
+				}
+				return s
+			},
+		),
+	),
 }
 
 var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStackCluster]{
@@ -111,6 +129,19 @@ func restorev1alpha8Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
 	}
 }
 
+func restorev1alpha7ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackClusterSpec) {
+	prevBastion := previous.Bastion
+	dstBastion := dst.Bastion
+	if prevBastion != nil && dstBastion != nil {
+		restorev1alpha7MachineSpec(&prevBastion.Instance, &dstBastion.Instance)
+	}
+
+	// We only restore DNSNameservers when these were lossly converted when NodeCIDR is empty.
+	if len(previous.DNSNameservers) > 0 && dst.NodeCIDR == "" {
+		dst.DNSNameservers = previous.DNSNameservers
+	}
+}
+
 func restorev1alpha8ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infrav1.OpenStackClusterSpec) {
 	prevBastion := previous.Bastion
 	dstBastion := dst.Bastion
@@ -132,6 +163,8 @@ func restorev1alpha8ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *inf
 	if len(previous.Subnets) > 1 {
 		dst.Subnets = append(dst.Subnets, previous.Subnets[1:]...)
 	}
+
+	dst.ManagedSubnets = previous.ManagedSubnets
 }
 
 func (r *OpenStackCluster) ConvertTo(dstRaw ctrlconversion.Hub) error {
@@ -171,6 +204,7 @@ func (r *OpenStackClusterList) ConvertFrom(srcRaw ctrlconversion.Hub) error {
 var _ ctrlconversion.Convertible = &OpenStackClusterTemplate{}
 
 func restorev1alpha7ClusterTemplateSpec(previous *OpenStackClusterTemplateSpec, dst *OpenStackClusterTemplateSpec) {
+	restorev1alpha7ClusterSpec(&previous.Template.Spec, &dst.Template.Spec)
 	restorev1alpha7Bastion(&previous.Template.Spec.Bastion, &dst.Template.Spec.Bastion)
 }
 
@@ -465,6 +499,16 @@ func Convert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in *
 		out.Subnets = []infrav1.SubnetFilter{subnet}
 	}
 
+	// DNSNameservers without NodeCIDR doesn't make sense, so we drop that.
+	if len(in.NodeCIDR) > 0 {
+		out.ManagedSubnets = []infrav1.SubnetSpec{
+			{
+				CIDR:           in.NodeCIDR,
+				DNSNameservers: in.DNSNameservers,
+			},
+		}
+	}
+
 	return nil
 }
 
@@ -484,5 +528,10 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in *
 		}
 	}
 
+	if len(in.ManagedSubnets) > 0 {
+		out.NodeCIDR = in.ManagedSubnets[0].CIDR
+		out.DNSNameservers = in.ManagedSubnets[0].DNSNameservers
+	}
+
 	return nil
 }

api/v1alpha6/zz_generated.conversion.go

@@ -100,6 +100,16 @@ func TestFuzzyConversion(t *testing.T) {
 					}
 				}
 			},
+
+			func(spec *infrav1.SubnetSpec, c fuzz.Continue) {
+				c.FuzzNoCustom(spec)
+
+				// CIDR is required and API validates that it's present, so
+				// we force it to always be set.
+				for spec.CIDR == "" {
+					spec.CIDR = c.RandString()
+				}
+			},
 		}
 	}
 

api/v1alpha7/conversion.go

@@ -34,10 +34,11 @@ type OpenStackClusterSpec struct {
 	// +optional
 	CloudName string `json:"cloudName"`
 
-	// NodeCIDR is the OpenStack Subnet to be created. Cluster actuator will create a
-	// network, a subnet with NodeCIDR, and a router connected to this subnet.
-	// If you leave this empty, no network will be created.
-	NodeCIDR string `json:"nodeCidr,omitempty"`
+	// ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network,
+	// subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4
+	// subnet is supported. If you leave this empty, no network will be created.
+	// +kubebuilder:validation:MaxItems=1
+	ManagedSubnets []SubnetSpec `json:"managedSubnets,omitempty"`
 
 	// If NodeCIDR is set this option can be used to detect an existing router.
 	// If specified, no new router will be created.
@@ -58,11 +59,6 @@ type OpenStackClusterSpec struct {
 	// +optional
 	NetworkMTU int `json:"networkMtu,omitempty"`
 
-	// DNSNameservers is the list of nameservers for OpenStack Subnet being created.
-	// Set this value when you need create a new network/subnet while the access
-	// through DNS is required.
-	// +listType=set
-	DNSNameservers []string `json:"dnsNameservers,omitempty"`
 	// ExternalRouterIPs is an array of externalIPs on the respective subnets.
 	// This is necessary if the router needs a fixed ip in a specific subnet.
 	ExternalRouterIPs []ExternalRouterIPParam `json:"externalRouterIPs,omitempty"`

api/v1alpha7/conversion_test.go

@@ -87,6 +87,16 @@ type RouterFilter struct {
 	NotTagsAny  string `json:"notTagsAny,omitempty"`
 }
 
+type SubnetSpec struct {
+	// CIDR is representing the IP address range used to create the subnet, e.g. 10.0.0.0/24.
+	// This field is required when defining a subnet.
+	// +required
+	CIDR string `json:"cidr"`
+	// DNSNameservers holds a list of DNS server addresses that will be provided when creating
+	// the subnet. These addresses need to have the same IP version as CIDR.
+	DNSNameservers []string `json:"dnsNameservers,omitempty"`
+}
+
 type PortOpts struct {
 	// Network is a query for an openstack network that the port will be created or discovered on.
 	// This will fail if the query returns more than one network.