Merge pull request #245 from averdagu/feat/dns-cleanup

Clean redundant DNS records

controllers/ovndbcluster_controller.go

@@ -734,12 +734,6 @@ func (r *OVNDBClusterReconciler) reconcileServices(
 				err = fmt.Errorf("Error while deleting service with name %s: %w", fullServiceName, err)
 				return ctrl.Result{}, err
 			}
-			// Delete DNS records for deleted services/pods
-			namespace := helper.GetBeforeObject().GetNamespace()
-			err = deleteDNSData(ctx, helper, fullServiceName, namespace)
-			if err != nil {
-				return ctrl.Result{}, err
-			}
 		}
 	}
 
@@ -748,6 +742,7 @@ 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 != "" {
+		var dnsIPsList []string
 		for _, ovnPod := range podList.Items[:*(instance.Spec.Replicas)] {
 			svc, err = service.GetServiceWithName(
 				ctx,
@@ -760,23 +755,23 @@ func (r *OVNDBClusterReconciler) reconcileServices(
 			}
 
 			dnsIP, err := getPodIPInNetwork(ovnPod, instance.Namespace, instance.Spec.NetworkAttachment)
+			dnsIPsList = append(dnsIPsList, dnsIP)
 			if err != nil {
 				return ctrl.Result{}, err
 			}
 
-			// Create DNSData CR
-			err = ovndbcluster.DNSData(
-				ctx,
-				helper,
-				serviceName,
-				dnsIP,
-				instance,
-				ovnPod,
-				serviceLabels,
-			)
-			if err != nil {
-				return ctrl.Result{}, err
-			}
+		}
+		// Create DNSData CR
+		err = ovndbcluster.DNSData(
+			ctx,
+			helper,
+			serviceName,
+			dnsIPsList,
+			instance,
+			serviceLabels,
+		)
+		if err != nil {
+			return ctrl.Result{}, err
 		}
 	}
 	// dbAddress will contain ovsdbserver-(nb|sb).openstack.svc or empty

pkg/ovndbcluster/dnsdata.go

@@ -17,29 +17,32 @@ func DNSData(
 	ctx context.Context,
 	helper *helper.Helper,
 	serviceName string,
-	ip string,
+	ipList []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,
-		},
+	dnsHosts := []infranetworkv1.DNSHost{}
+	for _, ip := range ipList {
+		record := infranetworkv1.DNSHost{
+			IP: ip,
+			Hostnames: []string{
+				headlessDNSHostname,
+			},
+		}
+		dnsHosts = append(dnsHosts, record)
+
 	}
 
 	// Create DNSData object
 	dnsData := &infranetworkv1.DNSData{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      ovnPod.Name,
-			Namespace: ovnPod.Namespace,
+			Name:      serviceName,
+			Namespace: instance.Namespace,
 			Labels:    serviceLabels,
 		},
 	}
