Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DNS entry for ovsdbserver-sb- services #154

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/v1beta1/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func getDBClusters(
return ovnDBList, nil
}

// GetDBClusterByType - return OVNDBCluster for the given dbType
func GetDBClusterByType(
ctx context.Context,
h *helper.Helper,
Expand Down
6 changes: 6 additions & 0 deletions api/v1beta1/ovndbcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ const (
// SBDBType - Southbound database type
SBDBType = "SB"

// DNSSuffix : hardcoded value on how DNSCore domain is configured
DNSSuffix = "cluster.local"
// TODO: retrieve it from environment

// Container image fall-back defaults

// OvnNBContainerImage is the fall-back container image for OVNDBCluster NB
Expand Down Expand Up @@ -189,13 +193,15 @@ func (instance OVNDBCluster) RbacResourceName() string {
return "ovncluster-" + instance.Name
}

// GetInternalEndpoint - return the DNS name that openshift coreDNS can resolve
func (instance OVNDBCluster) GetInternalEndpoint() (string, error) {
if instance.Status.InternalDBAddress == "" {
return "", fmt.Errorf("internal DBEndpoint not ready yet for %s", instance.Spec.DBType)
}
return instance.Status.InternalDBAddress, nil
}

// GetExternalEndpoint - return the DNS that openstack dnsmasq can resolve
func (instance OVNDBCluster) GetExternalEndpoint() (string, error) {
if instance.Spec.NetworkAttachment != "" && instance.Status.DBAddress == "" {
return "", fmt.Errorf("external DBEndpoint not ready yet for %s", instance.Spec.DBType)
Expand Down
12 changes: 12 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,18 @@ rules:
- patch
- update
- watch
- apiGroups:
- network.openstack.org
resources:
- dnsdata
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- ovn.openstack.org
resources:
Expand Down
115 changes: 94 additions & 21 deletions controllers/ovndbcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ import (
"strings"
"time"

"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/go-logr/logr"
infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
"github.com/openstack-k8s-operators/lib-common/modules/common"
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
"github.com/openstack-k8s-operators/lib-common/modules/common/configmap"
Expand All @@ -48,6 +49,7 @@ import (
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// OVNDBClusterReconciler reconciles a OVNDBCluster object
Expand Down Expand Up @@ -85,6 +87,7 @@ func (r *OVNDBClusterReconciler) GetLogger(ctx context.Context) logr.Logger {
//+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;patch;update;delete;
//+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;
//+kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch
//+kubebuilder:rbac:groups=network.openstack.org,resources=dnsdata,verbs=get;list;watch;create;update;patch;delete

// service account, role, rolebinding
// +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update
Expand Down Expand Up @@ -194,6 +197,7 @@ func (r *OVNDBClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
Owns(&corev1.ServiceAccount{}).
Owns(&rbacv1.Role{}).
Owns(&rbacv1.RoleBinding{}).
Owns(&infranetworkv1.DNSData{}).
Complete(r)
}

Expand Down Expand Up @@ -458,7 +462,6 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
if instance.Status.ReadyCount > 0 && len(svcList.Items) > 0 {
averdagu marked this conversation as resolved.
Show resolved Hide resolved
instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
karelyatin marked this conversation as resolved.
Show resolved Hide resolved
instance.Status.Conditions.MarkTrue(condition.ExposeServiceReadyCondition, condition.ExposeServiceReadyMessage)
averdagu marked this conversation as resolved.
Show resolved Hide resolved
dbAddress := []string{}
internalDbAddress := []string{}
raftAddress := []string{}
var svcPort int32
Expand All @@ -467,35 +470,55 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *

// Filter out headless services
if svc.Spec.ClusterIP != "None" {
// Test using hostname instead of ip for dbAddress connection
//serviceHostname := fmt.Sprintf("%s.%s.svc", svc.Name, svc.GetNamespace())
//dbAddress = append(dbAddress, fmt.Sprintf("tcp:%s:%d", serviceHostname, svc.Spec.Ports[0].Port))

internalDbAddress = append(internalDbAddress, fmt.Sprintf("tcp:%s:%d", svc.Spec.ClusterIP, svcPort))
raftAddress = append(raftAddress, fmt.Sprintf("tcp:%s:%d", svc.Spec.ClusterIP, svc.Spec.Ports[1].Port))
}
}

// External dbAddress if networkAttachment is used
if instance.Spec.NetworkAttachment != "" {
net := instance.Namespace + "/" + instance.Spec.NetworkAttachment
if netStat, ok := instance.Status.NetworkAttachments[net]; ok {
for _, instanceIP := range netStat {
dbAddress = append(dbAddress, fmt.Sprintf("tcp:%s:%d", instanceIP, svcPort))
}
internalDbAddress = append(internalDbAddress, fmt.Sprintf("tcp:%s.%s.svc.%s:%d", svc.Name, svc.Namespace, v1beta1.DNSSuffix, svcPort))
raftAddress = append(raftAddress, fmt.Sprintf("tcp:%s.%s.svc.%s:%d", svc.Name, svc.Namespace, v1beta1.DNSSuffix, svc.Spec.Ports[1].Port))
}
}

// Set DB Addresses
// Set DB Address
instance.Status.InternalDBAddress = strings.Join(internalDbAddress, ",")
instance.Status.DBAddress = strings.Join(dbAddress, ",")
// Set RaftAddress
instance.Status.RaftAddress = strings.Join(raftAddress, ",")
}
Log.Info("Reconciled Service successfully")
return ctrl.Result{}, nil
}

func getPodIPv4InNetwork(ovnPod corev1.Pod, namespace string, networkAttachment string) (string, error) {
netStat, err := nad.GetNetworkStatusFromAnnotation(ovnPod.Annotations)
if err != nil {
err = fmt.Errorf("Error while getting the Network Status for pod %s: %v", ovnPod.Name, err)
return "", err
}
for _, v := range netStat {
if v.Name == namespace+"/"+networkAttachment {
karelyatin marked this conversation as resolved.
Show resolved Hide resolved
for _, ip := range v.IPs {
if !strings.Contains(ip, ":") {
return ip, nil
}
}
}
}
// If this is reached it means that no IP was found, construct error and return
err = fmt.Errorf("Error while getting IPv4 address from pod %s in network %s, IP is empty", ovnPod.Name, networkAttachment)
return "", err
}

func deleteDNSData(ctx context.Context, helper *helper.Helper, dnsName string, namespace string) error {
// Delete DNS records for deleted services/pods
dnsData := &infranetworkv1.DNSData{
ObjectMeta: metav1.ObjectMeta{
Name: dnsName,
Namespace: namespace,
},
}
err := helper.GetClient().Delete(ctx, dnsData)
if err != nil && !k8s_errors.IsNotFound(err) {
return fmt.Errorf("Error while cleaning up DNS record %s: %w", dnsName, err)
}
return nil
}

func (r *OVNDBClusterReconciler) reconcileServices(
ctx context.Context,
instance *ovnv1.OVNDBCluster,
Expand Down Expand Up @@ -566,21 +589,71 @@ func (r *OVNDBClusterReconciler) reconcileServices(
)
if err == nil && len(svcList.Items) > int(*(instance.Spec.Replicas)) {
for i := len(svcList.Items) - 1; i >= int(*(instance.Spec.Replicas)); i-- {
fullServiceName := fmt.Sprintf("%s-%d", serviceName, i)
svcLabels := map[string]string{
common.AppSelector: serviceName,
"statefulset.kubernetes.io/pod-name": serviceName + fmt.Sprintf("-%d", i),
"statefulset.kubernetes.io/pod-name": fullServiceName,
}
err = service.DeleteServicesWithLabel(
ctx,
helper,
instance,
svcLabels,
)
if err != nil {
err = fmt.Errorf("Error while deleting service with name %s: %w", fullServiceName, err)
return ctrl.Result{}, err
}
// Delete DNS records for deleted services/pods
booxter marked this conversation as resolved.
Show resolved Hide resolved
namespace := helper.GetBeforeObject().GetNamespace()
err = deleteDNSData(ctx, helper, fullServiceName, namespace)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the dnsdata no longer exists for fulServiceName so this call and definition of function deleteDNSData not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it does exist.
For each service a DNSData will be created. Inside this DNSData entry it will be the resolution ovsdbserver-sb.openstack.svc -> SB_POD_INTERNALAPI_IP
But at the end you'll have the same number of entries as services. What it was deleted was the resolution ovsdbserver-sb-X.openstack.svc -> SB_POD_INTERNALAPI_IP

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okk i expected it to be single dnsdata per service from past discussion, keeping dnsdata per service just looks unnecessary at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's something that can be done, but it would miss dp3. Can create jira to do a follow up if necessary

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok can be checked/done later

if err != nil {
return ctrl.Result{}, err
}
}
}

var svc *corev1.Service

// 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 != "" {
for _, ovnPod := range podList.Items[:*(instance.Spec.Replicas)] {
svc, err = service.GetServiceWithName(
ctx,
helper,
ovnPod.Name,
ovnPod.Namespace,
)
if err != nil {
return ctrl.Result{}, err
averdagu marked this conversation as resolved.
Show resolved Hide resolved
}

// Currently only IPv4 is supported
dnsIP, err := getPodIPv4InNetwork(ovnPod, instance.Namespace, instance.Spec.NetworkAttachment)
if err != nil {
return ctrl.Result{}, err
}

averdagu marked this conversation as resolved.
Show resolved Hide resolved
// Create DNSData CR
err = ovndbcluster.DNSData(
ctx,
helper,
serviceName,
dnsIP,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fixed as not handling ips from multiple replica, just the ip of last pod will persist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't, This will create a DNSData object and it will call CreateOrPatch. So since this is called on every loop iteration, there will be N objects of DNSData, each one resolving that pod's IP to ovsdbserver-(nb|sb).namespace.svc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, reading it again got it as those created with podname. But looks unnecessary to maintain those extra dnsdata per pod. If it could be avoid it's good else can also cleanup later if others are ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok can be checked/done later

instance,
ovnPod,
serviceLabels,
)
if err != nil {
return ctrl.Result{}, err
}
}
}
// dbAddress will contain ovsdbserver-(nb|sb).openstack.svc or empty
// IPv6 is not handled
instance.Status.DBAddress = ovndbcluster.GetDBAddress(svc, serviceName, instance.Namespace)

Log.Info("Reconciled OVN DB Cluster Service successfully")
karelyatin marked this conversation as resolved.
Show resolved Hide resolved
return ctrl.Result{}, nil
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
github.com/onsi/ginkgo/v2 v2.14.0
github.com/onsi/gomega v1.30.0
github.com/openstack-k8s-operators/infra-operator/apis v0.3.0
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240106101723-5f7aa263457f
github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240106101723-5f7aa263457f
github.com/openstack-k8s-operators/ovn-operator/api v0.0.0-20230418071801-b5843d9e05fb
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8=
github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ=
github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxCMwNRnMjhhIDOWHJowi6q8G6koI=
github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
github.com/openstack-k8s-operators/infra-operator/apis v0.3.0 h1:omqNm2mG5YOXdNLuUs4fNCvi/2B13njLXfbS2Z4GNUE=
github.com/openstack-k8s-operators/infra-operator/apis v0.3.0/go.mod h1:zqFs5MrBKeaE4HQroUgMWwIkBwmmcygg6sghcidSdCA=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240106101723-5f7aa263457f h1:ifW2n0TkrS1Wa58DK/N+zAKdGH5XQh4Llk/hefsXzN8=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240106101723-5f7aa263457f/go.mod h1:ov4lAbniNUsLqZCBp1RTixpqXc8JlzA5B+yTcCkJXQg=
github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240106101723-5f7aa263457f h1:Tbw6HGO793AyaxhheNV7r+ftunFHzVBJKtgFG/RNLc0=
Expand Down
2 changes: 2 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"

networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"

ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1"
"github.com/openstack-k8s-operators/ovn-operator/controllers"
Expand All @@ -53,6 +54,7 @@ func init() {
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
utilruntime.Must(ovnv1.AddToScheme(scheme))
utilruntime.Must(networkv1.AddToScheme(scheme))
utilruntime.Must(infranetworkv1.AddToScheme(scheme))
//+kubebuilder:scaffold:scheme
}

Expand Down
67 changes: 67 additions & 0 deletions pkg/ovndbcluster/dnsdata.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package ovndbcluster

import (
"context"
"fmt"

infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

// DNSData - Create DNS entry that openstack dnsmasq will resolve
func DNSData(
ctx context.Context,
helper *helper.Helper,
serviceName string,
ip string,
instance *ovnv1.OVNDBCluster,
ovnPod corev1.Pod,
serviceLabels map[string]string,
) error {
// ovsdbserver-(sb|nb) entry
headlessDNSHostname := serviceName + "." + instance.Namespace + ".svc"
dnsHostCname := infranetworkv1.DNSHost{
IP: ip,
Hostnames: []string{
headlessDNSHostname,
},
}

// Create DNSData object
dnsData := &infranetworkv1.DNSData{
ObjectMeta: metav1.ObjectMeta{
Name: ovnPod.Name,
Namespace: ovnPod.Namespace,
Labels: serviceLabels,
},
}
dnsHosts := []infranetworkv1.DNSHost{dnsHostCname}

_, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), dnsData, func() error {
dnsData.Spec.Hosts = dnsHosts
// TODO: use value from DNSMasq instance instead of hardcode
dnsData.Spec.DNSDataLabelSelectorValue = "dnsdata"
booxter marked this conversation as resolved.
Show resolved Hide resolved
err := controllerutil.SetControllerReference(helper.GetBeforeObject(), dnsData, helper.GetScheme())
if err != nil {
return err
}
return nil
})
if err != nil {
return fmt.Errorf("Error creating DNSData %s: %w", dnsData.Name, err)
}
return nil
}

// GetDBAddress - return string connection for the given service
func GetDBAddress(svc *corev1.Service, serviceName string, namespace string) string {
if svc == nil {
averdagu marked this conversation as resolved.
Show resolved Hide resolved
return ""
}
headlessDNSHostname := serviceName + "." + namespace + ".svc"
return fmt.Sprintf("tcp:%s:%d", headlessDNSHostname, svc.Spec.Ports[0].Port)
}
Loading