clustermesh: remove GlobalServiceCache in cilium-agent

This remove the usage of GlobalServiceCache in the agent which was only useful
to count the number of global Service. This count didn't accounted the local
cluster and thus is misleading. While performance impact was not tested this
removes managing two level of nested maps and a global lock on each
remote endpoints updates which should certainly be valuable.

The global services count reported through cilium-dbg and the CLI
is no longer supported/exposed. Users with an older version of the CLI would
always see a count of 0 reported. Global Service counts will continue to be
reported per cluster along the count with other resources though.

Signed-off-by: Arthur Outhenin-Chalandre <git@mrfreezeex.fr>

Author: MrFreezeex

Documentation/operations/troubleshooting_clustermesh.rst

@@ -73,7 +73,7 @@ you may perform the following steps to troubleshoot ClusterMesh issues.
 
  #. Validate that ClusterMesh is healthy running ``cilium-dbg status --all-clusters`` inside each Cilium agent::
 
-        ClusterMesh:   1/1 remote clusters ready, 10 global-services
+        ClusterMesh:   1/1 remote clusters ready
            k8s-c2: ready, 3 nodes, 25 endpoints, 8 identities, 10 services, 0 MCS-API service exports, 0 reconnections (last: never)
            └  etcd: 1/1 connected, leases=0, lock lease-ID=7c028201b53de662, has-quorum=true: https://k8s-c2.mesh.cilium.io:2379 - 3.5.4 (Leader)
            └  remote configuration: expected=true, retrieved=true, cluster-id=3, kvstoremesh=false, sync-canaries=true, service-exports=disabled

Documentation/operations/upgrade.rst

@@ -305,7 +305,8 @@ communicating via the proxy must reconnect to re-establish connections.
   to update your network policies.
 * Kafka Network Policy support is deprecated and will be removed in Cilium v1.20.
 * Hubble field mask support was stabilized. In the Observer gRPC API, ``GetFlowsRequest.Experimental.field_mask`` was removed in favor of ``GetFlowsRequest.field_mask``. In the Hubble CLI, the ``--experimental-field-mask`` has been renamed to ``--field-mask`` and ``--experimental-use-default-field-mask`` renamed to ``-use-default-field-mask`` (now ``true`` by default).
-
+* Cilium-agent ClusterMesh status will no longer report the global services count. When using the CLI
+  with a version lower than 1.19, the global services count will be reported as 0.
 * ``enable-remote-node-masquerade`` config option is introduced.
   To masquerade traffic to remote nodes in BPF masquerading mode,
   use the option ``enable-remote-node-masquerade: "true"``.

api/v1/models/cluster_mesh_status.go

@@ -2339,9 +2339,6 @@ definitions:
         type: array
         items:
           "$ref": "#/definitions/RemoteCluster"
-      num-global-services:
-        description: Number of global services
-        type: integer
   RemoteCluster:
     description: |-
       Status of remote cluster

api/v1/openapi.yaml

@@ -682,15 +682,6 @@ func remoteClusterStatusToError(status *models.RemoteCluster) error {
 }
 
 func (c *ConnectivityStatus) parseAgentStatus(name string, expected []string, s *status.ClusterMeshAgentConnectivityStatus) {
-	if c.GlobalServices.Min < 0 || c.GlobalServices.Min > s.GlobalServices {
-		c.GlobalServices.Min = s.GlobalServices
-	}
-
-	if c.GlobalServices.Max < s.GlobalServices {
-		c.GlobalServices.Max = s.GlobalServices
-	}
-
-	c.GlobalServices.Avg += float64(s.GlobalServices)
 	c.Total++
 
 	ready := int64(0)

api/v1/server/embedded_spec.go

@@ -92,9 +92,8 @@ func NewK8sStatusCollector(client k8sImplementation, params K8sStatusParameters)
 }
 
 type ClusterMeshAgentConnectivityStatus struct {
-	GlobalServices int64
-	Clusters       map[string]*models.RemoteCluster
-	Errors         ErrorCountMap
+	Clusters map[string]*models.RemoteCluster
+	Errors   ErrorCountMap
 }
 
 // ErrClusterMeshStatusNotAvailable is a sentinel.
@@ -117,7 +116,6 @@ func (k *K8sStatusCollector) ClusterMeshConnectivity(ctx context.Context, cilium
 		return nil, ErrClusterMeshStatusNotAvailable
 	}
 
-	c.GlobalServices = status.ClusterMesh.NumGlobalServices
 	for _, cluster := range status.ClusterMesh.Clusters {
 		c.Clusters[cluster.Name] = cluster
 	}

cilium-cli/clustermesh/clustermesh.go