-	dnsHosts := []infranetworkv1.DNSHost{dnsHostCname}
 
 	_, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), dnsData, func() error {
 		dnsData.Spec.Hosts = dnsHosts

tests/functional/base_test.go

@@ -311,32 +311,20 @@ func GetDNSData(name types.NamespacedName) *infranetworkv1.DNSData {
 	return dns
 }
 
-func GetDNSDataList(name types.NamespacedName, labelSelector string) *infranetworkv1.DNSDataList {
-	dnsList := &infranetworkv1.DNSDataList{}
-	dnsListOpts := client.ListOptions{
-		Namespace: name.Namespace,
-	}
-	ml := client.MatchingLabels{
-		"service": labelSelector,
-	}
-	ml.ApplyToList(&dnsListOpts)
-	Eventually(func(g Gomega) {
-		g.Expect(k8sClient.List(ctx, dnsList, &dnsListOpts)).Should(Succeed())
-	}).Should(Succeed())
+func GetDNSDataHostsList(namespace string, dnsEntryName string) []infranetworkv1.DNSHost {
+	dnsEntry := GetDNSData(types.NamespacedName{Name: dnsEntryName, Namespace: namespace})
 
-	return dnsList
+	return dnsEntry.Spec.Hosts
 }
 
-func GetDNSDataHostnameIP(dnsDataName string, namespace string, dnsHostname string) string {
-	dnsEntry := GetDNSData(types.NamespacedName{Name: dnsDataName, Namespace: namespace})
-	for _, host := range dnsEntry.Spec.Hosts {
-		for i, hostname := range host.Hostnames {
-			if hostname == dnsHostname {
-				return dnsEntry.Spec.Hosts[i].IP
-			}
+func CheckDNSDataContainsIp(namespace string, dnsEntryName string, ip string) bool {
+	hostList := GetDNSDataHostsList(namespace, dnsEntryName)
+	for _, host := range hostList {
+		if host.IP == ip {
+			return true
 		}
 	}
-	return ""
+	return false
 }
 
 func GetPod(name types.NamespacedName) *corev1.Pod {

tests/functional/ovndbcluster_controller_test.go

@@ -19,7 +19,6 @@ package functional_test
 import (
 	"encoding/json"
 	"fmt"
-	"strings"
 
 	networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	. "github.com/onsi/ginkgo/v2"
@@ -58,8 +57,8 @@ var _ = Describe("OVNDBCluster controller", func() {
 					}
 
 					Eventually(func(g Gomega) int {
-						listDns := GetDNSDataList(statefulSetName, "ovsdbserver-"+DnsName)
-						return len(listDns.Items)
+						DnsHostsList := GetDNSDataHostsList(statefulSetName.Namespace, "ovsdbserver-"+DnsName)
+						return len(DnsHostsList)
 					}).Should(BeNumerically("==", 3))
 
 					// Scale down to 1
@@ -71,24 +70,12 @@ var _ = Describe("OVNDBCluster controller", func() {
 
 					// Check if dnsdata CR is down to 1
 					Eventually(func() int {
-						targetedDnsOcurrences := 0
-						listDns := GetDNSDataList(statefulSetName, "ovsdbserver-"+DnsName)
-						// This calls CreateOVNDBClusters at the BeforeEach, which will
-						// create NB and SB cluster, both belonging at the same namespace
-						// GetDNSDataList will return ALL occurrences of DNSData (will include
-						// the NB and the SB occurences). Since both clusters were created with 3
-						// replicas and only the targeted one has been scaled down to 1 if we don't
-						// filter by name the result would be 4 (3+1)
-						for _, d := range listDns.Items {
-							if strings.Contains(d.Name, DnsName) {
-								targetedDnsOcurrences++
-							}
-						}
-						return targetedDnsOcurrences
+						listDns := GetDNSDataHostsList(statefulSetName.Namespace, "ovsdbserver-"+DnsName)
+						return len(listDns)
 					}).Should(BeNumerically("==", 1))
 				},
-				Entry("DNS entry NB", "sb"),
-				Entry("DNS entry SB", "nb"),
+				Entry("DNS entry NB", "nb"),
+				Entry("DNS entry SB", "sb"),
 			)
 			DescribeTable("Should update DNSData IP if pod IP changes",
 				func(DNSEntryName string) {
@@ -102,12 +89,10 @@ var _ = Describe("OVNDBCluster controller", func() {
 					OVNSBDBClusterName = types.NamespacedName{Name: dbs[1].Name, Namespace: dbs[1].Namespace}
 					cluster = GetOVNDBCluster(OVNSBDBClusterName)
 					clusterName = OVNSBDBClusterName
-					fullDNSEntryName := DNSEntryName + "." + cluster.Namespace + ".svc"
 
 					// Check that DNSData info has been created with correct IP (10.0.0.1)
 					Eventually(func(g Gomega) {
-						ip := GetDNSDataHostnameIP("ovsdbserver-sb-0", cluster.Namespace, fullDNSEntryName)
-						g.Expect(ip).Should(Equal("10.0.0.1"))
+						g.Expect(CheckDNSDataContainsIp(cluster.Namespace, "ovsdbserver-sb", "10.0.0.1")).Should(BeTrue())
 					}).Should(Succeed())
 
 					// Modify pod IP section
@@ -135,8 +120,7 @@ var _ = Describe("OVNDBCluster controller", func() {
 
 					// Check that DNSData info has been modified with correct IP (10.0.0.10)
 					Eventually(func(g Gomega) {
-						ip := GetDNSDataHostnameIP("ovsdbserver-sb-0", cluster.Namespace, fullDNSEntryName)
-						g.Expect(ip).Should(Equal("10.0.0.10"))
+						g.Expect(CheckDNSDataContainsIp(cluster.Namespace, "ovsdbserver-sb", "10.0.0.1")).Should(BeTrue())
 					}).Should(Succeed())
 
 				},