Prevent flapping of external DNS configuration

Makefile

@@ -44,7 +44,7 @@ HELM_ARGS ?=
 K8GB_COREDNS_IP ?= kubectl get svc k8gb-coredns -n k8gb -o custom-columns='IP:spec.clusterIP' --no-headers
 LOG_FORMAT ?= simple
 LOG_LEVEL ?= debug
-CONTROLLER_GEN_VERSION  ?= v0.15.0
+CONTROLLER_GEN_VERSION  ?= v0.16.5
 GOLIC_VERSION  ?= v0.7.2
 GOLANGCI_VERSION ?= v1.59.1
 POD_NAMESPACE ?= k8gb

chart/k8gb/README.md

@@ -105,6 +105,7 @@ For Kubernetes `< 1.19` use this chart and k8gb in version `0.8.8` or lower.
 | k8gb.log.format | string | `"simple"` | log format (simple,json) |
 | k8gb.log.level | string | `"info"` | log level (panic,fatal,error,warn,info,debug,trace) |
 | k8gb.metricsAddress | string | `"0.0.0.0:8080"` | Metrics server address |
+| k8gb.nsRecordTTL | int | `30` | TTL of the NS and respective glue record used by external DNS |
 | k8gb.podAnnotations | object | `{}` | pod annotations |
 | k8gb.podLabels | object | `{}` | pod labels |
 | k8gb.reconcileRequeueSeconds | int | `30` | Reconcile time in seconds |

chart/k8gb/crd/k8gb.absa.oss_gslbs.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.15.0
+    controller-gen.kubebuilder.io/version: v0.16.5
   name: gslbs.k8gb.absa.oss
 spec:
   group: k8gb.absa.oss
@@ -111,6 +111,7 @@ spec:
                                 format: int32
                                 type: integer
                             type: object
+                            x-kubernetes-map-type: atomic
                         required:
                         - name
                         type: object
@@ -150,8 +151,8 @@ spec:
                             these may change in the future.\nIncoming requests are
                             matched against the host before the\nIngressRuleValue.
                             If the host is unspecified, the Ingress routes all\ntraffic
