Allow explicitly empty volume AZ

Replaces an AvailabilityZone string for volumes with a
VolumeAvailabilityZone struct which allows more flexibility in
defaulting behaviour. Specifically it enables us to express both the
current default behaviour where we take the volume AZ from the Machine,
and a new default behaviour where to don't specify a volume AZ at all.

In making this change to both RootVolume and AdditionalBlockDevices we
use common code for both APIs. This has the result of updating
RootVolume to be consistent with AdditionalBlockDevices.

Author: mdbooth

.golangci.yml

@@ -172,6 +172,10 @@ issues:
     - stylecheck
     text: "ST1003: should not use underscores in Go names;"
     path: (api\/.*|pkg/utils/optional)\/.*conversion.*\.go$
+  - linters:
+    - stylecheck
+    text: "ST1003: should not use underscores in Go names;"
+    path: pkg/utils/conversioncommon/.*.go
 
 run:
   timeout: 10m

Makefile

@@ -269,6 +269,7 @@ generate-conversion-gen: $(CONVERSION_GEN)
 		--input-dirs=$(capo_module)/api/v1alpha6 \
 		--input-dirs=$(capo_module)/api/v1alpha7 \
 		--extra-dirs=$(capo_module)/pkg/utils/optional \
+		--extra-dirs=$(capo_module)/pkg/utils/conversioncommon \
 		--output-file-base=zz_generated.conversion \
 		--trim-path-prefix=$(capo_module)/ \
 		--go-header-file=./hack/boilerplate/boilerplate.generatego.txt

api/v1alpha5/conversion.go

@@ -26,6 +26,7 @@ import (
 	ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/conversioncommon"
 )
 
 var _ ctrlconversion.Convertible = &OpenStackCluster{}
@@ -813,6 +814,18 @@ func Convert_v1beta1_NetworkParam_To_v1alpha5_NetworkFilter(in *infrav1.NetworkP
 	return nil
 }
 
+func Convert_v1alpha5_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *infrav1.RootVolume, s conversion.Scope) error {
+	out.SizeGiB = in.Size
+	out.Type = in.VolumeType
+	return conversioncommon.Convert_string_To_Pointer_v1beta1_VolumeAvailabilityZone(&in.AvailabilityZone, &out.AvailabilityZone, s)
+}
+
+func Convert_v1beta1_RootVolume_To_v1alpha5_RootVolume(in *infrav1.RootVolume, out *RootVolume, s conversion.Scope) error {
+	out.Size = in.SizeGiB
+	out.VolumeType = in.Type
+	return conversioncommon.Convert_Pointer_v1beta1_VolumeAvailabilityZone_To_string(&in.AvailabilityZone, &out.AvailabilityZone, s)
+}
+
 // conversion-gen registers the following functions so we have to define them, but nothing should ever call them.
 func Convert_v1alpha5_NetworkFilter_To_v1beta1_NetworkFilter(_ *NetworkFilter, _ *infrav1.NetworkFilter, _ conversion.Scope) error {
 	return errors.New("Convert_v1alpha6_NetworkFilter_To_v1beta1_NetworkFilter should not be called")

api/v1alpha5/zz_generated.conversion.go

@@ -792,6 +792,7 @@ func Test_FuzzRestorers(t *testing.T) {
 	testhelpers.FuzzRestorer(t, "restorev1alpha6SubnetParam", restorev1alpha6SubnetParam)
 	testhelpers.FuzzRestorer(t, "restorev1beta1SubnetParam", restorev1beta1SubnetParam)
 	testhelpers.FuzzRestorer(t, "restorev1alpha6Port", restorev1alpha6Port)
+	testhelpers.FuzzRestorer(t, "restorev1beta1BlockDeviceVolume", restorev1beta1BlockDeviceVolume)
 	testhelpers.FuzzRestorer(t, "restorev1alpha6SecurityGroup", restorev1alpha6SecurityGroup)
 	testhelpers.FuzzRestorer(t, "restorev1beta1APIServerLoadBalancer", restorev1beta1APIServerLoadBalancer)
 }

api/v1alpha6/conversion_test.go

@@ -196,6 +196,22 @@ func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infr
 			restorev1beta1SecurityGroupParam(&previous.SecurityGroups[i], &dst.SecurityGroups[i])
 		}
 	}
