diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index 9fb404b9e7..cd66e62bd3 100644 --- a/api/v1alpha5/conversion.go +++ b/api/v1alpha5/conversion.go @@ -205,7 +205,7 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in *i return err } - if in.ExternalNetwork.ID != "" { + if in.ExternalNetwork != nil && in.ExternalNetwork.ID != "" { out.ExternalNetworkID = in.ExternalNetwork.ID } @@ -227,6 +227,12 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in *i out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic } + if in.APIServerLoadBalancer != nil { + if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha5_APIServerLoadBalancer(in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + return err + } + } + out.CloudName = in.IdentityRef.CloudName out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name} @@ -240,7 +246,7 @@ func Convert_v1alpha5_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O } if in.ExternalNetworkID != "" { - out.ExternalNetwork = infrav1.NetworkFilter{ + out.ExternalNetwork = &infrav1.NetworkFilter{ ID: in.ExternalNetworkID, } } @@ -273,11 +279,30 @@ func Convert_v1alpha5_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O } } + if in.APIServerLoadBalancer.Enabled { + out.APIServerLoadBalancer = &infrav1.APIServerLoadBalancer{} + if err := Convert_v1alpha5_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(&in.APIServerLoadBalancer, out.APIServerLoadBalancer, s); err != nil { + return err + } + } + out.IdentityRef.CloudName = in.CloudName if in.IdentityRef != nil { out.IdentityRef.Name = in.IdentityRef.Name } + // The generated conversion function converts "" to &"" which is not what we want + if in.APIServerFloatingIP == "" { + out.APIServerFloatingIP = nil + } + if in.APIServerFixedIP == "" { + out.APIServerFixedIP = nil + } + + if in.APIServerPort != 0 { + out.APIServerPort = pointer.Int(in.APIServerPort) + } + return nil } diff --git a/api/v1alpha5/conversion_test.go b/api/v1alpha5/conversion_test.go index 1ecbf00a13..72b3c62851 100644 --- a/api/v1alpha5/conversion_test.go +++ b/api/v1alpha5/conversion_test.go @@ -40,7 +40,7 @@ func TestConvertFrom(t *testing.T) { want ctrlconversion.Convertible }{ { - name: "conversion must have conversion-data annotation", + name: "cluster conversion must have conversion-data annotation", spoke: &OpenStackCluster{}, hub: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{}, @@ -51,13 +51,13 @@ func TestConvertFrom(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"apiServerLoadBalancer\":{},\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"identityRef\":{\"cloudName\":\"\",\"name\":\"\"},\"network\":{}},\"status\":{\"ready\":false}}", + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"identityRef\":{\"cloudName\":\"\",\"name\":\"\"}},\"status\":{\"ready\":false}}", }, }, }, }, { - name: "conversion must have conversion-data annotation", + name: "cluster template conversion must have conversion-data annotation", spoke: &OpenStackClusterTemplate{}, hub: &infrav1.OpenStackClusterTemplate{ Spec: infrav1.OpenStackClusterTemplateSpec{}, @@ -72,13 +72,13 @@ func TestConvertFrom(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"apiServerLoadBalancer\":{},\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"identityRef\":{\"cloudName\":\"\",\"name\":\"\"},\"network\":{}}}}}", + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"identityRef\":{\"cloudName\":\"\",\"name\":\"\"}}}}}", }, }, }, }, { - name: "conversion must have conversion-data annotation", + name: "machine conversion must have conversion-data annotation", spoke: &OpenStackMachine{}, hub: &infrav1.OpenStackMachine{ Spec: infrav1.OpenStackMachineSpec{}, @@ -93,7 +93,7 @@ func TestConvertFrom(t *testing.T) { }, }, { - name: "conversion must have conversion-data annotation", + name: "machine template conversion must have conversion-data annotation", spoke: &OpenStackMachineTemplate{}, hub: &infrav1.OpenStackMachineTemplate{ Spec: infrav1.OpenStackMachineTemplateSpec{}, diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index 646ce0a938..3b4f8e916c 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -659,9 +659,7 @@ func Convert_v1beta1_OpenStackClusterList_To_v1alpha5_OpenStackClusterList(in *v func autoConvert_v1alpha5_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *OpenStackClusterSpec, out *v1beta1.OpenStackClusterSpec, s conversion.Scope) error { // WARNING: in.CloudName requires manual conversion: does not exist in peer-type // WARNING: in.NodeCIDR requires manual conversion: does not exist in peer-type - if err := Convert_v1alpha5_NetworkFilter_To_v1beta1_NetworkFilter(&in.Network, &out.Network, s); err != nil { - return err - } + // WARNING: in.Network requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha5.NetworkFilter vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.NetworkFilter) // WARNING: in.Subnet requires manual conversion: does not exist in peer-type // WARNING: in.DNSNameservers requires manual conversion: does not exist in peer-type if in.ExternalRouterIPs != nil { @@ -676,18 +674,26 @@ func autoConvert_v1alpha5_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(i out.ExternalRouterIPs = nil } // WARNING: in.ExternalNetworkID requires manual conversion: does not exist in peer-type - if err := Convert_v1alpha5_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(&in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + // WARNING: in.APIServerLoadBalancer requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha5.APIServerLoadBalancer vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.APIServerLoadBalancer) + if err := optional.Convert_bool_To_optional_Bool(&in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_string_To_optional_String(&in.APIServerFloatingIP, &out.APIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_string_To_optional_String(&in.APIServerFixedIP, &out.APIServerFixedIP, s); err != nil { + return err + } + if err := optional.Convert_int_To_optional_Int(&in.APIServerPort, &out.APIServerPort, s); err != nil { return err } - out.DisableAPIServerFloatingIP = in.DisableAPIServerFloatingIP - out.APIServerFloatingIP = in.APIServerFloatingIP - out.APIServerFixedIP = in.APIServerFixedIP - out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ManagedSecurityGroups) // WARNING: in.AllowAllInClusterTraffic requires manual conversion: does not exist in peer-type - out.DisablePortSecurity = in.DisablePortSecurity + if err := optional.Convert_bool_To_optional_Bool(&in.DisablePortSecurity, &out.DisablePortSecurity, s); err != nil { + return err + } out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + // WARNING: in.ControlPlaneEndpoint requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint vs *sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint) out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion @@ -705,9 +711,7 @@ func autoConvert_v1alpha5_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(i func autoConvert_v1beta1_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in *v1beta1.OpenStackClusterSpec, out *OpenStackClusterSpec, s conversion.Scope) error { // WARNING: in.ManagedSubnets requires manual conversion: does not exist in peer-type // WARNING: in.Router requires manual conversion: does not exist in peer-type - if err := Convert_v1beta1_NetworkFilter_To_v1alpha5_NetworkFilter(&in.Network, &out.Network, s); err != nil { - return err - } + // WARNING: in.Network requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.NetworkFilter vs sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha5.NetworkFilter) // WARNING: in.Subnets requires manual conversion: does not exist in peer-type // WARNING: in.NetworkMTU requires manual conversion: does not exist in peer-type if in.ExternalRouterIPs != nil { @@ -723,17 +727,25 @@ func autoConvert_v1beta1_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(i } // WARNING: in.ExternalNetwork requires manual conversion: does not exist in peer-type // WARNING: in.DisableExternalNetwork requires manual conversion: does not exist in peer-type - if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha5_APIServerLoadBalancer(&in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + // WARNING: in.APIServerLoadBalancer requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.APIServerLoadBalancer vs sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha5.APIServerLoadBalancer) + if err := optional.Convert_optional_Bool_To_bool(&in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_optional_String_To_string(&in.APIServerFloatingIP, &out.APIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_optional_String_To_string(&in.APIServerFixedIP, &out.APIServerFixedIP, s); err != nil { + return err + } + if err := optional.Convert_optional_Int_To_int(&in.APIServerPort, &out.APIServerPort, s); err != nil { return err } - out.DisableAPIServerFloatingIP = in.DisableAPIServerFloatingIP - out.APIServerFloatingIP = in.APIServerFloatingIP - out.APIServerFixedIP = in.APIServerFixedIP - out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ManagedSecurityGroups vs bool) - out.DisablePortSecurity = in.DisablePortSecurity + if err := optional.Convert_optional_Bool_To_bool(&in.DisablePortSecurity, &out.DisablePortSecurity, s); err != nil { + return err + } out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + // WARNING: in.ControlPlaneEndpoint requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint vs sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint) out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) // WARNING: in.ControlPlaneOmitAvailabilityZone requires manual conversion: does not exist in peer-type if in.Bastion != nil { diff --git a/api/v1alpha6/openstackcluster_conversion.go b/api/v1alpha6/openstackcluster_conversion.go index b8aac5aa5a..b1ecec275e 100644 --- a/api/v1alpha6/openstackcluster_conversion.go +++ b/api/v1alpha6/openstackcluster_conversion.go @@ -20,10 +20,12 @@ import ( "reflect" apiconversion "k8s.io/apimachinery/pkg/conversion" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" 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/conversion" + optional "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional" ) var _ ctrlconversion.Convertible = &OpenStackCluster{} @@ -162,14 +164,24 @@ func restorev1alpha6ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackCl func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infrav1.OpenStackClusterSpec) { // Bastion is restored separately + if dst.Network.IsEmpty() { + dst.Network = previous.Network + } + // Restore all fields except ID, which should have been copied over in conversion - dst.ExternalNetwork.Name = previous.ExternalNetwork.Name - dst.ExternalNetwork.Description = previous.ExternalNetwork.Description - dst.ExternalNetwork.ProjectID = previous.ExternalNetwork.ProjectID - dst.ExternalNetwork.Tags = previous.ExternalNetwork.Tags - dst.ExternalNetwork.TagsAny = previous.ExternalNetwork.TagsAny - dst.ExternalNetwork.NotTags = previous.ExternalNetwork.NotTags - dst.ExternalNetwork.NotTagsAny = previous.ExternalNetwork.NotTagsAny + if previous.ExternalNetwork != nil { + if dst.ExternalNetwork == nil { + dst.ExternalNetwork = &infrav1.NetworkFilter{} + } + + dst.ExternalNetwork.Name = previous.ExternalNetwork.Name + dst.ExternalNetwork.Description = previous.ExternalNetwork.Description + dst.ExternalNetwork.ProjectID = previous.ExternalNetwork.ProjectID + dst.ExternalNetwork.Tags = previous.ExternalNetwork.Tags + dst.ExternalNetwork.TagsAny = previous.ExternalNetwork.TagsAny + dst.ExternalNetwork.NotTags = previous.ExternalNetwork.NotTags + dst.ExternalNetwork.NotTagsAny = previous.ExternalNetwork.NotTagsAny + } // Restore fields not present in v1alpha6 dst.Router = previous.Router @@ -185,6 +197,21 @@ func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infr if previous.ManagedSecurityGroups != nil { dst.ManagedSecurityGroups.AllNodesSecurityGroupRules = previous.ManagedSecurityGroups.AllNodesSecurityGroupRules } + + if dst.APIServerLoadBalancer.IsZero() { + dst.APIServerLoadBalancer = previous.APIServerLoadBalancer + } + + if dst.ControlPlaneEndpoint == nil || *dst.ControlPlaneEndpoint == (clusterv1.APIEndpoint{}) { + dst.ControlPlaneEndpoint = previous.ControlPlaneEndpoint + } + + optional.RestoreString(&previous.APIServerFloatingIP, &dst.APIServerFloatingIP) + optional.RestoreString(&previous.APIServerFixedIP, &dst.APIServerFixedIP) + optional.RestoreInt(&previous.APIServerPort, &dst.APIServerPort) + optional.RestoreBool(&previous.DisableAPIServerFloatingIP, &dst.DisableAPIServerFloatingIP) + optional.RestoreBool(&previous.ControlPlaneOmitAvailabilityZone, &dst.ControlPlaneOmitAvailabilityZone) + optional.RestoreBool(&previous.DisablePortSecurity, &dst.DisablePortSecurity) } func Convert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *OpenStackClusterSpec, out *infrav1.OpenStackClusterSpec, s apiconversion.Scope) error { @@ -193,8 +220,15 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O return err } + if in.Network != (NetworkFilter{}) { + out.Network = &infrav1.NetworkFilter{} + if err := Convert_v1alpha6_NetworkFilter_To_v1beta1_NetworkFilter(&in.Network, out.Network, s); err != nil { + return err + } + } + if in.ExternalNetworkID != "" { - out.ExternalNetwork = infrav1.NetworkFilter{ + out.ExternalNetwork = &infrav1.NetworkFilter{ ID: in.ExternalNetworkID, } } @@ -227,11 +261,23 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O } } + if in.ControlPlaneEndpoint != (clusterv1.APIEndpoint{}) { + out.ControlPlaneEndpoint = &in.ControlPlaneEndpoint + } + out.IdentityRef.CloudName = in.CloudName if in.IdentityRef != nil { out.IdentityRef.Name = in.IdentityRef.Name } + apiServerLoadBalancer := &infrav1.APIServerLoadBalancer{} + if err := Convert_v1alpha6_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(&in.APIServerLoadBalancer, apiServerLoadBalancer, s); err != nil { + return err + } + if !apiServerLoadBalancer.IsZero() { + out.APIServerLoadBalancer = apiServerLoadBalancer + } + return nil } @@ -241,7 +287,13 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *i return err } - if in.ExternalNetwork.ID != "" { + if in.Network != nil { + if err := Convert_v1beta1_NetworkFilter_To_v1alpha6_NetworkFilter(in.Network, &out.Network, s); err != nil { + return err + } + } + + if in.ExternalNetwork != nil && in.ExternalNetwork.ID != "" { out.ExternalNetworkID = in.ExternalNetwork.ID } @@ -261,9 +313,19 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *i out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic } + if in.ControlPlaneEndpoint != nil { + out.ControlPlaneEndpoint = *in.ControlPlaneEndpoint + } + out.CloudName = in.IdentityRef.CloudName out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name} + if in.APIServerLoadBalancer != nil { + if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha6_APIServerLoadBalancer(in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + return err + } + } + return nil } diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index dafa54171b..b0ea770cdd 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -685,9 +685,7 @@ func Convert_v1beta1_OpenStackClusterList_To_v1alpha6_OpenStackClusterList(in *v func autoConvert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *OpenStackClusterSpec, out *v1beta1.OpenStackClusterSpec, s conversion.Scope) error { // WARNING: in.CloudName requires manual conversion: does not exist in peer-type // WARNING: in.NodeCIDR requires manual conversion: does not exist in peer-type - if err := Convert_v1alpha6_NetworkFilter_To_v1beta1_NetworkFilter(&in.Network, &out.Network, s); err != nil { - return err - } + // WARNING: in.Network requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6.NetworkFilter vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.NetworkFilter) // WARNING: in.Subnet requires manual conversion: does not exist in peer-type // WARNING: in.DNSNameservers requires manual conversion: does not exist in peer-type if in.ExternalRouterIPs != nil { @@ -702,20 +700,30 @@ func autoConvert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(i out.ExternalRouterIPs = nil } // WARNING: in.ExternalNetworkID requires manual conversion: does not exist in peer-type - if err := Convert_v1alpha6_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(&in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + // WARNING: in.APIServerLoadBalancer requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6.APIServerLoadBalancer vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.APIServerLoadBalancer) + if err := optional.Convert_bool_To_optional_Bool(&in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_string_To_optional_String(&in.APIServerFloatingIP, &out.APIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_string_To_optional_String(&in.APIServerFixedIP, &out.APIServerFixedIP, s); err != nil { + return err + } + if err := optional.Convert_int_To_optional_Int(&in.APIServerPort, &out.APIServerPort, s); err != nil { return err } - out.DisableAPIServerFloatingIP = in.DisableAPIServerFloatingIP - out.APIServerFloatingIP = in.APIServerFloatingIP - out.APIServerFixedIP = in.APIServerFixedIP - out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ManagedSecurityGroups) // WARNING: in.AllowAllInClusterTraffic requires manual conversion: does not exist in peer-type - out.DisablePortSecurity = in.DisablePortSecurity + if err := optional.Convert_bool_To_optional_Bool(&in.DisablePortSecurity, &out.DisablePortSecurity, s); err != nil { + return err + } out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + // WARNING: in.ControlPlaneEndpoint requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint vs *sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint) out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) - out.ControlPlaneOmitAvailabilityZone = in.ControlPlaneOmitAvailabilityZone + if err := optional.Convert_bool_To_optional_Bool(&in.ControlPlaneOmitAvailabilityZone, &out.ControlPlaneOmitAvailabilityZone, s); err != nil { + return err + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(v1beta1.Bastion) @@ -732,9 +740,7 @@ func autoConvert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(i func autoConvert_v1beta1_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *v1beta1.OpenStackClusterSpec, out *OpenStackClusterSpec, s conversion.Scope) error { // WARNING: in.ManagedSubnets requires manual conversion: does not exist in peer-type // WARNING: in.Router requires manual conversion: does not exist in peer-type - if err := Convert_v1beta1_NetworkFilter_To_v1alpha6_NetworkFilter(&in.Network, &out.Network, s); err != nil { - return err - } + // WARNING: in.Network requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.NetworkFilter vs sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6.NetworkFilter) // WARNING: in.Subnets requires manual conversion: does not exist in peer-type // WARNING: in.NetworkMTU requires manual conversion: does not exist in peer-type if in.ExternalRouterIPs != nil { @@ -750,19 +756,29 @@ func autoConvert_v1beta1_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(i } // WARNING: in.ExternalNetwork requires manual conversion: does not exist in peer-type // WARNING: in.DisableExternalNetwork requires manual conversion: does not exist in peer-type - if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha6_APIServerLoadBalancer(&in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + // WARNING: in.APIServerLoadBalancer requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.APIServerLoadBalancer vs sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6.APIServerLoadBalancer) + if err := optional.Convert_optional_Bool_To_bool(&in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_optional_String_To_string(&in.APIServerFloatingIP, &out.APIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_optional_String_To_string(&in.APIServerFixedIP, &out.APIServerFixedIP, s); err != nil { + return err + } + if err := optional.Convert_optional_Int_To_int(&in.APIServerPort, &out.APIServerPort, s); err != nil { return err } - out.DisableAPIServerFloatingIP = in.DisableAPIServerFloatingIP - out.APIServerFloatingIP = in.APIServerFloatingIP - out.APIServerFixedIP = in.APIServerFixedIP - out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ManagedSecurityGroups vs bool) - out.DisablePortSecurity = in.DisablePortSecurity + if err := optional.Convert_optional_Bool_To_bool(&in.DisablePortSecurity, &out.DisablePortSecurity, s); err != nil { + return err + } out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + // WARNING: in.ControlPlaneEndpoint requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint vs sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint) out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) - out.ControlPlaneOmitAvailabilityZone = in.ControlPlaneOmitAvailabilityZone + if err := optional.Convert_optional_Bool_To_bool(&in.ControlPlaneOmitAvailabilityZone, &out.ControlPlaneOmitAvailabilityZone, s); err != nil { + return err + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(Bastion) diff --git a/api/v1alpha7/openstackcluster_conversion.go b/api/v1alpha7/openstackcluster_conversion.go index 5761f11f8c..c8b5e29507 100644 --- a/api/v1alpha7/openstackcluster_conversion.go +++ b/api/v1alpha7/openstackcluster_conversion.go @@ -18,10 +18,12 @@ package v1alpha7 import ( apiconversion "k8s.io/apimachinery/pkg/conversion" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" 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/conversion" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional" ) var _ ctrlconversion.Convertible = &OpenStackCluster{} @@ -167,14 +169,24 @@ func restorev1alpha7ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackCl func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infrav1.OpenStackClusterSpec) { // Bastion is restored separately + if dst.Network.IsEmpty() { + dst.Network = previous.Network + } + // Restore all fields except ID, which should have been copied over in conversion - dst.ExternalNetwork.Name = previous.ExternalNetwork.Name - dst.ExternalNetwork.Description = previous.ExternalNetwork.Description - dst.ExternalNetwork.ProjectID = previous.ExternalNetwork.ProjectID - dst.ExternalNetwork.Tags = previous.ExternalNetwork.Tags - dst.ExternalNetwork.TagsAny = previous.ExternalNetwork.TagsAny - dst.ExternalNetwork.NotTags = previous.ExternalNetwork.NotTags - dst.ExternalNetwork.NotTagsAny = previous.ExternalNetwork.NotTagsAny + if previous.ExternalNetwork != nil { + if dst.ExternalNetwork == nil { + dst.ExternalNetwork = &infrav1.NetworkFilter{} + } + + dst.ExternalNetwork.Name = previous.ExternalNetwork.Name + dst.ExternalNetwork.Description = previous.ExternalNetwork.Description + dst.ExternalNetwork.ProjectID = previous.ExternalNetwork.ProjectID + dst.ExternalNetwork.Tags = previous.ExternalNetwork.Tags + dst.ExternalNetwork.TagsAny = previous.ExternalNetwork.TagsAny + dst.ExternalNetwork.NotTags = previous.ExternalNetwork.NotTags + dst.ExternalNetwork.NotTagsAny = previous.ExternalNetwork.NotTagsAny + } dst.DisableExternalNetwork = previous.DisableExternalNetwork @@ -187,6 +199,21 @@ func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infr if previous.ManagedSecurityGroups != nil { dst.ManagedSecurityGroups.AllNodesSecurityGroupRules = previous.ManagedSecurityGroups.AllNodesSecurityGroupRules } + + if dst.APIServerLoadBalancer.IsZero() { + dst.APIServerLoadBalancer = previous.APIServerLoadBalancer + } + + if dst.ControlPlaneEndpoint == nil || *dst.ControlPlaneEndpoint == (clusterv1.APIEndpoint{}) { + dst.ControlPlaneEndpoint = previous.ControlPlaneEndpoint + } + + optional.RestoreString(&previous.APIServerFloatingIP, &dst.APIServerFloatingIP) + optional.RestoreString(&previous.APIServerFixedIP, &dst.APIServerFixedIP) + optional.RestoreInt(&previous.APIServerPort, &dst.APIServerPort) + optional.RestoreBool(&previous.DisableAPIServerFloatingIP, &dst.DisableAPIServerFloatingIP) + optional.RestoreBool(&previous.ControlPlaneOmitAvailabilityZone, &dst.ControlPlaneOmitAvailabilityZone) + optional.RestoreBool(&previous.DisablePortSecurity, &dst.DisablePortSecurity) } func Convert_v1alpha7_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *OpenStackClusterSpec, out *infrav1.OpenStackClusterSpec, s apiconversion.Scope) error { @@ -195,8 +222,15 @@ func Convert_v1alpha7_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O return err } + if in.Network != (NetworkFilter{}) { + out.Network = &infrav1.NetworkFilter{} + if err := Convert_v1alpha7_NetworkFilter_To_v1beta1_NetworkFilter(&in.Network, out.Network, s); err != nil { + return err + } + } + if in.ExternalNetworkID != "" { - out.ExternalNetwork = infrav1.NetworkFilter{ + out.ExternalNetwork = &infrav1.NetworkFilter{ ID: in.ExternalNetworkID, } } @@ -229,11 +263,23 @@ func Convert_v1alpha7_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O } } + if in.ControlPlaneEndpoint != (clusterv1.APIEndpoint{}) { + out.ControlPlaneEndpoint = &in.ControlPlaneEndpoint + } + out.IdentityRef.CloudName = in.CloudName if in.IdentityRef != nil { out.IdentityRef.Name = in.IdentityRef.Name } + apiServerLoadBalancer := &infrav1.APIServerLoadBalancer{} + if err := Convert_v1alpha7_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(&in.APIServerLoadBalancer, apiServerLoadBalancer, s); err != nil { + return err + } + if !apiServerLoadBalancer.IsZero() { + out.APIServerLoadBalancer = apiServerLoadBalancer + } + return nil } @@ -243,7 +289,13 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in *i return err } - if in.ExternalNetwork.ID != "" { + if in.Network != nil { + if err := Convert_v1beta1_NetworkFilter_To_v1alpha7_NetworkFilter(in.Network, &out.Network, s); err != nil { + return err + } + } + + if in.ExternalNetwork != nil && in.ExternalNetwork.ID != "" { out.ExternalNetworkID = in.ExternalNetwork.ID } @@ -263,9 +315,19 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in *i out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic } + if in.ControlPlaneEndpoint != nil { + out.ControlPlaneEndpoint = *in.ControlPlaneEndpoint + } + out.CloudName = in.IdentityRef.CloudName out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name} + if in.APIServerLoadBalancer != nil { + if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha7_APIServerLoadBalancer(in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + return err + } + } + return nil } diff --git a/api/v1alpha7/types_conversion.go b/api/v1alpha7/types_conversion.go index f18f77269d..7b966d1320 100644 --- a/api/v1alpha7/types_conversion.go +++ b/api/v1alpha7/types_conversion.go @@ -21,6 +21,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/optional" ) /* SecurityGroupFilter */ @@ -200,17 +201,9 @@ func restorev1alpha7Port(previous *PortOpts, dst *PortOpts) { } func restorev1beta1Port(previous *infrav1.PortOpts, dst *infrav1.PortOpts) { - if dst.NameSuffix == nil || *dst.NameSuffix == "" { - dst.NameSuffix = previous.NameSuffix - } - - if dst.Description == nil || *dst.Description == "" { - dst.Description = previous.Description - } - - if dst.MACAddress == nil || *dst.MACAddress == "" { - dst.MACAddress = previous.MACAddress - } + 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 { @@ -234,13 +227,8 @@ func restorev1beta1Port(previous *infrav1.PortOpts, dst *infrav1.PortOpts) { } } - if dst.HostID == nil || *dst.HostID == "" { - dst.HostID = previous.HostID - } - - if dst.VNICType == nil || *dst.VNICType == "" { - dst.VNICType = previous.VNICType - } + optional.RestoreString(&previous.HostID, &dst.HostID) + optional.RestoreString(&previous.VNICType, &dst.VNICType) if dst.Profile == nil && previous.Profile != nil { dst.Profile = &infrav1.BindingProfile{} diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index 2cbd5b0184..3ce6bde5ca 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -899,11 +899,11 @@ func autoConvert_v1alpha7_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(i } else { out.Router = nil } - if err := Convert_v1alpha7_NetworkFilter_To_v1beta1_NetworkFilter(&in.Network, &out.Network, s); err != nil { + // WARNING: in.Network requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7.NetworkFilter vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.NetworkFilter) + // WARNING: in.Subnet requires manual conversion: does not exist in peer-type + if err := optional.Convert_int_To_optional_Int(&in.NetworkMTU, &out.NetworkMTU, s); err != nil { return err } - // WARNING: in.Subnet requires manual conversion: does not exist in peer-type - out.NetworkMTU = in.NetworkMTU // WARNING: in.DNSNameservers requires manual conversion: does not exist in peer-type if in.ExternalRouterIPs != nil { in, out := &in.ExternalRouterIPs, &out.ExternalRouterIPs @@ -917,20 +917,30 @@ func autoConvert_v1alpha7_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(i out.ExternalRouterIPs = nil } // WARNING: in.ExternalNetworkID requires manual conversion: does not exist in peer-type - if err := Convert_v1alpha7_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(&in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + // WARNING: in.APIServerLoadBalancer requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7.APIServerLoadBalancer vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.APIServerLoadBalancer) + if err := optional.Convert_bool_To_optional_Bool(&in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_string_To_optional_String(&in.APIServerFloatingIP, &out.APIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_string_To_optional_String(&in.APIServerFixedIP, &out.APIServerFixedIP, s); err != nil { + return err + } + if err := optional.Convert_int_To_optional_Int(&in.APIServerPort, &out.APIServerPort, s); err != nil { return err } - out.DisableAPIServerFloatingIP = in.DisableAPIServerFloatingIP - out.APIServerFloatingIP = in.APIServerFloatingIP - out.APIServerFixedIP = in.APIServerFixedIP - out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ManagedSecurityGroups) // WARNING: in.AllowAllInClusterTraffic requires manual conversion: does not exist in peer-type - out.DisablePortSecurity = in.DisablePortSecurity + if err := optional.Convert_bool_To_optional_Bool(&in.DisablePortSecurity, &out.DisablePortSecurity, s); err != nil { + return err + } out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + // WARNING: in.ControlPlaneEndpoint requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint vs *sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint) out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) - out.ControlPlaneOmitAvailabilityZone = in.ControlPlaneOmitAvailabilityZone + if err := optional.Convert_bool_To_optional_Bool(&in.ControlPlaneOmitAvailabilityZone, &out.ControlPlaneOmitAvailabilityZone, s); err != nil { + return err + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(v1beta1.Bastion) @@ -955,11 +965,11 @@ func autoConvert_v1beta1_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(i } else { out.Router = nil } - if err := Convert_v1beta1_NetworkFilter_To_v1alpha7_NetworkFilter(&in.Network, &out.Network, s); err != nil { + // WARNING: in.Network requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.NetworkFilter vs sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7.NetworkFilter) + // WARNING: in.Subnets requires manual conversion: does not exist in peer-type + if err := optional.Convert_optional_Int_To_int(&in.NetworkMTU, &out.NetworkMTU, s); err != nil { return err } - // WARNING: in.Subnets requires manual conversion: does not exist in peer-type - out.NetworkMTU = in.NetworkMTU if in.ExternalRouterIPs != nil { in, out := &in.ExternalRouterIPs, &out.ExternalRouterIPs *out = make([]ExternalRouterIPParam, len(*in)) @@ -973,19 +983,29 @@ func autoConvert_v1beta1_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(i } // WARNING: in.ExternalNetwork requires manual conversion: does not exist in peer-type // WARNING: in.DisableExternalNetwork requires manual conversion: does not exist in peer-type - if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha7_APIServerLoadBalancer(&in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { + // WARNING: in.APIServerLoadBalancer requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.APIServerLoadBalancer vs sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7.APIServerLoadBalancer) + if err := optional.Convert_optional_Bool_To_bool(&in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_optional_String_To_string(&in.APIServerFloatingIP, &out.APIServerFloatingIP, s); err != nil { + return err + } + if err := optional.Convert_optional_String_To_string(&in.APIServerFixedIP, &out.APIServerFixedIP, s); err != nil { + return err + } + if err := optional.Convert_optional_Int_To_int(&in.APIServerPort, &out.APIServerPort, s); err != nil { return err } - out.DisableAPIServerFloatingIP = in.DisableAPIServerFloatingIP - out.APIServerFloatingIP = in.APIServerFloatingIP - out.APIServerFixedIP = in.APIServerFixedIP - out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1.ManagedSecurityGroups vs bool) - out.DisablePortSecurity = in.DisablePortSecurity + if err := optional.Convert_optional_Bool_To_bool(&in.DisablePortSecurity, &out.DisablePortSecurity, s); err != nil { + return err + } out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + // WARNING: in.ControlPlaneEndpoint requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint vs sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint) out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) - out.ControlPlaneOmitAvailabilityZone = in.ControlPlaneOmitAvailabilityZone + if err := optional.Convert_optional_Bool_To_bool(&in.ControlPlaneOmitAvailabilityZone, &out.ControlPlaneOmitAvailabilityZone, s); err != nil { + return err + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(Bastion) diff --git a/api/v1beta1/openstackcluster_types.go b/api/v1beta1/openstackcluster_types.go index 3a3fe803c0..2aeb112c85 100644 --- a/api/v1beta1/openstackcluster_types.go +++ b/api/v1beta1/openstackcluster_types.go @@ -20,6 +20,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" capierrors "sigs.k8s.io/cluster-api/errors" + + "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional" ) const ( @@ -34,6 +36,8 @@ type OpenStackClusterSpec struct { // subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4 // subnet is supported. If you leave this empty, no network will be created. // +kubebuilder:validation:MaxItems=1 + // +listType=atomic + // +optional ManagedSubnets []SubnetSpec `json:"managedSubnets,omitempty"` // Router specifies an existing router to be used if ManagedSubnets are @@ -43,7 +47,8 @@ type OpenStackClusterSpec struct { // Network specifies an existing network to use if no ManagedSubnets // are specified. - Network NetworkFilter `json:"network,omitempty"` + // +optional + Network *NetworkFilter `json:"network,omitempty"` // Subnets specifies existing subnets to use if not ManagedSubnets are // specified. All subnets must be in the network specified by Network. @@ -51,6 +56,7 @@ type OpenStackClusterSpec struct { // all subnets in Network will be used. If 2 subnets are specified, one // must be IPv4 and the other IPv6. // +kubebuilder:validation:MaxItems=2 + // +listType=atomic // +optional Subnets []SubnetFilter `json:"subnets,omitempty"` @@ -59,26 +65,39 @@ type OpenStackClusterSpec struct { // If left empty, the network will have the default MTU defined in Openstack network service. // To use this field, the Openstack installation requires the net-mtu neutron API extension. // +optional - NetworkMTU int `json:"networkMTU,omitempty"` + NetworkMTU optional.Int `json:"networkMTU,omitempty"` // ExternalRouterIPs is an array of externalIPs on the respective subnets. // This is necessary if the router needs a fixed ip in a specific subnet. + // +listType=atomic + // +optional ExternalRouterIPs []ExternalRouterIPParam `json:"externalRouterIPs,omitempty"` // ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs. + // This option is ignored if DisableExternalNetwork is set to true. + // + // If ExternalNetwork is defined it must refer to exactly one external network. + // + // If ExternalNetwork is not defined or is empty the controller will use any + // existing external network as long as there is only one. It is an + // error if ExternalNetwork is not defined and there are multiple + // external networks unless DisableExternalNetwork is also set. + // + // If ExternalNetwork is not defined and there are no external networks + // the controller will proceed as though DisableExternalNetwork was set. // +optional - ExternalNetwork NetworkFilter `json:"externalNetwork,omitempty"` + ExternalNetwork *NetworkFilter `json:"externalNetwork,omitempty"` - // DisableExternalNetwork determines whether or not to attempt to connect the cluster + // DisableExternalNetwork specifies whether or not to attempt to connect the cluster // to an external network. This allows for the creation of clusters when connecting // to an external network is not possible or desirable, e.g. if using a provider network. // +optional - DisableExternalNetwork bool `json:"disableExternalNetwork"` + DisableExternalNetwork optional.Bool `json:"disableExternalNetwork,omitempty"` // APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. // It must be activated by setting `enabled: true`. // +optional - APIServerLoadBalancer APIServerLoadBalancer `json:"apiServerLoadBalancer,omitempty"` + APIServerLoadBalancer *APIServerLoadBalancer `json:"apiServerLoadBalancer,omitempty"` // DisableAPIServerFloatingIP determines whether or not to attempt to attach a floating // IP to the API server. This allows for the creation of clusters when attaching a floating @@ -93,13 +112,14 @@ type OpenStackClusterSpec struct { // configuration to manage the VIP on the control plane machines, which falls outside of // the scope of this controller. // +optional - DisableAPIServerFloatingIP bool `json:"disableAPIServerFloatingIP"` + DisableAPIServerFloatingIP optional.Bool `json:"disableAPIServerFloatingIP,omitempty"` // APIServerFloatingIP is the floatingIP which will be associated with the API server. // The floatingIP will be created if it does not already exist. // If not specified, a new floatingIP is allocated. // This field is not used if DisableAPIServerFloatingIP is set to true. - APIServerFloatingIP string `json:"apiServerFloatingIP,omitempty"` + // +optional + APIServerFloatingIP optional.String `json:"apiServerFloatingIP,omitempty"` // APIServerFixedIP is the fixed IP which will be associated with the API server. // In the case where the API server has a floating IP but not a managed load balancer, @@ -109,11 +129,13 @@ type OpenStackClusterSpec struct { // If a managed load balancer is not used AND the API server floating IP is disabled, // this field MUST be specified and should correspond to a pre-allocated port that // holds the fixed IP to be used as a VIP. - APIServerFixedIP string `json:"apiServerFixedIP,omitempty"` + // +optional + APIServerFixedIP optional.String `json:"apiServerFixedIP,omitempty"` // APIServerPort is the port on which the listener on the APIServer // will be created - APIServerPort int `json:"apiServerPort,omitempty"` + // +optional + APIServerPort optional.Int `json:"apiServerPort,omitempty"` // ManagedSecurityGroups determines whether OpenStack security groups for the cluster // will be managed by the OpenStack provider or whether pre-existing security groups will @@ -127,23 +149,35 @@ type OpenStackClusterSpec struct { // DisablePortSecurity disables the port security of the network created for the // Kubernetes cluster, which also disables SecurityGroups - DisablePortSecurity bool `json:"disablePortSecurity,omitempty"` + // +optional + DisablePortSecurity optional.Bool `json:"disablePortSecurity,omitempty"` - // Tags for all resources in cluster + // Tags to set on all resources in cluster which support tags // +listType=set + // +optional Tags []string `json:"tags,omitempty"` // ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. + // It is normally populated automatically by the OpenStackCluster + // controller during cluster provisioning. If it is set on creation the + // control plane endpoint will use the values set here in preference to + // values set elsewhere. + // ControlPlaneEndpoint cannot be modified after ControlPlaneEndpoint.Host has been set. // +optional - ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"` + ControlPlaneEndpoint *clusterv1.APIEndpoint `json:"controlPlaneEndpoint,omitempty"` - // ControlPlaneAvailabilityZones is the az to deploy control plane to + // ControlPlaneAvailabilityZones is the set of availability zones which + // control plane machines may be deployed to. // +listType=set + // +optional ControlPlaneAvailabilityZones []string `json:"controlPlaneAvailabilityZones,omitempty"` - // Indicates whether to omit the az for control plane nodes, allowing the Nova scheduler - // to make a decision on which az to use based on other scheduling constraints - ControlPlaneOmitAvailabilityZone bool `json:"controlPlaneOmitAvailabilityZone,omitempty"` + // ControlPlaneOmitAvailabilityZone causes availability zone to be + // omitted when creating control plane nodes, allowing the Nova + // scheduler to make a decision on which availability zone to use based + // on other scheduling constraints + // +optional + ControlPlaneOmitAvailabilityZone optional.Bool `json:"controlPlaneOmitAvailabilityZone,omitempty"` // Bastion is the OpenStack instance to login the nodes // @@ -167,31 +201,42 @@ type OpenStackClusterStatus struct { Ready bool `json:"ready"` // Network contains information about the created OpenStack Network. + // +optional Network *NetworkStatusWithSubnets `json:"network,omitempty"` - // externalNetwork contains information about the external network used for default ingress and egress traffic. + // ExternalNetwork contains information about the external network used for default ingress and egress traffic. + // +optional ExternalNetwork *NetworkStatus `json:"externalNetwork,omitempty"` // Router describes the default cluster router + // +optional Router *Router `json:"router,omitempty"` // APIServerLoadBalancer describes the api server load balancer if one exists + // +optional APIServerLoadBalancer *LoadBalancer `json:"apiServerLoadBalancer,omitempty"` // FailureDomains represent OpenStack availability zones FailureDomains clusterv1.FailureDomains `json:"failureDomains,omitempty"` - // ControlPlaneSecurityGroups contains all the information about the OpenStack - // Security Group that needs to be applied to control plane nodes. - // TODO: Maybe instead of two properties, we add a property to the group? + // ControlPlaneSecurityGroup contains the information about the + // OpenStack Security Group that needs to be applied to control plane + // nodes. + // +optional ControlPlaneSecurityGroup *SecurityGroupStatus `json:"controlPlaneSecurityGroup,omitempty"` - // WorkerSecurityGroup contains all the information about the OpenStack Security - // Group that needs to be applied to worker nodes. + // WorkerSecurityGroup contains the information about the OpenStack + // Security Group that needs to be applied to worker nodes. + // +optional WorkerSecurityGroup *SecurityGroupStatus `json:"workerSecurityGroup,omitempty"` + // BastionSecurityGroup contains the information about the OpenStack + // Security Group that needs to be applied to worker nodes. + // +optional BastionSecurityGroup *SecurityGroupStatus `json:"bastionSecurityGroup,omitempty"` + // Bastion contains the information about the deployed bastion host + // +optional Bastion *BastionStatus `json:"bastion,omitempty"` // FailureReason will be set in the event that there is a terminal problem diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 4b31458b4b..b242a9585d 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -94,6 +94,9 @@ type NetworkFilter struct { } func (networkFilter *NetworkFilter) IsEmpty() bool { + if networkFilter == nil { + return true + } return networkFilter.Name == "" && networkFilter.Description == "" && networkFilter.ProjectID == "" && @@ -612,6 +615,14 @@ type APIServerLoadBalancer struct { Provider string `json:"provider,omitempty"` } +func (s *APIServerLoadBalancer) IsZero() bool { + return s == nil || (!s.Enabled && len(s.AdditionalPorts) == 0 && len(s.AllowedCIDRs) == 0 && s.Provider == "") +} + +func (s *APIServerLoadBalancer) IsEnabled() bool { + return s != nil && s.Enabled +} + // ReferencedMachineResources contains resolved references to resources required by the machine. type ReferencedMachineResources struct { // ServerGroupID is the ID of the server group the machine should be added to and is calculated based on ServerGroupFilter. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index cd5c538ffa..3e41b4104d 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -492,7 +492,11 @@ func (in *OpenStackClusterSpec) DeepCopyInto(out *OpenStackClusterSpec) { *out = new(RouterFilter) (*in).DeepCopyInto(*out) } - in.Network.DeepCopyInto(&out.Network) + if in.Network != nil { + in, out := &in.Network, &out.Network + *out = new(NetworkFilter) + (*in).DeepCopyInto(*out) + } if in.Subnets != nil { in, out := &in.Subnets, &out.Subnets *out = make([]SubnetFilter, len(*in)) @@ -500,6 +504,11 @@ func (in *OpenStackClusterSpec) DeepCopyInto(out *OpenStackClusterSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NetworkMTU != nil { + in, out := &in.NetworkMTU, &out.NetworkMTU + *out = new(int) + **out = **in + } if in.ExternalRouterIPs != nil { in, out := &in.ExternalRouterIPs, &out.ExternalRouterIPs *out = make([]ExternalRouterIPParam, len(*in)) @@ -507,24 +516,71 @@ func (in *OpenStackClusterSpec) DeepCopyInto(out *OpenStackClusterSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - in.ExternalNetwork.DeepCopyInto(&out.ExternalNetwork) - in.APIServerLoadBalancer.DeepCopyInto(&out.APIServerLoadBalancer) + if in.ExternalNetwork != nil { + in, out := &in.ExternalNetwork, &out.ExternalNetwork + *out = new(NetworkFilter) + (*in).DeepCopyInto(*out) + } + if in.DisableExternalNetwork != nil { + in, out := &in.DisableExternalNetwork, &out.DisableExternalNetwork + *out = new(bool) + **out = **in + } + if in.APIServerLoadBalancer != nil { + in, out := &in.APIServerLoadBalancer, &out.APIServerLoadBalancer + *out = new(APIServerLoadBalancer) + (*in).DeepCopyInto(*out) + } + if in.DisableAPIServerFloatingIP != nil { + in, out := &in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP + *out = new(bool) + **out = **in + } + if in.APIServerFloatingIP != nil { + in, out := &in.APIServerFloatingIP, &out.APIServerFloatingIP + *out = new(string) + **out = **in + } + if in.APIServerFixedIP != nil { + in, out := &in.APIServerFixedIP, &out.APIServerFixedIP + *out = new(string) + **out = **in + } + if in.APIServerPort != nil { + in, out := &in.APIServerPort, &out.APIServerPort + *out = new(int) + **out = **in + } if in.ManagedSecurityGroups != nil { in, out := &in.ManagedSecurityGroups, &out.ManagedSecurityGroups *out = new(ManagedSecurityGroups) (*in).DeepCopyInto(*out) } + if in.DisablePortSecurity != nil { + in, out := &in.DisablePortSecurity, &out.DisablePortSecurity + *out = new(bool) + **out = **in + } if in.Tags != nil { in, out := &in.Tags, &out.Tags *out = make([]string, len(*in)) copy(*out, *in) } - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + if in.ControlPlaneEndpoint != nil { + in, out := &in.ControlPlaneEndpoint, &out.ControlPlaneEndpoint + *out = new(apiv1beta1.APIEndpoint) + **out = **in + } if in.ControlPlaneAvailabilityZones != nil { in, out := &in.ControlPlaneAvailabilityZones, &out.ControlPlaneAvailabilityZones *out = make([]string, len(*in)) copy(*out, *in) } + if in.ControlPlaneOmitAvailabilityZone != nil { + in, out := &in.ControlPlaneOmitAvailabilityZone, &out.ControlPlaneOmitAvailabilityZone + *out = new(bool) + **out = **in + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(Bastion) 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 afe2b5c726..ff569270af 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5529,15 +5529,21 @@ spec: type: object type: object controlPlaneAvailabilityZones: - description: ControlPlaneAvailabilityZones is the az to deploy control - plane to + description: |- + ControlPlaneAvailabilityZones is the set of availability zones which + control plane machines may be deployed to. items: type: string type: array x-kubernetes-list-type: set controlPlaneEndpoint: - description: ControlPlaneEndpoint represents the endpoint used to - communicate with the control plane. + description: |- + ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. + It is normally populated automatically by the OpenStackCluster + controller during cluster provisioning. If it is set on creation the + control plane endpoint will use the values set here in preference to + values set elsewhere. + ControlPlaneEndpoint cannot be modified after ControlPlaneEndpoint.Host has been set. properties: host: description: The hostname on which the API server is serving. @@ -5552,8 +5558,10 @@ spec: type: object controlPlaneOmitAvailabilityZone: description: |- - Indicates whether to omit the az for control plane nodes, allowing the Nova scheduler - to make a decision on which az to use based on other scheduling constraints + ControlPlaneOmitAvailabilityZone causes availability zone to be + omitted when creating control plane nodes, allowing the Nova + scheduler to make a decision on which availability zone to use based + on other scheduling constraints type: boolean disableAPIServerFloatingIP: description: |- @@ -5572,7 +5580,7 @@ spec: type: boolean disableExternalNetwork: description: |- - DisableExternalNetwork determines whether or not to attempt to connect the cluster + DisableExternalNetwork specifies whether or not to attempt to connect the cluster to an external network. This allows for the creation of clusters when connecting to an external network is not possible or desirable, e.g. if using a provider network. type: boolean @@ -5582,8 +5590,22 @@ spec: Kubernetes cluster, which also disables SecurityGroups type: boolean externalNetwork: - description: ExternalNetwork is the OpenStack Network to be used to - get public internet to the VMs. + description: |- + ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs. + This option is ignored if DisableExternalNetwork is set to true. + + + If ExternalNetwork is defined it must refer to exactly one external network. + + + If ExternalNetwork is not defined or is empty the controller will use any + existing external network as long as there is only one. It is an + error if ExternalNetwork is not defined and there are multiple + external networks unless DisableExternalNetwork is also set. + + + If ExternalNetwork is not defined and there are no external networks + the controller will proceed as though DisableExternalNetwork was set. properties: description: type: string @@ -5736,6 +5758,7 @@ spec: - subnet type: object type: array + x-kubernetes-list-type: atomic identityRef: description: |- IdentityRef is a reference to a secret holding OpenStack credentials @@ -5893,6 +5916,7 @@ spec: type: object maxItems: 1 type: array + x-kubernetes-list-type: atomic network: description: |- Network specifies an existing network to use if no ManagedSubnets @@ -6117,8 +6141,10 @@ spec: type: object maxItems: 2 type: array + x-kubernetes-list-type: atomic tags: - description: Tags for all resources in cluster + description: Tags to set on all resources in cluster which support + tags items: type: string type: array @@ -6156,6 +6182,8 @@ spec: - name type: object bastion: + description: Bastion contains the information about the deployed bastion + host properties: dependentResources: properties: @@ -6569,8 +6597,8 @@ spec: type: object bastionSecurityGroup: description: |- - SecurityGroupStatus represents the basic information of the associated - OpenStack Neutron Security Group. + BastionSecurityGroup contains the information about the OpenStack + Security Group that needs to be applied to worker nodes. properties: id: description: id of the security group @@ -6636,9 +6664,9 @@ spec: type: object controlPlaneSecurityGroup: description: |- - ControlPlaneSecurityGroups contains all the information about the OpenStack - Security Group that needs to be applied to control plane nodes. - TODO: Maybe instead of two properties, we add a property to the group? + ControlPlaneSecurityGroup contains the information about the + OpenStack Security Group that needs to be applied to control plane + nodes. properties: id: description: id of the security group @@ -6703,7 +6731,7 @@ spec: - name type: object externalNetwork: - description: externalNetwork contains information about the external + description: ExternalNetwork contains information about the external network used for default ingress and egress traffic. properties: id: @@ -6844,8 +6872,8 @@ spec: type: object workerSecurityGroup: description: |- - WorkerSecurityGroup contains all the information about the OpenStack Security - Group that needs to be applied to worker nodes. + WorkerSecurityGroup contains the information about the OpenStack + Security Group that needs to be applied to worker nodes. properties: id: description: id of the security group 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 550887a8a6..cefd394be1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2961,15 +2961,21 @@ spec: type: object type: object controlPlaneAvailabilityZones: - description: ControlPlaneAvailabilityZones is the az to deploy - control plane to + description: |- + ControlPlaneAvailabilityZones is the set of availability zones which + control plane machines may be deployed to. items: type: string type: array x-kubernetes-list-type: set controlPlaneEndpoint: - description: ControlPlaneEndpoint represents the endpoint - used to communicate with the control plane. + description: |- + ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. + It is normally populated automatically by the OpenStackCluster + controller during cluster provisioning. If it is set on creation the + control plane endpoint will use the values set here in preference to + values set elsewhere. + ControlPlaneEndpoint cannot be modified after ControlPlaneEndpoint.Host has been set. properties: host: description: The hostname on which the API server is serving. @@ -2984,8 +2990,10 @@ spec: type: object controlPlaneOmitAvailabilityZone: description: |- - Indicates whether to omit the az for control plane nodes, allowing the Nova scheduler - to make a decision on which az to use based on other scheduling constraints + ControlPlaneOmitAvailabilityZone causes availability zone to be + omitted when creating control plane nodes, allowing the Nova + scheduler to make a decision on which availability zone to use based + on other scheduling constraints type: boolean disableAPIServerFloatingIP: description: |- @@ -3004,7 +3012,7 @@ spec: type: boolean disableExternalNetwork: description: |- - DisableExternalNetwork determines whether or not to attempt to connect the cluster + DisableExternalNetwork specifies whether or not to attempt to connect the cluster to an external network. This allows for the creation of clusters when connecting to an external network is not possible or desirable, e.g. if using a provider network. type: boolean @@ -3014,8 +3022,22 @@ spec: Kubernetes cluster, which also disables SecurityGroups type: boolean externalNetwork: - description: ExternalNetwork is the OpenStack Network to be - used to get public internet to the VMs. + description: |- + ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs. + This option is ignored if DisableExternalNetwork is set to true. + + + If ExternalNetwork is defined it must refer to exactly one external network. + + + If ExternalNetwork is not defined or is empty the controller will use any + existing external network as long as there is only one. It is an + error if ExternalNetwork is not defined and there are multiple + external networks unless DisableExternalNetwork is also set. + + + If ExternalNetwork is not defined and there are no external networks + the controller will proceed as though DisableExternalNetwork was set. properties: description: type: string @@ -3168,6 +3190,7 @@ spec: - subnet type: object type: array + x-kubernetes-list-type: atomic identityRef: description: |- IdentityRef is a reference to a secret holding OpenStack credentials @@ -3327,6 +3350,7 @@ spec: type: object maxItems: 1 type: array + x-kubernetes-list-type: atomic network: description: |- Network specifies an existing network to use if no ManagedSubnets @@ -3551,8 +3575,10 @@ spec: type: object maxItems: 2 type: array + x-kubernetes-list-type: atomic tags: - description: Tags for all resources in cluster + description: Tags to set on all resources in cluster which + support tags items: type: string type: array diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index eea38455f2..0ce97f12d5 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -184,7 +184,7 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name) - if openStackCluster.Spec.APIServerLoadBalancer.Enabled { + if openStackCluster.Spec.APIServerLoadBalancer != nil && openStackCluster.Spec.APIServerLoadBalancer.Enabled { loadBalancerService, err := loadbalancer.NewService(scope) if err != nil { return reconcile.Result{}, err @@ -348,7 +348,7 @@ func reconcileNormal(scope *scope.WithLogger, cluster *clusterv1.Cluster, openSt openStackCluster.Status.FailureDomains = make(clusterv1.FailureDomains) for _, az := range availabilityZones { // By default, the AZ is used or not used for control plane nodes depending on the flag - found := !openStackCluster.Spec.ControlPlaneOmitAvailabilityZone + found := !pointer.BoolDeref(openStackCluster.Spec.ControlPlaneOmitAvailabilityZone, false) // If explicit AZs for control plane nodes are given, they override the value if len(openStackCluster.Spec.ControlPlaneAvailabilityZones) > 0 { found = contains(openStackCluster.Spec.ControlPlaneAvailabilityZones, az.ZoneName) @@ -464,10 +464,14 @@ func reconcileBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openS return ctrl.Result{}, nil } - floatingIP := openStackCluster.Spec.Bastion.FloatingIP - if openStackCluster.Status.Bastion.FloatingIP != "" { + var floatingIP *string + switch { + case openStackCluster.Status.Bastion.FloatingIP != "": // Some floating IP has already been created for this bastion, make sure we re-use it - floatingIP = openStackCluster.Status.Bastion.FloatingIP + floatingIP = &openStackCluster.Status.Bastion.FloatingIP + case openStackCluster.Spec.Bastion.FloatingIP != "": + // Use floating IP from the spec + floatingIP = &openStackCluster.Spec.Bastion.FloatingIP } // Check if there is an existing floating IP attached to bastion, in case where FloatingIP would not yet have been stored in cluster status fp, err = networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, floatingIP) @@ -634,7 +638,7 @@ func reconcilePreExistingNetworkComponents(scope *scope.WithLogger, networkingSe } if !openStackCluster.Spec.Network.IsEmpty() { - netOpts := filterconvert.NetworkFilterToListOpts(&openStackCluster.Spec.Network) + netOpts := filterconvert.NetworkFilterToListOpts(openStackCluster.Spec.Network) networkList, err := networkingService.GetNetworksByFilter(&netOpts) if err != nil { handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find network: %w", err)) @@ -718,7 +722,7 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n // API server load balancer is enabled. Create an Octavia load balancer. // Note that we reconcile the load balancer even if the control plane // endpoint is already set. - case openStackCluster.Spec.APIServerLoadBalancer.Enabled: + case openStackCluster.Spec.APIServerLoadBalancer != nil && openStackCluster.Spec.APIServerLoadBalancer.Enabled: loadBalancerService, err := loadbalancer.NewService(scope) if err != nil { return err @@ -743,12 +747,12 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n // Control plane endpoint is already set // Note that checking this here means that we don't re-execute any of // the branches below if the control plane endpoint is already set. - case openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): + case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): host = openStackCluster.Spec.ControlPlaneEndpoint.Host // API server load balancer is disabled, but floating IP is not. Create // a floating IP to be attached directly to a control plane host. - case !openStackCluster.Spec.DisableAPIServerFloatingIP: + case !pointer.BoolDeref(openStackCluster.Spec.DisableAPIServerFloatingIP, false): fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, openStackCluster.Spec.APIServerFloatingIP) if err != nil { handleUpdateOSCError(openStackCluster, fmt.Errorf("floating IP cannot be got or created: %w", err)) @@ -760,8 +764,8 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n // plane floating IP. In this case we configure APIServerFixedIP as the // control plane endpoint and leave it to the user to configure load // balancing. - case openStackCluster.Spec.APIServerFixedIP != "": - host = openStackCluster.Spec.APIServerFixedIP + case openStackCluster.Spec.APIServerFixedIP != nil: + host = *openStackCluster.Spec.APIServerFixedIP // Control plane endpoint is not set, and none can be created default: @@ -770,7 +774,7 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n return err } - openStackCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{ + openStackCluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{ Host: host, Port: int32(apiServerPort), } @@ -781,10 +785,10 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n // getAPIServerPort returns the port to use for the API server based on the cluster spec. func getAPIServerPort(openStackCluster *infrav1.OpenStackCluster) int { switch { - case openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): + case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): return int(openStackCluster.Spec.ControlPlaneEndpoint.Port) - case openStackCluster.Spec.APIServerPort != 0: - return openStackCluster.Spec.APIServerPort + case openStackCluster.Spec.APIServerPort != nil: + return *openStackCluster.Spec.APIServerPort } return 6443 } diff --git a/controllers/openstackcluster_controller_test.go b/controllers/openstackcluster_controller_test.go index 971d0308d2..9688ffb968 100644 --- a/controllers/openstackcluster_controller_test.go +++ b/controllers/openstackcluster_controller_test.go @@ -34,6 +34,7 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/test/framework" "sigs.k8s.io/cluster-api/util/annotations" @@ -519,12 +520,12 @@ var _ = Describe("OpenStackCluster controller", func() { Bastion: &infrav1.Bastion{ Enabled: true, }, - DisableAPIServerFloatingIP: true, - APIServerFixedIP: "10.0.0.1", - ExternalNetwork: infrav1.NetworkFilter{ + DisableAPIServerFloatingIP: pointer.Bool(true), + APIServerFixedIP: pointer.String("10.0.0.1"), + ExternalNetwork: &infrav1.NetworkFilter{ ID: externalNetworkID, }, - Network: infrav1.NetworkFilter{ + Network: &infrav1.NetworkFilter{ ID: clusterNetworkID, }, } @@ -556,6 +557,7 @@ var _ = Describe("OpenStackCluster controller", func() { ListOptsBuilder: networks.ListOpts{ ID: externalNetworkID, }, + External: pointer.Bool(true), }).Return([]networks.Network{ { ID: externalNetworkID, @@ -598,12 +600,12 @@ var _ = Describe("OpenStackCluster controller", func() { Bastion: &infrav1.Bastion{ Enabled: true, }, - DisableAPIServerFloatingIP: true, - APIServerFixedIP: "10.0.0.1", - ExternalNetwork: infrav1.NetworkFilter{ + DisableAPIServerFloatingIP: pointer.Bool(true), + APIServerFixedIP: pointer.String("10.0.0.1"), + ExternalNetwork: &infrav1.NetworkFilter{ ID: externalNetworkID, }, - Network: infrav1.NetworkFilter{ + Network: &infrav1.NetworkFilter{ ID: clusterNetworkID, }, Subnets: []infrav1.SubnetFilter{ @@ -639,6 +641,7 @@ var _ = Describe("OpenStackCluster controller", func() { ListOptsBuilder: networks.ListOpts{ ID: externalNetworkID, }, + External: pointer.Bool(true), }).Return([]networks.Network{ { ID: externalNetworkID, @@ -679,9 +682,9 @@ var _ = Describe("OpenStackCluster controller", func() { testCluster.SetName("subnet-filtering") testCluster.Spec = infrav1.OpenStackClusterSpec{ - DisableAPIServerFloatingIP: true, - APIServerFixedIP: "10.0.0.1", - DisableExternalNetwork: true, + DisableAPIServerFloatingIP: pointer.Bool(true), + APIServerFixedIP: pointer.String("10.0.0.1"), + DisableExternalNetwork: pointer.Bool(true), Subnets: []infrav1.SubnetFilter{ {ID: clusterSubnetID}, }, @@ -762,7 +765,7 @@ func Test_getAPIServerPort(t *testing.T) { name: "with a control plane endpoint", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ControlPlaneEndpoint: clusterv1.APIEndpoint{ + ControlPlaneEndpoint: &clusterv1.APIEndpoint{ Host: "192.168.0.1", Port: 6444, }, @@ -774,7 +777,7 @@ func Test_getAPIServerPort(t *testing.T) { name: "with API server port", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - APIServerPort: 6445, + APIServerPort: pointer.Int(6445), }, }, want: 6445, diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 2a296bcfed..0195b007ee 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -256,7 +256,7 @@ func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, cl return ctrl.Result{}, err } - if openStackCluster.Spec.APIServerLoadBalancer.Enabled { + if openStackCluster.Spec.APIServerLoadBalancer.IsEnabled() { loadBalancerService, err := loadbalancer.NewService(scope) if err != nil { return ctrl.Result{}, err @@ -278,7 +278,7 @@ func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, cl } else if instanceStatus, err = computeService.GetInstanceStatusByName(openStackMachine, openStackMachine.Name); err != nil { return ctrl.Result{}, err } - if !openStackCluster.Spec.APIServerLoadBalancer.Enabled && util.IsControlPlaneMachine(machine) && openStackCluster.Spec.APIServerFloatingIP == "" { + if !openStackCluster.Spec.APIServerLoadBalancer.IsEnabled() && util.IsControlPlaneMachine(machine) && openStackCluster.Spec.APIServerFloatingIP == nil { if instanceStatus != nil { instanceNS, err := instanceStatus.NetworkStatus() if err != nil { @@ -452,21 +452,24 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope return ctrl.Result{}, nil } - if openStackCluster.Spec.APIServerLoadBalancer.Enabled { + if openStackCluster.Spec.APIServerLoadBalancer.IsEnabled() { err = r.reconcileLoadBalancerMember(scope, openStackCluster, openStackMachine, instanceNS, clusterName) if err != nil { conditions.MarkFalse(openStackMachine, infrav1.APIServerIngressReadyCondition, infrav1.LoadBalancerMemberErrorReason, clusterv1.ConditionSeverityError, "Reconciling load balancer member failed: %v", err) return ctrl.Result{}, fmt.Errorf("reconcile load balancer member: %w", err) } - } else if !openStackCluster.Spec.DisableAPIServerFloatingIP { - floatingIPAddress := openStackCluster.Spec.ControlPlaneEndpoint.Host - if openStackCluster.Spec.APIServerFloatingIP != "" { + } else if !pointer.BoolDeref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) { + var floatingIPAddress *string + switch { + case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): + floatingIPAddress = &openStackCluster.Spec.ControlPlaneEndpoint.Host + case openStackCluster.Spec.APIServerFloatingIP != nil: floatingIPAddress = openStackCluster.Spec.APIServerFloatingIP } fp, err := networkingService.GetOrCreateFloatingIP(openStackMachine, openStackCluster, clusterName, floatingIPAddress) if err != nil { conditions.MarkFalse(openStackMachine, infrav1.APIServerIngressReadyCondition, infrav1.FloatingIPErrorReason, clusterv1.ConditionSeverityError, "Floating IP cannot be obtained or created: %v", err) - return ctrl.Result{}, fmt.Errorf("get or create floating IP %q: %w", floatingIPAddress, err) + return ctrl.Result{}, fmt.Errorf("get or create floating IP %v: %w", floatingIPAddress, err) } port, err := computeService.GetManagementPort(openStackCluster, instanceStatus) if err != nil { diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md index 311b73e3cf..7995b52c2a 100644 --- a/docs/book/src/api/v1beta1/api.md +++ b/docs/book/src/api/v1beta1/api.md @@ -77,6 +77,7 @@ OpenStackClusterSpec +(Optional)

ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network, subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4 subnet is supported. If you leave this empty, no network will be created.

