diff --git a/Makefile b/Makefile index eef7206c..db30de70 100644 --- a/Makefile +++ b/Makefile @@ -241,14 +241,12 @@ run: DIRTY=$(shell hack/check-git-dirty.sh || echo "unknown") run: manifests generate fmt vet ## Run a controller from your host. go run -ldflags "-X main.gitSHA=${GIT_SHA} -X main.dirty=${DIRTY}" ./cmd/main.go --zap-devel --provider inmemory,aws,google,azure - .PHONY: run-with-probes run-with-probes: GIT_SHA=$(shell git rev-parse HEAD || echo "unknown") run-with-probes: DIRTY=$(shell hack/check-git-dirty.sh || echo "unknown") run-with-probes: manifests generate fmt vet ## Run a controller from your host. go run -ldflags "-X main.gitSHA=${GIT_SHA} -X main.dirty=${DIRTY}" ./cmd/main.go --zap-devel --provider inmemory,aws,google,azure - # If you wish built the manager image targeting other platforms you can use the --platform flag. # (i.e. docker build --platform linux/arm64 ). However, you must enable docker buildKit for it. # More info: https://docs.docker.com/develop/develop-images/build_enhancements/ diff --git a/cmd/main.go b/cmd/main.go index 72a674cf..f8c44985 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -79,8 +79,10 @@ func main() { var maxRequeueTime time.Duration var providers stringSliceFlags var dnsProbesEnabled bool + var allowInsecureCerts bool flag.BoolVar(&dnsProbesEnabled, "enable-probes", true, "Enable DNSHealthProbes controller.") + flag.BoolVar(&allowInsecureCerts, "insecure-health-checks", true, "Allow DNSHealthProbes to use insecure certificates") flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -153,7 +155,7 @@ func main() { Client: mgr.GetClient(), Scheme: mgr.GetScheme(), ProviderFactory: providerFactory, - }).SetupWithManager(mgr, maxRequeueTime, validFor, minRequeueTime); err != nil { + }).SetupWithManager(mgr, maxRequeueTime, validFor, minRequeueTime, dnsProbesEnabled, allowInsecureCerts); err != nil { setupLog.Error(err, "unable to create controller", "controller", "DNSRecord") os.Exit(1) } diff --git a/internal/controller/dnshealthcheckprobe_reconciler.go b/internal/controller/dnshealthcheckprobe_reconciler.go index f42c471a..c77c202a 100644 --- a/internal/controller/dnshealthcheckprobe_reconciler.go +++ b/internal/controller/dnshealthcheckprobe_reconciler.go @@ -22,6 +22,7 @@ import ( ) const ( + ProbeOwnerLabel = "kuadrant.io/health-probes-owner" DNSHealthCheckFinalizer = "kuadrant.io/dns-health-check-probe" ) diff --git a/internal/controller/dnsrecord_controller.go b/internal/controller/dnsrecord_controller.go index 9d2b2255..f42b98bf 100644 --- a/internal/controller/dnsrecord_controller.go +++ b/internal/controller/dnsrecord_controller.go @@ -66,6 +66,9 @@ var ( randomizedValidationRequeue time.Duration validFor time.Duration reconcileStart metav1.Time + + probesEnabled bool + allowInsecureCert bool ) // DNSRecordReconciler reconciles a DNSRecord object @@ -125,8 +128,10 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return r.updateStatus(ctx, previous, dnsRecord, false, err) } - if err = r.ReconcileHealthChecks(ctx, dnsRecord); client.IgnoreNotFound(err) != nil { - return ctrl.Result{}, err + if probesEnabled { + if err = r.DeleteHealthChecks(ctx, dnsRecord); client.IgnoreNotFound(err) != nil { + return ctrl.Result{}, err + } } hadChanges, err := r.deleteRecord(ctx, dnsRecord, dnsProvider) if err != nil { @@ -227,8 +232,10 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return r.updateStatus(ctx, previous, dnsRecord, hadChanges, err) } - if err = r.ReconcileHealthChecks(ctx, dnsRecord); err != nil { - return ctrl.Result{}, err + if probesEnabled { + if err = r.ReconcileHealthChecks(ctx, dnsRecord, allowInsecureCert); err != nil { + return ctrl.Result{}, err + } } return r.updateStatus(ctx, previous, dnsRecord, hadChanges, nil) @@ -318,10 +325,12 @@ func (r *DNSRecordReconciler) updateStatus(ctx context.Context, previous, curren } // SetupWithManager sets up the controller with the Manager. -func (r *DNSRecordReconciler) SetupWithManager(mgr ctrl.Manager, maxRequeue, validForDuration, minRequeue time.Duration) error { +func (r *DNSRecordReconciler) SetupWithManager(mgr ctrl.Manager, maxRequeue, validForDuration, minRequeue time.Duration, healthProbesEnabled, allowInsecureHealthCert bool) error { defaultRequeueTime = maxRequeue validFor = validForDuration defaultValidationRequeue = minRequeue + probesEnabled = healthProbesEnabled + allowInsecureCert = allowInsecureHealthCert return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.DNSRecord{}). diff --git a/internal/controller/dnsrecord_healthchecks.go b/internal/controller/dnsrecord_healthchecks.go index 29b37fde..67cbed58 100644 --- a/internal/controller/dnsrecord_healthchecks.go +++ b/internal/controller/dnsrecord_healthchecks.go @@ -2,180 +2,167 @@ package controller import ( "context" - "crypto/md5" "fmt" - "io" "reflect" + "strings" - "k8s.io/apimachinery/pkg/api/meta" + "github.com/go-logr/logr" + "github.com/hashicorp/go-multierror" + + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - externaldns "sigs.k8s.io/external-dns/endpoint" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/utils/ptr" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" "github.com/kuadrant/dns-operator/api/v1alpha1" - "github.com/kuadrant/dns-operator/internal/provider" + "github.com/kuadrant/dns-operator/internal/common" ) -// healthChecksConfig represents the user configuration for the health checks -type healthChecksConfig struct { - Endpoint string - Port *int64 - FailureThreshold *int64 - Protocol *provider.HealthCheckProtocol -} - -func (r *DNSRecordReconciler) ReconcileHealthChecks(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) error { - var results []provider.HealthCheckResult - var err error +func (r *DNSRecordReconciler) ReconcileHealthChecks(ctx context.Context, dnsRecord *v1alpha1.DNSRecord, allowInsecureCerts bool) error { + logger := log.FromContext(ctx).WithName("healthchecks") + logger.Info("Reconciling healthchecks") - dnsProvider, err := r.getDNSProvider(ctx, dnsRecord) - if err != nil { - return err + // Probes enabled but no health check spec yet. Nothing to do + if dnsRecord.Spec.HealthCheck == nil { + return nil } - healthCheckReconciler := dnsProvider.HealthCheckReconciler() - - // Get the configuration for the health checks. If no configuration is - // set, ensure that the health checks are deleted - config := getHealthChecksConfig(dnsRecord) + desiredProbes := buildDesiredProbes(dnsRecord, common.GetLeafsTargets(common.MakeTreeFromDNSRecord(dnsRecord), ptr.To([]string{})), allowInsecureCerts) - for _, dnsEndpoint := range dnsRecord.Spec.Endpoints { - addresses := provider.GetExternalAddresses(dnsEndpoint, dnsRecord) - for _, address := range addresses { - probeStatus := r.getProbeStatus(address, dnsRecord) - - // no config means delete the health checks - if config == nil { - result, err := healthCheckReconciler.Delete(ctx, dnsEndpoint, probeStatus) - if err != nil { - return err - } - - results = append(results, result) - continue - } - - // creating / updating health checks - endpointId, err := idForEndpoint(dnsRecord, dnsEndpoint, address) - if err != nil { - return err - } - - spec := provider.HealthCheckSpec{ - Id: endpointId, - Name: fmt.Sprintf("%s-%s-%s", dnsRecord.Spec.RootHost, dnsEndpoint.DNSName, address), - Host: &dnsRecord.Spec.RootHost, - Path: config.Endpoint, - Port: config.Port, - Protocol: config.Protocol, - FailureThreshold: config.FailureThreshold, - } - - result := healthCheckReconciler.Reconcile(ctx, spec, dnsEndpoint, probeStatus, address) - results = append(results, result) + for _, probe := range desiredProbes { + // if one of them fails - health checks for this record are invalid anyway, so no sense to continue + if err := controllerruntime.SetControllerReference(dnsRecord, probe, r.Scheme); err != nil { + return err } - } - - result := r.reconcileHealthCheckStatus(results, dnsRecord) - return result -} - -func (r *DNSRecordReconciler) getProbeStatus(address string, dnsRecord *v1alpha1.DNSRecord) *v1alpha1.HealthCheckStatusProbe { - if dnsRecord.Status.HealthCheck == nil || dnsRecord.Status.HealthCheck.Probes == nil { - return nil - } - for _, probeStatus := range dnsRecord.Status.HealthCheck.Probes { - if probeStatus.IPAddress == address { - return &probeStatus + if err := r.ensureProbe(ctx, probe, logger); err != nil { + return err } } - + logger.Info("Healthecks reconciled") return nil } -func (r *DNSRecordReconciler) reconcileHealthCheckStatus(results []provider.HealthCheckResult, dnsRecord *v1alpha1.DNSRecord) error { - var previousCondition *metav1.Condition - probesCondition := &metav1.Condition{ - Reason: "AllProbesSynced", - Type: "healthProbesSynced", - } +// DeleteHealthChecks deletes all v1alpha1.DNSHealthCheckProbe that have ProbeOwnerLabel of passed in DNSRecord +func (r *DNSRecordReconciler) DeleteHealthChecks(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) error { + logger := log.FromContext(ctx).WithName("healthchecks") + logger.Info("Deleting healthchecks") - var allSynced = metav1.ConditionTrue + healthProbes := v1alpha1.DNSHealthCheckProbeList{} - if dnsRecord.Status.HealthCheck == nil { - dnsRecord.Status.HealthCheck = &v1alpha1.HealthCheckStatus{ - Conditions: []metav1.Condition{}, - Probes: []v1alpha1.HealthCheckStatusProbe{}, - } + if err := r.List(ctx, &healthProbes, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord), + }), + Namespace: dnsRecord.Namespace, + }); err != nil { + return err } - previousCondition = meta.FindStatusCondition(dnsRecord.Status.HealthCheck.Conditions, "HealthProbesSynced") - if previousCondition != nil { - probesCondition = previousCondition + var deleteErrors error + for _, probe := range healthProbes.Items { + logger.V(1).Info(fmt.Sprintf("Deleting probe: %s", probe.Name)) + if err := r.Delete(ctx, &probe); err != nil { + deleteErrors = multierror.Append(deleteErrors, err) + } } + return deleteErrors +} - dnsRecord.Status.HealthCheck.Probes = []v1alpha1.HealthCheckStatusProbe{} +func (r *DNSRecordReconciler) ensureProbe(ctx context.Context, generated *v1alpha1.DNSHealthCheckProbe, logger logr.Logger) error { + current := &v1alpha1.DNSHealthCheckProbe{} - for _, result := range results { - if result.Host == "" { - continue + if err := r.Get(ctx, client.ObjectKeyFromObject(generated), current); err != nil { + if errors.IsNotFound(err) { + logger.V(1).Info(fmt.Sprintf("Creating probe: %s", generated.Name)) + return r.Create(ctx, generated) } - status := true - if result.Result == provider.HealthCheckFailed { - status = false - allSynced = metav1.ConditionFalse - } - - dnsRecord.Status.HealthCheck.Probes = append(dnsRecord.Status.HealthCheck.Probes, v1alpha1.HealthCheckStatusProbe{ - ID: result.ID, - IPAddress: result.IPAddress, - Host: result.Host, - Synced: status, - Conditions: []metav1.Condition{result.Condition}, - }) + return err } - probesCondition.ObservedGeneration = dnsRecord.Generation - probesCondition.Status = allSynced - - if allSynced == metav1.ConditionTrue { - probesCondition.Message = fmt.Sprintf("all %v probes synced successfully", len(dnsRecord.Status.HealthCheck.Probes)) - probesCondition.Reason = "AllProbesSynced" - } else { - probesCondition.Reason = "UnsyncedProbes" - probesCondition.Message = "some probes have not yet successfully synced to the DNS Provider" - } + desired := current.DeepCopy() + desired.Spec = generated.Spec - //probe condition changed? - update transition time - if !reflect.DeepEqual(previousCondition, probesCondition) { - probesCondition.LastTransitionTime = metav1.Now() + if !reflect.DeepEqual(current, desired) { + logger.V(1).Info(fmt.Sprintf("Updating probe: %s", desired.Name)) + if err := r.Update(ctx, desired); err != nil { + return err + } } - - dnsRecord.Status.HealthCheck.Conditions = []metav1.Condition{*probesCondition} - + logger.V(1).Info(fmt.Sprintf("No updates needed for probe: %s", desired.Name)) return nil } -func getHealthChecksConfig(dnsRecord *v1alpha1.DNSRecord) *healthChecksConfig { - if dnsRecord.Spec.HealthCheck == nil || dnsRecord.DeletionTimestamp != nil { - return nil - } +func buildDesiredProbes(dnsRecord *v1alpha1.DNSRecord, leafs *[]string, allowInsecureCerts bool) []*v1alpha1.DNSHealthCheckProbe { + var probes []*v1alpha1.DNSHealthCheckProbe - port := int64(dnsRecord.Spec.HealthCheck.Port) - failureThreshold := int64(dnsRecord.Spec.HealthCheck.FailureThreshold) + if leafs == nil { + return probes + } - return &healthChecksConfig{ - Endpoint: dnsRecord.Spec.HealthCheck.Path, - Port: &port, - FailureThreshold: &failureThreshold, - Protocol: (*provider.HealthCheckProtocol)(&dnsRecord.Spec.HealthCheck.Protocol), + for _, leaf := range *leafs { + probes = append(probes, &v1alpha1.DNSHealthCheckProbe{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-%s", dnsRecord.Name, leaf), + Namespace: dnsRecord.Namespace, + Labels: map[string]string{ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord)}, + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + Port: dnsRecord.Spec.HealthCheck.Port, + Hostname: dnsRecord.Spec.RootHost, + Address: leaf, + Path: dnsRecord.Spec.HealthCheck.Path, + Protocol: dnsRecord.Spec.HealthCheck.Protocol, + Interval: dnsRecord.Spec.HealthCheck.Interval, + AdditionalHeadersRef: dnsRecord.Spec.HealthCheck.AdditionalHeadersRef, + FailureThreshold: dnsRecord.Spec.HealthCheck.FailureThreshold, + AllowInsecureCertificate: allowInsecureCerts, + }, + }) } + return probes } -// idForEndpoint returns a unique identifier for an endpoint -func idForEndpoint(dnsRecord *v1alpha1.DNSRecord, endpoint *externaldns.Endpoint, address string) (string, error) { - hash := md5.New() - if _, err := io.WriteString(hash, fmt.Sprintf("%s/%s@%s:%s-%v", dnsRecord.Name, endpoint.SetIdentifier, endpoint.DNSName, address, dnsRecord.Generation)); err != nil { - return "", fmt.Errorf("unexpected error creating ID for endpoint %s", endpoint.SetIdentifier) +// BuildOwnerLabelValue ensures label value does not exceed the 63 char limit +// It adds namespace and name of the record, +// if the resulting string longer than 63 chars it attempts the following in +// a stated order and uses the first solution that produces value of less than 63 characters: +// +// 1. Trims namespace part to get under the limit. +// Result: "short-namespace_hostname" +// +// 2. Uses the first two subdomains of host instead of the name (e.g. will use "pat.the" from "pat.the.cat.com"). +// Result: "original-namespace_pat.the" +// +// 3. Uses GetUIDHash() of v1alpha1.DNSRecord struct +// Result: "UIDHash" +func BuildOwnerLabelValue(record *v1alpha1.DNSRecord) string { + value := fmt.Sprintf("%s_%s", record.Namespace, record.Name) + + // using the name of the dns record (hostname) and namespace is likely to exceed a 63 char limit + if len(value) > 63 { + // determine how much we need to remove + overshoot := len(value) - 63 + + // if we can fix it by trimming NS + if len(record.Namespace) > overshoot { + value = fmt.Sprintf("%s_%s", record.Namespace[:len(record.Namespace)-overshoot], record.Name) + } else { + // trimming namespace is not an option - too long hostname + // the name of the probe will be fine - it has a limit of 253 + shortHost := strings.Join(strings.Split(record.Name, ".")[:3], ".") + + // if this the case we can reduce just host + if len(record.Name)-len(shortHost) > overshoot { + value = fmt.Sprintf("%s_%s", record.Namespace, shortHost) + } else { + // we can't deal with it just by shortening one of them + // default to UID of record + value = record.GetUIDHash() + } + } } - return fmt.Sprintf("%x", hash.Sum(nil)), nil + return value } diff --git a/internal/controller/dnsrecord_healthchecks_test.go b/internal/controller/dnsrecord_healthchecks_test.go new file mode 100644 index 00000000..c1cc36bc --- /dev/null +++ b/internal/controller/dnsrecord_healthchecks_test.go @@ -0,0 +1,181 @@ +//go:build integration + +package controller + +import ( + "fmt" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kuadrant/dns-operator/api/v1alpha1" + "github.com/kuadrant/dns-operator/pkg/builder" +) + +var _ = Describe("DNSRecordReconciler_HealthChecks", func() { + var ( + dnsRecord *v1alpha1.DNSRecord + + testNamespace, testHostname string + ) + + BeforeEach(func() { + CreateNamespace(&testNamespace) + + testZoneDomainName := strings.Join([]string{GenerateName(), "example.com"}, ".") + testHostname = strings.Join([]string{"foo", testZoneDomainName}, ".") + + dnsProviderSecret := builder.NewProviderBuilder("inmemory-credentials", testNamespace). + For(v1alpha1.SecretTypeKuadrantInmemory). + WithZonesInitialisedFor(testZoneDomainName). + Build() + Expect(k8sClient.Create(ctx, dnsProviderSecret)).To(Succeed()) + + dnsRecord = &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: testHostname, + Namespace: testNamespace, + }, + Spec: v1alpha1.DNSRecordSpec{ + RootHost: testHostname, + ProviderRef: v1alpha1.ProviderRef{ + Name: dnsProviderSecret.Name, + }, + Endpoints: getTestLBEndpoints(testHostname), + HealthCheck: getTestHealthCheckSpec(), + }, + } + }) + + It("Should create valid probe CRs and remove them on DNSRecord deletion", func() { + //Create default test dnsrecord + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal("ProviderSuccess"), + "Message": Equal("Provider ensured the dns record"), + })), + ) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + + By("Validating created probes") + Eventually(func(g Gomega) { + probes := &v1alpha1.DNSHealthCheckProbeList{} + + g.Expect(k8sClient.List(ctx, probes, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord), + }), + Namespace: dnsRecord.Namespace, + })).To(Succeed()) + + g.Expect(probes.Items).To(HaveLen(2)) + g.Expect(probes.Items).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal(fmt.Sprintf("%s-%s", dnsRecord.Name, "172.32.200.1")), + "Namespace": Equal(testNamespace), + }), + "Spec": MatchFields(IgnoreExtras, Fields{ + "Port": Equal(443), + "Hostname": Equal(testHostname), + "Address": Equal("172.32.200.1"), + "Path": Equal("/healthz"), + "Protocol": Equal(v1alpha1.Protocol("HTTPS")), + "Interval": Equal(metav1.Duration{Duration: time.Minute}), + "AdditionalHeadersRef": PointTo(MatchFields(IgnoreExtras, Fields{ + "Name": Equal("headers"), + })), + "FailureThreshold": Equal(5), + "AllowInsecureCertificate": Equal(true), + }), + }), + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal(fmt.Sprintf("%s-%s", dnsRecord.Name, "172.32.200.2")), + "Namespace": Equal(testNamespace), + }), + "Spec": MatchFields(IgnoreExtras, Fields{ + "Port": Equal(443), + "Hostname": Equal(testHostname), + "Address": Equal("172.32.200.2"), + "Path": Equal("/healthz"), + "Protocol": Equal(v1alpha1.Protocol("HTTPS")), + "Interval": Equal(metav1.Duration{Duration: time.Minute}), + "AdditionalHeadersRef": PointTo(MatchFields(IgnoreExtras, Fields{ + "Name": Equal("headers"), + })), + "FailureThreshold": Equal(5), + "AllowInsecureCertificate": Equal(true), + }), + }), + )) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + + By("Ensuring probes block DNSRecord deletion and correctly removed") + Eventually(func(g Gomega) { + probes := &v1alpha1.DNSHealthCheckProbeList{} + + g.Expect(k8sClient.List(ctx, probes, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord), + }), + Namespace: dnsRecord.Namespace, + })).To(Succeed()) + + g.Expect(probes.Items).To(HaveLen(2)) + g.Expect(probes.Items).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal(fmt.Sprintf("%s-%s", dnsRecord.Name, "172.32.200.1")), + "Namespace": Equal(testNamespace), + "OwnerReferences": ConsistOf(MatchFields(IgnoreExtras, Fields{ + "Name": Equal(dnsRecord.Name), + "UID": Equal(dnsRecord.UID), + "Controller": PointTo(Equal(true)), + "BlockOwnerDeletion": PointTo(Equal(true)), + })), + }), + }), + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal(fmt.Sprintf("%s-%s", dnsRecord.Name, "172.32.200.2")), + "Namespace": Equal(testNamespace), + "OwnerReferences": ConsistOf(MatchFields(IgnoreExtras, Fields{ + "Name": Equal(dnsRecord.Name), + "UID": Equal(dnsRecord.UID), + "Controller": PointTo(Equal(true)), + "BlockOwnerDeletion": PointTo(Equal(true)), + })), + }), + }), + )) + + g.Expect(k8sClient.Delete(ctx, dnsRecord)).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(k8sClient.List(ctx, probes, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord), + }), + Namespace: dnsRecord.Namespace, + })).To(Succeed()) + + g.Expect(probes.Items).To(HaveLen(0)) + }, TestTimeoutShort, time.Second) + + }, TestTimeoutMedium, time.Second).Should(Succeed()) + }) + +}) diff --git a/internal/controller/helper_test.go b/internal/controller/helper_test.go index dd90b04c..76c7b2af 100644 --- a/internal/controller/helper_test.go +++ b/internal/controller/helper_test.go @@ -9,7 +9,10 @@ import ( "github.com/goombaio/namegenerator" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" externaldnsendpoint "sigs.k8s.io/external-dns/endpoint" + + "github.com/kuadrant/dns-operator/api/v1alpha1" ) const ( @@ -42,3 +45,105 @@ func getTestEndpoints(dnsName, ip string) []*externaldnsendpoint.Endpoint { }, } } + +func getTestHealthCheckSpec() *v1alpha1.HealthCheckSpec { + return &v1alpha1.HealthCheckSpec{ + Path: "/healthz", + Port: 443, + Protocol: "HTTPS", + FailureThreshold: 5, + Interval: metav1.Duration{Duration: time.Minute}, + AdditionalHeadersRef: &v1alpha1.AdditionalHeadersRef{ + Name: "headers", + }, + } +} +func getTestLBEndpoints(testDomain string) []*externaldnsendpoint.Endpoint { + return []*externaldnsendpoint.Endpoint{ + { + DNSName: testDomain, + RecordTTL: 300, + RecordType: "CNAME", + Targets: []string{ + "klb." + testDomain, + }, + }, + { + DNSName: "ip1." + testDomain, + RecordTTL: 60, + RecordType: "A", + Targets: []string{ + "172.32.200.1", + }, + }, + { + DNSName: "ip2." + testDomain, + RecordTTL: 60, + RecordType: "A", + Targets: []string{ + "172.32.200.2", + }, + }, + { + DNSName: "eu.klb." + testDomain, + RecordTTL: 60, + RecordType: "CNAME", + Targets: []string{ + "ip2." + testDomain, + }, + }, + { + DNSName: "us.klb." + testDomain, + RecordTTL: 60, + RecordType: "CNAME", + Targets: []string{ + "ip1." + testDomain, + }, + }, + { + DNSName: "klb." + testDomain, + ProviderSpecific: []externaldnsendpoint.ProviderSpecificProperty{ + { + Name: "geo-code", + Value: "*", + }, + }, + RecordTTL: 300, + RecordType: "CNAME", + SetIdentifier: "", + Targets: []string{ + "eu.klb." + testDomain, + }, + }, + { + DNSName: "klb." + testDomain, + ProviderSpecific: []externaldnsendpoint.ProviderSpecificProperty{ + { + Name: "geo-code", + Value: "GEO-NA", + }, + }, + RecordTTL: 300, + RecordType: "CNAME", + SetIdentifier: "", + Targets: []string{ + "us.klb." + testDomain, + }, + }, + { + DNSName: "klb." + testDomain, + ProviderSpecific: []externaldnsendpoint.ProviderSpecificProperty{ + { + Name: "geo-code", + Value: "GEO-EU", + }, + }, + RecordTTL: 300, + RecordType: "CNAME", + SetIdentifier: "", + Targets: []string{ + "eu.klb." + testDomain, + }, + }, + } +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index adc88ad2..3f0ed7ff 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -104,7 +104,7 @@ var _ = BeforeSuite(func() { Client: mgr.GetClient(), Scheme: mgr.GetScheme(), ProviderFactory: providerFactory, - }).SetupWithManager(mgr, RequeueDuration, ValidityDuration, DefaultValidationDuration) + }).SetupWithManager(mgr, RequeueDuration, ValidityDuration, DefaultValidationDuration, true, true) Expect(err).ToNot(HaveOccurred()) go func() { diff --git a/test/e2e/healthcheck_test.go b/test/e2e/healthcheck_test.go index a4cc3bb7..bb134ccc 100644 --- a/test/e2e/healthcheck_test.go +++ b/test/e2e/healthcheck_test.go @@ -3,18 +3,13 @@ package e2e import ( - "fmt" "strings" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/strings/slices" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kuadrant/dns-operator/api/v1alpha1" @@ -54,181 +49,7 @@ var _ = Describe("Health Check Test", Serial, Labels{"health_checks"}, func() { } }) - Context("DNS Provider health checks", func() { - It("creates health checks for a health check spec", func(ctx SpecContext) { - healthChecksSupported := false - if slices.Contains(supportedHealthCheckProviders, strings.ToLower(testDNSProvider)) { - healthChecksSupported = true - } - - dnsRecord = &v1alpha1.DNSRecord{} - err := ResourceFromFile("./fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml", dnsRecord, GetTestEnv) - Expect(err).ToNot(HaveOccurred()) - - By("creating dnsrecord " + dnsRecord.Name) - err = k8sClient.Create(ctx, dnsRecord) - Expect(err).ToNot(HaveOccurred()) - - By("checking " + dnsRecord.Name + " becomes ready") - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(dnsRecord.Status.Conditions).To( - ContainElement(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(string(v1alpha1.ConditionTypeReady)), - "Status": Equal(metav1.ConditionTrue), - })), - ) - }, recordsReadyMaxDuration, 10*time.Second, ctx).Should(Succeed()) - - By("Confirming the DNS Record status") - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) - g.Expect(err).ToNot(HaveOccurred()) - - if healthChecksSupported { - g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil()) - g.Expect(&dnsRecord.Status.HealthCheck.Probes).ToNot(BeNil()) - g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).ToNot(BeZero()) - for _, condition := range dnsRecord.Status.HealthCheck.Conditions { - if condition.Type == "healthProbesSynced" { - g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(condition.Reason).To(Equal("AllProbesSynced")) - } - } - } else { - g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil()) - g.Expect(dnsRecord.Status.HealthCheck.Probes).To(BeNil()) - } - - for _, probe := range dnsRecord.Status.HealthCheck.Probes { - g.Expect(probe.Host).To(Equal(testHostname)) - g.Expect(probe.IPAddress).To(Equal("172.32.200.1")) - g.Expect(probe.ID).ToNot(Equal("")) - - for _, probeCondition := range probe.Conditions { - g.Expect(probeCondition.Type).To(Equal("ProbeSynced")) - g.Expect(probeCondition.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(probeCondition.Message).To(ContainSubstring(fmt.Sprintf("id: %v, address: %v, host: %v", probe.ID, probe.IPAddress, probe.Host))) - } - } - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - provider, err := ProviderForDNSRecord(ctx, dnsRecord, k8sClient) - Expect(err).To(BeNil()) - - By("confirming the health checks exist in the provider") - Eventually(func(g Gomega) { - if !healthChecksSupported { - g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).To(BeZero()) - } - for _, healthCheck := range dnsRecord.Status.HealthCheck.Probes { - exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck) - g.Expect(err).To(BeNil()) - g.Expect(exists).To(BeTrue()) - } - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - By("Removing the health check") - oldHealthCheckStatus := dnsRecord.Status.HealthCheck.DeepCopy() - Eventually(func(g Gomega) { - patchFrom := client.MergeFrom(dnsRecord.DeepCopy()) - dnsRecord.Spec.HealthCheck = nil - err := k8sClient.Patch(ctx, dnsRecord, patchFrom) - g.Expect(err).To(BeNil()) - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - By("Confirming the DNS Record status") - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(dnsRecord.Status.HealthCheck).To(BeNil()) - }) - - By("confirming the health checks were removed in the provider") - Eventually(func(g Gomega) { - for _, healthCheck := range oldHealthCheckStatus.Probes { - exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck) - g.Expect(err).NotTo(BeNil()) - g.Expect(exists).To(BeFalse()) - } - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - By("Adding a health check spec") - Eventually(func(g Gomega) { - patchFrom := client.MergeFrom(dnsRecord.DeepCopy()) - err = ResourceFromFile("./fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml", dnsRecord, GetTestEnv) - g.Expect(err).ToNot(HaveOccurred()) - err := k8sClient.Patch(ctx, dnsRecord, patchFrom) - g.Expect(err).To(BeNil()) - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - By("Confirming the DNS Record status") - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) - g.Expect(err).ToNot(HaveOccurred()) - - if healthChecksSupported { - g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil()) - g.Expect(&dnsRecord.Status.HealthCheck.Probes).ToNot(BeNil()) - g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).ToNot(BeZero()) - for _, condition := range dnsRecord.Status.HealthCheck.Conditions { - if condition.Type == "healthProbesSynced" { - g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(condition.Reason).To(Equal("AllProbesSynced")) - } - } - } else { - g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil()) - g.Expect(dnsRecord.Status.HealthCheck.Probes).To(BeNil()) - } - - for _, probe := range dnsRecord.Status.HealthCheck.Probes { - g.Expect(probe.Host).To(Equal(testHostname)) - g.Expect(probe.IPAddress).To(Equal("172.32.200.1")) - g.Expect(probe.ID).ToNot(Equal("")) - - for _, probeCondition := range probe.Conditions { - g.Expect(probeCondition.Type).To(Equal("ProbeSynced")) - g.Expect(probeCondition.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(probeCondition.Message).To(ContainSubstring(fmt.Sprintf("id: %v, address: %v, host: %v", probe.ID, probe.IPAddress, probe.Host))) - } - } - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - By("confirming the health checks exist in the provider") - Eventually(func(g Gomega) { - if !healthChecksSupported { - g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).To(BeZero()) - } - for _, healthCheck := range dnsRecord.Status.HealthCheck.Probes { - exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck) - g.Expect(err).To(BeNil()) - g.Expect(exists).To(BeTrue()) - } - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - By("Deleting the DNS Record") - oldHealthCheckStatus = dnsRecord.Status.HealthCheck.DeepCopy() - err = ResourceFromFile("./fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml", dnsRecord, GetTestEnv) - Expect(err).ToNot(HaveOccurred()) - Eventually(func(g Gomega) { - err := k8sClient.Delete(ctx, dnsRecord) - g.Expect(client.IgnoreNotFound(err)).To(BeNil()) - - err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) - g.Expect(errors.IsNotFound(err)).Should(BeTrue()) - }, recordsRemovedMaxDuration, time.Second).Should(Succeed()) - - By("confirming the health checks were removed in the provider") - Eventually(func(g Gomega) { - for _, healthCheck := range oldHealthCheckStatus.Probes { - exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck) - g.Expect(err).NotTo(BeNil()) - g.Expect(exists).To(BeFalse()) - } - }, TestTimeoutMedium, time.Second).Should(Succeed()) - - }) + Context("On-cluster healthchecks", func() { + // TODO test case }) })