+
+	if dst.RootVolume != nil && previous.RootVolume != nil {
+		restorev1beta1BlockDeviceVolume(
+			&previous.RootVolume.BlockDeviceVolume,
+			&dst.RootVolume.BlockDeviceVolume,
+		)
+	}
+
+	if len(dst.AdditionalBlockDevices) == len(previous.AdditionalBlockDevices) {
+		for i := range dst.AdditionalBlockDevices {
+			restorev1beta1BlockDeviceVolume(
+				previous.AdditionalBlockDevices[i].Storage.Volume,
+				dst.AdditionalBlockDevices[i].Storage.Volume,
+			)
+		}
+	}
 }
 
 func convertNetworksToPorts(networks []NetworkParam, s apiconversion.Scope) ([]infrav1.PortOpts, error) {

api/v1alpha6/openstackmachine_conversion.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/utils/pointer"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/conversioncommon"
 	optional "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional"
 )
 
@@ -426,6 +427,33 @@ func Convert_v1beta1_BindingProfile_To_Map_string_To_Interface(in *infrav1.Bindi
 /* AddressPair */
 /* Instance */
 /* RootVolume */
+
+func restorev1beta1BlockDeviceVolume(previous *infrav1.BlockDeviceVolume, dst *infrav1.BlockDeviceVolume) {
+	if previous == nil || dst == nil {
+		return
+	}
+
+	dstAZ := dst.AvailabilityZone
+	previousAZ := previous.AvailabilityZone
+
+	// Empty From (the default) will be converted to the explicit "Name"
+	if dstAZ != nil && previousAZ != nil && dstAZ.From == "Name" {
+		dstAZ.From = previousAZ.From
+	}
+}
+
+func Convert_v1alpha6_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *infrav1.RootVolume, s apiconversion.Scope) error {
+	out.SizeGiB = in.Size
+	out.Type = in.VolumeType
+	return conversioncommon.Convert_string_To_Pointer_v1beta1_VolumeAvailabilityZone(&in.AvailabilityZone, &out.AvailabilityZone, s)
+}
+
+func Convert_v1beta1_RootVolume_To_v1alpha6_RootVolume(in *infrav1.RootVolume, out *RootVolume, s apiconversion.Scope) error {
+	out.Size = in.SizeGiB
+	out.VolumeType = in.Type
+	return conversioncommon.Convert_Pointer_v1beta1_VolumeAvailabilityZone_To_string(&in.AvailabilityZone, &out.AvailabilityZone, s)
+}
+
 /* Network */
 
 func Convert_v1alpha6_Network_To_v1beta1_NetworkStatusWithSubnets(in *Network, out *infrav1.NetworkStatusWithSubnets, s apiconversion.Scope) error {

api/v1alpha6/types_conversion.go

@@ -406,4 +406,5 @@ func Test_FuzzRestorers(t *testing.T) {
 	testhelpers.FuzzRestorer(t, "restorev1alpha7Port", restorev1alpha7Port)
 	testhelpers.FuzzRestorer(t, "restorev1beta1Port", restorev1beta1Port)
 	testhelpers.FuzzRestorer(t, "restorev1beta1APIServerLoadBalancer", restorev1beta1APIServerLoadBalancer)
+	testhelpers.FuzzRestorer(t, "restorev1beta1BlockDeviceVolume", restorev1beta1BlockDeviceVolume)
 }

api/v1alpha6/zz_generated.conversion.go

@@ -191,6 +191,22 @@ func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infr
 		}
 	}
 	dst.FloatingIPPoolRef = previous.FloatingIPPoolRef
+
+	if dst.RootVolume != nil && previous.RootVolume != nil {
+		restorev1beta1BlockDeviceVolume(
+			&previous.RootVolume.BlockDeviceVolume,
+			&dst.RootVolume.BlockDeviceVolume,
+		)
+	}
+
+	if len(dst.AdditionalBlockDevices) == len(previous.AdditionalBlockDevices) {
+		for i := range dst.AdditionalBlockDevices {
+			restorev1beta1BlockDeviceVolume(
+				previous.AdditionalBlockDevices[i].Storage.Volume,
+				dst.AdditionalBlockDevices[i].Storage.Volume,
+			)
+		}
+	}
 }
 
 func Convert_v1alpha7_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *OpenStackMachineSpec, out *infrav1.OpenStackMachineSpec, s apiconversion.Scope) error {