From 5de7c4cea02f7098009e69b16e4a25e3dc921e16 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Thu, 7 Dec 2023 21:26:09 +0000 Subject: [PATCH 1/4] feat: Remove ocm dependency from policy controller Remove all dependencies related to OCM from the policy controller and associated code. Instead of labeling the OCM ManagedCluster resources with weight and geo metadata, the labels are now added to the gateway itself with the cluster name embedded. ``` "kuadrant.io/kind-mgc-control-plane_lb-attribute-weight": "TSTATTR", "kuadrant.io/kind-mgc-control-plane_lb-attribute-geo-code": "EU" ``` --- cmd/policy_controller/main.go | 7 +- hack/make/policy_controller.make | 3 +- pkg/controllers/dnspolicy/dns_helper.go | 4 +- pkg/controllers/dnspolicy/dns_helper_test.go | 224 +++++------ .../dnspolicy/dnspolicy_controller.go | 18 +- .../dnspolicy/dnspolicy_dnsrecords.go | 68 +--- pkg/controllers/events/cluster_eventmapper.go | 81 ---- pkg/controllers/gateway/gateway_controller.go | 3 - pkg/dns/target.go | 39 +- pkg/dns/target_test.go | 359 +++++++++++------- pkg/placement/fake/ocm.go | 5 - pkg/placement/ocm.go | 27 -- pkg/utils/gateway_wrapper.go | 138 +++++-- pkg/utils/gateway_wrapper_test.go | 219 ++++++----- test/gateway_integration/placement_fake.go | 16 - test/policy_integration/suite_test.go | 2 +- 16 files changed, 611 insertions(+), 602 deletions(-) delete mode 100644 pkg/controllers/events/cluster_eventmapper.go diff --git a/cmd/policy_controller/main.go b/cmd/policy_controller/main.go index 96255233f..2cc81ce0f 100644 --- a/cmd/policy_controller/main.go +++ b/cmd/policy_controller/main.go @@ -64,16 +64,11 @@ func main() { var metricsAddr string var enableLeaderElection bool var probeAddr string - var ocmHub bool 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.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") - - flag.BoolVar(&ocmHub, "ocm-hub", true, - "tells the controller if it is in an ocm hub"+ - "setting this to false will cause the controller to stop watching for managedcluster resources") opts := zap.Options{ Development: true, } @@ -130,7 +125,7 @@ func main() { BaseReconciler: dnsPolicyBaseReconciler, }, DNSProvider: provider.DNSProviderFactory, - }).SetupWithManager(mgr, ocmHub); err != nil { + }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "DNSPolicy") os.Exit(1) } diff --git a/hack/make/policy_controller.make b/hack/make/policy_controller.make index 86d3bc6db..18b970ba1 100644 --- a/hack/make/policy_controller.make +++ b/hack/make/policy_controller.make @@ -23,8 +23,7 @@ run-policy-controller: manifests generate fmt vet install go run ./cmd/policy_controller/main.go \ --metrics-bind-address=:8090 \ --health-probe-bind-address=:8091 \ - --zap-log-level=$(LOG_LEVEL) \ - --ocm-hub=$(OCM) + --zap-log-level=$(LOG_LEVEL) .PHONY: docker-build-policy-controller docker-build-policy-controller: ## Build docker image with the controller. diff --git a/pkg/controllers/dnspolicy/dns_helper.go b/pkg/controllers/dnspolicy/dns_helper.go index 48410a71d..a38a7e452 100644 --- a/pkg/controllers/dnspolicy/dns_helper.go +++ b/pkg/controllers/dnspolicy/dns_helper.go @@ -190,7 +190,7 @@ func (dh *dnsHelper) getSimpleEndpoints(mcgTarget *dns.MultiClusterGatewayTarget ) for _, cgwTarget := range mcgTarget.ClusterGatewayTargets { - for _, gwa := range cgwTarget.GatewayAddresses { + for _, gwa := range cgwTarget.Status.Addresses { if *gwa.Type == gatewayapiv1.IPAddressType { ipValues = append(ipValues, gwa.Value) } else { @@ -273,7 +273,7 @@ func (dh *dnsHelper) getLoadBalancedEndpoints(mcgTarget *dns.MultiClusterGateway var ipValues []string var hostValues []string - for _, gwa := range cgwTarget.GatewayAddresses { + for _, gwa := range cgwTarget.Status.Addresses { if *gwa.Type == gatewayapiv1.IPAddressType { ipValues = append(ipValues, gwa.Value) } else { diff --git a/pkg/controllers/dnspolicy/dns_helper_test.go b/pkg/controllers/dnspolicy/dns_helper_test.go index a51f02f7a..198a7dd35 100644 --- a/pkg/controllers/dnspolicy/dns_helper_test.go +++ b/pkg/controllers/dnspolicy/dns_helper_test.go @@ -18,6 +18,7 @@ import ( "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" + "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" testutil "github.com/Kuadrant/multicluster-gateway-controller/test/util" ) @@ -421,40 +422,42 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { }, ClusterGatewayTargets: []dns.ClusterGatewayTarget{ { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", - }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, + ClusterName: "test-cluster-1", }, Geo: testutil.Pointer(dns.GeoCode("default")), Weight: testutil.Pointer(120), }, { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), - Value: "mylb.example.com", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), + Value: "mylb.example.com", + }, + }, }, }, + ClusterName: "test-cluster-2", }, Geo: testutil.Pointer(dns.GeoCode("default")), Weight: testutil.Pointer(120), @@ -531,41 +534,42 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { }, ClusterGatewayTargets: []dns.ClusterGatewayTarget{ { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", - }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, + ClusterName: "test-cluster-1", }, Geo: testutil.Pointer(dns.GeoCode("NA")), Weight: testutil.Pointer(120), }, { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), - Value: "mylb.example.com", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), + Value: "mylb.example.com", + }, + }, }, }, + ClusterName: "test-cluster-2", }, Geo: testutil.Pointer(dns.GeoCode("IE")), Weight: testutil.Pointer(120), @@ -677,40 +681,42 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { ClusterGatewayTargets: []dns.ClusterGatewayTarget{ { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", - }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, + ClusterName: "test-cluster-1", }, Geo: testutil.Pointer(dns.GeoCode("default")), Weight: testutil.Pointer(120), }, { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), - Value: "mylb.example.com", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), + Value: "mylb.example.com", + }, + }, }, }, + ClusterName: "test-cluster-2", }, Geo: testutil.Pointer(dns.GeoCode("default")), Weight: testutil.Pointer(120), @@ -788,40 +794,42 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { ClusterGatewayTargets: []dns.ClusterGatewayTarget{ { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", - }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, + ClusterName: "test-cluster-1", }, Geo: testutil.Pointer(dns.GeoCode("NA")), Weight: testutil.Pointer(120), }, { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", - }, - }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), - Value: "mylb.example.com", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), + Value: "mylb.example.com", + }, + }, }, }, + ClusterName: "test-cluster-2", }, Geo: testutil.Pointer(dns.GeoCode("IE")), Weight: testutil.Pointer(120), @@ -930,26 +938,28 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { ClusterGatewayTargets: []dns.ClusterGatewayTarget{ { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{}, }, }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{}, + ClusterName: "test-cluster-1", }, Geo: testutil.Pointer(dns.GeoCode("NA")), Weight: testutil.Pointer(120), }, { - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{}, }, }, - GatewayAddresses: []gatewayapiv1.GatewayAddress{}, + ClusterName: "test-cluster-2", }, Geo: testutil.Pointer(dns.GeoCode("IE")), Weight: testutil.Pointer(120), diff --git a/pkg/controllers/dnspolicy/dnspolicy_controller.go b/pkg/controllers/dnspolicy/dnspolicy_controller.go index 7eef988c2..bcba1eb3d 100644 --- a/pkg/controllers/dnspolicy/dnspolicy_controller.go +++ b/pkg/controllers/dnspolicy/dnspolicy_controller.go @@ -22,8 +22,6 @@ import ( "fmt" "reflect" - clusterv1 "open-cluster-management.io/api/cluster/v1" - "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -67,9 +65,9 @@ type DNSPolicyReconciler struct { //+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/finalizers,verbs=update //+kubebuilder:rbac:groups=cluster.open-cluster-management.io,resources=managedclusters,verbs=get;list;watch -// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;update;patch -// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/finalizers,verbs=update +//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;update;patch +//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/finalizers,verbs=update func (r *DNSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := r.Logger().WithValues("DNSPolicy", req.NamespacedName) @@ -301,9 +299,8 @@ func (r *DNSPolicyReconciler) updateGatewayCondition(ctx context.Context, condit return nil } -func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager, ocmHub bool) error { +func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { gatewayEventMapper := events.NewGatewayEventMapper(r.Logger(), &DNSPolicyRefsConfig{}, "dnspolicy") - clusterEventMapper := events.NewClusterEventMapper(r.Logger(), r.Client(), &DNSPolicyRefsConfig{}, "dnspolicy") probeEventMapper := events.NewProbeEventMapper(r.Logger(), DNSPolicyBackRefAnnotation, "dnspolicy") r.dnsHelper = dnsHelper{Client: r.Client()} ctrlr := ctrl.NewControllerManagedBy(mgr). @@ -316,12 +313,5 @@ func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager, ocmHub bool) er &v1alpha1.DNSHealthCheckProbe{}, handler.EnqueueRequestsFromMapFunc(probeEventMapper.MapToPolicy), ) - if ocmHub { - r.Logger().Info("ocm enabled turning on managed cluster watch") - ctrlr.Watches( - &clusterv1.ManagedCluster{}, - handler.EnqueueRequestsFromMapFunc(clusterEventMapper.MapToPolicy), - ) - } return ctrlr.Complete(r) } diff --git a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go index eccaefbce..1b058a5b2 100644 --- a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go +++ b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go @@ -4,8 +4,6 @@ import ( "context" "fmt" - clusterv1 "open-cluster-management.io/api/cluster/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" @@ -14,6 +12,7 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" + "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/slice" "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" @@ -53,12 +52,11 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gw return err } - clusterGatewayAddresses := gatewayWrapper.GetClusterGatewayAddresses() + clusterGateways := gatewayWrapper.GetClusterGateways() - log.V(3).Info("checking gateway for attached routes ", "gateway", gatewayWrapper.Name, "clusters", clusterGatewayAddresses) + log.V(3).Info("checking gateway for attached routes ", "gateway", gatewayWrapper.Name, "clusterGateways", clusterGateways) for _, listener := range gatewayWrapper.Spec.Listeners { - var clusterGateways []dns.ClusterGateway var mz, err = r.dnsHelper.getManagedZoneForListener(ctx, gatewayWrapper.Namespace, listener) if err != nil { return err @@ -68,29 +66,21 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gw log.Info("skipping listener no hostname assigned", listener.Name, "in ns ", gatewayWrapper.Namespace) continue } - 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 := gatewayWrapper.ListenerTotalAttachedRoutes(clusterName, listener) - 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, gatewayAddresses, gatewayWrapper.Gateway) - if err != nil { - return fmt.Errorf("get cluster gateway failed: %s", err) + listenerGateways := slice.Filter(clusterGateways, func(cgw utils.ClusterGateway) bool { + hasAttachedRoute := false + for _, statusListener := range cgw.Status.Listeners { + if string(statusListener.Name) == string(listener.Name) { + hasAttachedRoute = int(statusListener.AttachedRoutes) > 0 + break + } } + return hasAttachedRoute + }) - clusterGateways = append(clusterGateways, cg) - } - - if len(clusterGateways) == 0 { + if len(listenerGateways) == 0 { // delete record - log.V(3).Info("no cluster gateways, deleting DNS record", " for listener ", listener.Name) + log.V(1).Info("no cluster gateways, deleting DNS record", " for listener ", listener.Name) if err := r.dnsHelper.deleteDNSRecordForListener(ctx, gatewayWrapper, listener); client.IgnoreNotFound(err) != nil { return fmt.Errorf("failed to delete dns record for listener %s : %s", listener.Name, err) } @@ -107,7 +97,7 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gw } } - mcgTarget, err := dns.NewMultiClusterGatewayTarget(gatewayWrapper.Gateway, clusterGateways, dnsPolicy.Spec.LoadBalancing) + mcgTarget, err := dns.NewMultiClusterGatewayTarget(gatewayWrapper.Gateway, listenerGateways, dnsPolicy.Spec.LoadBalancing) if err != nil { return fmt.Errorf("failed to create multi cluster gateway target for listener %s : %s ", listener.Name, err) } @@ -150,31 +140,3 @@ func (r *DNSPolicyReconciler) deleteDNSRecordsWithLabels(ctx context.Context, lb } return nil } - -func (r *DNSPolicyReconciler) buildClusterGateway(ctx context.Context, clusterName string, gatewayAddresses []gatewayapiv1.GatewayAddress, targetGW *gatewayapiv1.Gateway) (dns.ClusterGateway, error) { - var target dns.ClusterGateway - singleClusterAddresses := make([]gatewayapiv1.GatewayAddress, len(gatewayAddresses)) - - var metaObj client.Object - if clusterName != utils.SingleClusterNameValue { - mc := &clusterv1.ManagedCluster{} - 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 gatewayAddresses { - addrType, _ := utils.AddressTypeToSingleCluster(addr) - - singleClusterAddresses[i] = gatewayapiv1.GatewayAddress{ - Type: &addrType, - Value: addr.Value, - } - } - target = *dns.NewClusterGateway(metaObj, singleClusterAddresses) - - return target, nil -} diff --git a/pkg/controllers/events/cluster_eventmapper.go b/pkg/controllers/events/cluster_eventmapper.go deleted file mode 100644 index e88002b0e..000000000 --- a/pkg/controllers/events/cluster_eventmapper.go +++ /dev/null @@ -1,81 +0,0 @@ -package events - -import ( - "context" - "encoding/json" - - "github.com/go-logr/logr" - - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/kuadrant/kuadrant-operator/pkg/common" - - "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/metadata" - "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/slice" - "github.com/Kuadrant/multicluster-gateway-controller/pkg/controllers/gateway" -) - -// ClusterEventMapper is an EventHandler that maps Cluster object events to policy events. -// -// Cluster object can be anything that represents a cluster and has mgc attribute labels applied to (e.g. OCM ManagedCluster) -type ClusterEventMapper struct { - Logger logr.Logger - GatewayEventMapper *GatewayEventMapper - Client client.Client - PolicyRefsConfig common.PolicyRefsConfig - PolicyKind string -} - -func NewClusterEventMapper(logger logr.Logger, client client.Client, policyRefsConfig common.PolicyRefsConfig, policyKind string) *ClusterEventMapper { - log := logger.WithName("ClusterEventMapper") - return &ClusterEventMapper{ - Logger: log, - GatewayEventMapper: NewGatewayEventMapper(log, policyRefsConfig, policyKind), - Client: client, - PolicyRefsConfig: policyRefsConfig, - PolicyKind: policyKind, - } -} - -func (m *ClusterEventMapper) MapToPolicy(ctx context.Context, obj client.Object) []reconcile.Request { - return m.mapToPolicyRequest(ctx, obj, m.PolicyKind, m.PolicyRefsConfig) -} - -func (m *ClusterEventMapper) mapToPolicyRequest(ctx context.Context, obj client.Object, policyKind string, policyRefsConfig common.PolicyRefsConfig) []reconcile.Request { - logger := m.Logger.V(1).WithValues("object", client.ObjectKeyFromObject(obj)) - - if obj.GetDeletionTimestamp() != nil { - // Ignore ManagedCluster delete events. - // Create/Update events are OK as ManagedCluster custom attributes may change, affecting DNSPolicies. - // However, deleting a ManagedCluster shouldn't affect the DNSPolicy directly until the related - // Gateway is deleted from that ManagedCluster (and reconciled then via watching Gateways, not ManagedClusters) - return []reconcile.Request{} - } - - clusterName := obj.GetName() - - allGwList := &gatewayapiv1.GatewayList{} - err := m.Client.List(ctx, allGwList) - if err != nil { - logger.Info("mapToPolicyRequest:", "error", "failed to get gateways") - return []reconcile.Request{} - } - - requests := make([]reconcile.Request, 0) - for _, gw := range allGwList.Items { - val := metadata.GetAnnotation(&gw, gateway.GatewayClustersAnnotation) - if val == "" { - continue - } - var clusters []string - if err := json.Unmarshal([]byte(val), &clusters); err == nil { - if slice.ContainsString(clusters, clusterName) { - requests = append(requests, m.GatewayEventMapper.mapToPolicyRequest(ctx, &gw, policyKind, policyRefsConfig)[:]...) - } - } - } - - return requests -} diff --git a/pkg/controllers/gateway/gateway_controller.go b/pkg/controllers/gateway/gateway_controller.go index 36984d302..dfd244c0f 100644 --- a/pkg/controllers/gateway/gateway_controller.go +++ b/pkg/controllers/gateway/gateway_controller.go @@ -49,7 +49,6 @@ import ( "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/gracePeriod" "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/metadata" "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/slice" - "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" "github.com/Kuadrant/multicluster-gateway-controller/pkg/policysync" "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" ) @@ -73,8 +72,6 @@ type GatewayPlacer interface { ListenerTotalAttachedRoutes(ctx context.Context, gateway *gatewayapiv1.Gateway, listenerName string, downstream string) (int, error) // GetAddresses will look at the downstream view of the gateway and return the LB addresses used for these gateways GetAddresses(ctx context.Context, gateway *gatewayapiv1.Gateway, downstream string) ([]gatewayapiv1.GatewayAddress, error) - // GetClusterGateway - GetClusterGateway(ctx context.Context, gateway *gatewayapiv1.Gateway, clusterName string) (dns.ClusterGateway, error) } // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;create;update;patch;delete diff --git a/pkg/dns/target.go b/pkg/dns/target.go index 8c6788071..f00e6dd80 100644 --- a/pkg/dns/target.go +++ b/pkg/dns/target.go @@ -12,6 +12,7 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" + "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" ) const ( @@ -28,7 +29,7 @@ type MultiClusterGatewayTarget struct { LoadBalancing *v1alpha1.LoadBalancingSpec } -func NewMultiClusterGatewayTarget(gateway *gatewayapiv1.Gateway, clusterGateways []ClusterGateway, loadBalancing *v1alpha1.LoadBalancingSpec) (*MultiClusterGatewayTarget, error) { +func NewMultiClusterGatewayTarget(gateway *gatewayapiv1.Gateway, clusterGateways []utils.ClusterGateway, loadBalancing *v1alpha1.LoadBalancingSpec) (*MultiClusterGatewayTarget, error) { mcg := &MultiClusterGatewayTarget{Gateway: gateway, LoadBalancing: loadBalancing} err := mcg.setClusterGatewayTargets(clusterGateways) return mcg, err @@ -65,7 +66,7 @@ func (t *MultiClusterGatewayTarget) GetDefaultWeight() int { return DefaultWeight } -func (t *MultiClusterGatewayTarget) setClusterGatewayTargets(clusterGateways []ClusterGateway) error { +func (t *MultiClusterGatewayTarget) setClusterGatewayTargets(clusterGateways []utils.ClusterGateway) error { var cgTargets []ClusterGatewayTarget for _, cg := range clusterGateways { var customWeights []*v1alpha1.CustomWeight @@ -82,12 +83,6 @@ func (t *MultiClusterGatewayTarget) setClusterGatewayTargets(clusterGateways []C return nil } -// ClusterGateway contains the addresses of a Gateway on a single cluster and the attributes of the target cluster. -type ClusterGateway struct { - Cluster metav1.Object - GatewayAddresses []gatewayapiv1.GatewayAddress -} - type GeoCode string func (gc GeoCode) IsDefaultCode() bool { @@ -98,22 +93,14 @@ func (gc GeoCode) IsWildcard() bool { return gc == WildcardGeo } -func NewClusterGateway(cluster metav1.Object, gatewayAddresses []gatewayapiv1.GatewayAddress) *ClusterGateway { - cgw := &ClusterGateway{ - Cluster: cluster, - GatewayAddresses: gatewayAddresses, - } - return cgw -} - // ClusterGatewayTarget represents a cluster Gateway with geo and weighting info calculated type ClusterGatewayTarget struct { - *ClusterGateway + *utils.ClusterGateway Geo *GeoCode Weight *int } -func NewClusterGatewayTarget(cg ClusterGateway, defaultGeoCode GeoCode, defaultWeight int, customWeights []*v1alpha1.CustomWeight) (ClusterGatewayTarget, error) { +func NewClusterGatewayTarget(cg utils.ClusterGateway, defaultGeoCode GeoCode, defaultWeight int, customWeights []*v1alpha1.CustomWeight) (ClusterGatewayTarget, error) { target := ClusterGatewayTarget{ ClusterGateway: &cg, } @@ -134,7 +121,7 @@ func (t *ClusterGatewayTarget) GetWeight() int { } func (t *ClusterGatewayTarget) GetName() string { - return t.Cluster.GetName() + return t.ClusterName } func (t *ClusterGatewayTarget) GetShortCode() string { @@ -147,7 +134,7 @@ func (t *ClusterGatewayTarget) setGeo(defaultGeo GeoCode) { t.Geo = &geoCode return } - if gc, ok := t.Cluster.GetLabels()[LabelLBAttributeGeoCode]; ok { + if gc, ok := t.GetLabels()[LabelLBAttributeGeoCode]; ok { geoCode = GeoCode(gc) } t.Geo = &geoCode @@ -164,8 +151,8 @@ func (t *MultiClusterGatewayTarget) RemoveUnhealthyGatewayAddresses(probes []*v1 gwAddressHealth := map[string]bool{} allunhealthy := true for _, cgt := range t.ClusterGatewayTargets { - for _, gwa := range cgt.GatewayAddresses { - probe := getProbeForGatewayAddress(probes, gwa, t.Gateway.Name, string(listener.Name)) + for _, gwa := range cgt.Status.Addresses { + probe := getProbeForGatewayAddress(probes, gatewayapiv1.GatewayAddress(gwa), t.Gateway.Name, string(listener.Name)) if probe == nil { continue } @@ -187,13 +174,13 @@ func (t *MultiClusterGatewayTarget) RemoveUnhealthyGatewayAddresses(probes []*v1 // Remove all unhealthy addresses, we know by this point at least one of our addresses is healthy for _, cgt := range t.ClusterGatewayTargets { - healthyAddresses := []gatewayapiv1.GatewayAddress{} - for _, gwa := range cgt.GatewayAddresses { + healthyAddresses := []gatewayapiv1.GatewayStatusAddress{} + for _, gwa := range cgt.Status.Addresses { if healthy, exists := gwAddressHealth[gwa.Value]; exists && healthy { healthyAddresses = append(healthyAddresses, gwa) } } - cgt.GatewayAddresses = healthyAddresses + cgt.Status.Addresses = healthyAddresses } } @@ -218,7 +205,7 @@ func (t *ClusterGatewayTarget) setWeight(defaultWeight int, customWeights []*v1a if err != nil { return err } - if selector.Matches(labels.Set(t.Cluster.GetLabels())) { + if selector.Matches(labels.Set(t.GetLabels())) { customWeight := int(cw.Weight) weight = customWeight break diff --git a/pkg/dns/target_test.go b/pkg/dns/target_test.go index 5a0c33884..1b97e2d5a 100644 --- a/pkg/dns/target_test.go +++ b/pkg/dns/target_test.go @@ -12,6 +12,7 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" + "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" testutil "github.com/Kuadrant/multicluster-gateway-controller/test/util" ) @@ -25,7 +26,7 @@ const ( func TestNewClusterGatewayTarget(t *testing.T) { type args struct { - clusterGateway ClusterGateway + clusterGateway utils.ClusterGateway defaultGeoCode GeoCode defaultWeight int customWeights []*v1alpha1.CustomWeight @@ -39,26 +40,28 @@ func TestNewClusterGatewayTarget(t *testing.T) { { name: "set geo and weight from defaults", args: args{ - clusterGateway: ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + clusterGateway: utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, defaultWeight: 100, defaultGeoCode: GeoCode("IE"), customWeights: []*v1alpha1.CustomWeight{}, }, want: ClusterGatewayTarget{ - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, Geo: testutil.Pointer(GeoCode("IE")), Weight: testutil.Pointer(100), @@ -66,19 +69,22 @@ func TestNewClusterGatewayTarget(t *testing.T) { wantErr: false, }, { - name: "set geo and weight from cluster labels", + name: "set geo and weight from gateway labels", args: args{ - clusterGateway: ClusterGateway{ - Cluster: &testutil.TestResource{ + clusterGateway: utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", Labels: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", }, }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), + }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, defaultWeight: 100, defaultGeoCode: GeoCode("IE"), @@ -94,17 +100,20 @@ func TestNewClusterGatewayTarget(t *testing.T) { }, }, want: ClusterGatewayTarget{ - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", Labels: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", }, }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), + }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, Geo: testutil.Pointer(GeoCode("EU")), Weight: testutil.Pointer(60), @@ -132,7 +141,7 @@ func TestNewClusterGatewayTarget(t *testing.T) { func TestNewMultiClusterGatewayTarget(t *testing.T) { type args struct { gateway *gatewayapiv1.Gateway - clusterGateways []ClusterGateway + clusterGateways []utils.ClusterGateway loadBalancing *v1alpha1.LoadBalancingSpec } gateway := &gatewayapiv1.Gateway{ @@ -151,22 +160,28 @@ func TestNewMultiClusterGatewayTarget(t *testing.T) { name: "set cluster gateway targets with default geo and weight values", args: args{ gateway: gateway, - clusterGateways: []ClusterGateway{ + clusterGateways: []utils.ClusterGateway{ { - Cluster: &testutil.TestResource{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, { - Cluster: &testutil.TestResource{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName2, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress2), }, }, - GatewayAddresses: buildGatewayAddress(testAddress2), + ClusterName: clusterName2, }, }, loadBalancing: nil, @@ -175,25 +190,31 @@ func TestNewMultiClusterGatewayTarget(t *testing.T) { Gateway: gateway, ClusterGatewayTargets: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, Geo: testutil.Pointer(DefaultGeo), Weight: testutil.Pointer(DefaultWeight), }, { - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName2, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress2), }, }, - GatewayAddresses: buildGatewayAddress(testAddress2), + ClusterName: clusterName2, }, Geo: testutil.Pointer(DefaultGeo), Weight: testutil.Pointer(DefaultWeight), @@ -207,22 +228,28 @@ func TestNewMultiClusterGatewayTarget(t *testing.T) { name: "set cluster gateway targets with default geo and weight from load balancing config", args: args{ gateway: gateway, - clusterGateways: []ClusterGateway{ + clusterGateways: []utils.ClusterGateway{ { - Cluster: &testutil.TestResource{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, { - Cluster: &testutil.TestResource{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName2, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress2), }, }, - GatewayAddresses: buildGatewayAddress(testAddress2), + ClusterName: clusterName2, }, }, loadBalancing: &v1alpha1.LoadBalancingSpec{ @@ -238,25 +265,31 @@ func TestNewMultiClusterGatewayTarget(t *testing.T) { Gateway: gateway, ClusterGatewayTargets: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, Geo: testutil.Pointer(GeoCode("IE")), Weight: testutil.Pointer(255), }, { - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName2, + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress2), }, }, - GatewayAddresses: buildGatewayAddress(testAddress2), + ClusterName: clusterName2, }, Geo: testutil.Pointer(GeoCode("IE")), Weight: testutil.Pointer(255), @@ -277,26 +310,30 @@ func TestNewMultiClusterGatewayTarget(t *testing.T) { name: "set cluster gateway targets with default geo and weight from cluster labels", args: args{ gateway: gateway, - clusterGateways: []ClusterGateway{ + clusterGateways: []utils.ClusterGateway{ { - Cluster: &testutil.TestResource{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", Labels: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", }, }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), + }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, { - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: clusterName2, + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress2), }, }, - GatewayAddresses: buildGatewayAddress(testAddress2), + ClusterName: clusterName2, }, }, loadBalancing: &v1alpha1.LoadBalancingSpec{ @@ -322,29 +359,33 @@ func TestNewMultiClusterGatewayTarget(t *testing.T) { Gateway: gateway, ClusterGatewayTargets: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, + Name: "testgw", Labels: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", }, }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), + }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, Geo: testutil.Pointer(GeoCode("EU")), Weight: testutil.Pointer(60), }, { - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: clusterName2, + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress2), }, }, - GatewayAddresses: buildGatewayAddress(testAddress2), + ClusterName: clusterName2, }, Geo: testutil.Pointer(GeoCode("IE")), Weight: testutil.Pointer(255), @@ -418,19 +459,19 @@ func TestClusterGatewayTarget_setGeo(t *testing.T) { testCases := []struct { name string defaultGeo GeoCode - clusterLabels map[string]string + gatewayLabels map[string]string want GeoCode }{ { name: "sets geo from default", defaultGeo: "IE", - clusterLabels: nil, + gatewayLabels: nil, want: "IE", }, { name: "sets geo from label", defaultGeo: "IE", - clusterLabels: map[string]string{ + gatewayLabels: map[string]string{ "kuadrant.io/lb-attribute-geo-code": "EU", }, want: "EU", @@ -438,7 +479,7 @@ func TestClusterGatewayTarget_setGeo(t *testing.T) { { name: "sets geo to default for default geo value", defaultGeo: "default", - clusterLabels: map[string]string{ + gatewayLabels: map[string]string{ "kuadrant.io/lb-attribute-geo-code": "EU", }, want: "default", @@ -447,14 +488,17 @@ func TestClusterGatewayTarget_setGeo(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t1 *testing.T) { cgt := &ClusterGatewayTarget{ - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, - Labels: testCase.clusterLabels, + Name: "testgw", + Labels: testCase.gatewayLabels, + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress1), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, } cgt.setGeo(testCase.defaultGeo) @@ -470,14 +514,14 @@ func TestClusterGatewayTarget_setWeight(t *testing.T) { name string defaultWeight int customWeights []*v1alpha1.CustomWeight - clusterLabels map[string]string + gatewayLabels map[string]string want int wantErr bool }{ { name: "sets geo from default", defaultWeight: 255, - clusterLabels: nil, + gatewayLabels: nil, customWeights: []*v1alpha1.CustomWeight{}, want: 255, wantErr: false, @@ -485,7 +529,7 @@ func TestClusterGatewayTarget_setWeight(t *testing.T) { { name: "sets geo from custom weight", defaultWeight: 255, - clusterLabels: map[string]string{ + gatewayLabels: map[string]string{ "tstlabel1": "TSTATTR", }, customWeights: []*v1alpha1.CustomWeight{ @@ -504,7 +548,7 @@ func TestClusterGatewayTarget_setWeight(t *testing.T) { { name: "sets geo from from custom weight with selector with multiple matches", defaultWeight: 255, - clusterLabels: map[string]string{ + gatewayLabels: map[string]string{ "tstlabel1": "TSTATTR", "tstlabel2": "TSTATTR2", }, @@ -525,7 +569,7 @@ func TestClusterGatewayTarget_setWeight(t *testing.T) { { name: "sets geo from default when not all custom weight selectors match", defaultWeight: 255, - clusterLabels: map[string]string{ + gatewayLabels: map[string]string{ "tstlabel1": "TSTATTR", }, customWeights: []*v1alpha1.CustomWeight{ @@ -545,7 +589,7 @@ func TestClusterGatewayTarget_setWeight(t *testing.T) { { name: "returns error when label selector invalid", defaultWeight: 255, - clusterLabels: map[string]string{ + gatewayLabels: map[string]string{ "/tstlabel1": "TSTATTR", }, customWeights: []*v1alpha1.CustomWeight{ @@ -565,14 +609,17 @@ func TestClusterGatewayTarget_setWeight(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t1 *testing.T) { cgt := &ClusterGatewayTarget{ - ClusterGateway: &ClusterGateway{ - Cluster: &testutil.TestResource{ + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ ObjectMeta: v1.ObjectMeta{ - Name: clusterName1, - Labels: testCase.clusterLabels, + Name: "testgw", + Labels: testCase.gatewayLabels, + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: buildGatewayAddress(testAddress2), }, }, - GatewayAddresses: buildGatewayAddress(testAddress1), + ClusterName: clusterName1, }, Weight: &testCase.defaultWeight, } @@ -589,8 +636,8 @@ func TestClusterGatewayTarget_setWeight(t *testing.T) { } } -func buildGatewayAddress(value string) []gatewayapiv1.GatewayAddress { - return []gatewayapiv1.GatewayAddress{ +func buildGatewayAddress(value string) []gatewayapiv1.GatewayStatusAddress { + return []gatewayapiv1.GatewayStatusAddress{ { Type: testutil.Pointer(gatewayapiv1.IPAddressType), Value: value, @@ -619,15 +666,22 @@ func TestMultiClusterGatewayTarget_RemoveUnhealthyGatewayAddresses(t *testing.T) fields: fields{ ClusterGatewayTargets: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, }, @@ -670,15 +724,22 @@ func TestMultiClusterGatewayTarget_RemoveUnhealthyGatewayAddresses(t *testing.T) }, want: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, }, @@ -690,15 +751,22 @@ func TestMultiClusterGatewayTarget_RemoveUnhealthyGatewayAddresses(t *testing.T) fields: fields{ ClusterGatewayTargets: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, }, @@ -741,11 +809,18 @@ func TestMultiClusterGatewayTarget_RemoveUnhealthyGatewayAddresses(t *testing.T) }, want: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", + }, + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, }, @@ -757,15 +832,22 @@ func TestMultiClusterGatewayTarget_RemoveUnhealthyGatewayAddresses(t *testing.T) fields: fields{ ClusterGatewayTargets: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, }, @@ -808,15 +890,22 @@ func TestMultiClusterGatewayTarget_RemoveUnhealthyGatewayAddresses(t *testing.T) }, want: []ClusterGatewayTarget{ { - ClusterGateway: &ClusterGateway{ - GatewayAddresses: []gatewayapiv1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "1.1.1.1", + ClusterGateway: &utils.ClusterGateway{ + Gateway: gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", }, - { - Type: testutil.Pointer(gatewayapiv1.IPAddressType), - Value: "2.2.2.2", + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayapiv1.IPAddressType), + Value: "2.2.2.2", + }, + }, }, }, }, diff --git a/pkg/placement/fake/ocm.go b/pkg/placement/fake/ocm.go index 4ae8d9d58..990a38dce 100644 --- a/pkg/placement/fake/ocm.go +++ b/pkg/placement/fake/ocm.go @@ -10,7 +10,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" testutil "github.com/Kuadrant/multicluster-gateway-controller/test/util" ) @@ -20,10 +19,6 @@ func NewTestGatewayPlacer() *FakeGatewayPlacer { return &FakeGatewayPlacer{} } -func (p *FakeGatewayPlacer) GetClusterGateway(_ context.Context, _ *gatewayapiv1.Gateway, _ string) (dns.ClusterGateway, error) { - return dns.ClusterGateway{}, nil -} - func (p *FakeGatewayPlacer) Place(_ context.Context, upstream *gatewayapiv1.Gateway, _ *gatewayapiv1.Gateway, _ ...metav1.Object) (sets.Set[string], error) { if upstream.Labels == nil { return nil, nil diff --git a/pkg/placement/ocm.go b/pkg/placement/ocm.go index b8e8bcf2c..9a3d6c026 100644 --- a/pkg/placement/ocm.go +++ b/pkg/placement/ocm.go @@ -26,7 +26,6 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/gracePeriod" - "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" ) const ( @@ -266,32 +265,6 @@ func (op *ocmPlacer) GetClusters(ctx context.Context, gateway *gatewayapiv1.Gate return targetClusters, nil } -func (op *ocmPlacer) GetClusterGateway(ctx context.Context, gateway *gatewayapiv1.Gateway, clusterName string) (dns.ClusterGateway, error) { - var target dns.ClusterGateway - workname := WorkName(gateway) - mw := &workv1.ManifestWork{ - ObjectMeta: metav1.ObjectMeta{ - Name: workname, - Namespace: clusterName, - }, - } - if err := op.c.Get(ctx, client.ObjectKeyFromObject(mw), mw, &client.GetOptions{}); err != nil { - return target, err - } - - mc := &clusterv1.ManagedCluster{} - if err := op.c.Get(ctx, client.ObjectKey{Name: clusterName}, mc, &client.GetOptions{}); err != nil { - return target, err - } - - addresses, err := op.GetAddresses(ctx, gateway, clusterName) - if err != nil { - return target, err - } - target = *dns.NewClusterGateway(mc, addresses) - return target, nil -} - func (op *ocmPlacer) createUpdateClusterManifests(ctx context.Context, manifestName string, upstream *gatewayapiv1.Gateway, downstream *gatewayapiv1.Gateway, cluster string, obj ...metav1.Object) error { log := log.Log // set up gateway manifest diff --git a/pkg/utils/gateway_wrapper.go b/pkg/utils/gateway_wrapper.go index 15f3abc63..619f9034f 100644 --- a/pkg/utils/gateway_wrapper.go +++ b/pkg/utils/gateway_wrapper.go @@ -4,12 +4,11 @@ import ( "fmt" "strings" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) const ( - SingleClusterNameValue = "kudarant.io/single" - MultiClusterIPAddressType gatewayapiv1.AddressType = "kuadrant.io/MultiClusterIPAddress" MultiClusterHostnameAddressType gatewayapiv1.AddressType = "kuadrant.io/MultiClusterHostnameAddress" ) @@ -52,31 +51,30 @@ func (g *GatewayWrapper) Validate() error { // GetClusterGatewayAddresses constructs a map from Status.Addresses of underlying Gateway // with key being a cluster and value being an address in the cluster. -// In case of a single-cluster Gateway the key is SingleClusterNameValue -func (g *GatewayWrapper) GetClusterGatewayAddresses() map[string][]gatewayapiv1.GatewayAddress { - clusterAddrs := make(map[string][]gatewayapiv1.GatewayAddress, len(g.Status.Addresses)) +// In case of a single-cluster Gateway the key is the Gateway Name. +func (g *GatewayWrapper) GetClusterGatewayAddresses() map[string][]gatewayapiv1.GatewayStatusAddress { + + if !g.IsMultiCluster() { + // Single Cluster (Normal Gateway Status) + return map[string][]gatewayapiv1.GatewayStatusAddress{g.GetName(): g.Status.Addresses} + } + // Multi Cluster (MGC Gateway Status) + clusterAddrs := map[string][]gatewayapiv1.GatewayStatusAddress{} for _, address := range g.Status.Addresses { - //Default to Single Cluster (Normal Gateway Status) - cluster := SingleClusterNameValue - addressValue := address.Value - - //Check for Multi Cluster (MGC Gateway Status) - if g.IsMultiCluster() { - tmpCluster, tmpAddress, found := strings.Cut(address.Value, "/") - //If this fails something is wrong and the value hasn't been set correctly - if found { - cluster = tmpCluster - addressValue = tmpAddress - } + cluster, addressValue, found := strings.Cut(address.Value, "/") + //If this fails something is wrong and the value hasn't been set correctly + if !found { + continue } if _, ok := clusterAddrs[cluster]; !ok { - clusterAddrs[cluster] = []gatewayapiv1.GatewayAddress{} + clusterAddrs[cluster] = []gatewayapiv1.GatewayStatusAddress{} } - clusterAddrs[cluster] = append(clusterAddrs[cluster], gatewayapiv1.GatewayAddress{ - Type: address.Type, + addressType, _ := AddressTypeToSingleCluster(gatewayapiv1.GatewayAddress(address)) + clusterAddrs[cluster] = append(clusterAddrs[cluster], gatewayapiv1.GatewayStatusAddress{ + Type: &addressType, Value: addressValue, }) } @@ -84,27 +82,101 @@ func (g *GatewayWrapper) GetClusterGatewayAddresses() map[string][]gatewayapiv1. return clusterAddrs } -// ListenerTotalAttachedRoutes returns a count of attached routes from the Status.Listeners for a specified -// combination of downstreamClusterName and specListener.Name -func (g *GatewayWrapper) ListenerTotalAttachedRoutes(downstreamClusterName string, specListener gatewayapiv1.Listener) int { - for _, statusListener := range g.Status.Listeners { - // for Multi Cluster (MGC Gateway Status) - if g.IsMultiCluster() { - clusterName, listenerName, found := strings.Cut(string(statusListener.Name), ".") +// GetClusterGatewayLabels parses the labels of the wrapped Gateway and returns a list of labels for the given clusterName. +// In case of a single-cluster Gateway the wrapped Gateways labels are returned unmodified. +func (g *GatewayWrapper) GetClusterGatewayLabels(clusterName string) map[string]string { + if !g.IsMultiCluster() { + // Single Cluster (Normal Gateway Status) + return g.GetLabels() + } + + labels := map[string]string{} + for k, v := range g.GetLabels() { + attr, found := strings.CutPrefix(k, "kuadrant.io/"+clusterName+"_") + if found { + labels["kuadrant.io/"+attr] = v + } else { + // Only add a label if we haven't already found a cluster specific version of it + if _, ok := labels[k]; !ok { + labels[k] = v + } + } + } + return labels +} + +// GetClusterGatewayListeners processes the wrapped Gateway and returns a ListenerStatus for the given clusterName. +// In case of a single-cluster Gateway the wrapped Gateways status listeners are returned unmodified. +func (g *GatewayWrapper) GetClusterGatewayListeners(clusterName string) []gatewayapiv1.ListenerStatus { + + if !g.IsMultiCluster() { + // Single Cluster (Normal Gateway Status) + return g.Status.Listeners + } + + // Multi Cluster (MGC Gateway Status) + listeners := []gatewayapiv1.ListenerStatus{} + for _, specListener := range g.Spec.Listeners { + for _, statusListener := range g.Status.Listeners { + statusClusterName, statusListenerName, found := strings.Cut(string(statusListener.Name), ".") if !found { - return 0 + continue } - if clusterName == downstreamClusterName && listenerName == string(specListener.Name) { - return int(statusListener.AttachedRoutes) + if statusClusterName == clusterName && statusListenerName == string(specListener.Name) { + ls := gatewayapiv1.ListenerStatus{ + Name: specListener.Name, + AttachedRoutes: statusListener.AttachedRoutes, + } + listeners = append(listeners, ls) } } + } + return listeners +} + +// ClusterGateway contains a Gateway as it would be on a single cluster and the name of the cluster. +type ClusterGateway struct { + gatewayapiv1.Gateway + ClusterName string +} + +// GetClusterGateways parse the wrapped Gateway and returns a list of ClusterGateway resources. +// In case of a single-cluster Gateway a single ClusterGateway is returned with the unmodified wrapped Gateway and the +// Gateway name used as values. +func (g *GatewayWrapper) GetClusterGateways() []ClusterGateway { + + if !g.IsMultiCluster() { // Single Cluster (Normal Gateway Status) - if string(statusListener.Name) == string(specListener.Name) { - return int(statusListener.AttachedRoutes) + return []ClusterGateway{ + { + Gateway: *g.Gateway, + ClusterName: g.GetName(), + }, } } - return 0 + // Multi Cluster (MGC Gateway Status) + clusterAddrs := g.GetClusterGatewayAddresses() + clusterGateways := []ClusterGateway{} + for clusterName, addrs := range clusterAddrs { + gw := gatewayapiv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: g.GetName(), + Namespace: g.GetNamespace(), + Labels: g.GetClusterGatewayLabels(clusterName), + }, + Spec: g.Spec, + Status: gatewayapiv1.GatewayStatus{ + Addresses: addrs, + Listeners: g.GetClusterGatewayListeners(clusterName), + }, + } + clusterGateways = append(clusterGateways, ClusterGateway{ + Gateway: gw, + ClusterName: clusterName, + }) + } + return clusterGateways } // AddressTypeToMultiCluster returns a multi cluster version of the address type diff --git a/pkg/utils/gateway_wrapper_test.go b/pkg/utils/gateway_wrapper_test.go index da4190f23..49407f0ab 100644 --- a/pkg/utils/gateway_wrapper_test.go +++ b/pkg/utils/gateway_wrapper_test.go @@ -4,6 +4,7 @@ import ( "reflect" "testing" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -92,11 +93,14 @@ func TestGatewayWrapper_GetClusterGatewayAddresses(t *testing.T) { tests := []struct { name string Gateway *gatewayapiv1.Gateway - want map[string][]gatewayapiv1.GatewayAddress + want map[string][]gatewayapiv1.GatewayStatusAddress }{ { name: "single cluster Gateway", Gateway: &gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", + }, Status: gatewayapiv1.GatewayStatus{ Addresses: []gatewayapiv1.GatewayStatusAddress{ { @@ -106,8 +110,8 @@ func TestGatewayWrapper_GetClusterGatewayAddresses(t *testing.T) { }, }, }, - want: map[string][]gatewayapiv1.GatewayAddress{ - SingleClusterNameValue: { + want: map[string][]gatewayapiv1.GatewayStatusAddress{ + "testgw": { { Type: testutil.Pointer(v1beta1.IPAddressType), Value: "1.1.1.1", @@ -135,20 +139,20 @@ func TestGatewayWrapper_GetClusterGatewayAddresses(t *testing.T) { }, }, }, - want: map[string][]gatewayapiv1.GatewayAddress{ + want: map[string][]gatewayapiv1.GatewayStatusAddress{ "kind-mgc-control-plane": { { - Type: testutil.Pointer(MultiClusterIPAddressType), + Type: testutil.Pointer(gatewayapiv1.IPAddressType), Value: "1.1.1.1", }, }, "kind-mgc-workload-1": { { - Type: testutil.Pointer(MultiClusterIPAddressType), + Type: testutil.Pointer(gatewayapiv1.IPAddressType), Value: "2.2.2.2", }, { - Type: testutil.Pointer(MultiClusterHostnameAddressType), + Type: testutil.Pointer(gatewayapiv1.HostnameAddressType), Value: "boop.com", }, }, @@ -165,72 +169,98 @@ func TestGatewayWrapper_GetClusterGatewayAddresses(t *testing.T) { } } -func TestGatewayWrapper_ListenerTotalAttachedRoutes(t *testing.T) { - +func TestGatewayWrapper_Validate(t *testing.T) { tests := []struct { - name string - Gateway *gatewayapiv1.Gateway - downstreamClusterName string - want int + name string + Gateway *gatewayapiv1.Gateway + wantErr bool }{ { - name: "single cluster gateway", + name: "Valid Gateway", Gateway: &gatewayapiv1.Gateway{ - Spec: gatewayapiv1.GatewaySpec{ - Listeners: []gatewayapiv1.Listener{ + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ { - Name: "api", + Type: testutil.Pointer(MultiClusterIPAddressType), + Value: "kind-mgc-control-plane/1.1.1.1", }, - }, - }, - Status: gatewayapiv1.GatewayStatus{ - Listeners: []gatewayapiv1.ListenerStatus{ { - Name: "api", - AttachedRoutes: 1, + Type: testutil.Pointer(MultiClusterIPAddressType), + Value: "kind-mgc-workload-1/2.2.2.2", + }, + { + Type: testutil.Pointer(MultiClusterHostnameAddressType), + Value: "kind-mgc-workload-1/boop.com", }, }, }, }, - downstreamClusterName: SingleClusterNameValue, - want: 1, + wantErr: false, }, { - name: "multi cluster gateway", + name: "invalid Gateway: inconsistent addresses", Gateway: &gatewayapiv1.Gateway{ - Spec: gatewayapiv1.GatewaySpec{ - Listeners: []gatewayapiv1.Listener{ - { - Name: "api", - }, - }, - }, Status: gatewayapiv1.GatewayStatus{ Addresses: []gatewayapiv1.GatewayStatusAddress{ { Type: testutil.Pointer(MultiClusterIPAddressType), Value: "kind-mgc-control-plane/1.1.1.1", }, - }, - Listeners: []gatewayapiv1.ListenerStatus{ { - Name: "kind-mgc-control-plane.api", - AttachedRoutes: 1, + Type: testutil.Pointer(v1beta1.NamedAddressType), + Value: "kind-mgc-workload-1/boop.com", }, }, }, }, - downstreamClusterName: "kind-mgc-control-plane", - want: 1, + wantErr: true, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGatewayWrapper(tt.Gateway) + if err := g.Validate(); (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { + tests := []struct { + name string + Gateway *gatewayapiv1.Gateway + clusterName string + want map[string]string + }{ { - name: "invalid status listener name", + name: "single cluster gateway", Gateway: &gatewayapiv1.Gateway{ - Spec: gatewayapiv1.GatewaySpec{ - Listeners: []gatewayapiv1.Listener{ - { - Name: "api", - }, + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", + Labels: map[string]string{ + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "foo": "bar", + }, + }, + }, + clusterName: "foo", + want: map[string]string{ + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "foo": "bar", + }, + }, + { + name: "multi cluster gateway", + Gateway: &gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", + Labels: map[string]string{ + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "foo": "bar", }, }, Status: gatewayapiv1.GatewayStatus{ @@ -239,50 +269,33 @@ func TestGatewayWrapper_ListenerTotalAttachedRoutes(t *testing.T) { Type: testutil.Pointer(MultiClusterIPAddressType), Value: "kind-mgc-control-plane/1.1.1.1", }, - }, - Listeners: []gatewayapiv1.ListenerStatus{ { - Name: "kind-mgc-control-plane-api", - AttachedRoutes: 1, + Type: testutil.Pointer(MultiClusterIPAddressType), + Value: "kind-mgc-workload-1/2.2.2.2", }, }, }, }, - want: 0, + clusterName: "kind-mgc-control-plane", + want: map[string]string{ + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "foo": "bar", + }, }, { - name: "no status", + name: "multi cluster gateway with cluster labels", Gateway: &gatewayapiv1.Gateway{ - Spec: gatewayapiv1.GatewaySpec{ - Listeners: []gatewayapiv1.Listener{ - { - Name: "api", - }, + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", + Labels: map[string]string{ + "kuadrant.io/kind-mgc-control-plane_lb-attribute-weight": "TSTATTR", + "kuadrant.io/kind-mgc-control-plane_lb-attribute-geo-code": "EU", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", + "foo": "bar", }, }, - }, - want: 0, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewGatewayWrapper(tt.Gateway) - if got := g.ListenerTotalAttachedRoutes(tt.downstreamClusterName, tt.Gateway.Spec.Listeners[0]); got != tt.want { - t.Errorf("ListenerTotalAttachedRoutes() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestGatewayWrapper_Validate(t *testing.T) { - tests := []struct { - name string - Gateway *gatewayapiv1.Gateway - wantErr bool - }{ - { - name: "Valid Gateway", - Gateway: &gatewayapiv1.Gateway{ Status: gatewayapiv1.GatewayStatus{ Addresses: []gatewayapiv1.GatewayStatusAddress{ { @@ -293,18 +306,33 @@ func TestGatewayWrapper_Validate(t *testing.T) { Type: testutil.Pointer(MultiClusterIPAddressType), Value: "kind-mgc-workload-1/2.2.2.2", }, - { - Type: testutil.Pointer(MultiClusterHostnameAddressType), - Value: "kind-mgc-workload-1/boop.com", - }, }, }, }, - wantErr: false, + clusterName: "kind-mgc-control-plane", + want: map[string]string{ + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", + "foo": "bar", + }, }, { - name: "invalid Gateway: inconsistent addresses", + name: "multi cluster gateway with mix of cluster labels", Gateway: &gatewayapiv1.Gateway{ + ObjectMeta: v1.ObjectMeta{ + Name: "testgw", + Labels: map[string]string{ + "kuadrant.io/kind-mgc-control-plane_lb-attribute-weight": "TSTATTR", + "kuadrant.io/kind-mgc-control-plane_lb-attribute-geo-code": "EU", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", + "kuadrant.io/lb-attribute-weight": "TSTATTR3", + "kuadrant.io/lb-attribute-geo-code": "ES", + "foo": "bar", + }, + }, Status: gatewayapiv1.GatewayStatus{ Addresses: []gatewayapiv1.GatewayStatusAddress{ { @@ -312,20 +340,29 @@ func TestGatewayWrapper_Validate(t *testing.T) { Value: "kind-mgc-control-plane/1.1.1.1", }, { - Type: testutil.Pointer(v1beta1.NamedAddressType), - Value: "kind-mgc-workload-1/boop.com", + Type: testutil.Pointer(MultiClusterIPAddressType), + Value: "kind-mgc-workload-1/2.2.2.2", }, }, }, }, - wantErr: true, + clusterName: "kind-mgc-control-plane", + want: map[string]string{ + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", + "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", + "foo": "bar", + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - g := NewGatewayWrapper(tt.Gateway) - if err := g.Validate(); (err != nil) != tt.wantErr { - t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + g := &GatewayWrapper{ + Gateway: tt.Gateway, + } + if got := g.GetClusterGatewayLabels(tt.clusterName); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetClusterGatewayLabels() = %v, want %v", got, tt.want) } }) } diff --git a/test/gateway_integration/placement_fake.go b/test/gateway_integration/placement_fake.go index 04a987a89..f86b64536 100644 --- a/test/gateway_integration/placement_fake.go +++ b/test/gateway_integration/placement_fake.go @@ -9,9 +9,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" - testutil "github.com/Kuadrant/multicluster-gateway-controller/test/util" ) const ( @@ -114,16 +111,3 @@ func (f FakeOCMPlacer) GetAddresses(ctx context.Context, gateway *gatewayapiv1.G } return gwAddresses, nil } - -func (f FakeOCMPlacer) GetClusterGateway(ctx context.Context, gateway *gatewayapiv1.Gateway, clusterName string) (dns.ClusterGateway, error) { - gwAddresses, _ := f.GetAddresses(ctx, gateway, clusterName) - cgw := dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterName, - }, - }, - GatewayAddresses: gwAddresses, - } - return cgw, nil -} diff --git a/test/policy_integration/suite_test.go b/test/policy_integration/suite_test.go index 876464ad8..0b48c4685 100644 --- a/test/policy_integration/suite_test.go +++ b/test/policy_integration/suite_test.go @@ -160,7 +160,7 @@ var _ = BeforeSuite(func() { BaseReconciler: dnsPolicyBaseReconciler, }, DNSProvider: providerFactory, - }).SetupWithManager(k8sManager, true) + }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) tlsPolicyBaseReconciler := reconcilers.NewBaseReconciler( From bdc29dcebd9eb2b59b788c0e8c8c0f83d4af122b Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Fri, 8 Dec 2023 09:57:01 +0000 Subject: [PATCH 2/4] Init watch on ManagedCluster in gateway controller Adds the watch for ManagedCluster resources to the gateway controller and adds an additional step in the reconcile (reconcileClusterLabels) to map the kuadrant.io labels from cluster(s) to gateway. reconcileClusterLabels - Not Implemented --- pkg/controllers/events/cluster_eventmapper.go | 72 +++++++++++++++++++ pkg/controllers/gateway/gateway_controller.go | 25 ++++++- 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 pkg/controllers/events/cluster_eventmapper.go diff --git a/pkg/controllers/events/cluster_eventmapper.go b/pkg/controllers/events/cluster_eventmapper.go new file mode 100644 index 000000000..ca525f8de --- /dev/null +++ b/pkg/controllers/events/cluster_eventmapper.go @@ -0,0 +1,72 @@ +package events + +import ( + "context" + "encoding/json" + + "github.com/go-logr/logr" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/metadata" + "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/slice" +) + +// ClusterEventMapper is an EventHandler that maps Cluster object events to gateway events. +// +// Cluster object can be anything that represents a cluster and has mgc attribute labels applied to (e.g. OCM ManagedCluster) +type ClusterEventMapper struct { + Logger logr.Logger + Client client.Client +} + +func NewClusterEventMapper(logger logr.Logger, client client.Client) *ClusterEventMapper { + log := logger.WithName("ClusterEventMapper") + return &ClusterEventMapper{ + Logger: log, + Client: client, + } +} + +func (m *ClusterEventMapper) MapToGateway(ctx context.Context, obj client.Object) []reconcile.Request { + return m.mapToGatewayRequest(ctx, obj) +} + +func (m *ClusterEventMapper) mapToGatewayRequest(ctx context.Context, obj client.Object) []reconcile.Request { + logger := m.Logger.V(1).WithValues("object", client.ObjectKeyFromObject(obj)) + + if obj.GetDeletionTimestamp() != nil { + // Ignore ManagedCluster delete events. + // Create/Update events are OK as ManagedCluster custom attributes may change, affecting DNSPolicies. + // However, deleting a ManagedCluster shouldn't affect the DNSPolicy directly until the related + // Gateway is deleted from that ManagedCluster (and reconciled then via watching Gateways, not ManagedClusters) + return []reconcile.Request{} + } + + clusterName := obj.GetName() + + allGwList := &gatewayapiv1.GatewayList{} + err := m.Client.List(ctx, allGwList) + if err != nil { + logger.Info("mapToPolicyRequest:", "error", "failed to get gateways") + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, 0) + for _, gw := range allGwList.Items { + val := metadata.GetAnnotation(&gw, "kuadrant.io/gateway-clusters") + if val == "" { + continue + } + var clusters []string + if err = json.Unmarshal([]byte(val), &clusters); err == nil { + if slice.ContainsString(clusters, clusterName) { + requests = append(requests, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&gw)}) + } + } + } + + return requests +} diff --git a/pkg/controllers/gateway/gateway_controller.go b/pkg/controllers/gateway/gateway_controller.go index dfd244c0f..78acf282f 100644 --- a/pkg/controllers/gateway/gateway_controller.go +++ b/pkg/controllers/gateway/gateway_controller.go @@ -24,6 +24,7 @@ import ( "reflect" "time" + clusterv1 "open-cluster-management.io/api/cluster/v1" clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta1" workv1 "open-cluster-management.io/api/work/v1" @@ -49,6 +50,7 @@ import ( "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/gracePeriod" "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/metadata" "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/slice" + "github.com/Kuadrant/multicluster-gateway-controller/pkg/controllers/events" "github.com/Kuadrant/multicluster-gateway-controller/pkg/policysync" "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" ) @@ -195,6 +197,12 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } metadata.AddAnnotation(upstreamGateway, GatewayClustersAnnotation, string(serialized)) + // Map cluster labels onto the gateway + err = r.reconcileClusterLabels(ctx, upstreamGateway, clusters) + if err != nil { + return ctrl.Result{}, err + } + if reconcileErr == nil && !reflect.DeepEqual(upstreamGateway, previous) { log.Info("updating upstream gateway") return reconcile.Result{}, r.Update(ctx, upstreamGateway) @@ -264,6 +272,17 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, reconcileErr } +func (r *GatewayReconciler) reconcileClusterLabels(ctx context.Context, gateway *gatewayapiv1.Gateway, clusters []string) error { + //ToDo Implement me !! + + //Iterate clusters and for each find the ManagedCluster resource, get its labels and convert all "kuadrant.io/" labels to a cluster specific label and add it to the gateway + // (cluster kind-mgc-workload-1 label) kuadrant.io/lb-attribute-custom-weight=AWS = (gateway label) kuadrant.io/kind-mgc-workload-1_lb-attribute-custom-weight=AWS + // (cluster kind-mgc-workload-2 label) kuadrant.io/lb-attribute-geo-code=ES = (gateway label) kuadrant.io/kind-mgc-workload-2_lb-attribute-geo-code=ES + // etc ... + + return nil +} + // reconcileDownstreamGateway takes the upstream definition and transforms it as needed to apply it to the downstream spokes func (r *GatewayReconciler) reconcileDownstreamFromUpstreamGateway(ctx context.Context, upstreamGateway *gatewayapiv1.Gateway, params *Params) (bool, metav1.ConditionStatus, []string, error) { log := crlog.FromContext(ctx) @@ -475,7 +494,7 @@ func buildAcceptedCondition(generation int64, acceptedStatus metav1.ConditionSta // SetupWithManager sets up the controller with the Manager. func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager, ctx context.Context) error { log := crlog.FromContext(ctx) - + clusterEventMapper := events.NewClusterEventMapper(log, mgr.GetClient()) //TODO need to trigger gateway reconcile when gatewayclass params changes return ctrl.NewControllerManagedBy(mgr). For(&gatewayapiv1.Gateway{}). @@ -517,6 +536,10 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager, ctx context.Conte return req })). Watches(&corev1.Secret{}, &ClusterEventHandler{client: r.Client}). + Watches( + &clusterv1.ManagedCluster{}, + handler.EnqueueRequestsFromMapFunc(clusterEventMapper.MapToGateway), + ). WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool { gateway, ok := object.(*gatewayapiv1.Gateway) if ok { From c67d8917dc5e17c28eac442e7c7b1e9c8f5b9bb2 Mon Sep 17 00:00:00 2001 From: Maskym Vavilov Date: Fri, 15 Dec 2023 11:03:24 +0000 Subject: [PATCH 3/4] Enable GW controller to transfer label from ManagedCluster to the GW + integration test bump golangci-lint to 1.55.2 patch in e2e --- ...eway-controller.clusterserviceversion.yaml | 10 +- config/policy-controller/rbac/role.yaml | 8 - .../dnspolicy/dnspolicy_controller.go | 1 - pkg/controllers/gateway/gateway_controller.go | 32 ++- test/e2e/gateway_single_spoke_test.go | 9 +- ...dnspolicy_controller_multi_cluster_test.go | 189 ++++++++++++++++++ 6 files changed, 218 insertions(+), 31 deletions(-) diff --git a/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml b/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml index e5a073ed3..37cb742a8 100644 --- a/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml +++ b/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml @@ -4,7 +4,7 @@ metadata: annotations: alm-examples: '[]' capabilities: Basic Install - createdAt: "2023-12-08T11:41:09Z" + createdAt: "2023-12-21T13:08:31Z" operators.operatorframework.io/builder: operator-sdk-v1.28.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 name: multicluster-gateway-controller.v0.0.0 @@ -349,14 +349,6 @@ spec: - get - list - watch - - apiGroups: - - cluster.open-cluster-management.io - resources: - - managedclusters - verbs: - - get - - list - - watch - apiGroups: - gateway.networking.k8s.io resources: diff --git a/config/policy-controller/rbac/role.yaml b/config/policy-controller/rbac/role.yaml index 81909ce59..6c0ad91fa 100644 --- a/config/policy-controller/rbac/role.yaml +++ b/config/policy-controller/rbac/role.yaml @@ -41,14 +41,6 @@ rules: - get - list - watch -- apiGroups: - - cluster.open-cluster-management.io - resources: - - managedclusters - verbs: - - get - - list - - watch - apiGroups: - gateway.networking.k8s.io resources: diff --git a/pkg/controllers/dnspolicy/dnspolicy_controller.go b/pkg/controllers/dnspolicy/dnspolicy_controller.go index bcba1eb3d..ebdf574ff 100644 --- a/pkg/controllers/dnspolicy/dnspolicy_controller.go +++ b/pkg/controllers/dnspolicy/dnspolicy_controller.go @@ -64,7 +64,6 @@ type DNSPolicyReconciler struct { //+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies,verbs=get;list;watch;update;patch;delete //+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/finalizers,verbs=update -//+kubebuilder:rbac:groups=cluster.open-cluster-management.io,resources=managedclusters,verbs=get;list;watch //+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;update;patch //+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/status,verbs=get;update;patch //+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/finalizers,verbs=update diff --git a/pkg/controllers/gateway/gateway_controller.go b/pkg/controllers/gateway/gateway_controller.go index 78acf282f..c7686fb6b 100644 --- a/pkg/controllers/gateway/gateway_controller.go +++ b/pkg/controllers/gateway/gateway_controller.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "reflect" + "strings" "time" clusterv1 "open-cluster-management.io/api/cluster/v1" @@ -56,10 +57,11 @@ import ( ) const ( - GatewayClusterLabelSelectorAnnotation = "kuadrant.io/gateway-cluster-label-selector" - GatewayClustersAnnotation = "kuadrant.io/gateway-clusters" - GatewayFinalizer = "kuadrant.io/gateway" - ManagedLabel = "kuadrant.io/managed" + LabelPrefix = "kuadrant.io/" + GatewayClusterLabelSelectorAnnotation = LabelPrefix + "gateway-cluster-label-selector" + GatewayClustersAnnotation = LabelPrefix + "gateway-clusters" + GatewayFinalizer = LabelPrefix + "gateway" + ManagedLabel = LabelPrefix + "managed" ) type GatewayPlacer interface { @@ -272,14 +274,24 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, reconcileErr } +// reconcileClusterLabels fetches labels from ManagedCluster related to clusters array and adds them to the provided Gateway func (r *GatewayReconciler) reconcileClusterLabels(ctx context.Context, gateway *gatewayapiv1.Gateway, clusters []string) error { - //ToDo Implement me !! - - //Iterate clusters and for each find the ManagedCluster resource, get its labels and convert all "kuadrant.io/" labels to a cluster specific label and add it to the gateway - // (cluster kind-mgc-workload-1 label) kuadrant.io/lb-attribute-custom-weight=AWS = (gateway label) kuadrant.io/kind-mgc-workload-1_lb-attribute-custom-weight=AWS - // (cluster kind-mgc-workload-2 label) kuadrant.io/lb-attribute-geo-code=ES = (gateway label) kuadrant.io/kind-mgc-workload-2_lb-attribute-geo-code=ES - // etc ... + for _, cluster := range clusters { + managedCluster := &clusterv1.ManagedCluster{} + if err := r.Client.Get(ctx, client.ObjectKey{Name: cluster}, managedCluster); client.IgnoreNotFound(err) != nil { + return err + } + for key, value := range managedCluster.Labels { + if strings.Contains(key, LabelPrefix) { + _, attribute, found := strings.Cut(key, "/") + if !found { + continue + } + gateway.Labels[LabelPrefix+cluster+"_"+attribute] = value + } + } + } return nil } diff --git a/test/e2e/gateway_single_spoke_test.go b/test/e2e/gateway_single_spoke_test.go index 9ade94478..fcccb9691 100644 --- a/test/e2e/gateway_single_spoke_test.go +++ b/test/e2e/gateway_single_spoke_test.go @@ -373,8 +373,9 @@ var _ = Describe("Gateway single target cluster", func() { if gw.Spec.Listeners == nil { gw.Spec.Listeners = []gatewayapiv1.Listener{} } + patch := client.MergeFrom(gw.DeepCopy()) AddListener("wildcard", testHostnameWildcard, gatewayapiv1.ObjectName(testHostname), gw) - err = tconfig.HubClient().Update(ctx, gw) + err = tconfig.HubClient().Patch(ctx, gw, patch) Expect(err).ToNot(HaveOccurred()) Eventually(func(g Gomega, ctx SpecContext) { checkGateway := &gatewayapiv1.Gateway{} @@ -431,9 +432,10 @@ var _ = Describe("Gateway single target cluster", func() { if gw.Spec.Listeners == nil { gw.Spec.Listeners = []gatewayapiv1.Listener{} } + patch := client.MergeFrom(gw.DeepCopy()) AddListener("other", testHostnameOther, gatewayapiv1.ObjectName(testHostnameOther), gw) Eventually(func(g Gomega, ctx SpecContext) { - err = tconfig.HubClient().Update(ctx, gw) + err = tconfig.HubClient().Patch(ctx, gw, patch) Expect(err).ToNot(HaveOccurred()) checkGateway := &gatewayapiv1.Gateway{} err = tconfig.HubClient().Get(ctx, client.ObjectKey{Name: testID, Namespace: tconfig.HubNamespace()}, checkGateway) @@ -450,6 +452,7 @@ var _ = Describe("Gateway single target cluster", func() { // remove the listener err = tconfig.HubClient().Get(ctx, client.ObjectKey{Name: testID, Namespace: tconfig.HubNamespace()}, gw) Expect(err).ToNot(HaveOccurred()) + patch = client.MergeFrom(gw.DeepCopy()) if gw.Spec.Listeners == nil { gw.Spec.Listeners = []gatewayapiv1.Listener{} @@ -460,7 +463,7 @@ var _ = Describe("Gateway single target cluster", func() { gw.Spec.Listeners = append(gw.Spec.Listeners[:i], gw.Spec.Listeners[i+1:]...) } } - err = tconfig.HubClient().Update(ctx, gw) + err = tconfig.HubClient().Patch(ctx, gw, patch) Expect(err).ToNot(HaveOccurred()) Eventually(func(g Gomega, ctx SpecContext) { secret := &corev1.Secret{} diff --git a/test/policy_integration/dnspolicy_controller_multi_cluster_test.go b/test/policy_integration/dnspolicy_controller_multi_cluster_test.go index 7ecb9b0da..45d633b64 100644 --- a/test/policy_integration/dnspolicy_controller_multi_cluster_test.go +++ b/test/policy_integration/dnspolicy_controller_multi_cluster_test.go @@ -17,6 +17,7 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" + mgcgateway "github.com/Kuadrant/multicluster-gateway-controller/pkg/controllers/gateway" "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" testutil "github.com/Kuadrant/multicluster-gateway-controller/test/util" @@ -462,6 +463,194 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { }) + Context("custom geo+weighted", func() { + + BeforeEach(func() { + dnsPolicyBuilder. + WithLoadBalancingWeightedFor(120, []*v1alpha1.CustomWeight{ + { + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kuadrant.io/lb-attribute-custom-weight": "CAD", + }, + }, + Weight: 100, + }, + { + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kuadrant.io/lb-attribute-custom-weight": "ES", + }, + }, + Weight: 160, + }, + }). + WithLoadBalancingGeoFor("IE") + dnsPolicy = dnsPolicyBuilder.DNSPolicy + Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed()) + + Eventually(func() error { + gateway.Labels = map[string]string{} + gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameOne+"_lb-attribute-custom-weight"] = "CAD" + gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameTwo+"_lb-attribute-custom-weight"] = "ES" + gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameOne+"_lb-attribute-geo-code"] = "CAD" + gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameTwo+"_lb-attribute-geo-code"] = "ES" + return k8sClient.Update(ctx, gateway) + }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) + }) + + It("should create dns records", func() { + Eventually(func(g Gomega, ctx context.Context) { + recordList := &v1alpha1.DNSRecordList{} + err := k8sClient.List(ctx, recordList, &client.ListOptions{Namespace: testNamespace}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(recordList.Items).To(HaveLen(2)) + g.Expect(recordList.Items).To( + ContainElements( + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": HaveField("Name", recordName), + "Spec": MatchFields(IgnoreExtras, Fields{ + "ManagedZoneRef": HaveField("Name", "mz-example-com"), + "Endpoints": ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("2w705o.lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf(TestIPAddressTwo), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(v1alpha1.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("es.lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf("2w705o.lb-" + lbHash + ".test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("2w705o.lb-" + lbHash + ".test.example.com"), + "RecordTTL": Equal(v1alpha1.TTL(60)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "weight", Value: "160"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("cad.lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf("s07c46.lb-" + lbHash + ".test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("s07c46.lb-" + lbHash + ".test.example.com"), + "RecordTTL": Equal(v1alpha1.TTL(60)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "weight", Value: "100"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("s07c46.lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf(TestIPAddressOne), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(v1alpha1.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf("es.lb-" + lbHash + ".test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("ES"), + "RecordTTL": Equal(v1alpha1.TTL(300)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "ES"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf("cad.lb-" + lbHash + ".test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("CAD"), + "RecordTTL": Equal(v1alpha1.TTL(300)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "CAD"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf("cad.lb-" + lbHash + ".test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("default"), + "RecordTTL": Equal(v1alpha1.TTL(300)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "*"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(TestHostOne), + "Targets": ConsistOf("lb-" + lbHash + ".test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(v1alpha1.TTL(300)), + })), + ), + }), + }), + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": HaveField("Name", wildcardRecordName), + "Spec": MatchFields(IgnoreExtras, Fields{ + "ManagedZoneRef": HaveField("Name", "mz-example-com"), + "Endpoints": ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("2w705o.lb-" + lbHash + ".example.com"), + "Targets": ConsistOf(TestIPAddressTwo), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(v1alpha1.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("es.lb-" + lbHash + ".example.com"), + "Targets": ConsistOf("2w705o.lb-" + lbHash + ".example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("2w705o.lb-" + lbHash + ".example.com"), + "RecordTTL": Equal(v1alpha1.TTL(60)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "weight", Value: "160"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("cad.lb-" + lbHash + ".example.com"), + "Targets": ConsistOf("s07c46.lb-" + lbHash + ".example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("s07c46.lb-" + lbHash + ".example.com"), + "RecordTTL": Equal(v1alpha1.TTL(60)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "weight", Value: "100"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("s07c46.lb-" + lbHash + ".example.com"), + "Targets": ConsistOf(TestIPAddressOne), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(v1alpha1.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("lb-" + lbHash + ".example.com"), + "Targets": ConsistOf("cad.lb-" + lbHash + ".example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("CAD"), + "RecordTTL": Equal(v1alpha1.TTL(300)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "CAD"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("lb-" + lbHash + ".example.com"), + "Targets": ConsistOf("es.lb-" + lbHash + ".example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("ES"), + "RecordTTL": Equal(v1alpha1.TTL(300)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "ES"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("lb-" + lbHash + ".example.com"), + "Targets": ConsistOf("cad.lb-" + lbHash + ".example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal("default"), + "RecordTTL": Equal(v1alpha1.TTL(300)), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "*"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(TestHostWildcard), + "Targets": ConsistOf("lb-" + lbHash + ".example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(v1alpha1.TTL(300)), + })), + ), + }), + }), + )) + }, TestTimeoutMedium, TestRetryIntervalMedium, ctx).Should(Succeed()) + }) + + }) + }) }) From 18461559e26a07e420c2a140c1d6c6857ba5198b Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Thu, 18 Jan 2024 16:21:32 +0000 Subject: [PATCH 4/4] Move ClusterEventMapper back into gateway package. No longer relevant to policies, maps cluster events to gateways. Use "clusters.kuadrant.io" for all cluster labels on the target gateway and remove all existing cluster labels during reconciliation in order to remove labels removed from the cluster resource. Ensure only cluster labels for the current cluster are added when processing labels. --- .../cluster_eventmapper.go | 6 +-- pkg/controllers/gateway/gateway_controller.go | 22 +++++---- pkg/utils/gateway_wrapper.go | 22 +++++---- pkg/utils/gateway_wrapper_test.go | 48 ++++++++++--------- ...dnspolicy_controller_multi_cluster_test.go | 40 +++++++++------- 5 files changed, 78 insertions(+), 60 deletions(-) rename pkg/controllers/{events => gateway}/cluster_eventmapper.go (93%) diff --git a/pkg/controllers/events/cluster_eventmapper.go b/pkg/controllers/gateway/cluster_eventmapper.go similarity index 93% rename from pkg/controllers/events/cluster_eventmapper.go rename to pkg/controllers/gateway/cluster_eventmapper.go index ca525f8de..a6abe702d 100644 --- a/pkg/controllers/events/cluster_eventmapper.go +++ b/pkg/controllers/gateway/cluster_eventmapper.go @@ -1,4 +1,4 @@ -package events +package gateway import ( "context" @@ -50,13 +50,13 @@ func (m *ClusterEventMapper) mapToGatewayRequest(ctx context.Context, obj client allGwList := &gatewayapiv1.GatewayList{} err := m.Client.List(ctx, allGwList) if err != nil { - logger.Info("mapToPolicyRequest:", "error", "failed to get gateways") + logger.Info("mapToGatewayRequest:", "error", "failed to get gateways") return []reconcile.Request{} } requests := make([]reconcile.Request, 0) for _, gw := range allGwList.Items { - val := metadata.GetAnnotation(&gw, "kuadrant.io/gateway-clusters") + val := metadata.GetAnnotation(&gw, GatewayClustersAnnotation) if val == "" { continue } diff --git a/pkg/controllers/gateway/gateway_controller.go b/pkg/controllers/gateway/gateway_controller.go index c7686fb6b..b8a485fa5 100644 --- a/pkg/controllers/gateway/gateway_controller.go +++ b/pkg/controllers/gateway/gateway_controller.go @@ -51,13 +51,13 @@ import ( "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/gracePeriod" "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/metadata" "github.com/Kuadrant/multicluster-gateway-controller/pkg/_internal/slice" - "github.com/Kuadrant/multicluster-gateway-controller/pkg/controllers/events" "github.com/Kuadrant/multicluster-gateway-controller/pkg/policysync" "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" ) const ( LabelPrefix = "kuadrant.io/" + ClustersLabelPrefix = "clusters." + LabelPrefix GatewayClusterLabelSelectorAnnotation = LabelPrefix + "gateway-cluster-label-selector" GatewayClustersAnnotation = LabelPrefix + "gateway-clusters" GatewayFinalizer = LabelPrefix + "gateway" @@ -276,6 +276,14 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // reconcileClusterLabels fetches labels from ManagedCluster related to clusters array and adds them to the provided Gateway func (r *GatewayReconciler) reconcileClusterLabels(ctx context.Context, gateway *gatewayapiv1.Gateway, clusters []string) error { + //Remove all existing clusters.kuadrant.io labels + for key := range gateway.Labels { + if strings.HasPrefix(key, ClustersLabelPrefix) { + delete(gateway.Labels, key) + } + } + + //Add clusters.kuadrant.io labels for current clusters for _, cluster := range clusters { managedCluster := &clusterv1.ManagedCluster{} if err := r.Client.Get(ctx, client.ObjectKey{Name: cluster}, managedCluster); client.IgnoreNotFound(err) != nil { @@ -283,13 +291,11 @@ func (r *GatewayReconciler) reconcileClusterLabels(ctx context.Context, gateway } for key, value := range managedCluster.Labels { - if strings.Contains(key, LabelPrefix) { - _, attribute, found := strings.Cut(key, "/") - if !found { - continue - } - gateway.Labels[LabelPrefix+cluster+"_"+attribute] = value + attribute, found := strings.CutPrefix(key, LabelPrefix) + if !found { + continue } + gateway.Labels[ClustersLabelPrefix+cluster+"_"+attribute] = value } } return nil @@ -506,7 +512,7 @@ func buildAcceptedCondition(generation int64, acceptedStatus metav1.ConditionSta // SetupWithManager sets up the controller with the Manager. func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager, ctx context.Context) error { log := crlog.FromContext(ctx) - clusterEventMapper := events.NewClusterEventMapper(log, mgr.GetClient()) + clusterEventMapper := NewClusterEventMapper(log, mgr.GetClient()) //TODO need to trigger gateway reconcile when gatewayclass params changes return ctrl.NewControllerManagedBy(mgr). For(&gatewayapiv1.Gateway{}). diff --git a/pkg/utils/gateway_wrapper.go b/pkg/utils/gateway_wrapper.go index 619f9034f..3094050b3 100644 --- a/pkg/utils/gateway_wrapper.go +++ b/pkg/utils/gateway_wrapper.go @@ -9,8 +9,10 @@ import ( ) const ( - MultiClusterIPAddressType gatewayapiv1.AddressType = "kuadrant.io/MultiClusterIPAddress" - MultiClusterHostnameAddressType gatewayapiv1.AddressType = "kuadrant.io/MultiClusterHostnameAddress" + LabelPrefix = "kuadrant.io/" + ClustersLabelPrefix = "clusters." + LabelPrefix + MultiClusterIPAddressType gatewayapiv1.AddressType = LabelPrefix + "MultiClusterIPAddress" + MultiClusterHostnameAddressType gatewayapiv1.AddressType = LabelPrefix + "MultiClusterHostnameAddress" ) type GatewayWrapper struct { @@ -92,14 +94,16 @@ func (g *GatewayWrapper) GetClusterGatewayLabels(clusterName string) map[string] labels := map[string]string{} for k, v := range g.GetLabels() { - attr, found := strings.CutPrefix(k, "kuadrant.io/"+clusterName+"_") - if found { - labels["kuadrant.io/"+attr] = v - } else { - // Only add a label if we haven't already found a cluster specific version of it - if _, ok := labels[k]; !ok { - labels[k] = v + if strings.HasPrefix(k, ClustersLabelPrefix) { + attr, found := strings.CutPrefix(k, ClustersLabelPrefix+clusterName+"_") + if found { + labels[LabelPrefix+attr] = v } + continue + } + // Only add a label if we haven't already found a cluster specific version of it + if _, ok := labels[k]; !ok { + labels[k] = v } } return labels diff --git a/pkg/utils/gateway_wrapper_test.go b/pkg/utils/gateway_wrapper_test.go index 49407f0ab..77c6f2480 100644 --- a/pkg/utils/gateway_wrapper_test.go +++ b/pkg/utils/gateway_wrapper_test.go @@ -241,6 +241,7 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { Labels: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/foo": "bar", "foo": "bar", }, }, @@ -249,6 +250,7 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { want: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/foo": "bar", "foo": "bar", }, }, @@ -260,6 +262,7 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { Labels: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/foo": "bar", "foo": "bar", }, }, @@ -280,6 +283,7 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { want: map[string]string{ "kuadrant.io/lb-attribute-weight": "TSTATTR", "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/foo": "bar", "foo": "bar", }, }, @@ -289,11 +293,12 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { ObjectMeta: v1.ObjectMeta{ Name: "testgw", Labels: map[string]string{ - "kuadrant.io/kind-mgc-control-plane_lb-attribute-weight": "TSTATTR", - "kuadrant.io/kind-mgc-control-plane_lb-attribute-geo-code": "EU", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", - "foo": "bar", + "clusters.kuadrant.io/kind-mgc-control-plane_lb-attribute-weight": "TSTATTR", + "clusters.kuadrant.io/kind-mgc-control-plane_lb-attribute-geo-code": "EU", + "clusters.kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", + "clusters.kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", + "kuadrant.io/foo": "bar", + "foo": "bar", }, }, Status: gatewayapiv1.GatewayStatus{ @@ -311,11 +316,10 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { }, clusterName: "kind-mgc-control-plane", want: map[string]string{ - "kuadrant.io/lb-attribute-weight": "TSTATTR", - "kuadrant.io/lb-attribute-geo-code": "EU", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", - "foo": "bar", + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/foo": "bar", + "foo": "bar", }, }, { @@ -324,13 +328,14 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { ObjectMeta: v1.ObjectMeta{ Name: "testgw", Labels: map[string]string{ - "kuadrant.io/kind-mgc-control-plane_lb-attribute-weight": "TSTATTR", - "kuadrant.io/kind-mgc-control-plane_lb-attribute-geo-code": "EU", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", - "kuadrant.io/lb-attribute-weight": "TSTATTR3", - "kuadrant.io/lb-attribute-geo-code": "ES", - "foo": "bar", + "clusters.kuadrant.io/kind-mgc-control-plane_lb-attribute-weight": "TSTATTR", + "clusters.kuadrant.io/kind-mgc-control-plane_lb-attribute-geo-code": "EU", + "clusters.kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", + "clusters.kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", + "kuadrant.io/lb-attribute-weight": "TSTATTR3", + "kuadrant.io/lb-attribute-geo-code": "ES", + "kuadrant.io/foo": "bar", + "foo": "bar", }, }, Status: gatewayapiv1.GatewayStatus{ @@ -348,11 +353,10 @@ func TestGatewayWrapper_GetClusterGatewayLabels(t *testing.T) { }, clusterName: "kind-mgc-control-plane", want: map[string]string{ - "kuadrant.io/lb-attribute-weight": "TSTATTR", - "kuadrant.io/lb-attribute-geo-code": "EU", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-weight": "TSTATTR2", - "kuadrant.io/kind-mgc-workload-1_lb-attribute-geo-code": "US", - "foo": "bar", + "kuadrant.io/lb-attribute-weight": "TSTATTR", + "kuadrant.io/lb-attribute-geo-code": "EU", + "kuadrant.io/foo": "bar", + "foo": "bar", }, }, } diff --git a/test/policy_integration/dnspolicy_controller_multi_cluster_test.go b/test/policy_integration/dnspolicy_controller_multi_cluster_test.go index 45d633b64..2dd61d3d5 100644 --- a/test/policy_integration/dnspolicy_controller_multi_cluster_test.go +++ b/test/policy_integration/dnspolicy_controller_multi_cluster_test.go @@ -17,7 +17,6 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" - mgcgateway "github.com/Kuadrant/multicluster-gateway-controller/pkg/controllers/gateway" "github.com/Kuadrant/multicluster-gateway-controller/pkg/dns" "github.com/Kuadrant/multicluster-gateway-controller/pkg/utils" testutil "github.com/Kuadrant/multicluster-gateway-controller/test/util" @@ -463,7 +462,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { }) - Context("custom geo+weighted", func() { + Context("geo+weighted with custom weights", func() { BeforeEach(func() { dnsPolicyBuilder. @@ -471,7 +470,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { { Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "kuadrant.io/lb-attribute-custom-weight": "CAD", + "kuadrant.io/my-custom-weight-attr": "FOO", }, }, Weight: 100, @@ -479,7 +478,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { { Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "kuadrant.io/lb-attribute-custom-weight": "ES", + "kuadrant.io/my-custom-weight-attr": "BAR", }, }, Weight: 160, @@ -491,12 +490,17 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { Eventually(func() error { gateway.Labels = map[string]string{} - gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameOne+"_lb-attribute-custom-weight"] = "CAD" - gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameTwo+"_lb-attribute-custom-weight"] = "ES" - gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameOne+"_lb-attribute-geo-code"] = "CAD" - gateway.Labels[mgcgateway.LabelPrefix+TestClusterNameTwo+"_lb-attribute-geo-code"] = "ES" + gateway.Labels["clusters.kuadrant.io/"+TestClusterNameOne+"_my-custom-weight-attr"] = "FOO" + gateway.Labels["clusters.kuadrant.io/"+TestClusterNameTwo+"_my-custom-weight-attr"] = "BAR" + gateway.Labels["clusters.kuadrant.io/"+TestClusterNameOne+"_lb-attribute-geo-code"] = "IE" + gateway.Labels["clusters.kuadrant.io/"+TestClusterNameTwo+"_lb-attribute-geo-code"] = "ES" return k8sClient.Update(ctx, gateway) }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) + + Expect(gateway.Labels).To(HaveKeyWithValue("clusters.kuadrant.io/test-placed-control_my-custom-weight-attr", "FOO")) + Expect(gateway.Labels).To(HaveKeyWithValue("clusters.kuadrant.io/test-placed-control_lb-attribute-geo-code", "IE")) + Expect(gateway.Labels).To(HaveKeyWithValue("clusters.kuadrant.io/test-placed-workload-1_my-custom-weight-attr", "BAR")) + Expect(gateway.Labels).To(HaveKeyWithValue("clusters.kuadrant.io/test-placed-workload-1_lb-attribute-geo-code", "ES")) }) It("should create dns records", func() { @@ -528,7 +532,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "weight", Value: "160"}}), })), PointTo(MatchFields(IgnoreExtras, Fields{ - "DNSName": Equal("cad.lb-" + lbHash + ".test.example.com"), + "DNSName": Equal("ie.lb-" + lbHash + ".test.example.com"), "Targets": ConsistOf("s07c46.lb-" + lbHash + ".test.example.com"), "RecordType": Equal("CNAME"), "SetIdentifier": Equal("s07c46.lb-" + lbHash + ".test.example.com"), @@ -552,15 +556,15 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("lb-" + lbHash + ".test.example.com"), - "Targets": ConsistOf("cad.lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf("ie.lb-" + lbHash + ".test.example.com"), "RecordType": Equal("CNAME"), - "SetIdentifier": Equal("CAD"), + "SetIdentifier": Equal("IE"), "RecordTTL": Equal(v1alpha1.TTL(300)), - "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "CAD"}}), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "IE"}}), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("lb-" + lbHash + ".test.example.com"), - "Targets": ConsistOf("cad.lb-" + lbHash + ".test.example.com"), + "Targets": ConsistOf("ie.lb-" + lbHash + ".test.example.com"), "RecordType": Equal("CNAME"), "SetIdentifier": Equal("default"), "RecordTTL": Equal(v1alpha1.TTL(300)), @@ -597,7 +601,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "weight", Value: "160"}}), })), PointTo(MatchFields(IgnoreExtras, Fields{ - "DNSName": Equal("cad.lb-" + lbHash + ".example.com"), + "DNSName": Equal("ie.lb-" + lbHash + ".example.com"), "Targets": ConsistOf("s07c46.lb-" + lbHash + ".example.com"), "RecordType": Equal("CNAME"), "SetIdentifier": Equal("s07c46.lb-" + lbHash + ".example.com"), @@ -613,11 +617,11 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("lb-" + lbHash + ".example.com"), - "Targets": ConsistOf("cad.lb-" + lbHash + ".example.com"), + "Targets": ConsistOf("ie.lb-" + lbHash + ".example.com"), "RecordType": Equal("CNAME"), - "SetIdentifier": Equal("CAD"), + "SetIdentifier": Equal("IE"), "RecordTTL": Equal(v1alpha1.TTL(300)), - "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "CAD"}}), + "ProviderSpecific": Equal(v1alpha1.ProviderSpecific{{Name: "geo-code", Value: "IE"}}), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("lb-" + lbHash + ".example.com"), @@ -629,7 +633,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("lb-" + lbHash + ".example.com"), - "Targets": ConsistOf("cad.lb-" + lbHash + ".example.com"), + "Targets": ConsistOf("ie.lb-" + lbHash + ".example.com"), "RecordType": Equal("CNAME"), "SetIdentifier": Equal("default"), "RecordTTL": Equal(v1alpha1.TTL(300)),