diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index 3b4f8e916c..44dbc571e2 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -24,7 +24,8 @@ package v1alpha5 import ( unsafe "unsafe" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" conversion "k8s.io/apimachinery/pkg/conversion" runtime "k8s.io/apimachinery/pkg/runtime" v1alpha6 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" @@ -405,7 +406,9 @@ func RegisterConversions(s *runtime.Scheme) error { } func autoConvert_v1alpha5_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(in *APIServerLoadBalancer, out *v1beta1.APIServerLoadBalancer, s conversion.Scope) error { - out.Enabled = in.Enabled + if err := v1.Convert_bool_To_Pointer_bool(&in.Enabled, &out.Enabled, s); err != nil { + return err + } out.AdditionalPorts = *(*[]int)(unsafe.Pointer(&in.AdditionalPorts)) out.AllowedCIDRs = *(*[]string)(unsafe.Pointer(&in.AllowedCIDRs)) return nil @@ -417,7 +420,9 @@ func Convert_v1alpha5_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(in } func autoConvert_v1beta1_APIServerLoadBalancer_To_v1alpha5_APIServerLoadBalancer(in *v1beta1.APIServerLoadBalancer, out *APIServerLoadBalancer, s conversion.Scope) error { - out.Enabled = in.Enabled + if err := v1.Convert_Pointer_bool_To_bool(&in.Enabled, &out.Enabled, s); err != nil { + return err + } out.AdditionalPorts = *(*[]int)(unsafe.Pointer(&in.AdditionalPorts)) out.AllowedCIDRs = *(*[]string)(unsafe.Pointer(&in.AllowedCIDRs)) // WARNING: in.Provider requires manual conversion: does not exist in peer-type @@ -1189,7 +1194,7 @@ func autoConvert_v1beta1_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(i func autoConvert_v1alpha5_OpenStackMachineStatus_To_v1beta1_OpenStackMachineStatus(in *OpenStackMachineStatus, out *v1beta1.OpenStackMachineStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) + out.Addresses = *(*[]corev1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.InstanceState = (*v1beta1.InstanceState)(unsafe.Pointer(in.InstanceState)) out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) @@ -1204,7 +1209,7 @@ func Convert_v1alpha5_OpenStackMachineStatus_To_v1beta1_OpenStackMachineStatus(i func autoConvert_v1beta1_OpenStackMachineStatus_To_v1alpha5_OpenStackMachineStatus(in *v1beta1.OpenStackMachineStatus, out *OpenStackMachineStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) + out.Addresses = *(*[]corev1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.InstanceState = (*InstanceState)(unsafe.Pointer(in.InstanceState)) // WARNING: in.ReferencedResources requires manual conversion: does not exist in peer-type // WARNING: in.DependentResources requires manual conversion: does not exist in peer-type diff --git a/api/v1alpha6/openstackcluster_conversion.go b/api/v1alpha6/openstackcluster_conversion.go index b1ecec275e..cb295c3147 100644 --- a/api/v1alpha6/openstackcluster_conversion.go +++ b/api/v1alpha6/openstackcluster_conversion.go @@ -198,6 +198,12 @@ func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infr dst.ManagedSecurityGroups.AllNodesSecurityGroupRules = previous.ManagedSecurityGroups.AllNodesSecurityGroupRules } + if dst.APIServerLoadBalancer != nil && previous.APIServerLoadBalancer != nil { + if dst.APIServerLoadBalancer.Enabled == nil || !*dst.APIServerLoadBalancer.Enabled { + dst.APIServerLoadBalancer.Enabled = previous.APIServerLoadBalancer.Enabled + } + optional.RestoreString(&previous.APIServerLoadBalancer.Provider, &dst.APIServerLoadBalancer.Provider) + } if dst.APIServerLoadBalancer.IsZero() { dst.APIServerLoadBalancer = previous.APIServerLoadBalancer } diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index b0ea770cdd..e3d5251fae 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -24,7 +24,8 @@ package v1alpha6 import ( unsafe "unsafe" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" conversion "k8s.io/apimachinery/pkg/conversion" runtime "k8s.io/apimachinery/pkg/runtime" v1beta1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" @@ -419,10 +420,14 @@ func RegisterConversions(s *runtime.Scheme) error { } func autoConvert_v1alpha6_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(in *APIServerLoadBalancer, out *v1beta1.APIServerLoadBalancer, s conversion.Scope) error { - out.Enabled = in.Enabled + if err := v1.Convert_bool_To_Pointer_bool(&in.Enabled, &out.Enabled, s); err != nil { + return err + } out.AdditionalPorts = *(*[]int)(unsafe.Pointer(&in.AdditionalPorts)) out.AllowedCIDRs = *(*[]string)(unsafe.Pointer(&in.AllowedCIDRs)) - out.Provider = in.Provider + if err := optional.Convert_string_To_optional_String(&in.Provider, &out.Provider, s); err != nil { + return err + } return nil } @@ -432,10 +437,14 @@ func Convert_v1alpha6_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(in } func autoConvert_v1beta1_APIServerLoadBalancer_To_v1alpha6_APIServerLoadBalancer(in *v1beta1.APIServerLoadBalancer, out *APIServerLoadBalancer, s conversion.Scope) error { - out.Enabled = in.Enabled + if err := v1.Convert_Pointer_bool_To_bool(&in.Enabled, &out.Enabled, s); err != nil { + return err + } out.AdditionalPorts = *(*[]int)(unsafe.Pointer(&in.AdditionalPorts)) out.AllowedCIDRs = *(*[]string)(unsafe.Pointer(&in.AllowedCIDRs)) - out.Provider = in.Provider + if err := optional.Convert_optional_String_To_string(&in.Provider, &out.Provider, s); err != nil { + return err + } return nil } @@ -1220,7 +1229,7 @@ func autoConvert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(i func autoConvert_v1alpha6_OpenStackMachineStatus_To_v1beta1_OpenStackMachineStatus(in *OpenStackMachineStatus, out *v1beta1.OpenStackMachineStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) + out.Addresses = *(*[]corev1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.InstanceState = (*v1beta1.InstanceState)(unsafe.Pointer(in.InstanceState)) out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) @@ -1235,7 +1244,7 @@ func Convert_v1alpha6_OpenStackMachineStatus_To_v1beta1_OpenStackMachineStatus(i func autoConvert_v1beta1_OpenStackMachineStatus_To_v1alpha6_OpenStackMachineStatus(in *v1beta1.OpenStackMachineStatus, out *OpenStackMachineStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) + out.Addresses = *(*[]corev1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.InstanceState = (*InstanceState)(unsafe.Pointer(in.InstanceState)) // WARNING: in.ReferencedResources requires manual conversion: does not exist in peer-type // WARNING: in.DependentResources requires manual conversion: does not exist in peer-type diff --git a/api/v1alpha7/openstackcluster_conversion.go b/api/v1alpha7/openstackcluster_conversion.go index c8b5e29507..f36ab2cf33 100644 --- a/api/v1alpha7/openstackcluster_conversion.go +++ b/api/v1alpha7/openstackcluster_conversion.go @@ -200,6 +200,12 @@ func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infr dst.ManagedSecurityGroups.AllNodesSecurityGroupRules = previous.ManagedSecurityGroups.AllNodesSecurityGroupRules } + if dst.APIServerLoadBalancer != nil && previous.APIServerLoadBalancer != nil { + if dst.APIServerLoadBalancer.Enabled == nil || !*dst.APIServerLoadBalancer.Enabled { + dst.APIServerLoadBalancer.Enabled = previous.APIServerLoadBalancer.Enabled + } + optional.RestoreString(&previous.APIServerLoadBalancer.Provider, &dst.APIServerLoadBalancer.Provider) + } if dst.APIServerLoadBalancer.IsZero() { dst.APIServerLoadBalancer = previous.APIServerLoadBalancer } diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index 3ce6bde5ca..5f364226bb 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -445,10 +445,14 @@ func RegisterConversions(s *runtime.Scheme) error { } func autoConvert_v1alpha7_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(in *APIServerLoadBalancer, out *v1beta1.APIServerLoadBalancer, s conversion.Scope) error { - out.Enabled = in.Enabled + if err := v1.Convert_bool_To_Pointer_bool(&in.Enabled, &out.Enabled, s); err != nil { + return err + } out.AdditionalPorts = *(*[]int)(unsafe.Pointer(&in.AdditionalPorts)) out.AllowedCIDRs = *(*[]string)(unsafe.Pointer(&in.AllowedCIDRs)) - out.Provider = in.Provider + if err := optional.Convert_string_To_optional_String(&in.Provider, &out.Provider, s); err != nil { + return err + } return nil } @@ -458,10 +462,14 @@ func Convert_v1alpha7_APIServerLoadBalancer_To_v1beta1_APIServerLoadBalancer(in } func autoConvert_v1beta1_APIServerLoadBalancer_To_v1alpha7_APIServerLoadBalancer(in *v1beta1.APIServerLoadBalancer, out *APIServerLoadBalancer, s conversion.Scope) error { - out.Enabled = in.Enabled + if err := v1.Convert_Pointer_bool_To_bool(&in.Enabled, &out.Enabled, s); err != nil { + return err + } out.AdditionalPorts = *(*[]int)(unsafe.Pointer(&in.AdditionalPorts)) out.AllowedCIDRs = *(*[]string)(unsafe.Pointer(&in.AllowedCIDRs)) - out.Provider = in.Provider + if err := optional.Convert_optional_String_To_string(&in.Provider, &out.Provider, s); err != nil { + return err + } return nil } diff --git a/api/v1beta1/openstackcluster_types.go b/api/v1beta1/openstackcluster_types.go index 2aeb112c85..06d23de650 100644 --- a/api/v1beta1/openstackcluster_types.go +++ b/api/v1beta1/openstackcluster_types.go @@ -95,7 +95,7 @@ type OpenStackClusterSpec struct { DisableExternalNetwork optional.Bool `json:"disableExternalNetwork,omitempty"` // APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. - // It must be activated by setting `enabled: true`. + // If not specified, no load balancer will be created for the API server. // +optional APIServerLoadBalancer *APIServerLoadBalancer `json:"apiServerLoadBalancer,omitempty"` diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index b242a9585d..6b643b2f46 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "k8s.io/utils/pointer" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional" ) @@ -605,22 +607,41 @@ type Bastion struct { } type APIServerLoadBalancer struct { - // Enabled defines whether a load balancer should be created. - Enabled bool `json:"enabled,omitempty"` + // Enabled defines whether a load balancer should be created. This value + // defaults to true if an APIServerLoadBalancer is given. + // + // There is no reason to set this to false. To disable creation of the + // API server loadbalancer, omit the APIServerLoadBalancer field in the + // cluster spec instead. + // + // +kubebuilder:validation:Required + // +kubebuilder:default:=true + Enabled *bool `json:"enabled"` + // AdditionalPorts adds additional tcp ports to the load balancer. + // +optional + // +listType=set AdditionalPorts []int `json:"additionalPorts,omitempty"` + // AllowedCIDRs restrict access to all API-Server listeners to the given address CIDRs. + // +optional + // +listType=set AllowedCIDRs []string `json:"allowedCIDRs,omitempty"` - // Octavia Provider Used to create load balancer - Provider string `json:"provider,omitempty"` + + // Provider specifies name of a specific Octavia provider to use for the + // API load balancer. The Octavia default will be used if it is not + // specified. + // +optional + Provider optional.String `json:"provider,omitempty"` } func (s *APIServerLoadBalancer) IsZero() bool { - return s == nil || (!s.Enabled && len(s.AdditionalPorts) == 0 && len(s.AllowedCIDRs) == 0 && s.Provider == "") + return s == nil || ((s.Enabled == nil || !*s.Enabled) && len(s.AdditionalPorts) == 0 && len(s.AllowedCIDRs) == 0 && pointer.StringDeref(s.Provider, "") == "") } func (s *APIServerLoadBalancer) IsEnabled() bool { - return s != nil && s.Enabled + // The CRD default value for Enabled is true, so if the field is nil, it should be considered as true. + return s != nil && (s.Enabled == nil || *s.Enabled) } // ReferencedMachineResources contains resolved references to resources required by the machine. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 3e41b4104d..aa1a30e5eb 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -30,6 +30,11 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *APIServerLoadBalancer) DeepCopyInto(out *APIServerLoadBalancer) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } if in.AdditionalPorts != nil { in, out := &in.AdditionalPorts, &out.AdditionalPorts *out = make([]int, len(*in)) @@ -40,6 +45,11 @@ func (in *APIServerLoadBalancer) DeepCopyInto(out *APIServerLoadBalancer) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.Provider != nil { + in, out := &in.Provider, &out.Provider + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APIServerLoadBalancer. 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 ff569270af..db67158b60 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -4866,7 +4866,7 @@ spec: apiServerLoadBalancer: description: |- APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. - It must be activated by setting `enabled: true`. + If not specified, no load balancer will be created for the API server. properties: additionalPorts: description: AdditionalPorts adds additional tcp ports to the @@ -4874,19 +4874,33 @@ spec: items: type: integer type: array + x-kubernetes-list-type: set allowedCIDRs: description: AllowedCIDRs restrict access to all API-Server listeners to the given address CIDRs. items: type: string type: array + x-kubernetes-list-type: set enabled: - description: Enabled defines whether a load balancer should be - created. + default: true + description: |- + Enabled defines whether a load balancer should be created. This value + defaults to true if an APIServerLoadBalancer is given. + + + There is no reason to set this to false. To disable creation of the + API server loadbalancer, omit the APIServerLoadBalancer field in the + cluster spec instead. type: boolean provider: - description: Octavia Provider Used to create load balancer + description: |- + Provider specifies name of a specific Octavia provider to use for the + API load balancer. The Octavia default will be used if it is not + specified. type: string + required: + - enabled type: object apiServerPort: description: |- 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 cefd394be1..5dc38e4098 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2290,7 +2290,7 @@ spec: apiServerLoadBalancer: description: |- APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. - It must be activated by setting `enabled: true`. + If not specified, no load balancer will be created for the API server. properties: additionalPorts: description: AdditionalPorts adds additional tcp ports @@ -2298,19 +2298,33 @@ spec: items: type: integer type: array + x-kubernetes-list-type: set allowedCIDRs: description: AllowedCIDRs restrict access to all API-Server listeners to the given address CIDRs. items: type: string type: array + x-kubernetes-list-type: set enabled: - description: Enabled defines whether a load balancer should - be created. + default: true + description: |- + Enabled defines whether a load balancer should be created. This value + defaults to true if an APIServerLoadBalancer is given. + + + There is no reason to set this to false. To disable creation of the + API server loadbalancer, omit the APIServerLoadBalancer field in the + cluster spec instead. type: boolean provider: - description: Octavia Provider Used to create load balancer + description: |- + Provider specifies name of a specific Octavia provider to use for the + API load balancer. The Octavia default will be used if it is not + specified. type: string + required: + - enabled type: object apiServerPort: description: |- diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index 0ce97f12d5..794047ca47 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 != nil && openStackCluster.Spec.APIServerLoadBalancer.Enabled { + if openStackCluster.Spec.APIServerLoadBalancer.IsEnabled() { loadBalancerService, err := loadbalancer.NewService(scope) if err != nil { return reconcile.Result{}, err @@ -722,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 != nil && openStackCluster.Spec.APIServerLoadBalancer.Enabled: + case openStackCluster.Spec.APIServerLoadBalancer.IsEnabled(): loadBalancerService, err := loadbalancer.NewService(scope) if err != nil { return err diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md index 7995b52c2a..38003ddb7b 100644 --- a/docs/book/src/api/v1beta1/api.md +++ b/docs/book/src/api/v1beta1/api.md @@ -209,7 +209,7 @@ APIServerLoadBalancer (Optional)

APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. -It must be activated by setting enabled: true.

+If not specified, no load balancer will be created for the API server.

@@ -852,7 +852,11 @@ bool -

Enabled defines whether a load balancer should be created.

+

Enabled defines whether a load balancer should be created. This value +defaults to true if an APIServerLoadBalancer is given.

+

There is no reason to set this to false. To disable creation of the +API server loadbalancer, omit the APIServerLoadBalancer field in the +cluster spec instead.

@@ -863,6 +867,7 @@ bool +(Optional)

AdditionalPorts adds additional tcp ports to the load balancer.

@@ -874,6 +879,7 @@ bool +(Optional)

AllowedCIDRs restrict access to all API-Server listeners to the given address CIDRs.

@@ -885,7 +891,10 @@ string -

Octavia Provider Used to create load balancer

+(Optional) +

Provider specifies name of a specific Octavia provider to use for the +API load balancer. The Octavia default will be used if it is not +specified.

@@ -2109,7 +2118,7 @@ APIServerLoadBalancer (Optional)

APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. -It must be activated by setting enabled: true.

+If not specified, no load balancer will be created for the API server.

@@ -2691,7 +2700,7 @@ APIServerLoadBalancer (Optional)

APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. -It must be activated by setting enabled: true.

+If not specified, no load balancer will be created for the API server.

diff --git a/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md b/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md index 7c7484f573..7c98118078 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md @@ -11,13 +11,13 @@ - [`OpenStackMachine`](#openstackmachine) - [Removal of cloudName](#removal-of-cloudname) - [Change to serverGroupID](#change-to-servergroupid) + - [Change to image](#change-to-image) + - [Removal of imageUUID](#removal-of-imageuuid) - [Changes to ports](#changes-to-ports) - [`OpenStackCluster`](#openstackcluster) - [Removal of cloudName](#removal-of-cloudname-1) - [identityRef is now required](#identityref-is-now-required) - [Change to externalNetworkID](#change-to-externalnetworkid) - - [Change to image](#change-to-image) - - [Removal of imageUUID](#removal-of-imageuuid) - [Change to floatingIP](#change-to-floatingip) - [Change to subnet](#change-to-subnet) - [Change to nodeCidr and dnsNameservers](#change-to-nodecidr-and-dnsnameservers) @@ -26,6 +26,7 @@ - [Calico CNI](#calico-cni) - [Change to network](#change-to-network) - [Change to networkMtu](#change-to-networkmtu) + - [Changes to apiServerLoadBalancer](#changes-to-apiserverloadbalancer) - [Changes to filters](#changes-to-filters) - [Changes to filter tags](#changes-to-filter-tags) - [Field capitalization consistency](#field-capitalization-consistency) @@ -87,6 +88,39 @@ serverGroup: If a server group is provided and found, it'll be added to `OpenStackMachine.Status.ReferencedResources.ServerGroupID`. If the server group can't be found or filter matches multiple server groups, an error will be returned. If empty object or null is provided, Machine will not be added to any server group and `OpenStackMachine.Status.ReferencedResources.ServerGroupID` will be empty. +#### Change to image + +The field `image` is now an `ImageFilter` object rather than a string name. +The `ImageFilter` object allows selection of an image by name, by ID or by tags. + +```yaml +image: "test-image" +``` + +becomes + +```yaml +image: + name: "test-image" +``` + +The image ID will be added to `OpenStackMachine.Status.ReferencedResources.ImageID`. If the image can't be found or filter matches multiple images, an error will be returned. + +#### Removal of imageUUID + +The fild `imageUUID` has been removed in favor of the `image` field. + +```yaml +imageUUID: "72a6a1e6-3e0a-4a8b-9b4c-2d6f9e3e5f0a" +``` + +becomes + +```yaml +image: + id: "72a6a1e6-3e0a-4a8b-9b4c-2d6f9e3e5f0a" +``` + #### Changes to ports These changes apply to ports specified in both OpenStackMachines and the Bastion. @@ -150,39 +184,6 @@ It is now possible for a user to specify that no external network should be used disableExternalNetwork: true ``` -#### Change to image - -The field `image` is now an `ImageFilter` object rather than a string name. -The `ImageFilter` object allows selection of an image by name, by ID or by tags. - -```yaml -image: "test-image" -``` - -becomes - -```yaml -image: - name: "test-image" -``` - -The image ID will be added to `OpenStackMachine.Status.ReferencedResources.ImageID`. If the image can't be found or filter matches multiple images, an error will be returned. - -#### Removal of imageUUID - -The fild `imageUUID` has been removed in favor of the `image` field. - -```yaml -imageUUID: "72a6a1e6-3e0a-4a8b-9b4c-2d6f9e3e5f0a" -``` - -becomes - -```yaml -image: - id: "72a6a1e6-3e0a-4a8b-9b4c-2d6f9e3e5f0a" -``` - #### Change to floatingIP The `OpenStackMachineSpec.FloatingIP` field has moved to `OpenStackClusterSpec.Bastion.FloatingIP`. @@ -326,6 +327,23 @@ In v1beta1, when the `OpenStackCluster.Spec.Network` is not defined, the `Subnet In v1beta1, `OpenStackCluster.spec.NetworkMtu` becomes `OpenStackCluster.spec.NetworkMTU` in order to keep the name consistent with K8s API conventions. +#### Changes to apiServerLoadBalancer + +In v1beta1, `OpenStackCluster.spec.apiServerLoadBalancer` becomes optional and can be entirely omitted. Therefore `enabled` can be assumed by the presence of the field. Consequently `enabled` now defaults to `true` if `apiServerLoadBalancer` is specified. The following are now equivalent: + +```yaml +spec: + ... + apiServerLoadBalancer: + enabled: true +``` + +```yaml +spec: + ... + apiServerLoadBalancer: {} +``` + ### Changes to filters #### Changes to filter tags diff --git a/pkg/cloud/services/loadbalancer/loadbalancer.go b/pkg/cloud/services/loadbalancer/loadbalancer.go index 6b716e75cd..993a1dfb22 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer.go @@ -281,9 +281,9 @@ 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 != nil && openStackCluster.Spec.APIServerLoadBalancer.Provider != "" { + if openStackCluster.Spec.APIServerLoadBalancer != nil && openStackCluster.Spec.APIServerLoadBalancer.Provider != nil { for _, v := range providers { - if v.Name == openStackCluster.Spec.APIServerLoadBalancer.Provider { + if v.Name == *openStackCluster.Spec.APIServerLoadBalancer.Provider { lbProvider = v.Name break } diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go index cb5d26d2c0..ce46235d82 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go @@ -59,7 +59,7 @@ func Test_ReconcileLoadBalancer(t *testing.T) { openStackCluster := &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ - Enabled: true, + Enabled: pointer.Bool(true), }, DisableAPIServerFloatingIP: pointer.Bool(true), ControlPlaneEndpoint: &clusterv1.APIEndpoint{ diff --git a/pkg/cloud/services/networking/securitygroups.go b/pkg/cloud/services/networking/securitygroups.go index d1fd89d847..0b67e5fd92 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 != nil && openStackCluster.Spec.APIServerLoadBalancer.Enabled { + if openStackCluster.Spec.APIServerLoadBalancer.IsEnabled() { controlPlaneRules = append(controlPlaneRules, getSGControlPlaneAdditionalPorts(openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts)...) } diff --git a/pkg/webhooks/openstackcluster_webhook_test.go b/pkg/webhooks/openstackcluster_webhook_test.go index 8c9ed564ae..1d2a4cd6ad 100644 --- a/pkg/webhooks/openstackcluster_webhook_test.go +++ b/pkg/webhooks/openstackcluster_webhook_test.go @@ -167,7 +167,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { CloudName: "foobar", }, APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ - Enabled: true, + Enabled: pointer.Bool(true), AllowedCIDRs: []string{ "0.0.0.0/0", "192.168.10.0/24", @@ -182,7 +182,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { CloudName: "foobar", }, APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{ - Enabled: true, + Enabled: pointer.Bool(true), AllowedCIDRs: []string{ "0.0.0.0/0", "192.168.10.0/24", diff --git a/test/e2e/suites/apivalidations/openstackcluster_test.go b/test/e2e/suites/apivalidations/openstackcluster_test.go index 44bc8cbc40..ad1c0d502c 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" + "k8s.io/apimachinery/pkg/types" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" @@ -62,4 +63,27 @@ var _ = Describe("OpenStackCluster API validations", func() { cluster.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed") }) + + It("should default enabled to true if APIServerLoadBalancer is specified without enabled=true", func() { + cluster.Spec.APIServerLoadBalancer = &infrav1.APIServerLoadBalancer{} + Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + + // Fetch the cluster and check the defaulting + fetchedCluster := &infrav1.OpenStackCluster{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed") + + Expect(fetchedCluster.Spec.APIServerLoadBalancer.Enabled).ToNot(BeNil(), "APIServerLoadBalancer.Enabled should have been defaulted") + Expect(*fetchedCluster.Spec.APIServerLoadBalancer.Enabled).To(BeTrue(), "APIServerLoadBalancer.Enabled should default to true") + }) + + It("should not default APIServerLoadBalancer if it is not specifid", func() { + Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + + // Fetch the cluster and check the defaulting + fetchedCluster := &infrav1.OpenStackCluster{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed") + + Expect(fetchedCluster.Spec.APIServerLoadBalancer).To(BeNil(), "APIServerLoadBalancer should not have been defaulted") + Expect(fetchedCluster.Spec.APIServerLoadBalancer.IsEnabled()).To(BeFalse(), "APIServerLoadBalancer.Enabled should not have been defaulted") + }) })