From 094605f269c16281d606d015ac67cd33fc893755 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Thu, 18 Jan 2024 16:21:32 +0000 Subject: [PATCH] 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 | 9 ++-- 5 files changed, 60 insertions(+), 47 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 ca525f8d..a6abe702 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 c7686fb6..b8a485fa 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 619f9034..3094050b 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 49407f0a..77c6f248 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 45d633b6..899f2443 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" @@ -491,10 +490,10 @@ 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/"+TestClusterNameTwo+"_lb-attribute-custom-weight"] = "CAD" + gateway.Labels["clusters.kuadrant.io/"+TestClusterNameTwo+"_lb-attribute-custom-weight"] = "ES" + gateway.Labels["clusters.kuadrant.io/"+TestClusterNameOne+"_lb-attribute-geo-code"] = "CAD" + gateway.Labels["clusters.kuadrant.io/"+TestClusterNameTwo+"_lb-attribute-geo-code"] = "ES" return k8sClient.Update(ctx, gateway) }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) })