@@ -107,6 +108,7 @@ NetworkFilter +(Optional)

Network specifies an existing network to use if no ManagedSubnets are specified.

@@ -154,6 +156,7 @@ To use this field, the Openstack installation requires the net-mtu neutron API e +(Optional)

ExternalRouterIPs is an array of externalIPs on the respective subnets. This is necessary if the router needs a fixed ip in a specific subnet.

@@ -169,7 +172,15 @@ NetworkFilter (Optional) -

ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs.

+

ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs. +This option is ignored if DisableExternalNetwork is set to true.

+

If ExternalNetwork is defined it must refer to exactly one external network.

+

If ExternalNetwork is not defined or is empty the controller will use any +existing external network as long as there is only one. It is an +error if ExternalNetwork is not defined and there are multiple +external networks unless DisableExternalNetwork is also set.

+

If ExternalNetwork is not defined and there are no external networks +the controller will proceed as though DisableExternalNetwork was set.

@@ -181,7 +192,7 @@ bool (Optional) -

DisableExternalNetwork determines whether or not to attempt to connect the cluster +

DisableExternalNetwork specifies whether or not to attempt to connect the cluster to an external network. This allows for the creation of clusters when connecting to an external network is not possible or desirable, e.g. if using a provider network.

@@ -232,6 +243,7 @@ string +(Optional)