-                            based on the specified IngressRuleValue.\n\n\nHost can
-                            be \"precise\" which is a domain name without the terminating
+                            based on the specified IngressRuleValue.\n\nHost can be
+                            \"precise\" which is a domain name without the terminating
                             dot of\na network host (e.g. \"foo.bar.com\") or \"wildcard\",
                             which is a domain name\nprefixed with a single wildcard
                             label (e.g. \"*.foo.com\").\nThe wildcard character '*'
@@ -240,6 +241,7 @@ spec:
                                                 format: int32
                                                 type: integer
                                             type: object
+                                            x-kubernetes-map-type: atomic
                                         required:
                                         - name
                                         type: object

chart/k8gb/templates/deployment.yaml

@@ -66,7 +66,9 @@ spec:
             - name: DNS_ZONE
               value: {{ .Values.k8gb.dnsZone }}
             - name: RECONCILE_REQUEUE_SECONDS
-              value: {{ quote .Values.k8gb.reconcileRequeueSeconds}}
+              value: {{ quote .Values.k8gb.reconcileRequeueSeconds }}
+            - name: NS_RECORD_TTL
+              value: {{ quote .Values.k8gb.nsRecordTTL }}
             {{- if .Values.infoblox.enabled }}
             - name: INFOBLOX_GRID_HOST
               valueFrom:

chart/k8gb/values.schema.json

@@ -297,6 +297,10 @@
                     "type": "integer",
                     "minimum": 0
                 },
+                "nsRecordTTL": {
+                    "type": "integer",
+                    "minimum": 0
+                },
                 "log": {
                     "$ref": "#/definitions/k8gbLog"
                 },

chart/k8gb/values.yaml

@@ -29,6 +29,8 @@ k8gb:
   extGslbClustersGeoTags: "us"
   # -- Reconcile time in seconds
   reconcileRequeueSeconds: 30
+  # -- TTL of the NS and respective glue record used by external DNS
+  nsRecordTTL: 30
   coredns:
     # -- Extra CoreDNS server blocks
     extraServerBlocks: ""

controllers/depresolver/depresolver.go

@@ -120,6 +120,8 @@ type Infoblox struct {
 type Config struct {
 	// Reschedule of Reconcile loop to pickup external Gslb targets
 	ReconcileRequeueSeconds int `env:"RECONCILE_REQUEUE_SECONDS, default=30"`
+	// TTL of the NS and respective glue record used by external DNS
+	NSRecordTTL int `env:"NS_RECORD_TTL, default=30"`
 	// ClusterGeoTag to determine specific location
 	ClusterGeoTag string `env:"CLUSTER_GEO_TAG"`
 	// ExtClustersGeoTags to identify clusters in other locations in format separated by comma. i.e.: "eu,uk,us"

controllers/depresolver/depresolver_config.go

@@ -34,6 +34,7 @@ import (
 // Environment variables keys
 const (
 	ReconcileRequeueSecondsKey = "RECONCILE_REQUEUE_SECONDS"
+	NSRecordTTLKey             = "NS_RECORD_TTL"
 	ClusterGeoTagKey           = "CLUSTER_GEO_TAG"
 	ExtClustersGeoTagsKey      = "EXT_GSLB_CLUSTERS_GEO_TAGS"
 	ExtDNSEnabledKey           = "EXTDNS_ENABLED"
@@ -120,6 +121,10 @@ func (dr *DependencyResolver) validateConfig(config *Config, recognizedDNSTypes
 	if err != nil {
 		return err
 	}
+	err = field(NSRecordTTLKey, config.NSRecordTTL).isHigherThanZero().err
+	if err != nil {
+		return err
+	}
 	err = field(ClusterGeoTagKey, config.ClusterGeoTag).isNotEmpty().matchRegexp(geoTagRegex).err
 	if err != nil {
 		return err

controllers/depresolver/depresolver_test.go

@@ -55,6 +55,7 @@ const (
 
 var predefinedConfig = Config{
 	ReconcileRequeueSeconds: 30,
+	NSRecordTTL:             30,
 	ClusterGeoTag:           "us",
 	ExtClustersGeoTags:      []string{"za", "eu"},
 	EdgeDNSType:             DNSTypeInfoblox,
@@ -264,6 +265,24 @@ func TestResolveConfigWithZeroReconcileRequeueSecondsKey(t *testing.T) {
 	arrangeVariablesAndAssert(t, expected, assert.Error)
 }
 
+func TestResolveConfigWithNegativeNSRecordTTL(t *testing.T) {
+	// arrange
+	defer cleanup()
+	expected := predefinedConfig
+	expected.NSRecordTTL = -1
+	// act,assert
+	arrangeVariablesAndAssert(t, expected, assert.Error)
+}
+
+func TestResolveConfigWithZeroReconcileNSRecordTTL(t *testing.T) {
+	// arrange
+	defer cleanup()
+	expected := predefinedConfig
+	expected.NSRecordTTL = 0
+	// act,assert
+	arrangeVariablesAndAssert(t, expected, assert.Error)
+}
+
 // remove this test once the deprecated key is no longer supported
 func TestResolveConfigWithDeprecatedEdgeDNSServerKey(t *testing.T) {
 	// arrange
@@ -1504,8 +1523,8 @@ func arrangeVariablesAndAssert(t *testing.T, expected Config,
 }
 
 func cleanup() {
-	for _, s := range []string{ReconcileRequeueSecondsKey, ClusterGeoTagKey, ExtClustersGeoTagsKey, EdgeDNSZoneKey, DNSZoneKey, EdgeDNSServersKey,
-		ExtDNSEnabledKey, InfobloxGridHostKey, InfobloxVersionKey, InfobloxPortKey, InfobloxUsernameKey,
+	for _, s := range []string{ReconcileRequeueSecondsKey, NSRecordTTLKey, ClusterGeoTagKey, ExtClustersGeoTagsKey, EdgeDNSZoneKey, DNSZoneKey,
+		EdgeDNSServersKey, ExtDNSEnabledKey, InfobloxGridHostKey, InfobloxVersionKey, InfobloxPortKey, InfobloxUsernameKey,
 		InfobloxPasswordKey, K8gbNamespaceKey, CoreDNSExposedKey, InfobloxHTTPRequestTimeoutKey,
 		InfobloxHTTPPoolConnectionsKey, LogLevelKey, LogFormatKey, LogNoColorKey, MetricsAddressKey, SplitBrainCheckKey, TracingEnabled,
 		TracingSamplingRatio, OtelExporterOtlpEndpoint} {
@@ -1519,6 +1538,7 @@ func configureEnvVar(config Config) {
 	_ = os.Unsetenv(EdgeDNSServerKey)
 	_ = os.Unsetenv(EdgeDNSServerPortKey)
 	_ = os.Setenv(ReconcileRequeueSecondsKey, strconv.Itoa(config.ReconcileRequeueSeconds))
+	_ = os.Setenv(NSRecordTTLKey, strconv.Itoa(config.NSRecordTTL))
 	_ = os.Setenv(ClusterGeoTagKey, config.ClusterGeoTag)
 	_ = os.Setenv(ExtClustersGeoTagsKey, strings.Join(config.ExtClustersGeoTags, ","))
 	_ = os.Setenv(EdgeDNSServersKey, config.EdgeDNSServers.String())

controllers/finalize.go

@@ -28,7 +28,7 @@ func (r *GslbReconciler) finalizeGslb(gslb *k8gbv1beta1.Gslb) (err error) {
 	// needs to do before the CR can be deleted. Examples
 	// of finalizers include performing backups and deleting
 	// resources that are not owned by this CR, like a PVC.
-	err = r.DNSProvider.Finalize(gslb)
+	err = r.DNSProvider.Finalize(gslb, r.Client)
 	if err != nil {
 		log.Err(err).
 			Str("gslb", gslb.Name).

controllers/gslb_controller_reconciliation_test.go

@@ -75,6 +75,7 @@ var crSampleYaml = "../deploy/crds/k8gb.absa.oss_v1beta1_gslb_cr_roundrobin_ingr
 
 var predefinedConfig = depresolver.Config{
 	ReconcileRequeueSeconds: 30,
+	NSRecordTTL:             30,
 	ClusterGeoTag:           "us-west-1",
 	ExtClustersGeoTags:      []string{"us-east-1"},
 	EdgeDNSServers: []utils.DNSServer{

controllers/providers/dns/dns.go

@@ -21,6 +21,7 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic
 import (
 	k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1"
 	"github.com/k8gb-io/k8gb/controllers/providers/assistant"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	externaldns "sigs.k8s.io/external-dns/endpoint"
 )
 
@@ -32,7 +33,7 @@ type Provider interface {
 	// SaveDNSEndpoint update DNS endpoint in gslb or create new one if doesn't exist
 	SaveDNSEndpoint(*k8gbv1beta1.Gslb, *externaldns.DNSEndpoint) error
 	// Finalize finalize gslb in k8gbNamespace
-	Finalize(*k8gbv1beta1.Gslb) error
+	Finalize(*k8gbv1beta1.Gslb, client.Client) error
 	// String see: Stringer interface
 	String() string
 }

controllers/providers/dns/empty.go

@@ -22,6 +22,7 @@ import (
 	k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1"
 	"github.com/k8gb-io/k8gb/controllers/depresolver"
 	"github.com/k8gb-io/k8gb/controllers/providers/assistant"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	externaldns "sigs.k8s.io/external-dns/endpoint"
 )
 
@@ -50,7 +51,7 @@ func (p *EmptyDNSProvider) SaveDNSEndpoint(gslb *k8gbv1beta1.Gslb, i *externaldn
 	return p.assistant.SaveDNSEndpoint(gslb.Namespace, i)
 }
 
-func (p *EmptyDNSProvider) Finalize(gslb *k8gbv1beta1.Gslb) (err error) {
+func (p *EmptyDNSProvider) Finalize(gslb *k8gbv1beta1.Gslb, _ client.Client) (err error) {
 	return p.assistant.RemoveEndpoint(gslb.Name)
 }
 

controllers/providers/dns/external.go

@@ -19,6 +19,7 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic
 */
 
 import (
+	"context"
 	"fmt"
 	"sort"
 	"strings"
@@ -30,6 +31,7 @@ import (
 	k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1"
 	"github.com/k8gb-io/k8gb/controllers/depresolver"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	externaldns "sigs.k8s.io/external-dns/endpoint"
 )
 
@@ -52,7 +54,7 @@ func NewExternalDNS(config depresolver.Config, assistant assistant2.Assistant) *
 }
 
 func (p *ExternalDNSProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1beta1.Gslb) error {
-	ttl := externaldns.TTL(gslb.Spec.Strategy.DNSTtlSeconds)
+	ttl := externaldns.TTL(p.config.NSRecordTTL)
 	log.Info().
 		Interface("provider", p).
 		Msg("Creating/Updating DNSEndpoint CRDs")
@@ -66,6 +68,11 @@ func (p *ExternalDNSProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1bet
 	if p.config.CoreDNSExposed {
 		NSServerIPs, err = p.assistant.CoreDNSExposedIPs()
 	} else {
+		if len(gslb.Status.LoadBalancer.ExposedIPs) == 0 {
+			// do not update DNS Endpoint for External DNS if no IPs are exposed
+			// new GSLB resources may have this field empty
+			return nil
+		}
 		NSServerIPs = gslb.Status.LoadBalancer.ExposedIPs
 	}
 	if err != nil {
@@ -101,7 +108,19 @@ func (p *ExternalDNSProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1bet
 	return nil
 }
 
-func (p *ExternalDNSProvider) Finalize(*k8gbv1beta1.Gslb) error {
+func (p *ExternalDNSProvider) Finalize(_ *k8gbv1beta1.Gslb, k8sClient client.Client) error {
+	gslbList := &k8gbv1beta1.GslbList{}
+
+	err := k8sClient.List(context.TODO(), gslbList)
+	if err != nil {
+		return err
+	}
+
+	// only remove the DNSEndpoint if there are no more GSLB resources
+	if len(gslbList.Items) > 1 {
+		return nil
+	}
+
 	return p.assistant.RemoveEndpoint(p.endpointName)
 }
 

controllers/providers/dns/external_test.go

@@ -59,6 +59,7 @@ var a = struct {
 }{
 	Config: depresolver.Config{
 		ReconcileRequeueSeconds: 30,
+		NSRecordTTL:             30,
 		ClusterGeoTag:           "us",
 		ExtClustersGeoTags:      []string{"za", "eu"},
 		EdgeDNSServers: []utils2.DNSServer{
@@ -96,13 +97,13 @@ var expectedDNSEndpoint = &externaldns.DNSEndpoint{
 		Endpoints: []*externaldns.Endpoint{
 			{
 				DNSName:    a.Config.DNSZone,
-				RecordTTL:  30,
+				RecordTTL:  externaldns.TTL(a.Config.NSRecordTTL),
 				RecordType: "NS",
 				Targets:    a.TargetNSNamesSorted,
 			},
 			{
 				DNSName:    "gslb-ns-us-cloud.example.com",
-				RecordTTL:  30,
+				RecordTTL:  externaldns.TTL(a.Config.NSRecordTTL),
 				RecordType: "A",
 				Targets:    a.TargetIPs,
 			},

controllers/providers/dns/infoblox.go

@@ -23,6 +23,7 @@ import (
 	"reflect"
 	"time"
 
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	externaldns "sigs.k8s.io/external-dns/endpoint"
 
 	ibcl "github.com/infobloxopen/infoblox-go-client"
@@ -156,7 +157,7 @@ func (p *InfobloxProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1beta1.
 	return nil
 }
 
-func (p *InfobloxProvider) Finalize(gslb *k8gbv1beta1.Gslb) error {
+func (p *InfobloxProvider) Finalize(gslb *k8gbv1beta1.Gslb, _ client.Client) error {
 	objMgr, err := p.client.GetObjectManager()
 	if err != nil {
 		return err

controllers/providers/dns/infoblox_test.go

@@ -253,7 +253,7 @@ func TestInfobloxFinalize(t *testing.T) {
 	provider := NewInfobloxDNS(config, a, cl)
 
 	// act
-	err := provider.Finalize(defaultGslb)
+	err := provider.Finalize(defaultGslb, nil)
 
 	// assert
 	assert.NoError(t, err)