From eebf27b47e61c58c6f1188f862713c7c300736f1 Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Thu, 31 Oct 2024 15:34:52 +0100 Subject: [PATCH] Allow exposing ovn DB using a k8s service Using multus to attach the ovn db pods to the networks that the edpm nodes can access it won't work for BGP. A loadbalancer service, like metallb, could be used in this case. This introduces service override to the ovndbcluster, like we have for other services which use a LB service to expose them to the outside. Adding the AnnotationHostnameKey annotation the service hostname gets automatic registered in the DNS instance. In general this could replace all the current implementation of attaching the ovn db pods using multus, creating the headless service and creating the dnsdata to register it in dns. For update/backward compatability this is kept as if if no service override is provided. To expose the ovndbcluster-sb via a metallb loadbalancer would look like: ``` ovn: enabled: true template: ... ovnDBCluster: ovndbcluster-nb: ... ovndbcluster-sb: dbType: SB ... override: service: metadata: annotations: metallb.universe.tf/address-pool: internalapi metallb.universe.tf/allow-shared-ip: internalapi metallb.universe.tf/loadBalancerIPs: 172.17.0.80 spec: type: LoadBalancer ovnNorthd: ... ``` Jira: OSPRH-651 Signed-off-by: Martin Schuppert --- .../ovn.openstack.org_ovndbclusters.yaml | 164 ++++++++++ api/v1beta1/ovndbcluster_types.go | 13 +- api/v1beta1/zz_generated.deepcopy.go | 22 ++ .../ovn.openstack.org_ovndbclusters.yaml | 164 ++++++++++ controllers/ovndbcluster_controller.go | 111 +++++-- pkg/ovndbcluster/dnsdata.go | 6 +- .../ovndbcluster_controller_test.go | 304 ++++++++++++++++++ 7 files changed, 762 insertions(+), 22 deletions(-) diff --git a/api/bases/ovn.openstack.org_ovndbclusters.yaml b/api/bases/ovn.openstack.org_ovndbclusters.yaml index 845cc2f3..06e52f1b 100644 --- a/api/bases/ovn.openstack.org_ovndbclusters.yaml +++ b/api/bases/ovn.openstack.org_ovndbclusters.yaml @@ -83,6 +83,170 @@ spec: description: NodeSelector to target subset of worker nodes running this service type: object + override: + description: Override, provides the ability to override the generated + manifest of several child resources. + properties: + service: + description: Override configuration for the Service created to + serve traffic to the cluster. + properties: + metadata: + description: EmbeddedLabelsAnnotations is an embedded subset + of the fields included in k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta. + Only labels and annotations are included. + properties: + annotations: + additionalProperties: + type: string + description: 'Annotations is an unstructured key value + map stored with a resource that may be set by external + tools to store and retrieve arbitrary metadata. They + are not queryable and should be preserved when modifying + objects. More info: http://kubernetes.io/docs/user-guide/annotations' + type: object + labels: + additionalProperties: + type: string + description: 'Map of string keys and values that can be + used to organize and categorize (scope and select) objects. + May match selectors of replication controllers and services. + More info: http://kubernetes.io/docs/user-guide/labels' + type: object + type: object + spec: + description: OverrideServiceSpec is a subset of the fields + included in https://pkg.go.dev/k8s.io/api@v0.26.6/core/v1#ServiceSpec + Limited to Type, SessionAffinity, LoadBalancerSourceRanges, + ExternalName, ExternalTrafficPolicy, SessionAffinityConfig, + IPFamilyPolicy, LoadBalancerClass and InternalTrafficPolicy + properties: + externalName: + description: externalName is the external reference that + discovery mechanisms will return as an alias for this + service (e.g. a DNS CNAME record). No proxying will + be involved. Must be a lowercase RFC-1123 hostname + (https://tools.ietf.org/html/rfc1123) and requires `type` + to be "ExternalName". + type: string + externalTrafficPolicy: + description: externalTrafficPolicy describes how nodes + distribute service traffic they receive on one of the + Service's "externally-facing" addresses (NodePorts, + ExternalIPs, and LoadBalancer IPs). If set to "Local", + the proxy will configure the service in a way that assumes + that external load balancers will take care of balancing + the service traffic between nodes, and so each node + will deliver traffic only to the node-local endpoints + of the service, without masquerading the client source + IP. (Traffic mistakenly sent to a node with no endpoints + will be dropped.) The default value, "Cluster", uses + the standard behavior of routing to all endpoints evenly + (possibly modified by topology and other features). + Note that traffic sent to an External IP or LoadBalancer + IP from within the cluster will always get "Cluster" + semantics, but clients sending to a NodePort from within + the cluster may need to take traffic policy into account + when picking a node. + type: string + internalTrafficPolicy: + description: InternalTrafficPolicy describes how nodes + distribute service traffic they receive on the ClusterIP. + If set to "Local", the proxy will assume that pods only + want to talk to endpoints of the service on the same + node as the pod, dropping the traffic if there are no + local endpoints. The default value, "Cluster", uses + the standard behavior of routing to all endpoints evenly + (possibly modified by topology and other features). + type: string + ipFamilyPolicy: + description: IPFamilyPolicy represents the dual-stack-ness + requested or required by this Service. If there is no + value provided, then this field will be set to SingleStack. + Services can be "SingleStack" (a single IP family), + "PreferDualStack" (two IP families on dual-stack configured + clusters or a single IP family on single-stack clusters), + or "RequireDualStack" (two IP families on dual-stack + configured clusters, otherwise fail). The ipFamilies + and clusterIPs fields depend on the value of this field. + This field will be wiped when updating a service to + type ExternalName. + type: string + loadBalancerClass: + description: loadBalancerClass is the class of the load + balancer implementation this Service belongs to. If + specified, the value of this field must be a label-style + identifier, with an optional prefix, e.g. "internal-vip" + or "example.com/internal-vip". Unprefixed names are + reserved for end-users. This field can only be set when + the Service type is 'LoadBalancer'. If not set, the + default load balancer implementation is used, today + this is typically done through the cloud provider integration, + but should apply for any default implementation. If + set, it is assumed that a load balancer implementation + is watching for Services with a matching class. Any + default load balancer implementation (e.g. cloud providers) + should ignore Services that set this field. This field + can only be set when creating or updating a Service + to type 'LoadBalancer'. Once set, it can not be changed. + This field will be wiped when a service is updated to + a non 'LoadBalancer' type. + type: string + loadBalancerSourceRanges: + description: 'If specified and supported by the platform, + this will restrict traffic through the cloud-provider + load-balancer will be restricted to the specified client + IPs. This field will be ignored if the cloud-provider + does not support the feature." More info: https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/' + items: + type: string + type: array + sessionAffinity: + description: 'Supports "ClientIP" and "None". Used to + maintain session affinity. Enable client IP based session + affinity. Must be ClientIP or None. Defaults to None. + More info: https://kubernetes.io/docs/concepts/services-networking/service/#virtual-ips-and-service-proxies' + type: string + sessionAffinityConfig: + description: sessionAffinityConfig contains the configurations + of session affinity. + properties: + clientIP: + description: clientIP contains the configurations + of Client IP based session affinity. + properties: + timeoutSeconds: + description: timeoutSeconds specifies the seconds + of ClientIP type session sticky time. The value + must be >0 && <=86400(for 1 day) if ServiceAffinity + == "ClientIP". Default value is 10800(for 3 + hours). + format: int32 + type: integer + type: object + type: object + type: + description: 'type determines how the Service is exposed. + Defaults to ClusterIP. Valid options are ExternalName, + ClusterIP, NodePort, and LoadBalancer. "ClusterIP" allocates + a cluster-internal IP address for load-balancing to + endpoints. Endpoints are determined by the selector + or if that is not specified, by manual construction + of an Endpoints object or EndpointSlice objects. If + clusterIP is "None", no virtual IP is allocated and + the endpoints are published as a set of endpoints rather + than a virtual IP. "NodePort" builds on ClusterIP and + allocates a port on every node which routes to the same + endpoints as the clusterIP. "LoadBalancer" builds on + NodePort and creates an external load-balancer (if supported + in the current cloud) which routes to the same endpoints + as the clusterIP. "ExternalName" aliases this service + to the specified externalName. Several other fields + do not apply to ExternalName services. More info: https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services-service-types' + type: string + type: object + type: object + type: object probeIntervalToActive: default: 60000 description: Active probe interval from standby to active ovsdb-server diff --git a/api/v1beta1/ovndbcluster_types.go b/api/v1beta1/ovndbcluster_types.go index 91a31d61..af8287cc 100644 --- a/api/v1beta1/ovndbcluster_types.go +++ b/api/v1beta1/ovndbcluster_types.go @@ -20,6 +20,7 @@ import ( "fmt" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + "github.com/openstack-k8s-operators/lib-common/modules/common/service" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" corev1 "k8s.io/api/core/v1" @@ -122,6 +123,16 @@ type OVNDBClusterSpecCore struct { // +operator-sdk:csv:customresourcedefinitions:type=spec // TLS - Parameters related to TLS TLS tls.SimpleService `json:"tls,omitempty"` + + // +kubebuilder:validation:Optional + // Override, provides the ability to override the generated manifest of several child resources. + Override OVNDBClusterOverrideSpec `json:"override,omitempty"` +} + +// OVNDBClusterOverrideSpec to override the generated manifest of several child resources. +type OVNDBClusterOverrideSpec struct { + // Override configuration for the Service created to serve traffic to the cluster. + Service *service.OverrideSpec `json:"service,omitempty"` } // OVNDBClusterStatus defines the observed state of OVNDBCluster @@ -208,7 +219,7 @@ func (instance OVNDBCluster) GetInternalEndpoint() (string, error) { // GetExternalEndpoint - return the DNS that openstack dnsmasq can resolve func (instance OVNDBCluster) GetExternalEndpoint() (string, error) { - if instance.Spec.NetworkAttachment != "" && instance.Status.DBAddress == "" { + if (instance.Spec.NetworkAttachment != "" || instance.Spec.Override.Service != nil) && instance.Status.DBAddress == "" { return "", fmt.Errorf("external DBEndpoint not ready yet for %s", instance.Spec.DBType) } return instance.Status.DBAddress, nil diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index dd193ae3..71d7a349 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -23,6 +23,7 @@ package v1beta1 import ( "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + "github.com/openstack-k8s-operators/lib-common/modules/common/service" "k8s.io/apimachinery/pkg/runtime" ) @@ -266,6 +267,26 @@ func (in *OVNDBClusterList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OVNDBClusterOverrideSpec) DeepCopyInto(out *OVNDBClusterOverrideSpec) { + *out = *in + if in.Service != nil { + in, out := &in.Service, &out.Service + *out = new(service.OverrideSpec) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OVNDBClusterOverrideSpec. +func (in *OVNDBClusterOverrideSpec) DeepCopy() *OVNDBClusterOverrideSpec { + if in == nil { + return nil + } + out := new(OVNDBClusterOverrideSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OVNDBClusterSpec) DeepCopyInto(out *OVNDBClusterSpec) { *out = *in @@ -299,6 +320,7 @@ func (in *OVNDBClusterSpecCore) DeepCopyInto(out *OVNDBClusterSpecCore) { } in.Resources.DeepCopyInto(&out.Resources) in.TLS.DeepCopyInto(&out.TLS) + in.Override.DeepCopyInto(&out.Override) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OVNDBClusterSpecCore. diff --git a/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml b/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml index 845cc2f3..06e52f1b 100644 --- a/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml +++ b/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml @@ -83,6 +83,170 @@ spec: description: NodeSelector to target subset of worker nodes running this service type: object + override: + description: Override, provides the ability to override the generated + manifest of several child resources. + properties: + service: + description: Override configuration for the Service created to + serve traffic to the cluster. + properties: + metadata: + description: EmbeddedLabelsAnnotations is an embedded subset + of the fields included in k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta. + Only labels and annotations are included. + properties: + annotations: + additionalProperties: + type: string + description: 'Annotations is an unstructured key value + map stored with a resource that may be set by external + tools to store and retrieve arbitrary metadata. They + are not queryable and should be preserved when modifying + objects. More info: http://kubernetes.io/docs/user-guide/annotations' + type: object + labels: + additionalProperties: + type: string + description: 'Map of string keys and values that can be + used to organize and categorize (scope and select) objects. + May match selectors of replication controllers and services. + More info: http://kubernetes.io/docs/user-guide/labels' + type: object + type: object + spec: + description: OverrideServiceSpec is a subset of the fields + included in https://pkg.go.dev/k8s.io/api@v0.26.6/core/v1#ServiceSpec + Limited to Type, SessionAffinity, LoadBalancerSourceRanges, + ExternalName, ExternalTrafficPolicy, SessionAffinityConfig, + IPFamilyPolicy, LoadBalancerClass and InternalTrafficPolicy + properties: + externalName: + description: externalName is the external reference that + discovery mechanisms will return as an alias for this + service (e.g. a DNS CNAME record). No proxying will + be involved. Must be a lowercase RFC-1123 hostname + (https://tools.ietf.org/html/rfc1123) and requires `type` + to be "ExternalName". + type: string + externalTrafficPolicy: + description: externalTrafficPolicy describes how nodes + distribute service traffic they receive on one of the + Service's "externally-facing" addresses (NodePorts, + ExternalIPs, and LoadBalancer IPs). If set to "Local", + the proxy will configure the service in a way that assumes + that external load balancers will take care of balancing + the service traffic between nodes, and so each node + will deliver traffic only to the node-local endpoints + of the service, without masquerading the client source + IP. (Traffic mistakenly sent to a node with no endpoints + will be dropped.) The default value, "Cluster", uses + the standard behavior of routing to all endpoints evenly + (possibly modified by topology and other features). + Note that traffic sent to an External IP or LoadBalancer + IP from within the cluster will always get "Cluster" + semantics, but clients sending to a NodePort from within + the cluster may need to take traffic policy into account + when picking a node. + type: string + internalTrafficPolicy: + description: InternalTrafficPolicy describes how nodes + distribute service traffic they receive on the ClusterIP. + If set to "Local", the proxy will assume that pods only + want to talk to endpoints of the service on the same + node as the pod, dropping the traffic if there are no + local endpoints. The default value, "Cluster", uses + the standard behavior of routing to all endpoints evenly + (possibly modified by topology and other features). + type: string + ipFamilyPolicy: + description: IPFamilyPolicy represents the dual-stack-ness + requested or required by this Service. If there is no + value provided, then this field will be set to SingleStack. + Services can be "SingleStack" (a single IP family), + "PreferDualStack" (two IP families on dual-stack configured + clusters or a single IP family on single-stack clusters), + or "RequireDualStack" (two IP families on dual-stack + configured clusters, otherwise fail). The ipFamilies + and clusterIPs fields depend on the value of this field. + This field will be wiped when updating a service to + type ExternalName. + type: string + loadBalancerClass: + description: loadBalancerClass is the class of the load + balancer implementation this Service belongs to. If + specified, the value of this field must be a label-style + identifier, with an optional prefix, e.g. "internal-vip" + or "example.com/internal-vip". Unprefixed names are + reserved for end-users. This field can only be set when + the Service type is 'LoadBalancer'. If not set, the + default load balancer implementation is used, today + this is typically done through the cloud provider integration, + but should apply for any default implementation. If + set, it is assumed that a load balancer implementation + is watching for Services with a matching class. Any + default load balancer implementation (e.g. cloud providers) + should ignore Services that set this field. This field + can only be set when creating or updating a Service + to type 'LoadBalancer'. Once set, it can not be changed. + This field will be wiped when a service is updated to + a non 'LoadBalancer' type. + type: string + loadBalancerSourceRanges: + description: 'If specified and supported by the platform, + this will restrict traffic through the cloud-provider + load-balancer will be restricted to the specified client + IPs. This field will be ignored if the cloud-provider + does not support the feature." More info: https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/' + items: + type: string + type: array + sessionAffinity: + description: 'Supports "ClientIP" and "None". Used to + maintain session affinity. Enable client IP based session + affinity. Must be ClientIP or None. Defaults to None. + More info: https://kubernetes.io/docs/concepts/services-networking/service/#virtual-ips-and-service-proxies' + type: string + sessionAffinityConfig: + description: sessionAffinityConfig contains the configurations + of session affinity. + properties: + clientIP: + description: clientIP contains the configurations + of Client IP based session affinity. + properties: + timeoutSeconds: + description: timeoutSeconds specifies the seconds + of ClientIP type session sticky time. The value + must be >0 && <=86400(for 1 day) if ServiceAffinity + == "ClientIP". Default value is 10800(for 3 + hours). + format: int32 + type: integer + type: object + type: object + type: + description: 'type determines how the Service is exposed. + Defaults to ClusterIP. Valid options are ExternalName, + ClusterIP, NodePort, and LoadBalancer. "ClusterIP" allocates + a cluster-internal IP address for load-balancing to + endpoints. Endpoints are determined by the selector + or if that is not specified, by manual construction + of an Endpoints object or EndpointSlice objects. If + clusterIP is "None", no virtual IP is allocated and + the endpoints are published as a set of endpoints rather + than a virtual IP. "NodePort" builds on ClusterIP and + allocates a port on every node which routes to the same + endpoints as the clusterIP. "LoadBalancer" builds on + NodePort and creates an external load-balancer (if supported + in the current cloud) which routes to the same endpoints + as the clusterIP. "ExternalName" aliases this service + to the specified externalName. Several other fields + do not apply to ExternalName services. More info: https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services-service-types' + type: string + type: object + type: object + type: object probeIntervalToActive: default: 60000 description: Active probe interval from standby to active ovsdb-server diff --git a/controllers/ovndbcluster_controller.go b/controllers/ovndbcluster_controller.go index 4a21566a..14808a85 100644 --- a/controllers/ovndbcluster_controller.go +++ b/controllers/ovndbcluster_controller.go @@ -379,7 +379,7 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance * err.Error())) return ctrl.Result{}, err } - } else if instance.Spec.DBType == ovnv1.SBDBType { + } else if instance.Spec.DBType == ovnv1.SBDBType && instance.Spec.Override.Service == nil { // This config map was created by the SB and it only needs to be deleted once // since this reconcile loop can be done by the SB and the NB, filtering so only // one deletes it. @@ -613,10 +613,11 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance * for _, svc := range svcList.Items { svcPort = svc.Spec.Ports[0].Port - // Filter out headless services - if svc.Spec.ClusterIP != "None" { - internalDbAddress = append(internalDbAddress, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, ovnv1.DNSSuffix, svcPort)) + // Filter out headless and loadbalancer services + if svc.Spec.ClusterIP == "None" || svc.Spec.Type == corev1.ServiceTypeLoadBalancer { + continue } + internalDbAddress = append(internalDbAddress, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, ovnv1.DNSSuffix, svcPort)) } // Note setting this to the singular headless service address (e.g ssl:ovsdbserver-sb...) "works" but will not @@ -626,7 +627,7 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance * // Set DB Address instance.Status.InternalDBAddress = strings.Join(internalDbAddress, ",") - if instance.Spec.DBType == ovnv1.SBDBType && instance.Spec.NetworkAttachment != "" { + if instance.Spec.DBType == ovnv1.SBDBType && (instance.Spec.NetworkAttachment != "" || instance.Spec.Override.Service != nil) { // This config map will populate the sb db address to edpm, can't use the nb // If there's no networkAttachments the configMap is not needed configMapVars := make(map[string]env.Setter) @@ -670,21 +671,76 @@ func (r *OVNDBClusterReconciler) reconcileServices( Log.Info("Reconciling OVN DB Cluster Service") + var ssvc *service.Service + var err error + svcOverride := instance.Spec.Override.Service + if svcOverride != nil { + if svcOverride.EmbeddedLabelsAnnotations == nil { + svcOverride.EmbeddedLabelsAnnotations = &service.EmbeddedLabelsAnnotations{} + } + if svcOverride.Spec == nil { + svcOverride.Spec = &service.OverrideServiceSpec{} + } + } + // - // Ensure the ovndbcluster headless service Exists + // Ensure the ovndbcluster service Exists, if no override is provided, a headless service gets created // - headlessServiceLabels := util.MergeMaps(serviceLabels, map[string]string{"type": ovnv1.ServiceHeadlessType}) + if svcOverride != nil && svcOverride.Spec.Type == corev1.ServiceTypeLoadBalancer { + // When switch from headless service to a LoadBalancer type, we delete it and get it re-created + // because ClusterIP can not be None + svc, err := service.GetServiceWithName(ctx, helper, serviceName, instance.Namespace) + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, err + } + if svc != nil && svc.Spec.ClusterIP == "None" { + err = helper.GetClient().Delete(ctx, svc) + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("error deleting service for transition to use serviceOerrides %s: %w", serviceName, err) + } + } - headlesssvc, err := service.NewService( - ovndbcluster.HeadlessService(serviceName, instance, headlessServiceLabels, serviceLabels), - time.Duration(5)*time.Second, - nil, - ) - if err != nil { - return ctrl.Result{}, err + svcLabels := util.MergeMaps(serviceLabels, map[string]string{"type": strings.ToLower(string(svcOverride.Spec.Type))}) + ssvc, err = service.NewService( + ovndbcluster.Service(serviceName, instance, svcLabels, serviceLabels), + time.Duration(5)*time.Second, + svcOverride, + ) + if err != nil { + return ctrl.Result{}, err + } + + // add annotation to register service name in dnsmasq + if ssvc.GetServiceType() == corev1.ServiceTypeLoadBalancer { + ssvc.AddAnnotation(map[string]string{ + service.AnnotationHostnameKey: ssvc.GetServiceHostname(), + }) + } + } else { + // When switch from LoadBalancer to a headless service, we delete it and get it re-created + svc, err := service.GetServiceWithName(ctx, helper, serviceName, instance.Namespace) + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, err + } + if svc != nil && svc.Spec.Type == corev1.ServiceTypeLoadBalancer { + err = helper.GetClient().Delete(ctx, svc) + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("error deleting service for transition to use headless service %s: %w", serviceName, err) + } + } + + svcLabels := util.MergeMaps(serviceLabels, map[string]string{"type": ovnv1.ServiceHeadlessType}) + ssvc, err = service.NewService( + ovndbcluster.HeadlessService(serviceName, instance, svcLabels, serviceLabels), + time.Duration(5)*time.Second, + nil, + ) + if err != nil { + return ctrl.Result{}, err + } } - ctrlResult, err := headlesssvc.CreateOrPatch(ctx, helper) + ctrlResult, err := ssvc.CreateOrPatch(ctx, helper) if err != nil { return ctrl.Result{}, err } else if (ctrlResult != ctrl.Result{}) { @@ -754,7 +810,9 @@ func (r *OVNDBClusterReconciler) reconcileServices( // When the cluster is attached to an external network, create DNS record for every // cluster member so it can be resolved from outside cluster (edpm nodes) - if instance.Spec.NetworkAttachment != "" { + // This is not required for LoadBalancerType because we set the annotation there to + // get it automatically registered in DNS. + if instance.Spec.NetworkAttachment != "" && ssvc.GetServiceType() != corev1.ServiceTypeLoadBalancer { var dnsIPsList []string // TODO(averdagu): use built in Min once go1.21 is used minLen := ovn_common.Min(len(podList.Items), int(*(instance.Spec.Replicas))) @@ -774,7 +832,6 @@ func (r *OVNDBClusterReconciler) reconcileServices( if err != nil { return ctrl.Result{}, err } - } // DNSData info is called every reconcile loop to ensure that even if a pod gets // restarted and it's IP has changed, the DNSData CR will have the correct info. @@ -798,13 +855,31 @@ func (r *OVNDBClusterReconciler) reconcileServices( Log.Info(fmt.Sprintf("not all pods are yet created, number of expected pods: %v, current pods: %v", *(instance.Spec.Replicas), len(podList.Items))) return ctrl.Result{RequeueAfter: 1 * time.Second}, nil } + } else { + // cleanup dnsData either if there are no NAD, or LB k8s service + dnsdata := &infranetworkv1.DNSData{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceName, + Namespace: instance.Namespace, + }, + } + + err = helper.GetClient().Delete(ctx, dnsdata) + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("error deleting dnsdata %s: %w", serviceName, err) + } } + // dbAddress will contain ovsdbserver-(nb|sb).openstack.svc or empty scheme := "tcp" if instance.Spec.TLS.Enabled() { scheme = "ssl" } - instance.Status.DBAddress = ovndbcluster.GetDBAddress(svc, serviceName, instance.Namespace, scheme) + if ssvc.GetServiceType() == corev1.ServiceTypeLoadBalancer { + instance.Status.DBAddress = ovndbcluster.GetDBAddress(ssvc.GetSpec(), serviceName, instance.Namespace, scheme) + } else if svc != nil { + instance.Status.DBAddress = ovndbcluster.GetDBAddress(&svc.Spec, serviceName, instance.Namespace, scheme) + } Log.Info("Reconciled OVN DB Cluster Service successfully") return ctrl.Result{}, nil diff --git a/pkg/ovndbcluster/dnsdata.go b/pkg/ovndbcluster/dnsdata.go index 57880fc8..fcb9a181 100644 --- a/pkg/ovndbcluster/dnsdata.go +++ b/pkg/ovndbcluster/dnsdata.go @@ -61,10 +61,10 @@ func DNSData( } // GetDBAddress - return string connection for the given service -func GetDBAddress(svc *corev1.Service, serviceName string, namespace string, scheme string) string { - if svc == nil { +func GetDBAddress(svcSpec *corev1.ServiceSpec, serviceName string, namespace string, scheme string) string { + if svcSpec == nil { return "" } headlessDNSHostname := serviceName + "." + namespace + ".svc" - return fmt.Sprintf("%s:%s:%d", scheme, headlessDNSHostname, svc.Spec.Ports[0].Port) + return fmt.Sprintf("%s:%s:%d", scheme, headlessDNSHostname, svcSpec.Ports[0].Port) } diff --git a/tests/functional/ovndbcluster_controller_test.go b/tests/functional/ovndbcluster_controller_test.go index 966e4cf9..c6454d8c 100644 --- a/tests/functional/ovndbcluster_controller_test.go +++ b/tests/functional/ovndbcluster_controller_test.go @@ -24,6 +24,7 @@ import ( . "github.com/onsi/gomega" //revive:disable:dot-imports //revive:disable-next-line:dot-imports + "github.com/openstack-k8s-operators/lib-common/modules/common/service" . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" @@ -32,6 +33,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) var _ = Describe("OVNDBCluster controller", func() { @@ -624,6 +626,308 @@ var _ = Describe("OVNDBCluster controller", func() { corev1.ConditionTrue, ) }) + + When("OVNDBCluster gets migrated to use service override", func() { + BeforeEach(func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + nad := th.CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(th.DeleteInstance, nad) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + + th.ExpectCondition( + OVNDBClusterName, + ConditionGetterFunc(OVNDBClusterConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionTrue, + ) + + Eventually(func(g Gomega) { + OVNDBCluster := GetOVNDBCluster(OVNDBClusterName) + g.Expect(OVNDBCluster.Status.NetworkAttachments).To( + Equal(map[string][]string{namespace + "/internalapi": {"10.0.0.1"}})) + + }, timeout, interval).Should(Succeed()) + + th.ExpectCondition( + OVNDBClusterName, + ConditionGetterFunc(OVNDBClusterConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + }) + + It("creates a loadbalancer service for accessing the SB DB", func() { + ovnDB := GetOVNDBCluster(OVNDBClusterName) + Expect(ovnDB).ToNot(BeNil()) + + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, ovnDB, func() error { + ovnDB.Spec.Override.Service = &service.OverrideSpec{ + EmbeddedLabelsAnnotations: &service.EmbeddedLabelsAnnotations{ + Annotations: map[string]string{ + "metallb.universe.tf/address-pool": "osp-internalapi", + "metallb.universe.tf/allow-shared-ip": "osp-internalapi", + "metallb.universe.tf/loadBalancerIPs": "internal-lb-ip-1,internal-lb-ip-2", + }, + }, + Spec: &service.OverrideServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + }, + } + ovnDB.Spec.NetworkAttachment = "" + return nil + }) + Expect(err).ToNot(HaveOccurred()) + th.SimulateLoadBalancerServiceIP(types.NamespacedName{Namespace: namespace, Name: "ovsdbserver-sb"}) + + // Cluster is created with 1 replica, serviceListWithoutTypeLabel should be 2: + // - ovsdbserver-sb (loadbalancer type) + // - ovsdbserver-sb-0 (cluster type) + Eventually(func(g Gomega) { + serviceListWithoutTypeLabel := GetServicesListWithLabel(namespace) + g.Expect(serviceListWithoutTypeLabel.Items).To(HaveLen(2)) + }).Should(Succeed()) + + Eventually(func(g Gomega) { + serviceListWithClusterType := GetServicesListWithLabel(namespace, map[string]string{"type": "cluster"}) + g.Expect(serviceListWithClusterType.Items).To(HaveLen(1)) + g.Expect(serviceListWithClusterType.Items[0].Name).To(Equal("ovsdbserver-sb-0")) + }).Should(Succeed()) + + Eventually(func(g Gomega) { + serviceListWithLoadBalancerType := GetServicesListWithLabel(namespace, map[string]string{"type": "loadbalancer"}) + g.Expect(serviceListWithLoadBalancerType.Items).To(HaveLen(1)) + g.Expect(serviceListWithLoadBalancerType.Items[0].Name).To(Equal("ovsdbserver-sb")) + g.Expect(serviceListWithLoadBalancerType.Items[0].Spec.Type).To(Equal(corev1.ServiceTypeLoadBalancer)) + }).Should(Succeed()) + }) + }) + }) + + When("OVNDBCluster is created with a service override", func() { + var OVNDBClusterName types.NamespacedName + BeforeEach(func() { + spec := GetDefaultOVNDBClusterSpec() + spec.Override.Service = &service.OverrideSpec{ + EmbeddedLabelsAnnotations: &service.EmbeddedLabelsAnnotations{ + Annotations: map[string]string{ + "metallb.universe.tf/address-pool": "osp-internalapi", + "metallb.universe.tf/allow-shared-ip": "osp-internalapi", + "metallb.universe.tf/loadBalancerIPs": "internal-lb-ip-1,internal-lb-ip-2", + }, + }, + Spec: &service.OverrideServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + }, + } + + spec.DBType = ovnv1.SBDBType + instance := CreateOVNDBCluster(namespace, spec) + OVNDBClusterName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()} + DeferCleanup(th.DeleteInstance, instance) + }) + + It("should create services", func() { + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{}, + ) + th.SimulateLoadBalancerServiceIP(types.NamespacedName{Namespace: namespace, Name: "ovsdbserver-sb"}) + // Cluster is created with 1 replica, serviceListWithoutTypeLabel should be 2: + // - ovsdbserver-sb (loadbalancer type) + // - ovsdbserver-sb-0 (cluster type) + Eventually(func(g Gomega) { + serviceListWithoutTypeLabel := GetServicesListWithLabel(namespace) + g.Expect(serviceListWithoutTypeLabel.Items).To(HaveLen(2)) + }).Should(Succeed()) + + Eventually(func(g Gomega) { + serviceListWithClusterType := GetServicesListWithLabel(namespace, map[string]string{"type": "cluster"}) + g.Expect(serviceListWithClusterType.Items).To(HaveLen(1)) + g.Expect(serviceListWithClusterType.Items[0].Name).To(Equal("ovsdbserver-sb-0")) + }).Should(Succeed()) + + Eventually(func(g Gomega) { + serviceListWithLoadBalancerType := GetServicesListWithLabel(namespace, map[string]string{"type": "loadbalancer"}) + g.Expect(serviceListWithLoadBalancerType.Items).To(HaveLen(1)) + g.Expect(serviceListWithLoadBalancerType.Items[0].Name).To(Equal("ovsdbserver-sb")) + }).Should(Succeed()) + }) + + It("should create an external ConfigMap with expected key-value pairs and OwnerReferences set", func() { + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{}, + ) + th.SimulateLoadBalancerServiceIP(types.NamespacedName{Namespace: namespace, Name: "ovsdbserver-sb"}) + + externalCM := types.NamespacedName{ + Namespace: OVNDBClusterName.Namespace, + Name: "ovncontroller-config", + } + + ExpectedExternalSBEndpoint := "tcp:ovsdbserver-sb." + namespace + ".svc:6642" + + Eventually(func() corev1.ConfigMap { + return *th.GetConfigMap(externalCM) + }, timeout, interval).ShouldNot(BeNil()) + + // Check OwnerReferences set correctly for the Config Map + Expect(th.GetConfigMap(externalCM).ObjectMeta.OwnerReferences[0].Name).To(Equal(OVNDBClusterName.Name)) + Expect(th.GetConfigMap(externalCM).ObjectMeta.OwnerReferences[0].Kind).To(Equal("OVNDBCluster")) + + Eventually(func(g Gomega) { + g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should( + ContainSubstring("ovn-remote: %s", ExpectedExternalSBEndpoint)) + }, timeout, interval).Should(Succeed()) + }) + + It("should create an external ConfigMap with ovn-encap-type if OVNController is configured", func() { + ExpectedEncapType := "vxlan" + // Spawn OVNController with vxlan as ExternalIDs.OvnEncapType + ovncontrollerSpec := GetDefaultOVNControllerSpec() + ovncontrollerSpec.ExternalIDS.OvnEncapType = ExpectedEncapType + ovnController := CreateOVNController(namespace, ovncontrollerSpec) + DeferCleanup(th.DeleteInstance, ovnController) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{}, + ) + th.SimulateLoadBalancerServiceIP(types.NamespacedName{Namespace: namespace, Name: "ovsdbserver-sb"}) + + externalCM := types.NamespacedName{ + Namespace: OVNDBClusterName.Namespace, + Name: "ovncontroller-config", + } + + ExpectedExternalSBEndpoint := "tcp:ovsdbserver-sb." + namespace + ".svc:6642" + + Eventually(func() corev1.ConfigMap { + return *th.GetConfigMap(externalCM) + }, timeout, interval).ShouldNot(BeNil()) + + // Check OwnerReferences set correctly for the Config Map + Expect(th.GetConfigMap(externalCM).ObjectMeta.OwnerReferences[0].Name).To(Equal(OVNDBClusterName.Name)) + Expect(th.GetConfigMap(externalCM).ObjectMeta.OwnerReferences[0].Kind).To(Equal("OVNDBCluster")) + + Eventually(func(g Gomega) { + g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should( + ContainSubstring("ovn-remote: %s", ExpectedExternalSBEndpoint)) + }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should( + ContainSubstring("ovn-encap-type: %s", ExpectedEncapType)) + }, timeout, interval).Should(Succeed()) + }) + + It("should remove ovnEncapType if OVNController gets deleted", func() { + ExpectedEncapType := "vxlan" + // Spawn OVNController with vxlan as ExternalIDs.OvnEncapType + ovncontrollerSpec := GetDefaultOVNControllerSpec() + ovncontrollerSpec.ExternalIDS.OvnEncapType = ExpectedEncapType + ovnController := CreateOVNController(namespace, ovncontrollerSpec) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{}, + ) + th.SimulateLoadBalancerServiceIP(types.NamespacedName{Namespace: namespace, Name: "ovsdbserver-sb"}) + + externalCM := types.NamespacedName{ + Namespace: OVNDBClusterName.Namespace, + Name: "ovncontroller-config", + } + + ExpectedExternalSBEndpoint := "tcp:ovsdbserver-sb." + namespace + ".svc:6642" + + Eventually(func() corev1.ConfigMap { + return *th.GetConfigMap(externalCM) + }, timeout, interval).ShouldNot(BeNil()) + + // Check OwnerReferences set correctly for the Config Map + Expect(th.GetConfigMap(externalCM).ObjectMeta.OwnerReferences[0].Name).To(Equal(OVNDBClusterName.Name)) + Expect(th.GetConfigMap(externalCM).ObjectMeta.OwnerReferences[0].Kind).To(Equal("OVNDBCluster")) + + Eventually(func(g Gomega) { + g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should( + ContainSubstring("ovn-remote: %s", ExpectedExternalSBEndpoint)) + }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should( + ContainSubstring("ovn-encap-type: %s", ExpectedEncapType)) + }, timeout, interval).Should(Succeed()) + + // This should trigger an OVNDBCluster reconcile and update config map + // without ovn-encap-type + DeleteOVNController(types.NamespacedName{Name: ovnController.GetName(), Namespace: namespace}) + + Eventually(func() corev1.ConfigMap { + return *th.GetConfigMap(externalCM) + }, timeout, interval).ShouldNot(BeNil()) + Eventually(func(g Gomega) { + g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should( + ContainSubstring("ovn-remote: %s", ExpectedExternalSBEndpoint)) + }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).ShouldNot( + ContainSubstring("ovn-encap-type: %s", ExpectedEncapType)) + }, timeout, interval).Should(Succeed()) + }) + + It("should delete an external ConfigMap once SB DBCluster service override is removed", func() { + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{}, + ) + th.SimulateLoadBalancerServiceIP(types.NamespacedName{Namespace: namespace, Name: "ovsdbserver-sb"}) + + externalCM := types.NamespacedName{ + Namespace: OVNDBClusterName.Namespace, + Name: "ovncontroller-config", + } + + // Should exist externalCM + Eventually(func() corev1.ConfigMap { + return *th.GetConfigMap(externalCM) + }, timeout, interval).ShouldNot(BeNil()) + + // Detach SBCluster from NAD + Eventually(func(g Gomega) { + ovndbcluster := GetOVNDBCluster(OVNDBClusterName) + ovndbcluster.Spec.Override.Service = nil + g.Expect(k8sClient.Update(ctx, ovndbcluster)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + th.AssertConfigMapDoesNotExist(externalCM) + }) }) When("OVNDBCluster is created with TLS", func() {