APIServerFloatingIP is the floatingIP which will be associated with the API server. The floatingIP will be created if it does not already exist. If not specified, a new floatingIP is allocated. @@ -246,6 +258,7 @@ string +(Optional)

APIServerFixedIP is the fixed IP which will be associated with the API server. In the case where the API server has a floating IP but not a managed load balancer, this field is not used. @@ -264,6 +277,7 @@ int +(Optional)

APIServerPort is the port on which the listener on the APIServer will be created

@@ -296,6 +310,7 @@ bool +(Optional)

DisablePortSecurity disables the port security of the network created for the Kubernetes cluster, which also disables SecurityGroups

@@ -308,7 +323,8 @@ Kubernetes cluster, which also disables SecurityGroups

-

Tags for all resources in cluster

+(Optional) +

Tags to set on all resources in cluster which support tags

@@ -322,7 +338,12 @@ sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint (Optional) -

ControlPlaneEndpoint represents the endpoint used to communicate with the control plane.

+

ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. +It is normally populated automatically by the OpenStackCluster +controller during cluster provisioning. If it is set on creation the +control plane endpoint will use the values set here in preference to +values set elsewhere. +ControlPlaneEndpoint cannot be modified after ControlPlaneEndpoint.Host has been set.

@@ -333,7 +354,9 @@ sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint -

ControlPlaneAvailabilityZones is the az to deploy control plane to

+(Optional) +

ControlPlaneAvailabilityZones is the set of availability zones which +control plane machines may be deployed to.

@@ -344,8 +367,11 @@ bool -

Indicates whether to omit the az for control plane nodes, allowing the Nova scheduler -to make a decision on which az to use based on other scheduling constraints

+(Optional) +

ControlPlaneOmitAvailabilityZone causes availability zone to be +omitted when creating control plane nodes, allowing the Nova +scheduler to make a decision on which availability zone to use based +on other scheduling constraints

@@ -1951,6 +1977,7 @@ It may not be empty and may not contain commas.

+(Optional)

ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network, subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4 subnet is supported. If you leave this empty, no network will be created.

@@ -1981,6 +2008,7 @@ NetworkFilter +(Optional)

Network specifies an existing network to use if no ManagedSubnets are specified.

@@ -2028,6 +2056,7 @@ To use this field, the Openstack installation requires the net-mtu neutron API e +(Optional)

ExternalRouterIPs is an array of externalIPs on the respective subnets. This is necessary if the router needs a fixed ip in a specific subnet.

@@ -2043,7 +2072,15 @@ NetworkFilter (Optional) -

ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs.

+

ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs. +This option is ignored if DisableExternalNetwork is set to true.

