From 83916f0a6e1adbcde4328fe1392d63478fa77e9a Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Mon, 6 Nov 2023 16:26:29 +0000 Subject: [PATCH] Rename strategy -> routingStrategy --- config/crd/bases/kuadrant.io_dnspolicies.yaml | 7 +++--- pkg/apis/v1alpha1/dnspolicy_types.go | 11 ++++---- pkg/controllers/dnspolicy/dns_helper.go | 25 +++++++++---------- pkg/controllers/dnspolicy/dns_helper_test.go | 2 +- .../dnspolicy/dnspolicy_dnsrecords.go | 22 ++++++++-------- test/e2e/gateway_single_spoke_test.go | 1 + .../dnspolicy_controller_test.go | 2 ++ 7 files changed, 37 insertions(+), 33 deletions(-) diff --git a/config/crd/bases/kuadrant.io_dnspolicies.yaml b/config/crd/bases/kuadrant.io_dnspolicies.yaml index 98e617b8f..a6c6185ff 100644 --- a/config/crd/bases/kuadrant.io_dnspolicies.yaml +++ b/config/crd/bases/kuadrant.io_dnspolicies.yaml @@ -155,13 +155,14 @@ spec: type: integer type: object type: object - strategy: + routingStrategy: + default: loadbalanced enum: - simple - loadbalanced type: string x-kubernetes-validations: - - message: DNSPolicyStrategy is immutable + - message: RoutingStrategy is immutable rule: self == oldSelf targetRef: description: PolicyTargetReference identifies an API object to apply @@ -201,7 +202,7 @@ spec: - name type: object required: - - strategy + - routingStrategy - targetRef type: object status: diff --git a/pkg/apis/v1alpha1/dnspolicy_types.go b/pkg/apis/v1alpha1/dnspolicy_types.go index 96e016d9b..9a6a11eae 100644 --- a/pkg/apis/v1alpha1/dnspolicy_types.go +++ b/pkg/apis/v1alpha1/dnspolicy_types.go @@ -25,11 +25,11 @@ import ( gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) -type DNSPolicyStrategy string +type RoutingStrategy string const ( - SimpleStrategy DNSPolicyStrategy = "simple" - LoadBalancedStrategy DNSPolicyStrategy = "loadbalanced" + SimpleRoutingStrategy RoutingStrategy = "simple" + LoadBalancedRoutingStrategy RoutingStrategy = "loadbalanced" ) // DNSPolicySpec defines the desired state of DNSPolicy @@ -47,8 +47,9 @@ type DNSPolicySpec struct { // +required // +kubebuilder:validation:Enum=simple;loadbalanced - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="DNSPolicyStrategy is immutable" - Strategy DNSPolicyStrategy `json:"strategy"` + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="RoutingStrategy is immutable" + // +kubebuilder:default=loadbalanced + RoutingStrategy RoutingStrategy `json:"routingStrategy"` } type LoadBalancingSpec struct { diff --git a/pkg/controllers/dnspolicy/dns_helper.go b/pkg/controllers/dnspolicy/dns_helper.go index 53fc4ff9c..e1bd33f3d 100644 --- a/pkg/controllers/dnspolicy/dns_helper.go +++ b/pkg/controllers/dnspolicy/dns_helper.go @@ -33,9 +33,9 @@ const ( ) var ( - ErrUnknownDNSStrategy = fmt.Errorf("unknown dns policy strategy") - ErrNoManagedZoneForHost = fmt.Errorf("no managed zone for host") - ErrAlreadyAssigned = fmt.Errorf("managed host already assigned") + ErrUnknownRoutingStrategy = fmt.Errorf("unknown routing strategy") + ErrNoManagedZoneForHost = fmt.Errorf("no managed zone for host") + ErrAlreadyAssigned = fmt.Errorf("managed host already assigned") ) type dnsHelper struct { @@ -148,7 +148,7 @@ func withGatewayListener[T metav1.Object](gateway common.GatewayWrapper, listene return obj } -func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClusterGatewayTarget, dnsRecord *v1alpha1.DNSRecord, listener gatewayv1beta1.Listener, strategy v1alpha1.DNSPolicyStrategy) error { +func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClusterGatewayTarget, dnsRecord *v1alpha1.DNSRecord, listener gatewayv1beta1.Listener, strategy v1alpha1.RoutingStrategy) error { old := dnsRecord.DeepCopy() gwListenerHost := string(*listener.Hostname) var endpoints []*v1alpha1.Endpoint @@ -160,12 +160,12 @@ func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClust } switch strategy { - case v1alpha1.SimpleStrategy: + case v1alpha1.SimpleRoutingStrategy: endpoints = dh.getSimpleEndpoints(mcgTarget, gwListenerHost, currentEndpoints) - case v1alpha1.LoadBalancedStrategy: + case v1alpha1.LoadBalancedRoutingStrategy: endpoints = dh.getLoadBalancedEndpoints(mcgTarget, gwListenerHost, currentEndpoints) default: - return fmt.Errorf("%w : %s", ErrUnknownDNSStrategy, strategy) + return fmt.Errorf("%w : %s", ErrUnknownRoutingStrategy, strategy) } sort.Slice(endpoints, func(i, j int) bool { @@ -181,7 +181,7 @@ func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClust return nil } -// getSimpleEndpoints sets the endpoints for the given MultiClusterGatewayTarget using the simple DNS policy strategy +// getSimpleEndpoints returns the endpoints for the given MultiClusterGatewayTarget using the simple routing strategy func (dh *dnsHelper) getSimpleEndpoints(mcgTarget *dns.MultiClusterGatewayTarget, hostname string, currentEndpoints map[string]*v1alpha1.Endpoint) []*v1alpha1.Endpoint { @@ -189,7 +189,6 @@ func (dh *dnsHelper) getSimpleEndpoints(mcgTarget *dns.MultiClusterGatewayTarget endpoints []*v1alpha1.Endpoint ipValues []string hostValues []string - endpoint *v1alpha1.Endpoint ) for _, cgwTarget := range mcgTarget.ClusterGatewayTargets { @@ -203,20 +202,20 @@ func (dh *dnsHelper) getSimpleEndpoints(mcgTarget *dns.MultiClusterGatewayTarget } if len(ipValues) > 0 { - endpoint = createOrUpdateEndpoint(hostname, ipValues, v1alpha1.ARecordType, "", dns.DefaultTTL, currentEndpoints) + endpoint := createOrUpdateEndpoint(hostname, ipValues, v1alpha1.ARecordType, "", dns.DefaultTTL, currentEndpoints) endpoints = append(endpoints, endpoint) } - //ToDO This is what external-dns does, but not sure it will actually work since you can't have CNAME records with multiple values afaik + //ToDO This is what external-dns does, but not sure it will actually work since you can't have CNAME records with multiple values if len(hostValues) > 0 { - endpoint = createOrUpdateEndpoint(hostname, hostValues, v1alpha1.CNAMERecordType, "", dns.DefaultTTL, currentEndpoints) + endpoint := createOrUpdateEndpoint(hostname, hostValues, v1alpha1.CNAMERecordType, "", dns.DefaultTTL, currentEndpoints) endpoints = append(endpoints, endpoint) } return endpoints } -// getLoadBalancedEndpoints sets the endpoints for the given MultiClusterGatewayTarget using the loadbalanced DNS policy strategy +// getLoadBalancedEndpoints returns the endpoints for the given MultiClusterGatewayTarget using the loadbalanced routing strategy // // Builds an array of v1alpha1.Endpoint resources and sets them on the given DNSRecord. The endpoints expected are calculated // from the MultiClusterGatewayTarget using the target Gateway (MultiClusterGatewayTarget.Gateway), the LoadBalancing Spec diff --git a/pkg/controllers/dnspolicy/dns_helper_test.go b/pkg/controllers/dnspolicy/dns_helper_test.go index 61d07155d..c385aad24 100644 --- a/pkg/controllers/dnspolicy/dns_helper_test.go +++ b/pkg/controllers/dnspolicy/dns_helper_test.go @@ -975,7 +975,7 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { f := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(testCase.dnsRecord).Build() s := dnsHelper{Client: f} - if err := s.setEndpoints(context.TODO(), testCase.mcgTarget, testCase.dnsRecord, testCase.listener); (err != nil) != testCase.wantErr { + if err := s.setEndpoints(context.TODO(), testCase.mcgTarget, testCase.dnsRecord, testCase.listener, v1alpha1.LoadBalancedRoutingStrategy); (err != nil) != testCase.wantErr { t.Errorf("SetEndpoints() error = %v, wantErr %v", err, testCase.wantErr) } diff --git a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go index 81e5dc41e..75f8dd2be 100644 --- a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go +++ b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go @@ -53,9 +53,9 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga return err } - clusterAddresses := getClusterGatewayAddresses(gateway) + clusterGatewayAddresses := getClusterGatewayAddresses(gateway) - log.V(3).Info("checking gateway for attached routes ", "gateway", gateway.Name, "clusters", clusterAddresses) + log.V(3).Info("checking gateway for attached routes ", "gateway", gateway.Name, "clusters", clusterGatewayAddresses) for _, listener := range gateway.Spec.Listeners { var clusterGateways []dns.ClusterGateway @@ -68,11 +68,11 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga log.Info("skipping listener no hostname assigned", listener.Name, "in ns ", gateway.Namespace) continue } - for clusterName, clusterAddress := range clusterAddresses { + for clusterName, gatewayAddresses := range clusterGatewayAddresses { // Only consider host for dns if there's at least 1 attached route to the listener for this host in *any* gateway log.V(3).Info("checking downstream", "listener ", listener.Name) - attached := listenerTotalAttachedRoutes(gateway, clusterName, listener, clusterAddress) + attached := listenerTotalAttachedRoutes(gateway, clusterName, listener, gatewayAddresses) if attached == 0 { log.V(1).Info("no attached routes for ", "listener", listener, "cluster ", clusterName) @@ -80,7 +80,7 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga } log.V(3).Info("hostHasAttachedRoutes", "host", listener.Name, "hostHasAttachedRoutes", attached) - cg, err := r.buildClusterGateway(ctx, clusterName, clusterAddress, gateway) + cg, err := r.buildClusterGateway(ctx, clusterName, gatewayAddresses, gateway) if err != nil { return fmt.Errorf("get cluster gateway failed: %s", err) } @@ -118,7 +118,7 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga return err } mcgTarget.RemoveUnhealthyGatewayAddresses(probes, listener) - if err := r.dnsHelper.setEndpoints(ctx, mcgTarget, dnsRecord, listener, dnsPolicy.Spec.Strategy); err != nil { + if err := r.dnsHelper.setEndpoints(ctx, mcgTarget, dnsRecord, listener, dnsPolicy.Spec.RoutingStrategy); err != nil { return fmt.Errorf("failed to add dns record dnsTargets %s %v", err, mcgTarget) } } @@ -151,14 +151,14 @@ func (r *DNSPolicyReconciler) deleteDNSRecordsWithLabels(ctx context.Context, lb return nil } -func (r *DNSPolicyReconciler) buildClusterGateway(ctx context.Context, downstreamClusterName string, clusterAddress []gatewayv1beta1.GatewayAddress, targetGW *gatewayv1beta1.Gateway) (dns.ClusterGateway, error) { +func (r *DNSPolicyReconciler) buildClusterGateway(ctx context.Context, clusterName string, gatewayAddresses []gatewayv1beta1.GatewayAddress, targetGW *gatewayv1beta1.Gateway) (dns.ClusterGateway, error) { var target dns.ClusterGateway - singleClusterAddresses := make([]gatewayv1beta1.GatewayAddress, len(clusterAddress)) + singleClusterAddresses := make([]gatewayv1beta1.GatewayAddress, len(gatewayAddresses)) var metaObj client.Object - if downstreamClusterName != singleCluster { + if clusterName != singleCluster { mc := &clusterv1.ManagedCluster{} - if err := r.Client().Get(ctx, client.ObjectKey{Name: downstreamClusterName}, mc, &client.GetOptions{}); err != nil { + if err := r.Client().Get(ctx, client.ObjectKey{Name: clusterName}, mc, &client.GetOptions{}); err != nil { return target, err } metaObj = mc @@ -166,7 +166,7 @@ func (r *DNSPolicyReconciler) buildClusterGateway(ctx context.Context, downstrea metaObj = targetGW } - for i, addr := range clusterAddress { + for i, addr := range gatewayAddresses { addrType := *addr.Type if addrType == gateway.MultiClusterHostnameAddressType { addrType = gatewayv1beta1.HostnameAddressType diff --git a/test/e2e/gateway_single_spoke_test.go b/test/e2e/gateway_single_spoke_test.go index 22e1eff8b..8bcededdf 100644 --- a/test/e2e/gateway_single_spoke_test.go +++ b/test/e2e/gateway_single_spoke_test.go @@ -252,6 +252,7 @@ var _ = Describe("Gateway single target cluster", func() { Name: gatewayapi.ObjectName(testID), Namespace: Pointer(gatewayapi.Namespace(tconfig.HubNamespace())), }, + RoutingStrategy: v1alpha1.LoadBalancedRoutingStrategy, }, } err := tconfig.HubClient().Create(ctx, dnsPolicy) diff --git a/test/policy_integration/dnspolicy_controller_test.go b/test/policy_integration/dnspolicy_controller_test.go index 3d240b25c..7e3a25b3e 100644 --- a/test/policy_integration/dnspolicy_controller_test.go +++ b/test/policy_integration/dnspolicy_controller_test.go @@ -131,6 +131,7 @@ func testBuildDNSPolicyWithHealthCheck(policyName, gwName, ns string, threshold Namespace: ns, }, Spec: v1alpha1.DNSPolicySpec{ + RoutingStrategy: v1alpha1.LoadBalancedRoutingStrategy, TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ Group: "gateway.networking.k8s.io", Kind: "Gateway", @@ -159,6 +160,7 @@ func testBuildDNSPolicyWithGeo(policyName, gwName, ns string) *v1alpha1.DNSPolic Namespace: ns, }, Spec: v1alpha1.DNSPolicySpec{ + RoutingStrategy: v1alpha1.LoadBalancedRoutingStrategy, TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ Group: "gateway.networking.k8s.io", Kind: "Gateway",