Skip to content
This repository has been archived by the owner on Dec 16, 2024. It is now read-only.

Commit

Permalink
Rename strategy -> routingStrategy
Browse files Browse the repository at this point in the history
  • Loading branch information
mikenairn committed Nov 6, 2023
1 parent 66668bd commit 83916f0
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 33 deletions.
7 changes: 4 additions & 3 deletions config/crd/bases/kuadrant.io_dnspolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -201,7 +202,7 @@ spec:
- name
type: object
required:
- strategy
- routingStrategy
- targetRef
type: object
status:
Expand Down
11 changes: 6 additions & 5 deletions pkg/apis/v1alpha1/dnspolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
25 changes: 12 additions & 13 deletions pkg/controllers/dnspolicy/dns_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -181,15 +181,14 @@ 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 {

var (
endpoints []*v1alpha1.Endpoint
ipValues []string
hostValues []string
endpoint *v1alpha1.Endpoint
)

for _, cgwTarget := range mcgTarget.ClusterGatewayTargets {
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/dnspolicy/dns_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
22 changes: 11 additions & 11 deletions pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -68,19 +68,19 @@ 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)
continue
}
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)
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -151,22 +151,22 @@ 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
} else {
metaObj = targetGW
}

for i, addr := range clusterAddress {
for i, addr := range gatewayAddresses {
addrType := *addr.Type
if addrType == gateway.MultiClusterHostnameAddressType {
addrType = gatewayv1beta1.HostnameAddressType
Expand Down
1 change: 1 addition & 0 deletions test/e2e/gateway_single_spoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions test/policy_integration/dnspolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 83916f0

Please sign in to comment.