+

If ExternalNetwork is defined it must refer to exactly one external network.

+

If ExternalNetwork is not defined or is empty the controller will use any +existing external network as long as there is only one. It is an +error if ExternalNetwork is not defined and there are multiple +external networks unless DisableExternalNetwork is also set.

+

If ExternalNetwork is not defined and there are no external networks +the controller will proceed as though DisableExternalNetwork was set.

@@ -2055,7 +2092,7 @@ bool (Optional) -

DisableExternalNetwork determines whether or not to attempt to connect the cluster +

DisableExternalNetwork specifies whether or not to attempt to connect the cluster to an external network. This allows for the creation of clusters when connecting to an external network is not possible or desirable, e.g. if using a provider network.

@@ -2106,6 +2143,7 @@ string +(Optional)

APIServerFloatingIP is the floatingIP which will be associated with the API server. The floatingIP will be created if it does not already exist. If not specified, a new floatingIP is allocated. @@ -2120,6 +2158,7 @@ string +(Optional)

APIServerFixedIP is the fixed IP which will be associated with the API server. In the case where the API server has a floating IP but not a managed load balancer, this field is not used. @@ -2138,6 +2177,7 @@ int +(Optional)

APIServerPort is the port on which the listener on the APIServer will be created

@@ -2170,6 +2210,7 @@ bool +(Optional)

DisablePortSecurity disables the port security of the network created for the Kubernetes cluster, which also disables SecurityGroups

@@ -2182,7 +2223,8 @@ Kubernetes cluster, which also disables SecurityGroups

-

Tags for all resources in cluster

+(Optional) +

Tags to set on all resources in cluster which support tags

@@ -2196,7 +2238,12 @@ sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint (Optional) -

ControlPlaneEndpoint represents the endpoint used to communicate with the control plane.

+

ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. +It is normally populated automatically by the OpenStackCluster +controller during cluster provisioning. If it is set on creation the +control plane endpoint will use the values set here in preference to +values set elsewhere. +ControlPlaneEndpoint cannot be modified after ControlPlaneEndpoint.Host has been set.

@@ -2207,7 +2254,9 @@ sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint -

ControlPlaneAvailabilityZones is the az to deploy control plane to

+(Optional) +

ControlPlaneAvailabilityZones is the set of availability zones which +control plane machines may be deployed to.

@@ -2218,8 +2267,11 @@ bool -

Indicates whether to omit the az for control plane nodes, allowing the Nova scheduler -to make a decision on which az to use based on other scheduling constraints

+(Optional) +

ControlPlaneOmitAvailabilityZone causes availability zone to be +omitted when creating control plane nodes, allowing the Nova +scheduler to make a decision on which availability zone to use based +on other scheduling constraints

@@ -2294,6 +2346,7 @@ NetworkStatusWithSubnets +(Optional)

Network contains information about the created OpenStack Network.

@@ -2307,7 +2360,8 @@ NetworkStatus -

externalNetwork contains information about the external network used for default ingress and egress traffic.

+(Optional) +

ExternalNetwork contains information about the external network used for default ingress and egress traffic.

@@ -2320,6 +2374,7 @@ Router +(Optional)

Router describes the default cluster router

@@ -2333,6 +2388,7 @@ LoadBalancer +(Optional)

APIServerLoadBalancer describes the api server load balancer if one exists

@@ -2359,9 +2415,10 @@ SecurityGroupStatus -

ControlPlaneSecurityGroups contains all the information about the OpenStack -Security Group that needs to be applied to control plane nodes. -TODO: Maybe instead of two properties, we add a property to the group?

+(Optional) +

ControlPlaneSecurityGroup contains the information about the +OpenStack Security Group that needs to be applied to control plane +nodes.

@@ -2374,8 +2431,9 @@ SecurityGroupStatus -

WorkerSecurityGroup contains all the information about the OpenStack Security -Group that needs to be applied to worker nodes.

+(Optional) +

WorkerSecurityGroup contains the information about the OpenStack +Security Group that needs to be applied to worker nodes.

@@ -2388,6 +2446,9 @@ SecurityGroupStatus +(Optional) +

BastionSecurityGroup contains the information about the OpenStack +Security Group that needs to be applied to worker nodes.

@@ -2400,6 +2461,8 @@ BastionStatus +(Optional) +

Bastion contains the information about the deployed bastion host

@@ -2496,6 +2559,7 @@ OpenStackClusterSpec +(Optional)

ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network, subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4 subnet is supported. If you leave this empty, no network will be created.

@@ -2526,6 +2590,7 @@ NetworkFilter +(Optional)

Network specifies an existing network to use if no ManagedSubnets are specified.

@@ -2573,6 +2638,7 @@ To use this field, the Openstack installation requires the net-mtu neutron API e +(Optional)

ExternalRouterIPs is an array of externalIPs on the respective subnets. This is necessary if the router needs a fixed ip in a specific subnet.

@@ -2588,7 +2654,15 @@ NetworkFilter (Optional) -

ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs.

+

ExternalNetwork is the OpenStack Network to be used to get public internet to the VMs. +This option is ignored if DisableExternalNetwork is set to true.

+

If ExternalNetwork is defined it must refer to exactly one external network.

+

If ExternalNetwork is not defined or is empty the controller will use any +existing external network as long as there is only one. It is an +error if ExternalNetwork is not defined and there are multiple +external networks unless DisableExternalNetwork is also set.

+

If ExternalNetwork is not defined and there are no external networks +the controller will proceed as though DisableExternalNetwork was set.

@@ -2600,7 +2674,7 @@ bool (Optional) -

DisableExternalNetwork determines whether or not to attempt to connect the cluster +

DisableExternalNetwork specifies whether or not to attempt to connect the cluster to an external network. This allows for the creation of clusters when connecting to an external network is not possible or desirable, e.g. if using a provider network.

@@ -2651,6 +2725,7 @@ string +(Optional)

APIServerFloatingIP is the floatingIP which will be associated with the API server. The floatingIP will be created if it does not already exist. If not specified, a new floatingIP is allocated. @@ -2665,6 +2740,7 @@ string +(Optional)

APIServerFixedIP is the fixed IP which will be associated with the API server. In the case where the API server has a floating IP but not a managed load balancer, this field is not used. @@ -2683,6 +2759,7 @@ int +(Optional)

APIServerPort is the port on which the listener on the APIServer will be created

