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.

Sign up for GitHub

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

Terratest checking Local DNSEndpoint #918

Closed
wants to merge 3 commits into from
Closed

Terratest checking Local DNSEndpoint #918

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bef0d68
Weight Round Robin (3/4) - Endpoint
kuritka Jun 16, 2022
994dc95
Terratest checking Local DNSEndpoint
kuritka Jun 30, 2022
5cd7593
Merge branch 'master' into wrr-3-annotation-merge-terratest
kuritka Jul 12, 2022
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
49 changes: 30 additions & 19 deletions controllers/dnsupdate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,17 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic

import (
"fmt"
"sort"
"strings"

"github.com/k8gb-io/k8gb/controllers/depresolver"
"github.com/k8gb-io/k8gb/controllers/providers/assistant"

k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
externaldns "sigs.k8s.io/external-dns/endpoint"
)

func sortTargets(targets []string) []string {
sort.Slice(targets, func(i, j int) bool {
return targets[i] < targets[j]
})
return targets
}

func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.DNSEndpoint, error) {
var gslbHosts []*externaldns.Endpoint
var ttl = externaldns.TTL(gslb.Spec.Strategy.DNSTtlSeconds)
Expand All @@ -53,7 +46,7 @@ func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.D
}

for host, health := range serviceHealth {
var finalTargets []string
var finalTargets = assistant.NewTargets()

if !strings.Contains(host, r.Config.EdgeDNSZone) {
return nil, fmt.Errorf("ingress host %s does not match delegated zone %s", host, r.Config.EdgeDNSZone)
Expand All @@ -63,7 +56,7 @@ func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.D
isHealthy := health == k8gbv1beta1.Healthy

if isHealthy {
finalTargets = append(finalTargets, localTargets...)
finalTargets.Append(r.Config.ClusterGeoTag, localTargets)
localTargetsHost := fmt.Sprintf("localtargets-%s", host)
dnsRecord := &externaldns.Endpoint{
DNSName: localTargetsHost,
Expand All @@ -75,14 +68,13 @@ func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.D
}

// Check if host is alive on external Gslb
externalTargets := r.DNSProvider.GetExternalTargets(host).GetIPs()

sortTargets(externalTargets)
externalTargets := r.DNSProvider.GetExternalTargets(host)
externalTargets.Sort()

if len(externalTargets) > 0 {
switch gslb.Spec.Strategy.Type {
case depresolver.RoundRobinStrategy, depresolver.GeoStrategy:
finalTargets = append(finalTargets, externalTargets...)
finalTargets.AppendTargets(externalTargets)
case depresolver.FailoverStrategy:
// If cluster is Primary
if isPrimary {
Expand All @@ -93,7 +85,7 @@ func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.D
log.Info().
Str("gslb", gslb.Name).
Str("cluster", gslb.Spec.Strategy.PrimaryGeoTag).
Strs("targets", finalTargets).
Strs("targets", finalTargets.GetIPs()).
Str("workload", k8gbv1beta1.Unhealthy.String()).
Msg("Executing failover strategy for primary cluster")
}
Expand All @@ -105,7 +97,7 @@ func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.D
log.Info().
Str("gslb", gslb.Name).
Str("cluster", gslb.Spec.Strategy.PrimaryGeoTag).
Strs("targets", finalTargets).
Strs("targets", finalTargets.GetIPs()).
Str("workload", k8gbv1beta1.Healthy.String()).
Msg("Executing failover strategy for secondary cluster")
}
Expand All @@ -116,22 +108,25 @@ func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.D
Msg("No external targets have been found for host")
}

r.updateRuntimeStatus(gslb, isPrimary, health, finalTargets)
r.updateRuntimeStatus(gslb, isPrimary, health, finalTargets.GetIPs())
log.Info().
Str("gslb", gslb.Name).
Strs("targets", finalTargets).
Strs("targets", finalTargets.GetIPs()).
Msg("Final target list")

if len(finalTargets) > 0 {
dnsRecord := &externaldns.Endpoint{
DNSName: host,
RecordTTL: ttl,
RecordType: "A",
Targets: finalTargets,
Targets: finalTargets.GetIPs(),
Labels: externaldns.Labels{
"strategy": gslb.Spec.Strategy.Type,
},
}
for k, v := range r.getLabels(gslb, finalTargets) {
dnsRecord.Labels[k] = v
}
gslbHosts = append(gslbHosts, dnsRecord)
}
}
Expand Down Expand Up @@ -166,3 +161,19 @@ func (r *GslbReconciler) updateRuntimeStatus(gslb *k8gbv1beta1.Gslb, isPrimary b
m.UpdateFailoverStatus(gslb, isPrimary, isHealthy, finalTargets)
}
}

// getLabels map of where key identifies region and weight, value identifies IP.
func (r *GslbReconciler) getLabels(gslb *k8gbv1beta1.Gslb, targets assistant.Targets) (labels map[string]string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't that be named genLabels instead? getLables quite confusing.

Copy link
Collaborator Author

@kuritka kuritka Jul 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @k0da, thx for good comment. I will try to explain: Yes it generates labels and therefore it should be called genLabels. getLabels is getter function that always returns a label map.

The main reason why I would choose get over gen. Overall practice is getters don't change state. A getter returns or generates a value, but it doesn't change the state (for example, if the getLabels function somewhere inside would somehow change the endpoint, or gslb etc....). A getter can't change anything, just read anything and return the value or modified or generated value.

This reduces the cognitive load a lot, because if something is a getter, the developer doesn't have to worry about the details of how the get works. He doesn't have to study the body of the function in order to have it change some instances.

Imagine, In the case of genLabels, as someone who did not write the code, I would be worried that the function inside might change the state (e.g: modify something). I'd have to go in and study the details of the code, to ensure it reads only. Even though it's not related to the thing I'm working on.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense, my assumption was getter gets it from external source

labels = make(map[string]string, 0)
for k, v := range gslb.Spec.Strategy.Weight {
t, found := targets[k]
if !found {
continue
}
for i, ip := range t.IPs {
l := fmt.Sprintf("weight-%s-%v-%v", k, i, v.Int())
labels[l] = ip
}
}
return labels
}
6 changes: 3 additions & 3 deletions controllers/providers/assistant/gslb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,8 @@ func dnsQuery(host string, nameservers utils.DNSList) (*dns.Msg, error) {
}

func (r *Gslb) GetExternalTargets(host string, extClusterNsNames map[string]string) (targets Targets) {
targets = Targets{}
for _, cluster := range extClusterNsNames {
targets = NewTargets()
for tag, cluster := range extClusterNsNames {
// Use edgeDNSServer for resolution of NS names and fallback to local nameservers
log.Info().
Str("cluster", cluster).
Expand Down Expand Up @@ -327,7 +327,7 @@ func (r *Gslb) GetExternalTargets(host string, extClusterNsNames map[string]stri
}
clusterTargets := getARecords(a)
if len(clusterTargets) > 0 {
targets = append(targets, Target{cluster, clusterTargets})
targets[tag] = &Target{clusterTargets}
log.Info().
Strs("clusterTargets", clusterTargets).
Str("cluster", cluster).
Expand Down
45 changes: 38 additions & 7 deletions controllers/providers/assistant/target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package assistant

import "sort"

/*
Copyright 2022 The k8gb Contributors.

Expand All @@ -19,17 +21,46 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic
*/

type Target struct {
Region string
IPs []string
IPs []string
}

type Targets []Target
type Targets map[string]*Target

func NewTargets() Targets {
return make(map[string]*Target, 0)
}

func (t Targets) GetIPs() (targets []string) {
func (t Targets) GetIPs() (ips []string) {
// initializing targets to avoid possible nil reference errors (serialization etc.)
targets = []string{}
ips = []string{}
for _, v := range t {
ips = append(ips, v.IPs...)
}
return ips
}

func (t Targets) Append(tag string, ips []string) {
if target, found := t[tag]; found {
target.IPs = append(target.IPs, ips...)
return
}
t[tag] = &Target{IPs: ips}
}

func (t Targets) AppendTargets(targets Targets) {
for k, v := range targets {
t.Append(k, v.IPs)
}
}

func (t Targets) Sort() {
sort := func(targets []string) []string {
sort.Slice(targets, func(i, j int) bool {
return targets[i] < targets[j]
})
return targets
}
for _, v := range t {
targets = append(targets, v.IPs...)
v.IPs = sort(v.IPs)
}
return targets
}
Loading