@@ -436,8 +436,8 @@ func FormatStatusResponse(w io.Writer, sr *models.StatusResponse, sd StatusDetai
 	}
 
 	if sr.ClusterMesh != nil {
-		fmt.Fprintf(w, "ClusterMesh:\t%d/%d remote clusters ready, %d global-services\n",
-			NumReadyClusters(sr.ClusterMesh.Clusters), len(sr.ClusterMesh.Clusters), sr.ClusterMesh.NumGlobalServices)
+		fmt.Fprintf(w, "ClusterMesh:\t%d/%d remote clusters ready\n",
+			NumReadyClusters(sr.ClusterMesh.Clusters), len(sr.ClusterMesh.Clusters))
 
 		verbosity := RemoteClustersStatusNotReadyOnly
 		if sd.AllClusters {

cilium-cli/status/k8s.go

@@ -110,10 +110,6 @@ type ClusterMesh struct {
 	// common implements the common logic to connect to remote clusters.
 	common common.ClusterMesh
 
-	// globalServices is a list of all global services. The datastructure
-	// is protected by its own mutex inside the structure.
-	globalServices *common.GlobalServiceCache
-
 	// nodeName is the name of the local node. This is used for logging and metrics
 	nodeName string
 
@@ -134,12 +130,8 @@ func NewClusterMesh(lifecycle cell.Lifecycle, c Configuration) *ClusterMesh {
 
 	nodeName := nodeTypes.GetName()
 	cm := &ClusterMesh{
-		conf:     c,
-		nodeName: nodeName,
-		globalServices: common.NewGlobalServiceCache(
-			c.Logger,
-			c.Metrics.TotalGlobalServices.WithLabelValues(c.ClusterInfo.Name, nodeName),
-		),
+		conf:           c,
+		nodeName:       nodeName,
 		FeatureMetrics: c.FeatureMetrics,
 	}
 
@@ -170,6 +162,20 @@ func NewClusterMesh(lifecycle cell.Lifecycle, c Configuration) *ClusterMesh {
 	return cm
 }
 
+type clusterServiceObserver struct {
+	serviceMerger ServiceMerger
+}
+
+func (obs *clusterServiceObserver) OnUpdate(key store.Key) {
+	svc := &(key.(*serviceStore.ValidatingClusterService).ClusterService)
+	obs.serviceMerger.MergeExternalServiceUpdate(svc)
+}
+
+func (obs *clusterServiceObserver) OnDelete(key store.NamedKey) {
+	svc := &(key.(*serviceStore.ValidatingClusterService).ClusterService)
+	obs.serviceMerger.MergeExternalServiceDelete(svc)
+}
+
 func (cm *ClusterMesh) NewRemoteCluster(name string, status common.StatusFunc) common.RemoteCluster {
 	rc := &remoteCluster{
 		name:                     name,
@@ -203,12 +209,7 @@ func (cm *ClusterMesh) NewRemoteCluster(name string, status common.StatusFunc) c
 			serviceStore.NamespacedNameValidator(),
 			serviceStore.ClusterIDValidator(&rc.clusterID),
 		),
-		common.NewSharedServicesObserver(
-			rc.log,
-			cm.globalServices,
-			cm.conf.ServiceMerger.MergeExternalServiceUpdate,
-			cm.conf.ServiceMerger.MergeExternalServiceDelete,
-		),
+		&clusterServiceObserver{serviceMerger: cm.conf.ServiceMerger},
 		store.RWSWithOnSyncCallback(func(ctx context.Context) { close(rc.synced.services) }),
 	)
 
@@ -281,9 +282,7 @@ func (cm *ClusterMesh) synced(ctx context.Context, toWaitFn func(*remoteCluster)
 
 // Status returns the status of the ClusterMesh subsystem
 func (cm *ClusterMesh) Status() (status *models.ClusterMeshStatus) {
-	status = &models.ClusterMeshStatus{
-		NumGlobalServices: int64(cm.globalServices.Size()),
-	}
+	status = &models.ClusterMeshStatus{}
 
 	cm.common.ForEachRemoteCluster(func(rci common.RemoteCluster) error {
 		rc := rci.(*remoteCluster)

pkg/client/client.go

@@ -16,15 +16,13 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/cilium/cilium/pkg/clustermesh/common"
 	serviceStore "github.com/cilium/cilium/pkg/clustermesh/store"
 	"github.com/cilium/cilium/pkg/clustermesh/types"
 	"github.com/cilium/cilium/pkg/identity"
 	"github.com/cilium/cilium/pkg/identity/cache"
 	"github.com/cilium/cilium/pkg/ipcache"
 	"github.com/cilium/cilium/pkg/kvstore"
 	"github.com/cilium/cilium/pkg/kvstore/store"
-	"github.com/cilium/cilium/pkg/metrics"
 	nodeTypes "github.com/cilium/cilium/pkg/node/types"
 	"github.com/cilium/cilium/pkg/source"
 	"github.com/cilium/cilium/pkg/testutils"
@@ -166,7 +164,6 @@ func TestRemoteClusterRun(t *testing.T) {
 					FeatureMetrics:        NewClusterMeshMetricsNoop(),
 					Logger:                logger,
 				},
-				globalServices: common.NewGlobalServiceCache(logger, metrics.NoOpGauge),
 				FeatureMetrics: NewClusterMeshMetricsNoop(),
 			}
 			rc := cm.NewRemoteCluster("foo", nil).(*remoteCluster)
@@ -297,7 +294,6 @@ func TestRemoteClusterClusterIDChange(t *testing.T) {
 			Logger:                logger,
 		},
 		FeatureMetrics: NewClusterMeshMetricsNoop(),
-		globalServices: common.NewGlobalServiceCache(logger, metrics.NoOpGauge),
 	}
 	rc := cm.NewRemoteCluster("foo", nil).(*remoteCluster)