@@ -2715,6 +2792,7 @@ bool +(Optional)

DisablePortSecurity disables the port security of the network created for the Kubernetes cluster, which also disables SecurityGroups

@@ -2727,7 +2805,8 @@ Kubernetes cluster, which also disables SecurityGroups

-

Tags for all resources in cluster

+(Optional) +

Tags to set on all resources in cluster which support tags

@@ -2741,7 +2820,12 @@ sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint (Optional) -

ControlPlaneEndpoint represents the endpoint used to communicate with the control plane.

+

ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. +It is normally populated automatically by the OpenStackCluster +controller during cluster provisioning. If it is set on creation the +control plane endpoint will use the values set here in preference to +values set elsewhere. +ControlPlaneEndpoint cannot be modified after ControlPlaneEndpoint.Host has been set.

@@ -2752,7 +2836,9 @@ sigs.k8s.io/cluster-api/api/v1beta1.APIEndpoint -

ControlPlaneAvailabilityZones is the az to deploy control plane to

+(Optional) +

ControlPlaneAvailabilityZones is the set of availability zones which +control plane machines may be deployed to.

@@ -2763,8 +2849,11 @@ bool -

Indicates whether to omit the az for control plane nodes, allowing the Nova scheduler -to make a decision on which az to use based on other scheduling constraints

+(Optional) +

ControlPlaneOmitAvailabilityZone causes availability zone to be +omitted when creating control plane nodes, allowing the Nova +scheduler to make a decision on which availability zone to use based +on other scheduling constraints

