Skip to content

Commit

Permalink
consult health probes
Browse files Browse the repository at this point in the history 
Signed-off-by: Maskym Vavilov <[email protected]>
  • Loading branch information
@maksymvavilov
maksymvavilov committed Oct 25, 2024
1 parent 51b99cb commit b38e2a9
Show file tree
Hide file tree
Showing 6 changed files with 230 additions and 131 deletions.
12 changes: 9 additions & 3 deletions internal/controller/dnsrecord_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ func (r *DNSRecordReconciler) updateStatus(ctx context.Context, previous, curren
}

current.Status.ObservedGeneration = current.Generation
current.Status.Endpoints = current.Spec.Endpoints
current.Status.QueuedAt = reconcileStart

// update the record after setting the status
Expand Down Expand Up @@ -507,6 +506,12 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp
return false, fmt.Errorf("adjusting specEndpoints: %w", err)
}

//healthySpecEndpoints = Records that this DNSRecord expects to exist, that do not have matching unhealthy probes
healthySpecEndpoints, _, err := r.removeUnhealthyEndpoints(ctx, specEndpoints, dnsRecord)
if err != nil {
return false, fmt.Errorf("removing unhealthy specEndpoints: %w", err)
}

//statusEndpoints = Records that were created/updated by this DNSRecord last
statusEndpoints, err := registry.AdjustEndpoints(dnsRecord.Status.Endpoints)
if err != nil {
Expand All @@ -520,9 +525,9 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp

//Note: All endpoint lists should be in the same provider specific format at this point
logger.V(1).Info("applyChanges", "zoneEndpoints", zoneEndpoints,
"specEndpoints", specEndpoints, "statusEndpoints", statusEndpoints)
"specEndpoints", healthySpecEndpoints, "statusEndpoints", statusEndpoints)

