diff --git a/.golangci.yml b/.golangci.yml index 944b2ad9fc..7e3d5aac16 100644 --- a/.golangci.yml +++ b/.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 diff --git a/Makefile b/Makefile index 197fb71297..1e9b096730 100644 --- a/Makefile +++ b/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 diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index 79b282dfeb..153adebd4e 100644 --- a/api/v1alpha5/conversion.go +++ b/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") diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index 95f181a279..a77967fb5a 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -207,16 +207,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*RootVolume)(nil), (*v1beta1.RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha5_RootVolume_To_v1beta1_RootVolume(a.(*RootVolume), b.(*v1beta1.RootVolume), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1beta1.RootVolume)(nil), (*RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_RootVolume_To_v1alpha5_RootVolume(a.(*v1beta1.RootVolume), b.(*RootVolume), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Router)(nil), (*v1beta1.Router)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha5_Router_To_v1beta1_Router(a.(*Router), b.(*v1beta1.Router), scope) }); err != nil { @@ -297,6 +287,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*RootVolume)(nil), (*v1beta1.RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha5_RootVolume_To_v1beta1_RootVolume(a.(*RootVolume), b.(*v1beta1.RootVolume), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*SecurityGroupFilter)(nil), (*v1beta1.SecurityGroupFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha5_SecurityGroupFilter_To_v1beta1_SecurityGroupFilter(a.(*SecurityGroupFilter), b.(*v1beta1.SecurityGroupFilter), scope) }); err != nil { @@ -402,6 +397,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.RootVolume)(nil), (*RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_RootVolume_To_v1alpha5_RootVolume(a.(*v1beta1.RootVolume), b.(*RootVolume), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.SecurityGroupFilter)(nil), (*SecurityGroupFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_SecurityGroupFilter_To_v1alpha5_SecurityGroupFilter(a.(*v1beta1.SecurityGroupFilter), b.(*SecurityGroupFilter), scope) }); err != nil { @@ -1182,7 +1182,15 @@ func autoConvert_v1alpha5_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(i out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) // WARNING: in.ServerMetadata requires manual conversion: inconvertible types (map[string]string vs []sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ServerMetadata) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) - out.RootVolume = (*v1beta1.RootVolume)(unsafe.Pointer(in.RootVolume)) + if in.RootVolume != nil { + in, out := &in.RootVolume, &out.RootVolume + *out = new(v1beta1.RootVolume) + if err := Convert_v1alpha5_RootVolume_To_v1beta1_RootVolume(*in, *out, s); err != nil { + return err + } + } else { + out.RootVolume = nil + } // WARNING: in.ServerGroupID requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef @@ -1227,7 +1235,15 @@ func autoConvert_v1beta1_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(i out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) // WARNING: in.ServerMetadata requires manual conversion: inconvertible types ([]sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ServerMetadata vs map[string]string) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) - out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) + if in.RootVolume != nil { + in, out := &in.RootVolume, &out.RootVolume + *out = new(RootVolume) + if err := Convert_v1beta1_RootVolume_To_v1alpha5_RootVolume(*in, *out, s); err != nil { + return err + } + } else { + out.RootVolume = nil + } // WARNING: in.AdditionalBlockDevices requires manual conversion: does not exist in peer-type // WARNING: in.ServerGroup requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { @@ -1475,29 +1491,18 @@ func autoConvert_v1beta1_PortOpts_To_v1alpha5_PortOpts(in *v1beta1.PortOpts, out } func autoConvert_v1alpha5_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *v1beta1.RootVolume, s conversion.Scope) error { - out.Size = in.Size - out.VolumeType = in.VolumeType - out.AvailabilityZone = in.AvailabilityZone + // WARNING: in.Size requires manual conversion: does not exist in peer-type + // WARNING: in.VolumeType requires manual conversion: does not exist in peer-type + // WARNING: in.AvailabilityZone requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha5_RootVolume_To_v1beta1_RootVolume is an autogenerated conversion function. -func Convert_v1alpha5_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *v1beta1.RootVolume, s conversion.Scope) error { - return autoConvert_v1alpha5_RootVolume_To_v1beta1_RootVolume(in, out, s) -} - func autoConvert_v1beta1_RootVolume_To_v1alpha5_RootVolume(in *v1beta1.RootVolume, out *RootVolume, s conversion.Scope) error { - out.Size = in.Size - out.VolumeType = in.VolumeType - out.AvailabilityZone = in.AvailabilityZone + // WARNING: in.SizeGiB requires manual conversion: does not exist in peer-type + // WARNING: in.BlockDeviceVolume requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_RootVolume_To_v1alpha5_RootVolume is an autogenerated conversion function. -func Convert_v1beta1_RootVolume_To_v1alpha5_RootVolume(in *v1beta1.RootVolume, out *RootVolume, s conversion.Scope) error { - return autoConvert_v1beta1_RootVolume_To_v1alpha5_RootVolume(in, out, s) -} - func autoConvert_v1alpha5_Router_To_v1beta1_Router(in *Router, out *v1beta1.Router, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID diff --git a/api/v1alpha6/conversion_test.go b/api/v1alpha6/conversion_test.go index c451091929..ffbe779937 100644 --- a/api/v1alpha6/conversion_test.go +++ b/api/v1alpha6/conversion_test.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) } diff --git a/api/v1alpha6/openstackmachine_conversion.go b/api/v1alpha6/openstackmachine_conversion.go index 7df76db0c5..aed2d3b7d4 100644 --- a/api/v1alpha6/openstackmachine_conversion.go +++ b/api/v1alpha6/openstackmachine_conversion.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) { diff --git a/api/v1alpha6/types_conversion.go b/api/v1alpha6/types_conversion.go index e9f1ea72a9..634e7c0194 100644 --- a/api/v1alpha6/types_conversion.go +++ b/api/v1alpha6/types_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 { diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index 541bef18aa..59dba6198a 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -191,16 +191,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*RootVolume)(nil), (*v1beta1.RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha6_RootVolume_To_v1beta1_RootVolume(a.(*RootVolume), b.(*v1beta1.RootVolume), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1beta1.RootVolume)(nil), (*RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_RootVolume_To_v1alpha6_RootVolume(a.(*v1beta1.RootVolume), b.(*RootVolume), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Router)(nil), (*v1beta1.Router)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha6_Router_To_v1beta1_Router(a.(*Router), b.(*v1beta1.Router), scope) }); err != nil { @@ -301,6 +291,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*RootVolume)(nil), (*v1beta1.RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha6_RootVolume_To_v1beta1_RootVolume(a.(*RootVolume), b.(*v1beta1.RootVolume), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*SecurityGroupFilter)(nil), (*v1beta1.SecurityGroupFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha6_SecurityGroupFilter_To_v1beta1_SecurityGroupFilter(a.(*SecurityGroupFilter), b.(*v1beta1.SecurityGroupFilter), scope) }); err != nil { @@ -411,6 +406,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.RootVolume)(nil), (*RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_RootVolume_To_v1alpha6_RootVolume(a.(*v1beta1.RootVolume), b.(*RootVolume), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.SecurityGroupFilter)(nil), (*SecurityGroupFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_SecurityGroupFilter_To_v1alpha6_SecurityGroupFilter(a.(*v1beta1.SecurityGroupFilter), b.(*SecurityGroupFilter), scope) }); err != nil { @@ -1192,7 +1192,15 @@ func autoConvert_v1alpha6_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(i out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) // WARNING: in.ServerMetadata requires manual conversion: inconvertible types (map[string]string vs []sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ServerMetadata) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) - out.RootVolume = (*v1beta1.RootVolume)(unsafe.Pointer(in.RootVolume)) + if in.RootVolume != nil { + in, out := &in.RootVolume, &out.RootVolume + *out = new(v1beta1.RootVolume) + if err := Convert_v1alpha6_RootVolume_To_v1beta1_RootVolume(*in, *out, s); err != nil { + return err + } + } else { + out.RootVolume = nil + } // WARNING: in.ServerGroupID requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef @@ -1237,7 +1245,15 @@ func autoConvert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(i out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) // WARNING: in.ServerMetadata requires manual conversion: inconvertible types ([]sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ServerMetadata vs map[string]string) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) - out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) + if in.RootVolume != nil { + in, out := &in.RootVolume, &out.RootVolume + *out = new(RootVolume) + if err := Convert_v1beta1_RootVolume_To_v1alpha6_RootVolume(*in, *out, s); err != nil { + return err + } + } else { + out.RootVolume = nil + } // WARNING: in.AdditionalBlockDevices requires manual conversion: does not exist in peer-type // WARNING: in.ServerGroup requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { @@ -1486,29 +1502,18 @@ func autoConvert_v1beta1_PortOpts_To_v1alpha6_PortOpts(in *v1beta1.PortOpts, out } func autoConvert_v1alpha6_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *v1beta1.RootVolume, s conversion.Scope) error { - out.Size = in.Size - out.VolumeType = in.VolumeType - out.AvailabilityZone = in.AvailabilityZone + // WARNING: in.Size requires manual conversion: does not exist in peer-type + // WARNING: in.VolumeType requires manual conversion: does not exist in peer-type + // WARNING: in.AvailabilityZone requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha6_RootVolume_To_v1beta1_RootVolume is an autogenerated conversion function. -func Convert_v1alpha6_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *v1beta1.RootVolume, s conversion.Scope) error { - return autoConvert_v1alpha6_RootVolume_To_v1beta1_RootVolume(in, out, s) -} - func autoConvert_v1beta1_RootVolume_To_v1alpha6_RootVolume(in *v1beta1.RootVolume, out *RootVolume, s conversion.Scope) error { - out.Size = in.Size - out.VolumeType = in.VolumeType - out.AvailabilityZone = in.AvailabilityZone + // WARNING: in.SizeGiB requires manual conversion: does not exist in peer-type + // WARNING: in.BlockDeviceVolume requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_RootVolume_To_v1alpha6_RootVolume is an autogenerated conversion function. -func Convert_v1beta1_RootVolume_To_v1alpha6_RootVolume(in *v1beta1.RootVolume, out *RootVolume, s conversion.Scope) error { - return autoConvert_v1beta1_RootVolume_To_v1alpha6_RootVolume(in, out, s) -} - func autoConvert_v1alpha6_Router_To_v1beta1_Router(in *Router, out *v1beta1.Router, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID diff --git a/api/v1alpha7/conversion_test.go b/api/v1alpha7/conversion_test.go index ac3d00cddf..8a7e9559b6 100644 --- a/api/v1alpha7/conversion_test.go +++ b/api/v1alpha7/conversion_test.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) } diff --git a/api/v1alpha7/openstackmachine_conversion.go b/api/v1alpha7/openstackmachine_conversion.go index 35a2cf6d83..5183b3fae2 100644 --- a/api/v1alpha7/openstackmachine_conversion.go +++ b/api/v1alpha7/openstackmachine_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 { diff --git a/api/v1alpha7/types_conversion.go b/api/v1alpha7/types_conversion.go index ac09a5d3dc..806e145806 100644 --- a/api/v1alpha7/types_conversion.go +++ b/api/v1alpha7/types_conversion.go @@ -22,6 +22,7 @@ import ( apiconversion "k8s.io/apimachinery/pkg/conversion" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/conversioncommon" "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional" ) @@ -484,6 +485,34 @@ func Convert_v1beta1_PortOpts_To_v1alpha7_PortOpts(in *infrav1.PortOpts, out *Po return nil } +/* 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_v1alpha7_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_v1alpha7_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) +} + /* SecurityGroup */ func Convert_v1alpha7_SecurityGroup_To_v1beta1_SecurityGroupStatus(in *SecurityGroup, out *infrav1.SecurityGroupStatus, _ apiconversion.Scope) error { diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index 554345da6c..f67c228983 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -29,6 +29,7 @@ import ( conversion "k8s.io/apimachinery/pkg/conversion" runtime "k8s.io/apimachinery/pkg/runtime" v1beta1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + conversioncommon "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/conversioncommon" optional "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional" apiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" errors "sigs.k8s.io/cluster-api/errors" @@ -261,16 +262,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*RootVolume)(nil), (*v1beta1.RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha7_RootVolume_To_v1beta1_RootVolume(a.(*RootVolume), b.(*v1beta1.RootVolume), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1beta1.RootVolume)(nil), (*RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_RootVolume_To_v1alpha7_RootVolume(a.(*v1beta1.RootVolume), b.(*RootVolume), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Router)(nil), (*v1beta1.Router)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha7_Router_To_v1beta1_Router(a.(*Router), b.(*v1beta1.Router), scope) }); err != nil { @@ -346,6 +337,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*RootVolume)(nil), (*v1beta1.RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha7_RootVolume_To_v1beta1_RootVolume(a.(*RootVolume), b.(*v1beta1.RootVolume), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*RouterFilter)(nil), (*v1beta1.RouterFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha7_RouterFilter_To_v1beta1_RouterFilter(a.(*RouterFilter), b.(*v1beta1.RouterFilter), scope) }); err != nil { @@ -446,6 +442,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.RootVolume)(nil), (*RootVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_RootVolume_To_v1alpha7_RootVolume(a.(*v1beta1.RootVolume), b.(*RootVolume), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.RouterFilter)(nil), (*RouterFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_RouterFilter_To_v1alpha7_RouterFilter(a.(*v1beta1.RouterFilter), b.(*RouterFilter), scope) }); err != nil { @@ -647,7 +648,15 @@ func Convert_v1beta1_BindingProfile_To_v1alpha7_BindingProfile(in *v1beta1.Bindi func autoConvert_v1alpha7_BlockDeviceStorage_To_v1beta1_BlockDeviceStorage(in *BlockDeviceStorage, out *v1beta1.BlockDeviceStorage, s conversion.Scope) error { out.Type = v1beta1.BlockDeviceType(in.Type) - out.Volume = (*v1beta1.BlockDeviceVolume)(unsafe.Pointer(in.Volume)) + if in.Volume != nil { + in, out := &in.Volume, &out.Volume + *out = new(v1beta1.BlockDeviceVolume) + if err := Convert_v1alpha7_BlockDeviceVolume_To_v1beta1_BlockDeviceVolume(*in, *out, s); err != nil { + return err + } + } else { + out.Volume = nil + } return nil } @@ -658,7 +667,15 @@ func Convert_v1alpha7_BlockDeviceStorage_To_v1beta1_BlockDeviceStorage(in *Block func autoConvert_v1beta1_BlockDeviceStorage_To_v1alpha7_BlockDeviceStorage(in *v1beta1.BlockDeviceStorage, out *BlockDeviceStorage, s conversion.Scope) error { out.Type = BlockDeviceType(in.Type) - out.Volume = (*BlockDeviceVolume)(unsafe.Pointer(in.Volume)) + if in.Volume != nil { + in, out := &in.Volume, &out.Volume + *out = new(BlockDeviceVolume) + if err := Convert_v1beta1_BlockDeviceVolume_To_v1alpha7_BlockDeviceVolume(*in, *out, s); err != nil { + return err + } + } else { + out.Volume = nil + } return nil } @@ -669,7 +686,9 @@ func Convert_v1beta1_BlockDeviceStorage_To_v1alpha7_BlockDeviceStorage(in *v1bet func autoConvert_v1alpha7_BlockDeviceVolume_To_v1beta1_BlockDeviceVolume(in *BlockDeviceVolume, out *v1beta1.BlockDeviceVolume, s conversion.Scope) error { out.Type = in.Type - out.AvailabilityZone = in.AvailabilityZone + if err := conversioncommon.Convert_string_To_Pointer_v1beta1_VolumeAvailabilityZone(&in.AvailabilityZone, &out.AvailabilityZone, s); err != nil { + return err + } return nil } @@ -680,7 +699,9 @@ func Convert_v1alpha7_BlockDeviceVolume_To_v1beta1_BlockDeviceVolume(in *BlockDe func autoConvert_v1beta1_BlockDeviceVolume_To_v1alpha7_BlockDeviceVolume(in *v1beta1.BlockDeviceVolume, out *BlockDeviceVolume, s conversion.Scope) error { out.Type = in.Type - out.AvailabilityZone = in.AvailabilityZone + if err := conversioncommon.Convert_Pointer_v1beta1_VolumeAvailabilityZone_To_string(&in.AvailabilityZone, &out.AvailabilityZone, s); err != nil { + return err + } return nil } @@ -1403,8 +1424,26 @@ func autoConvert_v1alpha7_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(i out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) // WARNING: in.ServerMetadata requires manual conversion: inconvertible types (map[string]string vs []sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ServerMetadata) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) - out.RootVolume = (*v1beta1.RootVolume)(unsafe.Pointer(in.RootVolume)) - out.AdditionalBlockDevices = *(*[]v1beta1.AdditionalBlockDevice)(unsafe.Pointer(&in.AdditionalBlockDevices)) + if in.RootVolume != nil { + in, out := &in.RootVolume, &out.RootVolume + *out = new(v1beta1.RootVolume) + if err := Convert_v1alpha7_RootVolume_To_v1beta1_RootVolume(*in, *out, s); err != nil { + return err + } + } else { + out.RootVolume = nil + } + if in.AdditionalBlockDevices != nil { + in, out := &in.AdditionalBlockDevices, &out.AdditionalBlockDevices + *out = make([]v1beta1.AdditionalBlockDevice, len(*in)) + for i := range *in { + if err := Convert_v1alpha7_AdditionalBlockDevice_To_v1beta1_AdditionalBlockDevice(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.AdditionalBlockDevices = nil + } // WARNING: in.ServerGroupID requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef @@ -1449,8 +1488,26 @@ func autoConvert_v1beta1_OpenStackMachineSpec_To_v1alpha7_OpenStackMachineSpec(i out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) // WARNING: in.ServerMetadata requires manual conversion: inconvertible types ([]sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ServerMetadata vs map[string]string) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) - out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) - out.AdditionalBlockDevices = *(*[]AdditionalBlockDevice)(unsafe.Pointer(&in.AdditionalBlockDevices)) + if in.RootVolume != nil { + in, out := &in.RootVolume, &out.RootVolume + *out = new(RootVolume) + if err := Convert_v1beta1_RootVolume_To_v1alpha7_RootVolume(*in, *out, s); err != nil { + return err + } + } else { + out.RootVolume = nil + } + if in.AdditionalBlockDevices != nil { + in, out := &in.AdditionalBlockDevices, &out.AdditionalBlockDevices + *out = make([]AdditionalBlockDevice, len(*in)) + for i := range *in { + if err := Convert_v1beta1_AdditionalBlockDevice_To_v1alpha7_AdditionalBlockDevice(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.AdditionalBlockDevices = nil + } // WARNING: in.ServerGroup requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef @@ -1686,29 +1743,18 @@ func autoConvert_v1beta1_PortOpts_To_v1alpha7_PortOpts(in *v1beta1.PortOpts, out } func autoConvert_v1alpha7_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *v1beta1.RootVolume, s conversion.Scope) error { - out.Size = in.Size - out.VolumeType = in.VolumeType - out.AvailabilityZone = in.AvailabilityZone + // WARNING: in.Size requires manual conversion: does not exist in peer-type + // WARNING: in.VolumeType requires manual conversion: does not exist in peer-type + // WARNING: in.AvailabilityZone requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha7_RootVolume_To_v1beta1_RootVolume is an autogenerated conversion function. -func Convert_v1alpha7_RootVolume_To_v1beta1_RootVolume(in *RootVolume, out *v1beta1.RootVolume, s conversion.Scope) error { - return autoConvert_v1alpha7_RootVolume_To_v1beta1_RootVolume(in, out, s) -} - func autoConvert_v1beta1_RootVolume_To_v1alpha7_RootVolume(in *v1beta1.RootVolume, out *RootVolume, s conversion.Scope) error { - out.Size = in.Size - out.VolumeType = in.VolumeType - out.AvailabilityZone = in.AvailabilityZone + // WARNING: in.SizeGiB requires manual conversion: does not exist in peer-type + // WARNING: in.BlockDeviceVolume requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_RootVolume_To_v1alpha7_RootVolume is an autogenerated conversion function. -func Convert_v1beta1_RootVolume_To_v1alpha7_RootVolume(in *v1beta1.RootVolume, out *RootVolume, s conversion.Scope) error { - return autoConvert_v1beta1_RootVolume_To_v1alpha7_RootVolume(in, out, s) -} - func autoConvert_v1alpha7_Router_To_v1beta1_Router(in *Router, out *v1beta1.Router, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index b6789489d1..3500482fdf 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -490,9 +490,12 @@ type BastionStatus struct { } type RootVolume struct { - Size int `json:"diskSize,omitempty"` - VolumeType string `json:"volumeType,omitempty"` - AvailabilityZone string `json:"availabilityZone,omitempty"` + // SizeGiB is the size of the block device in gibibytes (GiB). + // +kubebuilder:validation:Required + // +kubebuilder:validation:Minimum:=1 + SizeGiB int `json:"sizeGiB"` + + BlockDeviceVolume `json:",inline"` } // BlockDeviceStorage is the storage type of a block device to create and @@ -520,13 +523,44 @@ type BlockDeviceVolume struct { // +optional Type string `json:"type,omitempty"` - // AvailabilityZone is the volume availability zone to create the volume in. - // If omitted, the availability zone of the server will be used. - // The availability zone must NOT contain spaces otherwise it will lead to volume that belongs - // to this availability zone register failure, see kubernetes/cloud-provider-openstack#1379 for - // further information. + // AvailabilityZone is the volume availability zone to create the volume + // in. If not specified, the volume will be created without an explicit + // availability zone. // +optional - AvailabilityZone string `json:"availabilityZone,omitempty"` + AvailabilityZone *VolumeAvailabilityZone `json:"availabilityZone,omitempty"` +} + +// VolumeAZSource specifies where to obtain the availability zone for a volume. +// +kubebuilder:validation:Enum=Name;Machine +type VolumeAZSource string + +const ( + VolumeAZFromName VolumeAZSource = "Name" + VolumeAZFromMachine VolumeAZSource = "Machine" +) + +// VolumeAZName is the name of a volume availability zone. It may not contain spaces. +// +kubebuilder:validation:Pattern:="^[^ ]+$" +// +kubebuilder:validation:MinLength:=1 +type VolumeAZName string + +// VolumeAvailabilityZone specifies the availability zone for a volume. +// +kubebuilder:validation:XValidation:rule="!has(self.from) || self.from == 'Name' ? has(self.name) : !has(self.name)",message="name is required when from is 'Name' or default" +type VolumeAvailabilityZone struct { + // From specifies where we will obtain the availability zone for the + // volume. The options are "Name" and "Machine". If "Name" is specified + // then the Name field must also be specified. If "Machine" is specified + // the volume will use the value of FailureDomain, if any, from the + // associated Machine. + // +kubebuilder:default:=Name + // +optional + From VolumeAZSource `json:"from,omitempty"` + + // Name is the name of a volume availability zone to use. It is required + // if From is "Name". The volume availability zone name may not contain + // spaces. + // +optional + Name *VolumeAZName `json:"name,omitempty"` } // AdditionalBlockDevice is a block device to attach to the server. @@ -537,9 +571,13 @@ type AdditionalBlockDevice struct { // Also, this name will be used for tagging the block device. // Information about the block device tag can be obtained from the OpenStack // metadata API or the config drive. + // Name cannot be 'root', which is reserved for the root volume. + // +kubebuilder:validation:Required Name string `json:"name"` // SizeGiB is the size of the block device in gibibytes (GiB). + // +kubebuilder:validation:Required + // +kubebuilder:validation:Minimum:=1 SizeGiB int `json:"sizeGiB"` // Storage specifies the storage type of the block device and diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index fe06c1285c..e0e7e28ad2 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -221,7 +221,7 @@ func (in *BlockDeviceStorage) DeepCopyInto(out *BlockDeviceStorage) { if in.Volume != nil { in, out := &in.Volume, &out.Volume *out = new(BlockDeviceVolume) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -238,6 +238,11 @@ func (in *BlockDeviceStorage) DeepCopy() *BlockDeviceStorage { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BlockDeviceVolume) DeepCopyInto(out *BlockDeviceVolume) { *out = *in + if in.AvailabilityZone != nil { + in, out := &in.AvailabilityZone, &out.AvailabilityZone + *out = new(VolumeAvailabilityZone) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BlockDeviceVolume. @@ -990,7 +995,7 @@ func (in *OpenStackMachineSpec) DeepCopyInto(out *OpenStackMachineSpec) { if in.RootVolume != nil { in, out := &in.RootVolume, &out.RootVolume *out = new(RootVolume) - **out = **in + (*in).DeepCopyInto(*out) } if in.AdditionalBlockDevices != nil { in, out := &in.AdditionalBlockDevices, &out.AdditionalBlockDevices @@ -1393,6 +1398,7 @@ func (in *ResolvedPortSpecFields) DeepCopy() *ResolvedPortSpecFields { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RootVolume) DeepCopyInto(out *RootVolume) { *out = *in + in.BlockDeviceVolume.DeepCopyInto(&out.BlockDeviceVolume) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RootVolume. @@ -1742,3 +1748,23 @@ func (in *ValueSpec) DeepCopy() *ValueSpec { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VolumeAvailabilityZone) DeepCopyInto(out *VolumeAvailabilityZone) { + *out = *in + if in.Name != nil { + in, out := &in.Name, &out.Name + *out = new(VolumeAZName) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VolumeAvailabilityZone. +func (in *VolumeAvailabilityZone) DeepCopy() *VolumeAvailabilityZone { + if in == nil { + return nil + } + out := new(VolumeAvailabilityZone) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index 10074be068..fd240c51f5 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5125,10 +5125,12 @@ spec: Also, this name will be used for tagging the block device. Information about the block device tag can be obtained from the OpenStack metadata API or the config drive. + Name cannot be 'root', which is reserved for the root volume. type: string sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). + minimum: 1 type: integer storage: description: |- @@ -5146,12 +5148,36 @@ spec: properties: availabilityZone: description: |- - AvailabilityZone is the volume availability zone to create the volume in. - If omitted, the availability zone of the server will be used. - The availability zone must NOT contain spaces otherwise it will lead to volume that belongs - to this availability zone register failure, see kubernetes/cloud-provider-openstack#1379 for - further information. - type: string + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from is 'Name' + or default + rule: '!has(self.from) || self.from == ''Name'' + ? has(self.name) : !has(self.name)' type: description: |- Type is the Cinder volume type of the volume. @@ -5672,11 +5698,49 @@ spec: description: The volume metadata to boot from properties: availabilityZone: - type: string - diskSize: + description: |- + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from is 'Name' or default + rule: '!has(self.from) || self.from == ''Name'' ? has(self.name) + : !has(self.name)' + sizeGiB: + description: SizeGiB is the size of the block device in + gibibytes (GiB). + minimum: 1 type: integer - volumeType: + type: + description: |- + Type is the Cinder volume type of the volume. + If omitted, the default Cinder volume type that is configured in the OpenStack cloud + will be used. type: string + required: + - sizeGiB type: object securityGroups: description: The names of the security groups to assign to diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index 956f46dc3a..96680265b6 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2560,10 +2560,12 @@ spec: Also, this name will be used for tagging the block device. Information about the block device tag can be obtained from the OpenStack metadata API or the config drive. + Name cannot be 'root', which is reserved for the root volume. type: string sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). + minimum: 1 type: integer storage: description: |- @@ -2581,12 +2583,36 @@ spec: properties: availabilityZone: description: |- - AvailabilityZone is the volume availability zone to create the volume in. - If omitted, the availability zone of the server will be used. - The availability zone must NOT contain spaces otherwise it will lead to volume that belongs - to this availability zone register failure, see kubernetes/cloud-provider-openstack#1379 for - further information. - type: string + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from + is 'Name' or default + rule: '!has(self.from) || self.from + == ''Name'' ? has(self.name) : !has(self.name)' type: description: |- Type is the Cinder volume type of the volume. @@ -3116,11 +3142,50 @@ spec: description: The volume metadata to boot from properties: availabilityZone: - type: string - diskSize: + description: |- + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from is 'Name' + or default + rule: '!has(self.from) || self.from == ''Name'' + ? has(self.name) : !has(self.name)' + sizeGiB: + description: SizeGiB is the size of the block + device in gibibytes (GiB). + minimum: 1 type: integer - volumeType: + type: + description: |- + Type is the Cinder volume type of the volume. + If omitted, the default Cinder volume type that is configured in the OpenStack cloud + will be used. type: string + required: + - sizeGiB type: object securityGroups: description: The names of the security groups to assign diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index b518044f8f..243087e475 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1732,10 +1732,12 @@ spec: Also, this name will be used for tagging the block device. Information about the block device tag can be obtained from the OpenStack metadata API or the config drive. + Name cannot be 'root', which is reserved for the root volume. type: string sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). + minimum: 1 type: integer storage: description: |- @@ -1753,12 +1755,35 @@ spec: properties: availabilityZone: description: |- - AvailabilityZone is the volume availability zone to create the volume in. - If omitted, the availability zone of the server will be used. - The availability zone must NOT contain spaces otherwise it will lead to volume that belongs - to this availability zone register failure, see kubernetes/cloud-provider-openstack#1379 for - further information. - type: string + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from is 'Name' or default + rule: '!has(self.from) || self.from == ''Name'' ? + has(self.name) : !has(self.name)' type: description: |- Type is the Cinder volume type of the volume. @@ -2272,11 +2297,49 @@ spec: description: The volume metadata to boot from properties: availabilityZone: - type: string - diskSize: + description: |- + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from is 'Name' or default + rule: '!has(self.from) || self.from == ''Name'' ? has(self.name) + : !has(self.name)' + sizeGiB: + description: SizeGiB is the size of the block device in gibibytes + (GiB). + minimum: 1 type: integer - volumeType: + type: + description: |- + Type is the Cinder volume type of the volume. + If omitted, the default Cinder volume type that is configured in the OpenStack cloud + will be used. type: string + required: + - sizeGiB type: object securityGroups: description: The names of the security groups to assign to the instance diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index b6d3d4d62a..7e5c5bbe6f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -1404,10 +1404,12 @@ spec: Also, this name will be used for tagging the block device. Information about the block device tag can be obtained from the OpenStack metadata API or the config drive. + Name cannot be 'root', which is reserved for the root volume. type: string sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). + minimum: 1 type: integer storage: description: |- @@ -1425,12 +1427,36 @@ spec: properties: availabilityZone: description: |- - AvailabilityZone is the volume availability zone to create the volume in. - If omitted, the availability zone of the server will be used. - The availability zone must NOT contain spaces otherwise it will lead to volume that belongs - to this availability zone register failure, see kubernetes/cloud-provider-openstack#1379 for - further information. - type: string + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from is 'Name' + or default + rule: '!has(self.from) || self.from == ''Name'' + ? has(self.name) : !has(self.name)' type: description: |- Type is the Cinder volume type of the volume. @@ -1951,11 +1977,49 @@ spec: description: The volume metadata to boot from properties: availabilityZone: - type: string - diskSize: + description: |- + AvailabilityZone is the volume availability zone to create the volume + in. If not specified, the volume will be created without an explicit + availability zone. + properties: + from: + default: Name + description: |- + From specifies where we will obtain the availability zone for the + volume. The options are "Name" and "Machine". If "Name" is specified + then the Name field must also be specified. If "Machine" is specified + the volume will use the value of FailureDomain, if any, from the + associated Machine. + enum: + - Name + - Machine + type: string + name: + description: |- + Name is the name of a volume availability zone to use. It is required + if From is "Name". The volume availability zone name may not contain + spaces. + minLength: 1 + pattern: ^[^ ]+$ + type: string + type: object + x-kubernetes-validations: + - message: name is required when from is 'Name' or default + rule: '!has(self.from) || self.from == ''Name'' ? has(self.name) + : !has(self.name)' + sizeGiB: + description: SizeGiB is the size of the block device in + gibibytes (GiB). + minimum: 1 type: integer - volumeType: + type: + description: |- + Type is the Cinder volume type of the volume. + If omitted, the default Cinder volume type that is configured in the OpenStack cloud + will be used. type: string + required: + - sizeGiB type: object securityGroups: description: The names of the security groups to assign to diff --git a/controllers/openstackmachine_controller_test.go b/controllers/openstackmachine_controller_test.go index c77220bc4c..b191111702 100644 --- a/controllers/openstackmachine_controller_test.go +++ b/controllers/openstackmachine_controller_test.go @@ -366,7 +366,7 @@ func Test_reconcileDelete(t *testing.T) { Spec: infrav1.OpenStackMachineSpec{ Image: defaultImage, RootVolume: &infrav1.RootVolume{ - Size: 50, + SizeGiB: 50, }, }, Status: infrav1.OpenStackMachineStatus{ @@ -393,7 +393,7 @@ func Test_reconcileDelete(t *testing.T) { Spec: infrav1.OpenStackMachineSpec{ Image: defaultImage, RootVolume: &infrav1.RootVolume{ - Size: 50, + SizeGiB: 50, }, }, Status: infrav1.OpenStackMachineStatus{ diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md index 19dfeb31e3..8646bdebfa 100644 --- a/docs/book/src/api/v1beta1/api.md +++ b/docs/book/src/api/v1beta1/api.md @@ -977,7 +977,8 @@ If the block device is a volume, the Cinder volume will be named as a combination of the machine name and this name. Also, this name will be used for tagging the block device. Information about the block device tag can be obtained from the OpenStack -metadata API or the config drive.

+metadata API or the config drive. +Name cannot be ‘root’, which is reserved for the root volume.

@@ -1397,7 +1398,8 @@ BlockDeviceVolume

(Appears on: -BlockDeviceStorage) +BlockDeviceStorage, +RootVolume)

BlockDeviceVolume contains additional storage options for a volume block device.

@@ -1428,16 +1430,16 @@ will be used.

availabilityZone
-string + +VolumeAvailabilityZone + (Optional) -

AvailabilityZone is the volume availability zone to create the volume in. -If omitted, the availability zone of the server will be used. -The availability zone must NOT contain spaces otherwise it will lead to volume that belongs -to this availability zone register failure, see kubernetes/cloud-provider-openstack#1379 for -further information.

+

AvailabilityZone is the volume availability zone to create the volume +in. If not specified, the volume will be created without an explicit +availability zone.

@@ -4310,32 +4312,28 @@ depends on the specific OpenStack implementation.

-diskSize
+sizeGiB
int +

SizeGiB is the size of the block device in gibibytes (GiB).

-volumeType
- -string - - - - - - - -availabilityZone
+BlockDeviceVolume
-string + +BlockDeviceVolume + +

+(Members of BlockDeviceVolume are embedded into this type.) +

@@ -5260,6 +5258,90 @@ string +

VolumeAZName +(string alias)

+

+(Appears on: +VolumeAvailabilityZone) +

+

+

VolumeAZName is the name of a volume availability zone. It may not contain spaces.

+

+

VolumeAZSource +(string alias)

+

+(Appears on: +VolumeAvailabilityZone) +

+

+

VolumeAZSource specifies where to obtain the availability zone for a volume.

+

+ + + + + + + + + + + + +
ValueDescription

"Machine"

"Name"

+

VolumeAvailabilityZone +

+

+(Appears on: +BlockDeviceVolume) +

+

+

VolumeAvailabilityZone specifies the availability zone for a volume.

+

+ + + + + + + + + + + + + + + + + +
FieldDescription
+from
+ + +VolumeAZSource + + +		 +(Optional) +

From specifies where we will obtain the availability zone for the +volume. The options are “Name” and “Machine”. If “Name” is specified +then the Name field must also be specified. If “Machine” is specified +the volume will use the value of FailureDomain, if any, from the +associated Machine.

+
+name
+ + +VolumeAZName + + +		 +(Optional) +

Name is the name of a volume availability zone to use. It is required +if From is “Name”. The volume availability zone name may not contain +spaces.

+

Generated with gen-crd-api-reference-docs. diff --git a/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md b/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md index 598873cb0d..975d226275 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md @@ -15,6 +15,8 @@ - [Removal of imageUUID](#removal-of-imageuuid) - [Change to instanceID](#change-to-instanceid) - [Changes to ports](#changes-to-ports) + - [Change to `availabilityZone` for `additionalBlockDevices`](#change-to-availabilityzone-for-additionalblockdevices) + - [Changes to rootVolume](#changes-to-rootvolume) - [Addition of floatingIPPoolRef](#addition-of-floatingippoolref) - [`OpenStackCluster`](#openstackcluster) - [Removal of cloudName](#removal-of-cloudname-1) @@ -149,6 +151,101 @@ The following fields in `PortOpts` are renamed in order to keep them consistent * `hostId` becomes `hostID` * `allowedCidrs` becomes `allowedCIDRs` +#### Change to `availabilityZone` for `additionalBlockDevices` + +Prior to v1beta1 it was not possible to specify a volume with no availability zone, allowing Cinder to allocate the volume according to its default policy. Previously, if no availability zone was given CAPO would use the value of `failureDomain` from the associated `Machine`. + +In v1beta1 this default changes. In v1beta1, if no `availabilityZone` is specified, CAPO will not specify an availability zone for the created volume. + +To support this whilst continuing to support the previous behaviour, `availabilityZone` becomes a struct with 2 fields: + +`from` specifies where we get the availability zone from. It can be either `Name`, or `Machine`. It can be omitted, and the default is `Name`, in which case the value of the `name` field will be used. Specifying `Machine` provides the previous default behaviour. + +`name` is a specific availability zone to use. + +For example, a volume with an explicit AZ in: +* v1alpha7 + ```yaml + - name: extra + sizeGiB: 1000 + storage: + type: volume + volume: + type: ceph + availabilityZone: az1 + ``` +* v1beta1 + ```yaml + - name: extra + sizeGiB: 1000 + storage: + type: volume + volume: + type: ceph + availabilityZone: + name: az1 + ``` + +A volume which uses the value of `failureDomain` from the `Machine`: +* v1alpha7 + ```yaml + - name: extra + sizeGiB: 1000 + storage: + type: volume + volume: + type: ceph + ``` +* v1beta1 + ```yaml + - name: extra + sizeGiB: 1000 + storage: + type: volume + volume: + type: ceph + availabilityZone: + from: Machine + ``` + +A volume which explicitly has no availability zone: +* v1alpha7: Not possible +* v1beta1 + ```yaml + - name: extra + sizeGiB: 1000 + storage: + type: volume + volume: + type: ceph + ``` + +Volumes upgraded from prior versions will retain their current semantics, so volumes which previously specified no availabilityZone will have an availabilityZone with `from: Machine` added. + +#### Changes to rootVolume + +The `rootVolume` field has been updated to be consistent with the volume specification for `additionalBlockDevices`. Specifically: + +* `rootVolume.diskSize` becomes `rootVolume.sizeGiB` +* `rootVolume.volumeType` becomes `rootVolume.Type` +* `rootVolume.availabilityZone` becomes a struct with the same semantics as `availabilityZone` in `additionalBlockDevices`, described above. + +For example: +* v1alpha7 + ```yaml + rootVolume: + diskSize: 50 + volumeType: ceph + ``` +* v1beta1 + ```yaml + rootVolume: + sizeGiB: 50 + type: ceph + availabilityZone: + from: Machine + ``` + #### Addition of floatingIPPoolRef A new field, FloatingIPPoolRef, has been introduced. It is important to note that this feature requires the existence of an IPPool to operate seamlessly. This new field references an IPPool whice will be used for floating IP allocation. diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index ee808c05d6..cb7146ca4d 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -131,7 +131,7 @@ func volumeName(instanceName string, nameSuffix string) string { } func hasRootVolume(instanceSpec *InstanceSpec) bool { - return instanceSpec.RootVolume != nil && instanceSpec.RootVolume.Size > 0 + return instanceSpec.RootVolume != nil && instanceSpec.RootVolume.SizeGiB > 0 } func (s *Service) getVolumeByName(name string) (*volumes.Volume, error) { @@ -204,30 +204,46 @@ func (s *Service) waitForVolume(volumeID string, timeout time.Duration, retryInt // getOrCreateVolumeBuilder gets or creates a volume with the given options. It returns the volume that already exists or the newly created one. // It returns an error if the volume creation failed or if the expected volume is different from the one that already exists. -func (s *Service) getOrCreateVolumeBuilder(eventObject runtime.Object, instanceSpec *InstanceSpec, blockDevice infrav1.AdditionalBlockDevice, imageID string, description string) (*volumes.Volume, error) { - var volumeType string - availabilityZone := instanceSpec.FailureDomain - - if blockDevice.Storage.Volume != nil { - if blockDevice.Storage.Volume.AvailabilityZone != "" { - availabilityZone = blockDevice.Storage.Volume.AvailabilityZone - } - volumeType = blockDevice.Storage.Volume.Type - } +func (s *Service) getOrCreateVolumeBuilder(eventObject runtime.Object, instanceSpec *InstanceSpec, blockDeviceSpec *infrav1.AdditionalBlockDevice, imageID string, description string) (*volumes.Volume, error) { + availabilityZone, volType := resolveVolumeOpts(instanceSpec, blockDeviceSpec.Storage.Volume) createOpts := volumes.CreateOpts{ - Name: volumeName(instanceSpec.Name, blockDevice.Name), + Name: volumeName(instanceSpec.Name, blockDeviceSpec.Name), Description: description, - Size: blockDevice.SizeGiB, + Size: blockDeviceSpec.SizeGiB, ImageID: imageID, Multiattach: false, AvailabilityZone: availabilityZone, - VolumeType: volumeType, + VolumeType: volType, } return s.getOrCreateVolume(eventObject, createOpts) } +func resolveVolumeOpts(instanceSpec *InstanceSpec, volumeOpts *infrav1.BlockDeviceVolume) (az, volType string) { + if volumeOpts == nil { + return + } + + volType = volumeOpts.Type + + volumeAZ := volumeOpts.AvailabilityZone + if volumeAZ == nil { + return + } + + switch volumeAZ.From { + case "", infrav1.VolumeAZFromName: + // volumeAZ.Name is nil case should have been caught by validation + if volumeAZ.Name != nil { + az = string(*volumeAZ.Name) + } + case infrav1.VolumeAZFromMachine: + az = instanceSpec.FailureDomain + } + return +} + // getBlockDevices returns a list of block devices that were created and attached to the instance. It returns an error // if the root volume or any of the additional block devices could not be created. func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *InstanceSpec, imageID string, timeout time.Duration, retryInterval time.Duration) ([]bootfromvolume.BlockDevice, error) { @@ -236,18 +252,15 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst if hasRootVolume(instanceSpec) { rootVolumeToBlockDevice := infrav1.AdditionalBlockDevice{ Name: "root", - SizeGiB: instanceSpec.RootVolume.Size, + SizeGiB: instanceSpec.RootVolume.SizeGiB, Storage: infrav1.BlockDeviceStorage{ - Type: infrav1.VolumeBlockDevice, - Volume: &infrav1.BlockDeviceVolume{ - AvailabilityZone: instanceSpec.RootVolume.AvailabilityZone, - Type: instanceSpec.RootVolume.VolumeType, - }, + Type: infrav1.VolumeBlockDevice, + Volume: &instanceSpec.RootVolume.BlockDeviceVolume, }, } - rootVolume, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, rootVolumeToBlockDevice, imageID, fmt.Sprintf("Root volume for %s", instanceSpec.Name)) + rootVolume, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, &rootVolumeToBlockDevice, imageID, fmt.Sprintf("Root volume for %s", instanceSpec.Name)) if err != nil { - return []bootfromvolume.BlockDevice{}, err + return nil, err } blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ SourceType: bootfromvolume.SourceVolume, @@ -266,7 +279,9 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst }) } - for _, blockDeviceSpec := range instanceSpec.AdditionalBlockDevices { + for i := range instanceSpec.AdditionalBlockDevices { + blockDeviceSpec := instanceSpec.AdditionalBlockDevices[i] + var bdUUID string var localDiskSizeGiB int var sourceType bootfromvolume.SourceType @@ -274,13 +289,13 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst // There is also a validation in the openstackmachine webhook. if blockDeviceSpec.Name == "root" { - return []bootfromvolume.BlockDevice{}, fmt.Errorf("block device name 'root' is reserved") + return nil, fmt.Errorf("block device name 'root' is reserved") } if blockDeviceSpec.Storage.Type == infrav1.VolumeBlockDevice { - blockDevice, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, blockDeviceSpec, "", fmt.Sprintf("Additional block device for %s", instanceSpec.Name)) + blockDevice, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, &blockDeviceSpec, "", fmt.Sprintf("Additional block device for %s", instanceSpec.Name)) if err != nil { - return []bootfromvolume.BlockDevice{}, err + return nil, err } bdUUID = blockDevice.ID sourceType = bootfromvolume.SourceVolume @@ -290,7 +305,7 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst destinationType = bootfromvolume.DestinationLocal localDiskSizeGiB = blockDeviceSpec.SizeGiB } else { - return []bootfromvolume.BlockDevice{}, fmt.Errorf("invalid block device type %s", blockDeviceSpec.Storage.Type) + return nil, fmt.Errorf("invalid block device type %s", blockDeviceSpec.Storage.Type) } blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ @@ -309,7 +324,7 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst for _, bd := range blockDevices { if bd.SourceType == bootfromvolume.SourceVolume { if err := s.waitForVolume(bd.UUID, timeout, retryInterval); err != nil { - return []bootfromvolume.BlockDevice{}, fmt.Errorf("volume %s did not become available: %w", bd.UUID, err) + return nil, fmt.Errorf("volume %s did not become available: %w", bd.UUID, err) } } } diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 54677279aa..cc6335027e 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -331,7 +331,7 @@ func TestService_ReconcileInstance(t *testing.T) { getInstanceSpec: func() *InstanceSpec { s := getDefaultInstanceSpec() s.RootVolume = &infrav1.RootVolume{ - Size: 50, + SizeGiB: 50, } return s }, @@ -341,12 +341,11 @@ func TestService_ReconcileInstance(t *testing.T) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ - Size: 50, - AvailabilityZone: failureDomain, - Description: fmt.Sprintf("Root volume for %s", openStackMachineName), - Name: fmt.Sprintf("%s-root", openStackMachineName), - ImageID: imageUUID, - Multiattach: false, + Size: 50, + Description: fmt.Sprintf("Root volume for %s", openStackMachineName), + Name: fmt.Sprintf("%s-root", openStackMachineName), + ImageID: imageUUID, + Multiattach: false, }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) expectVolumePollSuccess(r.volume, rootVolumeUUID) @@ -373,10 +372,13 @@ func TestService_ReconcileInstance(t *testing.T) { getInstanceSpec: func() *InstanceSpec { s := getDefaultInstanceSpec() s.RootVolume = &infrav1.RootVolume{ - Size: 50, - AvailabilityZone: "test-alternate-az", - VolumeType: "test-volume-type", + SizeGiB: 50, } + azName := infrav1.VolumeAZName("test-alternate-az") + s.RootVolume.AvailabilityZone = &infrav1.VolumeAvailabilityZone{ + Name: &azName, + } + s.RootVolume.Type = "test-volume-type" return s }, expect: func(r *recorders) { @@ -414,12 +416,16 @@ func TestService_ReconcileInstance(t *testing.T) { wantErr: false, }, { - name: "Boot from volume failure cleans up ports", + name: "Boot from volume with AZ from machine", getInstanceSpec: func() *InstanceSpec { s := getDefaultInstanceSpec() s.RootVolume = &infrav1.RootVolume{ - Size: 50, + SizeGiB: 50, } + s.RootVolume.AvailabilityZone = &infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromMachine, + } + s.RootVolume.Type = "test-volume-type" return s }, expect: func(r *recorders) { @@ -430,11 +436,53 @@ func TestService_ReconcileInstance(t *testing.T) { r.volume.CreateVolume(volumes.CreateOpts{ Size: 50, AvailabilityZone: failureDomain, + VolumeType: "test-volume-type", Description: fmt.Sprintf("Root volume for %s", openStackMachineName), Name: fmt.Sprintf("%s-root", openStackMachineName), ImageID: imageUUID, Multiattach: false, }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) + expectVolumePollSuccess(r.volume, rootVolumeUUID) + + createMap := getDefaultServerMap() + serverMap := createMap["server"].(map[string]interface{}) + serverMap["imageRef"] = "" + serverMap["block_device_mapping_v2"] = []map[string]interface{}{ + { + "delete_on_termination": true, + "destination_type": "volume", + "source_type": "volume", + "uuid": rootVolumeUUID, + "boot_index": float64(0), + }, + } + expectCreateServer(r.compute, createMap, false) + + // Don't delete ports because the server is created: DeleteInstance will do it + }, + wantErr: false, + }, + { + name: "Boot from volume failure cleans up ports", + getInstanceSpec: func() *InstanceSpec { + s := getDefaultInstanceSpec() + s.RootVolume = &infrav1.RootVolume{ + SizeGiB: 50, + } + return s + }, + expect: func(r *recorders) { + expectDefaultFlavor(r.compute) + + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). + Return([]volumes.Volume{}, nil) + r.volume.CreateVolume(volumes.CreateOpts{ + Size: 50, + Description: fmt.Sprintf("Root volume for %s", openStackMachineName), + Name: fmt.Sprintf("%s-root", openStackMachineName), + ImageID: imageUUID, + Multiattach: false, + }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) expectVolumePoll(r.volume, rootVolumeUUID, []string{"creating", "error"}) }, wantErr: true, @@ -444,7 +492,7 @@ func TestService_ReconcileInstance(t *testing.T) { getInstanceSpec: func() *InstanceSpec { s := getDefaultInstanceSpec() s.RootVolume = &infrav1.RootVolume{ - Size: 50, + SizeGiB: 50, } s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { @@ -473,24 +521,22 @@ func TestService_ReconcileInstance(t *testing.T) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ - Size: 50, - AvailabilityZone: failureDomain, - Description: fmt.Sprintf("Root volume for %s", openStackMachineName), - Name: fmt.Sprintf("%s-root", openStackMachineName), - ImageID: imageUUID, - Multiattach: false, + Size: 50, + Description: fmt.Sprintf("Root volume for %s", openStackMachineName), + Name: fmt.Sprintf("%s-root", openStackMachineName), + ImageID: imageUUID, + Multiattach: false, }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) expectVolumePollSuccess(r.volume, rootVolumeUUID) r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ - Size: 50, - AvailabilityZone: failureDomain, - Description: fmt.Sprintf("Additional block device for %s", openStackMachineName), - Name: fmt.Sprintf("%s-etcd", openStackMachineName), - Multiattach: false, - VolumeType: "test-volume-type", + Size: 50, + Description: fmt.Sprintf("Additional block device for %s", openStackMachineName), + Name: fmt.Sprintf("%s-etcd", openStackMachineName), + Multiattach: false, + VolumeType: "test-volume-type", }).Return(&volumes.Volume{ID: additionalBlockDeviceVolumeUUID}, nil) expectVolumePollSuccess(r.volume, additionalBlockDeviceVolumeUUID) @@ -559,12 +605,11 @@ func TestService_ReconcileInstance(t *testing.T) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ - Size: 50, - AvailabilityZone: failureDomain, - Description: fmt.Sprintf("Additional block device for %s", openStackMachineName), - Name: fmt.Sprintf("%s-etcd", openStackMachineName), - Multiattach: false, - VolumeType: "test-volume-type", + Size: 50, + Description: fmt.Sprintf("Additional block device for %s", openStackMachineName), + Name: fmt.Sprintf("%s-etcd", openStackMachineName), + Multiattach: false, + VolumeType: "test-volume-type", }).Return(&volumes.Volume{ID: additionalBlockDeviceVolumeUUID}, nil) expectVolumePollSuccess(r.volume, additionalBlockDeviceVolumeUUID) @@ -605,6 +650,7 @@ func TestService_ReconcileInstance(t *testing.T) { name: "Additional block device success with explicit AZ", getInstanceSpec: func() *InstanceSpec { s := getDefaultInstanceSpec() + azName := infrav1.VolumeAZName("test-alternate-az") s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { Name: "etcd", @@ -612,8 +658,10 @@ func TestService_ReconcileInstance(t *testing.T) { Storage: infrav1.BlockDeviceStorage{ Type: "Volume", Volume: &infrav1.BlockDeviceVolume{ - AvailabilityZone: "test-alternate-az", - Type: "test-volume-type", + Type: "test-volume-type", + AvailabilityZone: &infrav1.VolumeAvailabilityZone{ + Name: &azName, + }, }, }, }, diff --git a/pkg/utils/conversioncommon/volumeavailabilityzone.go b/pkg/utils/conversioncommon/volumeavailabilityzone.go new file mode 100644 index 0000000000..520a213a01 --- /dev/null +++ b/pkg/utils/conversioncommon/volumeavailabilityzone.go @@ -0,0 +1,62 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package conversioncommon + +import ( + "k8s.io/apimachinery/pkg/conversion" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" +) + +func Convert_string_To_Pointer_v1beta1_VolumeAvailabilityZone(in *string, out **infrav1.VolumeAvailabilityZone, _ conversion.Scope) error { + switch *in { + case "": + *out = &infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromMachine, + } + default: + azName := infrav1.VolumeAZName(*in) + *out = &infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromName, + Name: &azName, + } + } + + return nil +} + +func Convert_Pointer_v1beta1_VolumeAvailabilityZone_To_string(in **infrav1.VolumeAvailabilityZone, out *string, _ conversion.Scope) error { + // This is a lossy: can't specify no AZ prior to v1beta1 + if *in == nil { + *out = "" + return nil + } + + switch (*in).From { + case "", infrav1.VolumeAZFromName: + name := (*in).Name + if name != nil { + *out = string(*name) + } else { + *out = "" + } + case infrav1.VolumeAZFromMachine: + *out = "" + } + + return nil +} diff --git a/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml b/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml index fe803f27af..9db70c86a6 100644 --- a/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml +++ b/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml @@ -2,7 +2,9 @@ - op: add path: /spec/template/spec/rootVolume value: - diskSize: 25 + sizeGiB: 25 + availabilityZone: + from: Machine - op: add path: /spec/template/spec/additionalBlockDevices value: @@ -10,6 +12,9 @@ sizeGiB: 1 storage: type: Volume + volume: + availabilityZone: + from: Machine - name: etcd sizeGiB: 1 storage: diff --git a/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml b/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml index 83029a10f6..6d46c42e35 100644 --- a/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml +++ b/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml @@ -2,9 +2,10 @@ - op: add path: /spec/template/spec/rootVolume value: - diskSize: 25 - volumeType: ${OPENSTACK_VOLUME_TYPE_ALT} - availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} + sizeGiB: 25 + type: ${OPENSTACK_VOLUME_TYPE_ALT} + availabilityZone: + name: ${OPENSTACK_FAILURE_DOMAIN} - op: add path: /spec/template/spec/additionalBlockDevices value: @@ -14,7 +15,8 @@ type: Volume volume: type: ${OPENSTACK_VOLUME_TYPE_ALT} - availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} + availabilityZone: + name: ${OPENSTACK_FAILURE_DOMAIN} - name: etcd sizeGiB: 1 storage: diff --git a/test/e2e/suites/apivalidations/openstackmachine_test.go b/test/e2e/suites/apivalidations/openstackmachine_test.go index 4e888a3a41..49a59444fe 100644 --- a/test/e2e/suites/apivalidations/openstackmachine_test.go +++ b/test/e2e/suites/apivalidations/openstackmachine_test.go @@ -29,26 +29,30 @@ import ( var _ = Describe("OpenStackMachine API validations", func() { var namespace *corev1.Namespace - var machine *infrav1.OpenStackMachine - - BeforeEach(func() { - namespace = createNamespace() + defaultMachine := func() *infrav1.OpenStackMachine { // Initialise a basic machine object in the correct namespace - machine = &infrav1.OpenStackMachine{ + machine := &infrav1.OpenStackMachine{ Spec: infrav1.OpenStackMachineSpec{ Image: infrav1.ImageParam{Filter: &infrav1.ImageFilter{Name: pointer.String("test-image")}}, }, } machine.Namespace = namespace.Name machine.GenerateName = "machine-" + return machine + } + + BeforeEach(func() { + namespace = createNamespace() }) It("should allow the smallest permissible machine spec", func() { - Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "OpenStackMachine creation should succeed") + Expect(k8sClient.Create(ctx, defaultMachine())).To(Succeed(), "OpenStackMachine creation should succeed") }) It("should only allow the providerID to be set once", func() { + machine := defaultMachine() + By("Creating a bare machine") Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "OpenStackMachine creation should succeed") @@ -62,6 +66,8 @@ var _ = Describe("OpenStackMachine API validations", func() { }) It("should not allow server metadata to exceed 255 characters", func() { + machine := defaultMachine() + By("Creating a machine with a metadata key that is too long") machine.Spec.ServerMetadata = []infrav1.ServerMetadata{ { @@ -89,4 +95,187 @@ var _ = Describe("OpenStackMachine API validations", func() { } Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "Creating a machine with max metadata key and value should succeed") }) + + Context("volumes", func() { + It("should not allow volume with zero size", func() { + machine := defaultMachine() + machine.Spec.RootVolume = &infrav1.RootVolume{ + SizeGiB: 0, + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a zero size root volume should fail") + + machine = defaultMachine() + machine.Spec.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "test-volume", + SizeGiB: 0, + }, + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a zero size additional block device should fail") + }) + + /* FIXME: These tests are failing + It("should not allow additional volume with empty name", func() { + machine.Spec.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "", + SizeGiB: 1, + }, + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with an empty name additional block device should fail") + }) + + It("should not allow additional volume with name root", func() { + machine.Spec.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "root", + SizeGiB: 1, + }, + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a root named additional block device should fail") + }) + */ + + It("should not allow additional volume with duplicate name", func() { + machine := defaultMachine() + machine.Spec.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "test-volume", + SizeGiB: 1, + }, + { + Name: "test-volume", + SizeGiB: 2, + }, + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with duplicate named additional block device should fail") + }) + + defaultMachineWithRootVolumeAZ := func(az *infrav1.VolumeAvailabilityZone) *infrav1.OpenStackMachine { + machine := defaultMachine() + machine.Spec.RootVolume = &infrav1.RootVolume{ + SizeGiB: 1, + } + machine.Spec.RootVolume.AvailabilityZone = az + return machine + } + + defaultMachineWithAdditionBlockDeviceAZ := func(az *infrav1.VolumeAvailabilityZone) *infrav1.OpenStackMachine { + machine := defaultMachine() + machine.Spec.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "test-volume", + SizeGiB: 1, + Storage: infrav1.BlockDeviceStorage{ + Type: infrav1.VolumeBlockDevice, + Volume: &infrav1.BlockDeviceVolume{ + AvailabilityZone: az, + }, + }, + }, + } + return machine + } + + It("should allow volume with defaulted AZ from", func() { + azName := infrav1.VolumeAZName("test-az") + az := infrav1.VolumeAvailabilityZone{ + Name: &azName, + } + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "Creating a machine with a root volume with an availability zone should succeed") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "Creating a machine with an additional block device with an availability zone should succeed") + }) + + It("should allow volume with AZ from Name", func() { + azName := infrav1.VolumeAZName("test-az") + az := infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromName, + Name: &azName, + } + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "Creating a machine with a root volume with an availability zone should succeed") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "Creating a machine with an additional block device with an availability zone should succeed") + }) + + It("should allow volume AZ from Machine", func() { + az := infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromMachine, + } + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "Creating a machine with a root volume with an availability zone should succeed") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "Creating a machine with an additional block device with an availability zone should succeed") + }) + + It("should not allow volume AZ with invalid from", func() { + az := infrav1.VolumeAvailabilityZone{ + From: "invalid", + } + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a root volume with an invalid availability zone should fail") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with an additional block device with an invalid availability zone should fail") + }) + + It("should not allow empty volume AZ", func() { + az := infrav1.VolumeAvailabilityZone{} + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a root volume with an empty availability zone should fail") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with an additional block device with an empty availability zone should fail") + }) + + It("should not allow volume AZ from Name with missing name", func() { + az := infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromName, + } + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a root volume with a missing name availability zone should fail") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with an additional block device with a missing name availability zone should fail") + }) + + It("should not allow volume AZ from Machine with name", func() { + azName := infrav1.VolumeAZName("test-az") + az := infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromMachine, + Name: &azName, + } + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a root volume with a name availability zone should fail") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with an additional block device with a name availability zone should fail") + }) + + It("should not allow volume AZ from Name with empty name", func() { + azName := infrav1.VolumeAZName("") + az := infrav1.VolumeAvailabilityZone{ + From: infrav1.VolumeAZFromName, + Name: &azName, + } + + machine := defaultMachineWithRootVolumeAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with a root volume with an empty name availability zone should fail") + + machine = defaultMachineWithAdditionBlockDeviceAZ(&az) + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with an additional block device with an empty name availability zone should fail") + }) + }) }) diff --git a/test/helpers/fuzzerfuncs.go b/test/helpers/fuzzerfuncs.go index 063bfe9b58..8f40656831 100644 --- a/test/helpers/fuzzerfuncs.go +++ b/test/helpers/fuzzerfuncs.go @@ -153,5 +153,35 @@ func InfraV1FuzzerFuncs() []interface{} { func(param *infrav1.SecurityGroupParam, c fuzz.Continue) { fuzzFilterParam(¶m.ID, ¶m.Filter, c) }, + + // Ensure VolumeAZ type is valid + func(az *infrav1.VolumeAvailabilityZone, c fuzz.Continue) { + stringWithoutSpaces := func() string { + for { + s := c.RandString() + if !strings.Contains(s, " ") && len(s) > 0 { + return s + } + } + } + + // From is defaulted + if c.RandBool() { + name := infrav1.VolumeAZName(stringWithoutSpaces()) + az.Name = &name + return + } + + // From is Name + if c.RandBool() { + az.From = infrav1.VolumeAZFromName + name := infrav1.VolumeAZName(stringWithoutSpaces()) + az.Name = &name + return + } + + // From is Machine + az.From = infrav1.VolumeAZFromMachine + }, } }