diff --git a/pkg/cloud/services/loadbalancer/loadbalancer.go b/pkg/cloud/services/loadbalancer/loadbalancer.go index 38dda5ecb6..6b716e75cd 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer.go @@ -30,6 +30,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/pools" "k8s.io/apimachinery/pkg/util/wait" utilsnet "k8s.io/utils/net" + "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util" @@ -52,21 +53,26 @@ const loadBalancerProvisioningStatusActive = "ACTIVE" // We wrap the LookupHost function in a variable to allow overriding it in unit tests. // //nolint:gocritic -var lookupHost = func(host string) (string, error) { +var lookupHost = func(host string) (*string, error) { ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second) defer cancel() ips, err := net.DefaultResolver.LookupHost(ctx, host) if err != nil { - return "", err + return nil, err } if ip := net.ParseIP(ips[0]); ip == nil { - return "", fmt.Errorf("failed to resolve IP address for host %s", host) + return nil, fmt.Errorf("failed to resolve IP address for host %s", host) } - return ips[0], nil + return &ips[0], nil } // ReconcileLoadBalancer reconciles the load balancer for the given cluster. func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackCluster, clusterName string, apiServerPort int) (bool, error) { + lbSpec := openStackCluster.Spec.APIServerLoadBalancer + if !lbSpec.IsEnabled() { + return false, nil + } + loadBalancerName := getLoadBalancerName(clusterName) s.scope.Logger().Info("Reconciling load balancer", "name", loadBalancerName) @@ -94,7 +100,7 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust } } - if !openStackCluster.Spec.DisableAPIServerFloatingIP { + if !pointer.BoolDeref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) { floatingIPAddress, err := getAPIServerFloatingIP(openStackCluster) if err != nil { return false, err @@ -128,7 +134,7 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust } portList := []int{apiServerPort} - portList = append(portList, openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts...) + portList = append(portList, lbSpec.AdditionalPorts...) for _, port := range portList { if err := s.reconcileAPILoadBalancerListener(lb, openStackCluster, clusterName, port); err != nil { return false, err @@ -140,50 +146,50 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust // getAPIServerVIPAddress gets the VIP address for the API server from wherever it is specified. // Returns an empty string if the VIP address is not specified and it should be allocated automatically. -func getAPIServerVIPAddress(openStackCluster *infrav1.OpenStackCluster) (string, error) { +func getAPIServerVIPAddress(openStackCluster *infrav1.OpenStackCluster) (*string, error) { switch { // We only use call this function when creating the loadbalancer, so this case should never be used case openStackCluster.Status.APIServerLoadBalancer != nil && openStackCluster.Status.APIServerLoadBalancer.InternalIP != "": - return openStackCluster.Status.APIServerLoadBalancer.InternalIP, nil + return &openStackCluster.Status.APIServerLoadBalancer.InternalIP, nil // Explicit fixed IP in the cluster spec - case openStackCluster.Spec.APIServerFixedIP != "": + case openStackCluster.Spec.APIServerFixedIP != nil: return openStackCluster.Spec.APIServerFixedIP, nil // If we are using the VIP as the control plane endpoint, use any value explicitly set on the control plane endpoint - case openStackCluster.Spec.DisableAPIServerFloatingIP && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): + case pointer.BoolDeref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) && openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): fixedIPAddress, err := lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host) if err != nil { - return "", fmt.Errorf("lookup host: %w", err) + return nil, fmt.Errorf("lookup host: %w", err) } return fixedIPAddress, nil } - return "", nil + return nil, nil } // getAPIServerFloatingIP gets the floating IP from wherever it is specified. // Returns an empty string if the floating IP is not specified and it should be allocated automatically. -func getAPIServerFloatingIP(openStackCluster *infrav1.OpenStackCluster) (string, error) { +func getAPIServerFloatingIP(openStackCluster *infrav1.OpenStackCluster) (*string, error) { switch { // The floating IP was created previously case openStackCluster.Status.APIServerLoadBalancer != nil && openStackCluster.Status.APIServerLoadBalancer.IP != "": - return openStackCluster.Status.APIServerLoadBalancer.IP, nil + return &openStackCluster.Status.APIServerLoadBalancer.IP, nil // Explicit floating IP in the cluster spec - case openStackCluster.Spec.APIServerFloatingIP != "": + case openStackCluster.Spec.APIServerFloatingIP != nil: return openStackCluster.Spec.APIServerFloatingIP, nil // An IP address is specified explicitly in the control plane endpoint - case openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): + case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): floatingIPAddress, err := lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host) if err != nil { - return "", fmt.Errorf("lookup host: %w", err) + return nil, fmt.Errorf("lookup host: %w", err) } return floatingIPAddress, nil } - return "", nil + return nil, nil } // getCanonicalAllowedCIDRs gets a filtered list of CIDRs which should be allowed to access the API server loadbalancer. @@ -192,7 +198,7 @@ func getAPIServerFloatingIP(openStackCluster *infrav1.OpenStackCluster) (string, func getCanonicalAllowedCIDRs(openStackCluster *infrav1.OpenStackCluster) []string { allowedCIDRs := []string{} - if len(openStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs) > 0 { + if openStackCluster.Spec.APIServerLoadBalancer != nil && len(openStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs) > 0 { allowedCIDRs = append(allowedCIDRs, openStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs...) // In the first reconciliation loop, only the Ready field is set in openStackCluster.Status @@ -275,7 +281,7 @@ func (s *Service) getOrCreateAPILoadBalancer(openStackCluster *infrav1.OpenStack // Choose the selected provider if it is set in cluster spec, if not, omit the field and Octavia will use the default provider. lbProvider := "" - if openStackCluster.Spec.APIServerLoadBalancer.Provider != "" { + if openStackCluster.Spec.APIServerLoadBalancer != nil && openStackCluster.Spec.APIServerLoadBalancer.Provider != "" { for _, v := range providers { if v.Name == openStackCluster.Spec.APIServerLoadBalancer.Provider { lbProvider = v.Name @@ -296,11 +302,14 @@ func (s *Service) getOrCreateAPILoadBalancer(openStackCluster *infrav1.OpenStack lbCreateOpts := loadbalancers.CreateOpts{ Name: loadBalancerName, VipSubnetID: subnetID, - VipAddress: vipAddress, Description: names.GetDescription(clusterName), Provider: lbProvider, Tags: openStackCluster.Spec.Tags, } + if vipAddress != nil { + lbCreateOpts.VipAddress = *vipAddress + } + lb, err = s.loadbalancerClient.CreateLoadBalancer(lbCreateOpts) if err != nil { record.Warnf(openStackCluster, "FailedCreateLoadBalancer", "Failed to create load balancer %s: %v", loadBalancerName, err) @@ -509,13 +518,21 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac if openStackCluster.Status.APIServerLoadBalancer == nil { return errors.New("network.APIServerLoadBalancer is not yet available in openStackCluster.Status") } + if openStackCluster.Spec.ControlPlaneEndpoint == nil || !openStackCluster.Spec.ControlPlaneEndpoint.IsValid() { + return errors.New("ControlPlaneEndpoint is not yet set in openStackCluster.Spec") + } loadBalancerName := getLoadBalancerName(clusterName) s.scope.Logger().Info("Reconciling load balancer member", "loadBalancerName", loadBalancerName) lbID := openStackCluster.Status.APIServerLoadBalancer.ID - portList := []int{int(openStackCluster.Spec.ControlPlaneEndpoint.Port)} - portList = append(portList, openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts...) + var portList []int + if openStackCluster.Spec.ControlPlaneEndpoint != nil { + portList = append(portList, int(openStackCluster.Spec.ControlPlaneEndpoint.Port)) + } + if openStackCluster.Spec.APIServerLoadBalancer != nil { + portList = append(portList, openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts...) + } for _, port := range portList { lbPortObjectsName := fmt.Sprintf("%s-%d", loadBalancerName, port) name := lbPortObjectsName + "-" + openStackMachine.Name @@ -604,7 +621,7 @@ func (s *Service) DeleteLoadBalancer(openStackCluster *infrav1.OpenStackCluster, } // If the floating is user-provider (BYO floating IP), don't delete it. - if openStackCluster.Spec.APIServerFloatingIP != fip.FloatingIP { + if openStackCluster.Spec.APIServerFloatingIP == nil || *openStackCluster.Spec.APIServerFloatingIP != fip.FloatingIP { if err = s.networkingService.DeleteFloatingIP(openStackCluster, fip.FloatingIP); err != nil { return err } @@ -645,8 +662,13 @@ func (s *Service) DeleteLoadBalancerMember(openStackCluster *infrav1.OpenStackCl lbID := lb.ID - portList := []int{int(openStackCluster.Spec.ControlPlaneEndpoint.Port)} - portList = append(portList, openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts...) + var portList []int + if openStackCluster.Spec.ControlPlaneEndpoint != nil { + portList = append(portList, int(openStackCluster.Spec.ControlPlaneEndpoint.Port)) + } + if openStackCluster.Spec.APIServerLoadBalancer != nil { + portList = append(portList, openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts...) + } for _, port := range portList { lbPortObjectsName := fmt.Sprintf("%s-%d", loadBalancerName, port) name := lbPortObjectsName + "-" + openStackMachine.Name diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go index f264a83415..cb5d26d2c0 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go @@ -31,6 +31,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/pools" "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/providers" . "github.com/onsi/gomega" + "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" @@ -45,20 +46,23 @@ func Test_ReconcileLoadBalancer(t *testing.T) { defer mockCtrl.Finish() // Stub the call to net.LookupHost - lookupHost = func(host string) (addrs string, err error) { + lookupHost = func(host string) (addrs *string, err error) { if net.ParseIP(host) != nil { - return host, nil + return &host, nil } else if host == apiHostname { ips := []string{"192.168.100.10"} - return ips[0], nil + return &ips[0], nil } - return "", errors.New("Unknown Host " + host) + return nil, errors.New("Unknown Host " + host) } openStackCluster := &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - DisableAPIServerFloatingIP: true, - ControlPlaneEndpoint: clusterv1.APIEndpoint{ + APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ + Enabled: true, + }, + DisableAPIServerFloatingIP: pointer.Bool(true), + ControlPlaneEndpoint: &clusterv1.APIEndpoint{ Host: apiHostname, Port: 6443, }, @@ -159,25 +163,25 @@ func Test_ReconcileLoadBalancer(t *testing.T) { func Test_getAPIServerVIPAddress(t *testing.T) { // Stub the call to net.LookupHost - lookupHost = func(host string) (addrs string, err error) { + lookupHost = func(host string) (addrs *string, err error) { if net.ParseIP(host) != nil { - return host, nil + return &host, nil } else if host == apiHostname { ips := []string{"192.168.100.10"} - return ips[0], nil + return &ips[0], nil } - return "", errors.New("Unknown Host " + host) + return nil, errors.New("Unknown Host " + host) } tests := []struct { name string openStackCluster *infrav1.OpenStackCluster - want string + want *string wantError bool }{ { name: "empty cluster returns empty VIP", openStackCluster: &infrav1.OpenStackCluster{}, - want: "", + want: nil, wantError: false, }, { @@ -189,39 +193,39 @@ func Test_getAPIServerVIPAddress(t *testing.T) { }, }, }, - want: "1.2.3.4", + want: pointer.String("1.2.3.4"), wantError: false, }, { name: "API server VIP is API Server Fixed IP", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - APIServerFixedIP: "1.2.3.4", + APIServerFixedIP: pointer.String("1.2.3.4"), }, }, - want: "1.2.3.4", + want: pointer.String("1.2.3.4"), wantError: false, }, { name: "API server VIP with valid control plane endpoint", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - DisableAPIServerFloatingIP: true, - ControlPlaneEndpoint: clusterv1.APIEndpoint{ + DisableAPIServerFloatingIP: pointer.Bool(true), + ControlPlaneEndpoint: &clusterv1.APIEndpoint{ Host: apiHostname, Port: 6443, }, }, }, - want: "192.168.100.10", + want: pointer.String("192.168.100.10"), wantError: false, }, { name: "API server VIP with invalid control plane endpoint", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - DisableAPIServerFloatingIP: true, - ControlPlaneEndpoint: clusterv1.APIEndpoint{ + DisableAPIServerFloatingIP: pointer.Bool(true), + ControlPlaneEndpoint: &clusterv1.APIEndpoint{ Host: "invalid-api.test-cluster.test", Port: 6443, }, @@ -237,6 +241,7 @@ func Test_getAPIServerVIPAddress(t *testing.T) { got, err := getAPIServerVIPAddress(tt.openStackCluster) if tt.wantError { g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) } else { g.Expect(err).NotTo(HaveOccurred()) g.Expect(got).To(Equal(tt.want)) @@ -247,25 +252,25 @@ func Test_getAPIServerVIPAddress(t *testing.T) { func Test_getAPIServerFloatingIP(t *testing.T) { // Stub the call to net.LookupHost - lookupHost = func(host string) (addrs string, err error) { + lookupHost = func(host string) (addrs *string, err error) { if net.ParseIP(host) != nil { - return host, nil + return &host, nil } else if host == apiHostname { ips := []string{"192.168.100.10"} - return ips[0], nil + return &ips[0], nil } - return "", errors.New("Unknown Host " + host) + return nil, errors.New("Unknown Host " + host) } tests := []struct { name string openStackCluster *infrav1.OpenStackCluster - want string + want *string wantError bool }{ { name: "empty cluster returns empty FIP", openStackCluster: &infrav1.OpenStackCluster{}, - want: "", + want: nil, wantError: false, }, { @@ -277,37 +282,37 @@ func Test_getAPIServerFloatingIP(t *testing.T) { }, }, }, - want: "1.2.3.4", + want: pointer.String("1.2.3.4"), wantError: false, }, { name: "API server FIP is API Server Floating IP", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - APIServerFloatingIP: "1.2.3.4", + APIServerFloatingIP: pointer.String("1.2.3.4"), }, }, - want: "1.2.3.4", + want: pointer.String("1.2.3.4"), wantError: false, }, { name: "API server FIP with valid control plane endpoint", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ControlPlaneEndpoint: clusterv1.APIEndpoint{ + ControlPlaneEndpoint: &clusterv1.APIEndpoint{ Host: apiHostname, Port: 6443, }, }, }, - want: "192.168.100.10", + want: pointer.String("192.168.100.10"), wantError: false, }, { name: "API server FIP with invalid control plane endpoint", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ControlPlaneEndpoint: clusterv1.APIEndpoint{ + ControlPlaneEndpoint: &clusterv1.APIEndpoint{ Host: "invalid-api.test-cluster.test", Port: 6443, }, @@ -323,6 +328,7 @@ func Test_getAPIServerFloatingIP(t *testing.T) { got, err := getAPIServerFloatingIP(tt.openStackCluster) if tt.wantError { g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) } else { g.Expect(err).NotTo(HaveOccurred()) g.Expect(got).To(Equal(tt.want)) @@ -346,7 +352,7 @@ func Test_getCanonicalAllowedCIDRs(t *testing.T) { name: "allowed CIDRs are set", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ + APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ AllowedCIDRs: []string{"1.2.3.4/32"}, }, }, @@ -357,7 +363,7 @@ func Test_getCanonicalAllowedCIDRs(t *testing.T) { name: "allowed CIDRs are set with bastion", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ + APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ AllowedCIDRs: []string{"1.2.3.4/32"}, }, }, @@ -374,7 +380,7 @@ func Test_getCanonicalAllowedCIDRs(t *testing.T) { name: "allowed CIDRs are set with network status", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ + APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ AllowedCIDRs: []string{"1.2.3.4/32"}, }, }, @@ -394,7 +400,7 @@ func Test_getCanonicalAllowedCIDRs(t *testing.T) { name: "allowed CIDRs are set with network status and router IP", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ + APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ AllowedCIDRs: []string{"1.2.3.4/32"}, }, }, diff --git a/pkg/cloud/services/networking/floatingip.go b/pkg/cloud/services/networking/floatingip.go index f7fc8dc308..68f6cb2aac 100644 --- a/pkg/cloud/services/networking/floatingip.go +++ b/pkg/cloud/services/networking/floatingip.go @@ -24,6 +24,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/utils/pointer" "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" @@ -32,13 +33,13 @@ import ( "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/names" ) -func (s *Service) GetOrCreateFloatingIP(eventObject runtime.Object, openStackCluster *infrav1.OpenStackCluster, clusterName, ip string) (*floatingips.FloatingIP, error) { +func (s *Service) GetOrCreateFloatingIP(eventObject runtime.Object, openStackCluster *infrav1.OpenStackCluster, clusterName string, ip *string) (*floatingips.FloatingIP, error) { var fp *floatingips.FloatingIP var err error var fpCreateOpts floatingips.CreateOpts - if ip != "" { - fp, err = s.GetFloatingIP(ip) + if pointer.StringDeref(ip, "") != "" { + fp, err = s.GetFloatingIP(*ip) if err != nil { return nil, err } @@ -46,15 +47,17 @@ func (s *Service) GetOrCreateFloatingIP(eventObject runtime.Object, openStackClu return fp, nil } // only admin can add ip address - fpCreateOpts.FloatingIP = ip + fpCreateOpts.FloatingIP = *ip } fpCreateOpts.FloatingNetworkID = openStackCluster.Status.ExternalNetwork.ID fpCreateOpts.Description = names.GetDescription(clusterName) + s.scope.Logger().Info("Creating floating IP", "ip", fpCreateOpts.FloatingIP, "floatingNetworkID", openStackCluster.Status.ExternalNetwork.ID, "description", fpCreateOpts.Description) + fp, err = s.client.CreateFloatingIP(fpCreateOpts) if err != nil { - record.Warnf(eventObject, "FailedCreateFloatingIP", "Failed to create floating IP %s: %v", ip, err) + record.Warnf(eventObject, "FailedCreateFloatingIP", "Failed to create floating IP %s: %v", fpCreateOpts.FloatingIP, err) return nil, err } diff --git a/pkg/cloud/services/networking/floatingip_test.go b/pkg/cloud/services/networking/floatingip_test.go index e25a42ab53..60a9db4300 100644 --- a/pkg/cloud/services/networking/floatingip_test.go +++ b/pkg/cloud/services/networking/floatingip_test.go @@ -19,12 +19,15 @@ package networking import ( "testing" + "github.com/go-logr/logr/testr" "github.com/golang/mock/gomock" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips" . "github.com/onsi/gomega" + "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) func Test_GetOrCreateFloatingIP(t *testing.T) { @@ -72,13 +75,18 @@ func Test_GetOrCreateFloatingIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + log := testr.New(t) mockClient := mock.NewMockNetworkClient(mockCtrl) + mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "") tt.expect(mockClient.EXPECT()) + + scope := scope.NewWithLogger(mockScopeFactory, log) s := Service{ + scope: scope, client: mockClient, } eventObject := infrav1.OpenStackMachine{} - got, err := s.GetOrCreateFloatingIP(&eventObject, openStackCluster, "test-cluster", tt.ip) + got, err := s.GetOrCreateFloatingIP(&eventObject, openStackCluster, "test-cluster", pointer.String(tt.ip)) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(got).To(Equal(tt.want)) }) diff --git a/pkg/cloud/services/networking/network.go b/pkg/cloud/services/networking/network.go index 1904b3823c..b5c16b003c 100644 --- a/pkg/cloud/services/networking/network.go +++ b/pkg/cloud/services/networking/network.go @@ -24,6 +24,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/external" "github.com/gophercloud/gophercloud/openstack/networking/v2/networks" "github.com/gophercloud/gophercloud/openstack/networking/v2/subnets" + "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" @@ -78,28 +79,17 @@ func (c createOpts) ToNetworkCreateMap() (map[string]interface{}, error) { // - the user has set OpenStackCluster.Spec.DisableExternalNetwork to true. func (s *Service) ReconcileExternalNetwork(openStackCluster *infrav1.OpenStackCluster) error { var listOpts external.ListOptsExt - var isAutoDetecting bool - if openStackCluster.Spec.DisableExternalNetwork { + if pointer.BoolDeref(openStackCluster.Spec.DisableExternalNetwork, false) { s.scope.Logger().Info("External network is disabled - proceeding with internal network only") openStackCluster.Status.ExternalNetwork = nil return nil } - externalNetworkListOpts := filterconvert.NetworkFilterToListOpts(&openStackCluster.Spec.ExternalNetwork) - if externalNetworkListOpts != (networks.ListOpts{}) { - listOpts = external.ListOptsExt{ - ListOptsBuilder: externalNetworkListOpts, - } - } else { - // ExternalNetwork is not given so we'll list all networks and filter for external networks - isAutoDetecting = true - listOpts = external.ListOptsExt{ - ListOptsBuilder: networks.ListOpts{}, - External: &isAutoDetecting, - } + listOpts = external.ListOptsExt{ + ListOptsBuilder: filterconvert.NetworkFilterToListOpts(openStackCluster.Spec.ExternalNetwork), + External: pointer.Bool(true), } - networkList, err := s.client.ListNetwork(listOpts) if err != nil { return err @@ -107,7 +97,7 @@ func (s *Service) ReconcileExternalNetwork(openStackCluster *infrav1.OpenStackCl switch len(networkList) { case 0: - if isAutoDetecting { + if openStackCluster.Spec.ExternalNetwork == nil { // Not finding an external network is fine if ExternalNetwork is not set openStackCluster.Status.ExternalNetwork = nil s.scope.Logger().Info("No external network found - proceeding with internal network only") @@ -150,12 +140,12 @@ func (s *Service) ReconcileNetwork(openStackCluster *infrav1.OpenStackCluster, c Name: networkName, } - if openStackCluster.Spec.DisablePortSecurity { + if pointer.BoolDeref(openStackCluster.Spec.DisablePortSecurity, false) { opts.PortSecurityEnabled = gophercloud.Disabled } - if openStackCluster.Spec.NetworkMTU > 0 { - opts.MTU = &openStackCluster.Spec.NetworkMTU + if openStackCluster.Spec.NetworkMTU != nil { + opts.MTU = openStackCluster.Spec.NetworkMTU } network, err := s.client.CreateNetwork(opts) diff --git a/pkg/cloud/services/networking/network_test.go b/pkg/cloud/services/networking/network_test.go index 27a07580ba..52b0fdb0d4 100644 --- a/pkg/cloud/services/networking/network_test.go +++ b/pkg/cloud/services/networking/network_test.go @@ -113,7 +113,7 @@ func Test_ReconcileNetwork(t *testing.T) { name: "creation with disabled port security", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - DisablePortSecurity: true, + DisablePortSecurity: pointer.Bool(true), }, }, expect: func(m *mock.MockNetworkClientMockRecorder) { @@ -149,7 +149,7 @@ func Test_ReconcileNetwork(t *testing.T) { name: "creation with mtu set", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - NetworkMTU: 1500, + NetworkMTU: pointer.Int(1500), }, }, expect: func(m *mock.MockNetworkClientMockRecorder) { @@ -207,7 +207,6 @@ func Test_ReconcileExternalNetwork(t *testing.T) { fakeNetworkID := "d08803fc-2fa5-4179-b9f7-8c43d0af2fe6" fakeNetworkname := "external-network" - isAutodetecting := true tests := []struct { name string @@ -220,7 +219,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "reconcile external network by ID", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: infrav1.NetworkFilter{ + ExternalNetwork: &infrav1.NetworkFilter{ ID: fakeNetworkID, }, }, @@ -229,6 +228,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { m. ListNetwork(external.ListOptsExt{ ListOptsBuilder: networks.ListOpts{ID: fakeNetworkID}, + External: pointer.Bool(true), }). Return([]networks.Network{ { @@ -239,7 +239,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { }, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: infrav1.NetworkFilter{ + ExternalNetwork: &infrav1.NetworkFilter{ ID: fakeNetworkID, }, }, @@ -256,7 +256,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "reconcile external network by name", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: infrav1.NetworkFilter{ + ExternalNetwork: &infrav1.NetworkFilter{ Name: fakeNetworkname, }, }, @@ -265,6 +265,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { m. ListNetwork(external.ListOptsExt{ ListOptsBuilder: networks.ListOpts{Name: fakeNetworkname}, + External: pointer.Bool(true), }). Return([]networks.Network{ { @@ -275,7 +276,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { }, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: infrav1.NetworkFilter{ + ExternalNetwork: &infrav1.NetworkFilter{ Name: fakeNetworkname, }, }, @@ -292,7 +293,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "reconcile external network by ID when no external network found", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: infrav1.NetworkFilter{ + ExternalNetwork: &infrav1.NetworkFilter{ ID: fakeNetworkID, }, }, @@ -301,12 +302,13 @@ func Test_ReconcileExternalNetwork(t *testing.T) { m. ListNetwork(external.ListOptsExt{ ListOptsBuilder: networks.ListOpts{ID: fakeNetworkID}, + External: pointer.Bool(true), }). Return([]networks.Network{}, nil) }, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: infrav1.NetworkFilter{ + ExternalNetwork: &infrav1.NetworkFilter{ ID: fakeNetworkID, }, }, @@ -317,13 +319,13 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "not reconcile external network when external network disabled", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - DisableExternalNetwork: true, + DisableExternalNetwork: pointer.Bool(true), }, }, expect: func(m *mock.MockNetworkClientMockRecorder) {}, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - DisableExternalNetwork: true, + DisableExternalNetwork: pointer.Bool(true), }, Status: infrav1.OpenStackClusterStatus{ ExternalNetwork: nil, @@ -340,7 +342,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { m. ListNetwork(external.ListOptsExt{ ListOptsBuilder: networks.ListOpts{}, - External: &isAutodetecting, + External: pointer.Bool(true), }). Return([]networks.Network{}, nil) }, @@ -361,7 +363,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { m. ListNetwork(external.ListOptsExt{ ListOptsBuilder: networks.ListOpts{}, - External: &isAutodetecting, + External: pointer.Bool(true), }). Return([]networks.Network{ { @@ -390,7 +392,7 @@ func Test_ReconcileExternalNetwork(t *testing.T) { m. ListNetwork(external.ListOptsExt{ ListOptsBuilder: networks.ListOpts{}, - External: &isAutodetecting, + External: pointer.Bool(true), }). Return([]networks.Network{ { diff --git a/pkg/cloud/services/networking/port.go b/pkg/cloud/services/networking/port.go index 305c58d71a..e651ba992c 100644 --- a/pkg/cloud/services/networking/port.go +++ b/pkg/cloud/services/networking/port.go @@ -423,7 +423,7 @@ func (s *Service) normalizePorts(ports []infrav1.PortOpts, openStackCluster *inf // normalizePortTarget ensures that the port has a network ID. func (s *Service) normalizePortTarget(port *infrav1.PortOpts, openStackCluster *infrav1.OpenStackCluster, portIdx int) error { // Treat no Network and empty Network the same - noNetwork := port.Network == nil || port.Network.IsEmpty() + noNetwork := port.Network.IsEmpty() // No network or subnets defined: use cluster defaults if noNetwork && len(port.FixedIPs) == 0 { diff --git a/pkg/cloud/services/networking/securitygroups.go b/pkg/cloud/services/networking/securitygroups.go index cd1f64360b..d1fd89d847 100644 --- a/pkg/cloud/services/networking/securitygroups.go +++ b/pkg/cloud/services/networking/securitygroups.go @@ -165,7 +165,7 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl workerRules = append(workerRules, getSGWorkerNodePort()...) // If we set additional ports to LB, we need create secgroup rules those ports, this apply to controlPlaneRules only - if openStackCluster.Spec.APIServerLoadBalancer.Enabled { + if openStackCluster.Spec.APIServerLoadBalancer != nil && openStackCluster.Spec.APIServerLoadBalancer.Enabled { controlPlaneRules = append(controlPlaneRules, getSGControlPlaneAdditionalPorts(openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts)...) } diff --git a/pkg/utils/filterconvert/convert.go b/pkg/utils/filterconvert/convert.go index 3dedad67ac..a4ba88e44f 100644 --- a/pkg/utils/filterconvert/convert.go +++ b/pkg/utils/filterconvert/convert.go @@ -27,6 +27,9 @@ import ( ) func SecurityGroupFilterToListOpts(securityGroupFilter *infrav1.SecurityGroupFilter) securitygroups.ListOpts { + if securityGroupFilter == nil { + return securitygroups.ListOpts{} + } return securitygroups.ListOpts{ ID: securityGroupFilter.ID, Name: securityGroupFilter.Name, @@ -40,6 +43,9 @@ func SecurityGroupFilterToListOpts(securityGroupFilter *infrav1.SecurityGroupFil } func SubnetFilterToListOpts(subnetFilter *infrav1.SubnetFilter) subnets.ListOpts { + if subnetFilter == nil { + return subnets.ListOpts{} + } return subnets.ListOpts{ Name: subnetFilter.Name, Description: subnetFilter.Description, @@ -58,6 +64,9 @@ func SubnetFilterToListOpts(subnetFilter *infrav1.SubnetFilter) subnets.ListOpts } func NetworkFilterToListOpts(networkFilter *infrav1.NetworkFilter) networks.ListOpts { + if networkFilter == nil { + return networks.ListOpts{} + } return networks.ListOpts{ Name: networkFilter.Name, Description: networkFilter.Description, @@ -71,6 +80,9 @@ func NetworkFilterToListOpts(networkFilter *infrav1.NetworkFilter) networks.List } func RouterFilterToListOpts(routerFilter *infrav1.RouterFilter) routers.ListOpts { + if routerFilter == nil { + return routers.ListOpts{} + } return routers.ListOpts{ ID: routerFilter.ID, Name: routerFilter.Name, @@ -84,6 +96,9 @@ func RouterFilterToListOpts(routerFilter *infrav1.RouterFilter) routers.ListOpts } func ImageFilterToListOpts(imageFilter *infrav1.ImageFilter) images.ListOpts { + if imageFilter == nil { + return images.ListOpts{} + } return images.ListOpts{ ID: imageFilter.ID, Name: imageFilter.Name, diff --git a/pkg/utils/optional/conversion.go b/pkg/utils/optional/conversion.go index 1876927b7a..9e98c9916f 100644 --- a/pkg/utils/optional/conversion.go +++ b/pkg/utils/optional/conversion.go @@ -20,6 +20,24 @@ import ( "k8s.io/apimachinery/pkg/conversion" ) +func RestoreString(previous, dst *String) { + if *dst == nil || **dst == "" { + *dst = *previous + } +} + +func RestoreInt(previous, dst *Int) { + if *dst == nil || **dst == 0 { + *dst = *previous + } +} + +func RestoreBool(previous, dst *Bool) { + if *dst == nil || !**dst { + *dst = *previous + } +} + func Convert_string_To_optional_String(in *string, out *String, _ conversion.Scope) error { // NOTE: This function has the opposite defaulting behaviour to // Convert_string_to_Pointer_string defined in apimachinery: it converts @@ -41,3 +59,39 @@ func Convert_optional_String_To_string(in *String, out *string, _ conversion.Sco } return nil } + +func Convert_int_To_optional_Int(in *int, out *Int, _ conversion.Scope) error { + if *in == 0 { + *out = nil + } else { + *out = in + } + return nil +} + +func Convert_optional_Int_To_int(in *Int, out *int, _ conversion.Scope) error { + if *in == nil { + *out = 0 + } else { + *out = **in + } + return nil +} + +func Convert_bool_To_optional_Bool(in *bool, out *Bool, _ conversion.Scope) error { + if !*in { + *out = nil + } else { + *out = in + } + return nil +} + +func Convert_optional_Bool_To_bool(in *Bool, out *bool, _ conversion.Scope) error { + if *in == nil { + *out = false + } else { + *out = **in + } + return nil +} diff --git a/pkg/utils/optional/types.go b/pkg/utils/optional/types.go index c9d05ec91b..37815bfb27 100644 --- a/pkg/utils/optional/types.go +++ b/pkg/utils/optional/types.go @@ -19,4 +19,16 @@ package optional // String is a string that can be unspecified. strings which are converted to // optional.String during API conversion will be converted to nil if the value // was previously the empty string. +// +optional. type String *string + +// Int is an int that can be unspecified. ints which are converted to +// optional.Int during API conversion will be converted to nil if the value +// was previously 0. +// +optional. +type Int *int + +// Bool is a bool that can be unspecified. bools which are converted to +// optional.Bool during API conversion will be converted to nil if the value +// was previously false. +type Bool *bool diff --git a/pkg/webhooks/openstackcluster_webhook.go b/pkg/webhooks/openstackcluster_webhook.go index 6541e93681..ece642481f 100644 --- a/pkg/webhooks/openstackcluster_webhook.go +++ b/pkg/webhooks/openstackcluster_webhook.go @@ -23,7 +23,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -88,20 +88,24 @@ func (*openStackClusterWebhook) ValidateUpdate(_ context.Context, oldObjRaw, new oldObj.Spec.IdentityRef = infrav1.OpenStackIdentityReference{} newObj.Spec.IdentityRef = infrav1.OpenStackIdentityReference{} - // Allow change only for the first time. - if oldObj.Spec.ControlPlaneEndpoint.Host == "" { - oldObj.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{} - newObj.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{} + // Allow changes to ControlPlaneEndpoint fields only if it was not + // previously set, or did not previously have a host. + if oldObj.Spec.ControlPlaneEndpoint == nil { + newObj.Spec.ControlPlaneEndpoint = nil + } else if oldObj.Spec.ControlPlaneEndpoint.Host == "" { + oldObj.Spec.ControlPlaneEndpoint = nil + newObj.Spec.ControlPlaneEndpoint = nil } // Allow change only for the first time. - if oldObj.Spec.DisableAPIServerFloatingIP && oldObj.Spec.APIServerFixedIP == "" { - newObj.Spec.APIServerFixedIP = "" + if pointer.BoolDeref(oldObj.Spec.DisableAPIServerFloatingIP, false) && pointer.StringDeref(oldObj.Spec.APIServerFixedIP, "") == "" { + oldObj.Spec.APIServerFixedIP = nil + newObj.Spec.APIServerFixedIP = nil } // If API Server floating IP is disabled, allow the change of the API Server port only for the first time. - if oldObj.Spec.DisableAPIServerFloatingIP && oldObj.Spec.APIServerPort == 0 && newObj.Spec.APIServerPort > 0 { - newObj.Spec.APIServerPort = 0 + if pointer.BoolDeref(oldObj.Spec.DisableAPIServerFloatingIP, false) && oldObj.Spec.APIServerPort == nil && newObj.Spec.APIServerPort != nil { + newObj.Spec.APIServerPort = nil } // Allow to remove the bastion spec only if it was disabled before. @@ -126,7 +130,7 @@ func (*openStackClusterWebhook) ValidateUpdate(_ context.Context, oldObjRaw, new } // Allow changes on AllowedCIDRs - if newObj.Spec.APIServerLoadBalancer.Enabled { + if newObj.Spec.APIServerLoadBalancer != nil && oldObj.Spec.APIServerLoadBalancer != nil { oldObj.Spec.APIServerLoadBalancer.AllowedCIDRs = []string{} newObj.Spec.APIServerLoadBalancer.AllowedCIDRs = []string{} } @@ -137,13 +141,13 @@ func (*openStackClusterWebhook) ValidateUpdate(_ context.Context, oldObjRaw, new // Allow the scheduling to be changed from CAPI managed to Nova and // vice versa. - oldObj.Spec.ControlPlaneOmitAvailabilityZone = false - newObj.Spec.ControlPlaneOmitAvailabilityZone = false + oldObj.Spec.ControlPlaneOmitAvailabilityZone = nil + newObj.Spec.ControlPlaneOmitAvailabilityZone = nil // Allow change on the spec.APIServerFloatingIP only if it matches the current api server loadbalancer IP. - if oldObj.Status.APIServerLoadBalancer != nil && newObj.Spec.APIServerFloatingIP == oldObj.Status.APIServerLoadBalancer.IP { - newObj.Spec.APIServerFloatingIP = "" - oldObj.Spec.APIServerFloatingIP = "" + if oldObj.Status.APIServerLoadBalancer != nil && pointer.StringDeref(newObj.Spec.APIServerFloatingIP, "") == oldObj.Status.APIServerLoadBalancer.IP { + newObj.Spec.APIServerFloatingIP = nil + oldObj.Spec.APIServerFloatingIP = nil } if !reflect.DeepEqual(oldObj.Spec, newObj.Spec) { diff --git a/pkg/webhooks/openstackcluster_webhook_test.go b/pkg/webhooks/openstackcluster_webhook_test.go index 5a4c44269c..8c9ed564ae 100644 --- a/pkg/webhooks/openstackcluster_webhook_test.go +++ b/pkg/webhooks/openstackcluster_webhook_test.go @@ -166,7 +166,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ + APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ Enabled: true, AllowedCIDRs: []string{ "0.0.0.0/0", @@ -181,7 +181,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ + APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ Enabled: true, AllowedCIDRs: []string{ "0.0.0.0/0", @@ -286,7 +286,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - ControlPlaneOmitAvailabilityZone: true, + ControlPlaneOmitAvailabilityZone: pointer.Bool(true), }, }, wantErr: false, @@ -299,7 +299,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - DisableAPIServerFloatingIP: true, + DisableAPIServerFloatingIP: pointer.Bool(true), }, }, newTemplate: &infrav1.OpenStackCluster{ @@ -308,8 +308,8 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - DisableAPIServerFloatingIP: true, - APIServerFixedIP: "20.1.56.1", + DisableAPIServerFloatingIP: pointer.Bool(true), + APIServerFixedIP: pointer.String("20.1.56.1"), }, }, wantErr: false, @@ -322,7 +322,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - DisableAPIServerFloatingIP: false, + DisableAPIServerFloatingIP: pointer.Bool(false), }, }, newTemplate: &infrav1.OpenStackCluster{ @@ -331,8 +331,8 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - DisableAPIServerFloatingIP: false, - APIServerFixedIP: "20.1.56.1", + DisableAPIServerFloatingIP: pointer.Bool(false), + APIServerFixedIP: pointer.String("20.1.56.1"), }, }, wantErr: true, @@ -346,13 +346,13 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - DisableAPIServerFloatingIP: true, + DisableAPIServerFloatingIP: pointer.Bool(true), }, }, newTemplate: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - DisableAPIServerFloatingIP: true, - APIServerPort: 8443, + DisableAPIServerFloatingIP: pointer.Bool(true), + APIServerPort: pointer.Int(8443), }, }, wantErr: false, @@ -365,7 +365,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - DisableAPIServerFloatingIP: false, + DisableAPIServerFloatingIP: pointer.Bool(false), }, }, newTemplate: &infrav1.OpenStackCluster{ @@ -374,8 +374,8 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - DisableAPIServerFloatingIP: false, - APIServerPort: 8443, + DisableAPIServerFloatingIP: pointer.Bool(false), + APIServerPort: pointer.Int(8443), }, }, wantErr: true, @@ -388,7 +388,6 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - APIServerFloatingIP: "", }, Status: infrav1.OpenStackClusterStatus{ APIServerLoadBalancer: &infrav1.LoadBalancer{ @@ -402,7 +401,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - APIServerFloatingIP: "1.2.3.4", + APIServerFloatingIP: pointer.String("1.2.3.4"), }, Status: infrav1.OpenStackClusterStatus{ APIServerLoadBalancer: &infrav1.LoadBalancer{ @@ -420,7 +419,6 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - APIServerFloatingIP: "", }, Status: infrav1.OpenStackClusterStatus{ APIServerLoadBalancer: &infrav1.LoadBalancer{ @@ -434,7 +432,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Name: "foobar", CloudName: "foobar", }, - APIServerFloatingIP: "5.6.7.8", + APIServerFloatingIP: pointer.String("5.6.7.8"), }, Status: infrav1.OpenStackClusterStatus{ APIServerLoadBalancer: &infrav1.LoadBalancer{ diff --git a/test/e2e/suites/apivalidations/neutronfilters_test.go b/test/e2e/suites/apivalidations/neutronfilters_test.go index 540cb815c0..59fb80e9fd 100644 --- a/test/e2e/suites/apivalidations/neutronfilters_test.go +++ b/test/e2e/suites/apivalidations/neutronfilters_test.go @@ -73,8 +73,8 @@ var _ = Describe("Neutron filter API validations", func() { } cluster.Spec.Subnets = subnets if len(tags) > 0 { - cluster.Spec.Network = infrav1.NetworkFilter{FilterByNeutronTags: tags[0]} - cluster.Spec.ExternalNetwork = infrav1.NetworkFilter{FilterByNeutronTags: tags[0]} + cluster.Spec.Network = &infrav1.NetworkFilter{FilterByNeutronTags: tags[0]} + cluster.Spec.ExternalNetwork = &infrav1.NetworkFilter{FilterByNeutronTags: tags[0]} cluster.Spec.Router = &infrav1.RouterFilter{FilterByNeutronTags: tags[0]} } Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed") @@ -137,13 +137,13 @@ var _ = Describe("Neutron filter API validations", func() { { cluster := cluster.DeepCopy() - cluster.Spec.Network = infrav1.NetworkFilter{FilterByNeutronTags: tag} + cluster.Spec.Network = &infrav1.NetworkFilter{FilterByNeutronTags: tag} Expect(k8sClient.Create(ctx, cluster)).NotTo(Succeed(), "OpenStackCluster creation should fail with invalid network neutron tags") } { cluster := cluster.DeepCopy() - cluster.Spec.ExternalNetwork = infrav1.NetworkFilter{FilterByNeutronTags: tag} + cluster.Spec.ExternalNetwork = &infrav1.NetworkFilter{FilterByNeutronTags: tag} Expect(k8sClient.Create(ctx, cluster)).NotTo(Succeed(), "OpenStackCluster creation should fail with invalid external network neutron tags") } diff --git a/test/e2e/suites/apivalidations/openstackcluster_test.go b/test/e2e/suites/apivalidations/openstackcluster_test.go index 00fd97197b..44bc8cbc40 100644 --- a/test/e2e/suites/apivalidations/openstackcluster_test.go +++ b/test/e2e/suites/apivalidations/openstackcluster_test.go @@ -20,6 +20,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" ) @@ -46,8 +47,10 @@ var _ = Describe("OpenStackCluster API validations", func() { Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed") By("Setting the control plane endpoint") - cluster.Spec.ControlPlaneEndpoint.Host = "foo" - cluster.Spec.ControlPlaneEndpoint.Port = 1234 + cluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{ + Host: "foo", + Port: 1234, + } Expect(k8sClient.Update(ctx, cluster)).To(Succeed(), "Setting control plane endpoint should succeed") By("Modifying the control plane endpoint")