From 4bdb2b9583f31cc393786cecfd074917b77ca511 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Mon, 28 Oct 2024 12:22:21 +0000 Subject: [PATCH] Fix conversion of v1alpha6 PortOpts v1alpha6 PortOpts conversion was missing many fields, but this was hidden in the unit tests because the v1alpha6 restore function for the machine spec unconditionally restored all Ports. This change turns the unconditional restore of all ports in the machine spec into individual conversion of each port, then fixes all the resulting test failures by adding individual conversions and restores as required. --- api/v1alpha6/openstackmachine_conversion.go | 8 +- api/v1alpha6/types_conversion.go | 115 ++++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/api/v1alpha6/openstackmachine_conversion.go b/api/v1alpha6/openstackmachine_conversion.go index 47a757bd7d..8a52c5146f 100644 --- a/api/v1alpha6/openstackmachine_conversion.go +++ b/api/v1alpha6/openstackmachine_conversion.go @@ -184,8 +184,6 @@ func restorev1alpha6MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) { // PropagateUplinkStatus has been added in v1beta1. - // We restore the whole Ports since they are anyway immutable. - dst.Ports = previous.Ports dst.AdditionalBlockDevices = previous.AdditionalBlockDevices dst.ServerGroup = previous.ServerGroup dst.Image = previous.Image @@ -197,6 +195,12 @@ func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infr } } + if len(dst.Ports) == len(previous.Ports) { + for i := range dst.Ports { + restorev1beta1Port(&previous.Ports[i], &dst.Ports[i]) + } + } + if dst.RootVolume != nil && previous.RootVolume != nil { restorev1beta1BlockDeviceVolume( &previous.RootVolume.BlockDeviceVolume, diff --git a/api/v1alpha6/types_conversion.go b/api/v1alpha6/types_conversion.go index 30c8618cc8..def726cff1 100644 --- a/api/v1alpha6/types_conversion.go +++ b/api/v1alpha6/types_conversion.go @@ -330,11 +330,94 @@ func restorev1alpha6Port(previous *PortOpts, dst *PortOpts) { } } +func restorev1beta1Port(previous *infrav1.PortOpts, dst *infrav1.PortOpts) { + // PropagateUplinkStatus was not present in v1alpha6 + dst.PropagateUplinkStatus = previous.PropagateUplinkStatus + + optional.RestoreString(&previous.NameSuffix, &dst.NameSuffix) + optional.RestoreString(&previous.Description, &dst.Description) + optional.RestoreString(&previous.MACAddress, &dst.MACAddress) + + if len(dst.FixedIPs) == len(previous.FixedIPs) { + for j := range dst.FixedIPs { + prevFixedIP := &previous.FixedIPs[j] + dstFixedIP := &dst.FixedIPs[j] + + optional.RestoreString(&prevFixedIP.IPAddress, &dstFixedIP.IPAddress) + restorev1beta1SubnetParam(prevFixedIP.Subnet, dstFixedIP.Subnet) + } + } + + if len(dst.AllowedAddressPairs) == len(previous.AllowedAddressPairs) { + for j := range dst.AllowedAddressPairs { + prevAAP := &previous.AllowedAddressPairs[j] + dstAAP := &dst.AllowedAddressPairs[j] + + optional.RestoreString(&prevAAP.MACAddress, &dstAAP.MACAddress) + } + } + + optional.RestoreString(&previous.HostID, &dst.HostID) + optional.RestoreString(&previous.VNICType, &dst.VNICType) + + if previous.Profile != nil { + // A binding profile of {&false, &false} will be converted to a nil map. + // We still need to restore it, so substitute an empty BindProfile. + var dstProfile *infrav1.BindingProfile + if dst.Profile != nil { + dstProfile = dst.Profile + } else { + dstProfile = &infrav1.BindingProfile{} + dst.Profile = dstProfile + } + prevProfile := previous.Profile + + if dstProfile.OVSHWOffload == nil || !*dstProfile.OVSHWOffload { + dstProfile.OVSHWOffload = prevProfile.OVSHWOffload + } + + if dstProfile.TrustedVF == nil || !*dstProfile.TrustedVF { + dstProfile.TrustedVF = prevProfile.TrustedVF + } + } +} + func Convert_v1alpha6_PortOpts_To_v1beta1_PortOpts(in *PortOpts, out *infrav1.PortOpts, s apiconversion.Scope) error { if err := autoConvert_v1alpha6_PortOpts_To_v1beta1_PortOpts(in, out, s); err != nil { return err } + // Copy members of ResolvedPortSpecFields + var allowedAddressPairs []infrav1.AddressPair + if len(in.AllowedAddressPairs) > 0 { + allowedAddressPairs = make([]infrav1.AddressPair, len(in.AllowedAddressPairs)) + for i := range in.AllowedAddressPairs { + aap := &in.AllowedAddressPairs[i] + allowedAddressPairs[i] = infrav1.AddressPair{ + MACAddress: &aap.MACAddress, + IPAddress: aap.IPAddress, + } + } + } + var valueSpecs []infrav1.ValueSpec + if len(in.ValueSpecs) > 0 { + valueSpecs = make([]infrav1.ValueSpec, len(in.ValueSpecs)) + for i, vs := range in.ValueSpecs { + valueSpecs[i] = infrav1.ValueSpec(vs) + } + } + out.AdminStateUp = in.AdminStateUp + out.AllowedAddressPairs = allowedAddressPairs + out.DisablePortSecurity = in.DisablePortSecurity + out.ValueSpecs = valueSpecs + if err := errors.Join( + optional.Convert_string_To_optional_String(&in.MACAddress, &out.MACAddress, s), + optional.Convert_string_To_optional_String(&in.HostID, &out.HostID, s), + optional.Convert_string_To_optional_String(&in.VNICType, &out.VNICType, s), + ); err != nil { + return err + } + if len(in.SecurityGroups) > 0 || len(in.SecurityGroupFilters) > 0 { out.SecurityGroups = make([]infrav1.SecurityGroupParam, len(in.SecurityGroups)+len(in.SecurityGroupFilters)) for i := range in.SecurityGroupFilters { @@ -373,6 +456,38 @@ func Convert_v1beta1_PortOpts_To_v1alpha6_PortOpts(in *infrav1.PortOpts, out *Po return err } + // Copy members of ResolvedPortSpecFields + var allowedAddressPairs []AddressPair + if len(in.AllowedAddressPairs) > 0 { + allowedAddressPairs = make([]AddressPair, len(in.AllowedAddressPairs)) + for i := range in.AllowedAddressPairs { + inAAP := &in.AllowedAddressPairs[i] + outAAP := &allowedAddressPairs[i] + if err := optional.Convert_optional_String_To_string(&inAAP.MACAddress, &outAAP.MACAddress, s); err != nil { + return err + } + outAAP.IPAddress = inAAP.IPAddress + } + } + var valueSpecs []ValueSpec + if len(in.ValueSpecs) > 0 { + valueSpecs = make([]ValueSpec, len(in.ValueSpecs)) + for i, vs := range in.ValueSpecs { + valueSpecs[i] = ValueSpec(vs) + } + } + out.AdminStateUp = in.AdminStateUp + out.AllowedAddressPairs = allowedAddressPairs + out.DisablePortSecurity = in.DisablePortSecurity + out.ValueSpecs = valueSpecs + if err := errors.Join( + optional.Convert_optional_String_To_string(&in.MACAddress, &out.MACAddress, s), + optional.Convert_optional_String_To_string(&in.HostID, &out.HostID, s), + optional.Convert_optional_String_To_string(&in.VNICType, &out.VNICType, s), + ); err != nil { + return err + } + // The auto-generated function converts v1beta1 SecurityGroup to // v1alpha6 SecurityGroup, but v1alpha6 SecurityGroupFilter is more // appropriate. Unset them and convert to SecurityGroupFilter instead.