Merge pull request kubernetes-sigs#1930 from shiftstack/openstackcluster_apicleanup

⚠️ OpenStackCluster api general cleanup

Author: k8s-ci-robot

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
 }
 

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{},

api/v1alpha5/zz_generated.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
 }