From 9a065cb3455b14986dd471bd744e01d089110f15 Mon Sep 17 00:00:00 2001 From: Maskym Vavilov Date: Fri, 8 Nov 2024 11:12:21 +0000 Subject: [PATCH] add heatlhy cond to dns policy Signed-off-by: Maskym Vavilov --- api/v1/dnspolicy_types.go | 4 +- controllers/dns_workflow.go | 29 ++++ controllers/dnspolicy_status_updater.go | 37 +++++ go.mod | 2 +- go.sum | 4 +- .../dnspolicy/dnspolicy_controller_test.go | 140 +++++++++++++++++- 6 files changed, 210 insertions(+), 6 deletions(-) diff --git a/api/v1/dnspolicy_types.go b/api/v1/dnspolicy_types.go index 8ccc5dded..da5468f86 100644 --- a/api/v1/dnspolicy_types.go +++ b/api/v1/dnspolicy_types.go @@ -322,9 +322,9 @@ func (p *DNSPolicy) WithTargetGatewayListener(gwName string, lName string) *DNSP func (p *DNSPolicy) WithHealthCheckFor(endpoint string, port int, protocol string, failureThreshold int) *DNSPolicy { return p.WithHealthCheck(dnsv1alpha1.HealthCheckSpec{ Path: endpoint, - Port: &port, + Port: port, Protocol: dnsv1alpha1.Protocol(protocol), - FailureThreshold: &failureThreshold, + FailureThreshold: failureThreshold, }) } diff --git a/controllers/dns_workflow.go b/controllers/dns_workflow.go index 622fbddf8..08368a85e 100644 --- a/controllers/dns_workflow.go +++ b/controllers/dns_workflow.go @@ -1,6 +1,7 @@ package controllers import ( + "errors" "fmt" "sync" @@ -19,6 +20,7 @@ import ( "github.com/kuadrant/policy-machinery/machinery" kuadrantv1 "github.com/kuadrant/kuadrant-operator/api/v1" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) @@ -26,6 +28,9 @@ const ( DNSRecordKind = "DNSRecord" StateDNSPolicyAcceptedKey = "DNSPolicyValid" StateDNSPolicyErrorsKey = "DNSPolicyErrors" + + PolicyConditionSubResourcesHealthy gatewayapiv1alpha2.PolicyConditionType = "SubResourcesHealthy" + PolicyReasonSubResourcesHealthy gatewayapiv1alpha2.PolicyConditionReason = "SubResourcesHealthy" ) var ( @@ -138,3 +143,27 @@ func dnsPolicyTypeFilterFunc() func(item machinery.Policy, _ int) (*kuadrantv1.D return p, ok } } + +func dnsPolicyHealthyCondition(policy kuadrant.Policy, err kuadrant.PolicyError) *metav1.Condition { + cond := &metav1.Condition{ + Type: string(PolicyConditionSubResourcesHealthy), + Status: metav1.ConditionTrue, + Reason: string(PolicyReasonSubResourcesHealthy), + Message: fmt.Sprintf("all subresources of %s are healthy", policy.Kind()), + } + + if err == nil { + return cond + } + + // Wrap error into a PolicyError if it is not this type + var policyErr kuadrant.PolicyError + if !errors.As(err, &policyErr) { + policyErr = kuadrant.NewErrUnknown(policy.Kind(), err) + } + cond.Status = metav1.ConditionFalse + cond.Message = policyErr.Error() + cond.Reason = string(policyErr.Reason()) + + return cond +} diff --git a/controllers/dnspolicy_status_updater.go b/controllers/dnspolicy_status_updater.go index dfe11d5df..4ea20d6c8 100644 --- a/controllers/dnspolicy_status_updater.go +++ b/controllers/dnspolicy_status_updater.go @@ -93,6 +93,13 @@ func (r *DNSPolicyStatusUpdater) updateStatus(ctx context.Context, _ []controlle } meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond) + if policy.Spec.HealthCheck != nil { + healthyCond := healthyCondition(policyRecords, policy) + meta.SetStatusCondition(&newStatus.Conditions, *healthyCond) + } else { + meta.RemoveStatusCondition(&newStatus.Conditions, string(PolicyConditionSubResourcesHealthy)) + } + propagateRecordConditions(policyRecords, newStatus) newStatus.TotalRecords = int32(len(policyRecords)) @@ -155,6 +162,36 @@ func enforcedCondition(records []*kuadrantdnsv1alpha1.DNSRecord, dnsPolicy *kuad return kuadrant.EnforcedCondition(dnsPolicy, nil, true) } +func healthyCondition(records []*kuadrantdnsv1alpha1.DNSRecord, dnsPolicy *kuadrantv1.DNSPolicy) *metav1.Condition { + // if we don't have records - consider healthy + if len(records) == 0 { + cond := dnsPolicyHealthyCondition(dnsPolicy, nil) + cond.Message = "No sub-resources present" + return cond + } + + // filter not healthy records + notHealthyRecords := utils.Filter(records, func(record *kuadrantdnsv1alpha1.DNSRecord) bool { + return meta.IsStatusConditionFalse(record.Status.Conditions, string(kuadrantdnsv1alpha1.ConditionTypeHealthy)) + }) + + // all records are healthy + if len(notHealthyRecords) == 0 { + cond := dnsPolicyHealthyCondition(dnsPolicy, nil) + cond.Message = "All sub-resources are healthy" + return cond + } + + cond := dnsPolicyHealthyCondition(dnsPolicy, kuadrant.NewErrUnknown(kuadrantv1.DNSPolicyGroupKind.Kind, errors.New("not all sub-resources of policy are passing the policy defined health check"))) + additionalMessage := ". Not healthy DNSRecords are: " + for _, record := range notHealthyRecords { + additionalMessage += fmt.Sprintf("%s ", record.Name) + } + cond.Message += additionalMessage + + return cond +} + var NegativePolarityConditions []string func propagateRecordConditions(records []*kuadrantdnsv1alpha1.DNSRecord, policyStatus *kuadrantv1.DNSPolicyStatus) { diff --git a/go.mod b/go.mod index 9ff62ff8f..9ab1dd621 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/google/go-cmp v0.6.0 github.com/kuadrant/authorino v0.19.0 github.com/kuadrant/authorino-operator v0.11.1 - github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef + github.com/kuadrant/dns-operator v0.0.0-20241108101824-a0cca97b2ef8 github.com/kuadrant/limitador-operator v0.9.0 github.com/kuadrant/policy-machinery v0.6.4 github.com/martinlindhe/base36 v1.1.1 diff --git a/go.sum b/go.sum index b7becb253..bc4f1abc7 100644 --- a/go.sum +++ b/go.sum @@ -258,8 +258,8 @@ github.com/kuadrant/authorino v0.19.0 h1:vPkDWVgQPi5A91MfIyWe06rLW0ZaACd+8j2SZPK github.com/kuadrant/authorino v0.19.0/go.mod h1:J9B/8aZMSyim6CxdmSlS2kRfQt3xMrZub2V/qHnkNeA= github.com/kuadrant/authorino-operator v0.11.1 h1:jndTZhiHMU+2Dk0NU+KP2+MUSfvclrn+YtTCQDJj+1s= github.com/kuadrant/authorino-operator v0.11.1/go.mod h1:TeFFdX477vUTMushCojaHpvwPLga4DpErGI2oQbqFIs= -github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef h1:6P2pC1kOPcrT/22N23Mr3xr3CTHmQQkj3jmOlUEJvO0= -github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef/go.mod h1:LGG4R3KEz93Ep0CV1/tziCmRk+VtojWUHR9mXkOHZks= +github.com/kuadrant/dns-operator v0.0.0-20241108101824-a0cca97b2ef8 h1:z+pdlUIPjLCeytxmL5/NcORm3elZDAP8i+MLIpMba9U= +github.com/kuadrant/dns-operator v0.0.0-20241108101824-a0cca97b2ef8/go.mod h1:LGG4R3KEz93Ep0CV1/tziCmRk+VtojWUHR9mXkOHZks= github.com/kuadrant/limitador-operator v0.9.0 h1:hTQ6CFPayf/sL7cIzwWjCoU8uTn6fzWdsJgKbDlnFts= github.com/kuadrant/limitador-operator v0.9.0/go.mod h1:DQOlg9qFOcnWPrwO529JRCMLLOEXJQxkmOes952S/Hw= github.com/kuadrant/policy-machinery v0.6.4 h1:UMdZ2p7WyUdOKcWlJA2w2MzJnB8/Nn4dT6hE9cUcbeg= diff --git a/tests/common/dnspolicy/dnspolicy_controller_test.go b/tests/common/dnspolicy/dnspolicy_controller_test.go index e3b91bf04..7945dee91 100644 --- a/tests/common/dnspolicy/dnspolicy_controller_test.go +++ b/tests/common/dnspolicy/dnspolicy_controller_test.go @@ -6,13 +6,13 @@ import ( "fmt" "time" + "github.com/kuadrant/kuadrant-operator/controllers" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" "github.com/onsi/gomega/types" "k8s.io/apimachinery/pkg/util/rand" - kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -21,6 +21,7 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" kuadrantv1 "github.com/kuadrant/kuadrant-operator/api/v1" "github.com/kuadrant/kuadrant-operator/pkg/common" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" @@ -1244,6 +1245,143 @@ var _ = Describe("DNSPolicy controller", func() { }) + // there is no need to replicate cases form the "valid target and valid gateway status" context + // from the policy pov healthchecks can only affect the "SubResourcesHealthy" condition + Context("valid target and valid gateway status with healthchecks", func() { + BeforeEach(func(ctx SpecContext) { + gateway = tests.NewGatewayBuilder(tests.GatewayName, gatewayClass.Name, testNamespace). + WithHTTPListener(tests.ListenerNameOne, tests.HostOne(domain)). + WithHTTPListener(tests.ListenerNameWildcard, tests.HostWildcard(domain)). + Gateway + dnsPolicy = tests.NewDNSPolicy("test-dns-policy", testNamespace). + WithProviderSecret(*dnsProviderSecret). + WithTargetGateway(tests.GatewayName). + WithLoadBalancingFor(100, "foo", true). + WithHealthCheckFor("/health", 80, string(kuadrantdnsv1alpha1.HttpProtocol), 1) + + Expect(k8sClient.Create(ctx, gateway)).To(Succeed()) + Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway)).To(Succeed()) + gateway.Status.Addresses = []gatewayapiv1.GatewayStatusAddress{ + { + Type: ptr.To(gatewayapiv1.IPAddressType), + Value: tests.IPAddressOne, + }, + { + Type: ptr.To(gatewayapiv1.IPAddressType), + Value: tests.IPAddressTwo, + }, + } + gateway.Status.Listeners = []gatewayapiv1.ListenerStatus{ + { + Name: tests.ListenerNameOne, + SupportedKinds: []gatewayapiv1.RouteGroupKind{}, + AttachedRoutes: 1, + Conditions: []metav1.Condition{}, + }, + { + Name: tests.ListenerNameWildcard, + SupportedKinds: []gatewayapiv1.RouteGroupKind{}, + AttachedRoutes: 1, + Conditions: []metav1.Condition{}, + }, + } + g.Expect(k8sClient.Status().Update(ctx, gateway)).To(Succeed()) + }, tests.TimeoutMedium, tests.RetryIntervalMedium).Should(Succeed()) + + recordName = fmt.Sprintf("%s-%s", tests.GatewayName, tests.ListenerNameOne) + wildcardRecordName = fmt.Sprintf("%s-%s", tests.GatewayName, tests.ListenerNameWildcard) + }) + + It("should create records with enforced and not healthy status", func(ctx SpecContext) { + Eventually(func(g Gomega) { + //Check records + recordList := &kuadrantdnsv1alpha1.DNSRecordList{} + err := k8sClient.List(ctx, recordList, &client.ListOptions{Namespace: testNamespace}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(recordList.Items).To(HaveLen(2)) + + // This record should not be ready - we are not publishing unhealthy EPs + dnsRecord := &kuadrantdnsv1alpha1.DNSRecord{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: recordName, Namespace: testNamespace}, dnsRecord)).To(Succeed()) + g.Expect(dnsRecord.Status.Conditions).To( + ContainElements( + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(kuadrantdnsv1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(string(kuadrantdnsv1alpha1.ConditionReasonUnhealthy)), + "Message": Equal("Not publishing unhealthy records"), + }), + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(kuadrantdnsv1alpha1.ConditionTypeHealthy)), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(string(kuadrantdnsv1alpha1.ConditionReasonUnhealthy)), + "Message": And( + ContainSubstring("Not healthy addresses"), + ContainSubstring(tests.IPAddressOne), + ContainSubstring(tests.IPAddressTwo)), + }), + ), + ) + + // This record should be ready - we are not creating checks for wildcards + wildcardDnsRecord := &kuadrantdnsv1alpha1.DNSRecord{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: wildcardRecordName, Namespace: testNamespace}, wildcardDnsRecord)).To(Succeed()) + g.Expect(wildcardDnsRecord.Status.Conditions).To( + And( + ContainElement( + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(kuadrantdnsv1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal(string(kuadrantdnsv1alpha1.ConditionReasonProviderSuccess)), + "Message": Equal("Provider ensured the dns record"), + }), + ), + Not(ContainElement( + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(kuadrantdnsv1alpha1.ConditionTypeHealthy)), + }), + )), + ), + ) + + //Check policy status + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsPolicy.Status.Conditions).To( + ContainElements( + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(gatewayapiv1alpha2.PolicyConditionAccepted)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal(string(gatewayapiv1alpha2.PolicyConditionAccepted)), + "Message": Equal("DNSPolicy has been accepted"), + }), + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(kuadrant.PolicyConditionEnforced)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal(string(kuadrant.PolicyReasonEnforced)), + "Message": And( + ContainSubstring("DNSPolicy has been partially enforced. Not ready DNSRecords are:"), + ContainSubstring(recordName), + Not(ContainSubstring(wildcardRecordName))), + }), + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(controllers.PolicyConditionSubResourcesHealthy)), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(string(kuadrant.PolicyReasonUnknown)), + "Message": And( + ContainSubstring("DNSPolicy has encountered some issues: not all sub-resources of policy are passing the policy defined health check. Not healthy DNSRecords are:"), + ContainSubstring(recordName), + Not(ContainSubstring(wildcardRecordName))), // explicitly make sure that we have no probes for the wildcard record + })), + ) + g.Expect(dnsPolicy.Status.TotalRecords).To(Equal(int32(2))) + }, tests.TimeoutLong, tests.RetryIntervalMedium, ctx).Should(Succeed()) + }, testTimeOut) + }) + Context("cel validation", func() { It("should error targeting invalid group", func(ctx SpecContext) { p := tests.NewDNSPolicy("test-dns-policy", testNamespace).