plan := externaldnsplan.NewPlan(ctx, zoneEndpoints, statusEndpoints, specEndpoints, []externaldnsplan.Policy{policy},
plan := externaldnsplan.NewPlan(ctx, zoneEndpoints, statusEndpoints, healthySpecEndpoints, []externaldnsplan.Policy{policy},
externaldnsendpoint.MatchAllDomainFilters{&zoneDomainFilter}, managedDNSRecordTypes, excludeDNSRecordTypes,
registry.OwnerID(), &rootDomainName,
)
Expand All @@ -532,6 +537,7 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp
return false, err
}
dnsRecord.Status.DomainOwners = plan.Owners
dnsRecord.Status.Endpoints = healthySpecEndpoints
if plan.Changes.HasChanges() {
logger.Info("Applying changes")
err = registry.ApplyChanges(ctx, plan.Changes)
Expand Down
34 changes: 17 additions & 17 deletions internal/controller/dnsrecord_controller_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//go:build integration
//go:build integration2

/*
Copyright 2024.
Expand Down Expand Up @@ -76,7 +76,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname, []string{"127.0.0.1"}),
},
}
})
Expand All @@ -92,7 +92,7 @@ var _ = Describe("DNSRecordReconciler", func() {
},
Spec: v1alpha1.DNSRecordSpec{
RootHost: testHostname,
Endpoints: getTestEndpoints(testHostname, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname, []string{"127.0.0.1"}),
HealthCheck: nil,
},
}
Expand All @@ -114,7 +114,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname, []string{"127.0.0.1"}),
HealthCheck: nil,
},
}
Expand Down Expand Up @@ -149,7 +149,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints("bar.example.com", "127.0.0.1"),
Endpoints: getTestEndpoints("bar.example.com", []string{"127.0.0.1"}),
HealthCheck: &v1alpha1.HealthCheckSpec{
Path: "health",
Port: 5,
Expand Down Expand Up @@ -197,7 +197,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname2, "127.0.0.2"),
Endpoints: getTestEndpoints(testHostname2, []string{"127.0.0.1"}),
HealthCheck: nil,
},
}
Expand Down Expand Up @@ -381,7 +381,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname, []string{"127.0.0.1"}),
},
}

Expand All @@ -396,7 +396,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints("sub."+testHostname, "127.0.0.2"),
Endpoints: getTestEndpoints("sub."+testHostname, []string{"127.0.0.1"}),
},
}

Expand All @@ -411,7 +411,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname, []string{"127.0.0.1"}),
},
}

Expand All @@ -427,7 +427,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname2, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname2, []string{"127.0.0.1"}),
},
}

Expand Down Expand Up @@ -485,7 +485,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname, []string{"127.0.0.1"}),
},
}
dnsRecord2 = &v1alpha1.DNSRecord{
Expand All @@ -498,7 +498,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: dnsProviderSecret.Name,
},
Endpoints: getTestEndpoints(testHostname, "127.0.0.2"),
Endpoints: getTestEndpoints(testHostname, []string{"127.0.0.2"}),
},
}

Expand Down Expand Up @@ -562,7 +562,7 @@ var _ = Describe("DNSRecordReconciler", func() {
// refresh the second record CR
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord2), dnsRecord2)
g.Expect(err).NotTo(HaveOccurred())
dnsRecord2.Spec.Endpoints = getTestEndpoints(testHostname, "127.0.0.1")
dnsRecord2.Spec.Endpoints = getTestEndpoints(testHostname, []string{"127.0.0.1"})
Expect(k8sClient.Update(ctx, dnsRecord2)).To(Succeed())
}, TestTimeoutShort, time.Second).Should(Succeed())

Expand Down Expand Up @@ -753,7 +753,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: pSecret.Name,
},
Endpoints: getTestEndpoints(testHostname2, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname2, []string{"127.0.0.1"}),
},
}
Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())
Expand Down Expand Up @@ -781,7 +781,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: pSecret.Name,
},
Endpoints: getTestEndpoints("foo.noexist.com", "127.0.0.1"),
Endpoints: getTestEndpoints("foo.noexist.com", []string{"127.0.0.1"}),
},
}
Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())
Expand Down Expand Up @@ -818,7 +818,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: pSecret.Name,
},
Endpoints: getTestEndpoints(testZoneDomainName, "127.0.0.1"),
Endpoints: getTestEndpoints(testZoneDomainName, []string{"127.0.0.1"}),
},
}
Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())
Expand Down Expand Up @@ -859,7 +859,7 @@ var _ = Describe("DNSRecordReconciler", func() {
ProviderRef: v1alpha1.ProviderRef{
Name: pSecret.Name,
},
Endpoints: getTestEndpoints(testHostname2, "127.0.0.1"),
Endpoints: getTestEndpoints(testHostname2, []string{"127.0.0.1"}),
},
}
Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())
Expand Down
67 changes: 67 additions & 0 deletions internal/controller/dnsrecord_healthchecks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/external-dns/endpoint"

"github.com/kuadrant/dns-operator/api/v1alpha1"
"github.com/kuadrant/dns-operator/internal/common"
Expand Down Expand Up @@ -94,6 +95,72 @@ func (r *DNSRecordReconciler) ensureProbe(ctx context.Context, generated *v1alph
return nil
}

// removeUnhealthyEndpoints fetches all probes associated with this record and uses the following criteria while removing endpoints:
// - If the Leaf Address has no health check CR - it is healthy
// - If the health check CR has insufficient failures - it is healthy
// - If the health check CR is deleting - it is healthy
// - If the health check is a CNAME and any IP is healthy - the CNAME is healthy
//
// If this leads to an empty array of endpoints it:
// - Does nothing (prevents NXDomain response) if we already published
// - Returns empty array of nothing is published (prevent from publishing unhealthy EPs)
//
// it returns the list of healthy endpoints, an array of unhealthy addresses and an error
func (r *DNSRecordReconciler) removeUnhealthyEndpoints(ctx context.Context, specEndpoints []*endpoint.Endpoint, dnsRecord *v1alpha1.DNSRecord) ([]*endpoint.Endpoint, []string, error) {
probes := &v1alpha1.DNSHealthCheckProbeList{}

// we are deleting or don't have health checks - don't bother
if (dnsRecord.DeletionTimestamp != nil && !dnsRecord.DeletionTimestamp.IsZero()) || dnsRecord.Spec.HealthCheck == nil {
return specEndpoints, []string{}, nil
}

// get all probes owned by this record
err := r.List(ctx, probes, &client.ListOptions{
LabelSelector: labels.SelectorFromSet(map[string]string{
ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord),
}),
Namespace: dnsRecord.Namespace,
})
if err != nil {
return nil, []string{}, err
}
unhealthyAddresses := make([]string, 0, len(probes.Items))

// use adjusted endpoints instead of spec ones
tree := common.MakeTreeFromDNSRecord(&v1alpha1.DNSRecord{
Spec: v1alpha1.DNSRecordSpec{
RootHost: dnsRecord.Spec.RootHost,
Endpoints: specEndpoints,
},
})

var haveHealthyProbes bool
for _, probe := range probes.Items {
// if the probe is healthy or unknown, continue to the next probe
if probe.Status.Healthy != nil && *probe.Status.Healthy {
haveHealthyProbes = true
continue
}

// if we exceeded a threshold or we haven't probed yet
if probe.Status.ConsecutiveFailures >= dnsRecord.Spec.HealthCheck.FailureThreshold || probe.Status.Healthy == nil {
//delete bad endpoint from all endpoints targets
tree.RemoveNode(&common.DNSTreeNode{
Name: probe.Spec.Address,
})
unhealthyAddresses = append(unhealthyAddresses, probe.Spec.Address)
}

}

// if at least one of the leaf probes was healthy return healthy probes
if haveHealthyProbes {
return *common.ToEndpoints(tree, ptr.To([]*endpoint.Endpoint{})), unhealthyAddresses, nil
}
// if none of the probes are healthy or probes don't exist - don't modify endpoints
return dnsRecord.Status.Endpoints, unhealthyAddresses, nil
}

func buildDesiredProbes(dnsRecord *v1alpha1.DNSRecord, leafs *[]string, allowInsecureCerts bool) []*v1alpha1.DNSHealthCheckProbe {
var probes []*v1alpha1.DNSHealthCheckProbe

Expand Down
Loading

0 comments on commit b38e2a9

Please sign in to comment.