updating the sidecar config with local clusters (#437)

updating the sidecar config with local clusters

Author: shriramsharma

admiral/cmd/admiral/cmd/root.go

@@ -286,6 +286,8 @@ func GetRootCmd(args []string) *cobra.Command {
 	rootCmd.PersistentFlags().StringToStringVarP(&params.VSRoutingInClusterDisabledResources, "vs_routing_in_cluster_disabled_resources", "d", map[string]string{}, "The source clusters and corresponding source identities to disable VS based routing in-cluster on")
 	rootCmd.PersistentFlags().BoolVar(&params.EnableCustomVSMerge, "enable_custom_vs_merge", false, "Enable/Disable custom VS merge with in cluster VS")
 	rootCmd.PersistentFlags().StringVar(&params.ProcessVSCreatedBy, "process_vs_created_by", "", "process the VS that was createdBy. Add createdBy label and value provided here for admiral to process this VS")
+	rootCmd.PersistentFlags().BoolVar(&params.EnableSidecarCaching, "enable_sidecar_caching", false, "Enable/Disable sidecar caching")
+	rootCmd.PersistentFlags().IntVar(&params.MaxSidecarEgressHostsLimitToCache, "max_sidecar_egress_hosts_limit_to_cache", 100, "This is the max sidecar egress hosts limit to cache. If the number of egress hosts exceeds this limit, then the sidecar will not be cached. This is to limit the memory consumption of the sidecar cache")
 
 	rootCmd.PersistentFlags().BoolVar(&params.EnableClientDiscovery, "enable_client_discovery", true, "Enable/Disable Client (mesh egress) Discovery")
 	rootCmd.PersistentFlags().StringSliceVar(&params.ClientDiscoveryClustersForJobs, "client_discovery_clusters_for_jobs", []string{}, "List of clusters for client discovery for k8s jobs")

admiral/pkg/clusters/serviceentry.go

@@ -913,7 +913,7 @@ func modifyServiceEntryForNewServiceOrPod(
 	// Writing phase: We update the base in-cluster virtualservices with the RouteDestinations
 	// gathered during the discovery phase and write them to the source cluster
 	err = addUpdateInClusterVirtualServices(
-		ctx, ctxLogger, remoteRegistry, sourceClusterToInClusterDestinations, cname, sourceIdentity, env)
+		ctx, ctxLogger, remoteRegistry, sourceClusterToInClusterDestinations, cname, sourceIdentity, env, sourceClusterToEventNsCache)
 	if err != nil {
 		ctxLogger.Errorf(common.CtxLogFormat, "addUpdateInClusterVirtualServices",
 			deploymentOrRolloutName, namespace, "", err)

admiral/pkg/clusters/virtualservice_routing.go

@@ -28,6 +28,14 @@ import (
 	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+type addUpdateSidecarFunc func(
+	ctxLogger *log.Entry,
+	ctx context.Context,
+	newSidecarConfig *v1alpha3.Sidecar,
+	cachedSidecar *v1alpha3.Sidecar,
+	clientNamespace string,
+	rc *RemoteController)
+
 type envCustomVSTuple struct {
 	env      string
 	customVS *v1alpha3.VirtualService
@@ -712,7 +720,8 @@ func addUpdateInClusterVirtualServices(
 	sourceClusterToDestinations map[string]map[string][]*vsrouting.RouteDestination,
 	cname string,
 	sourceIdentity string,
-	env string) error {
+	env string,
+	sourceClusterToEventNsCache map[string]string) error {
 
 	if sourceIdentity == "" {
 		return fmt.Errorf("identity is empty")
@@ -748,6 +757,17 @@ func addUpdateInClusterVirtualServices(
 			return err
 		}
 
+		// Update the client's default sidecar with cluster local services
+		// This is to make sure that the client proxies have the necessary .local clusters
+		// available in its config
+		err = updateClientSidecarWithClusterLocalServices(ctx,
+			ctxLogger, rc, virtualService, sourceCluster, sourceClusterToEventNsCache, addUpdateSidecar)
+		if err != nil {
+			ctxLogger.Errorf(common.CtxLogFormat, "addUpdateInClusterVirtualServices",
+				virtualService.Name, virtualService.Namespace, sourceCluster,
+				fmt.Sprintf("updateClientSidecarWithClusterLocalServices failed due to %v", err.Error()))
+		}
+
 		virtualServicesToBeProcessed := []*v1alpha3.VirtualService{virtualService}
 
 		// Merge the incluster vs with custom virtualservice, if enabled
@@ -838,6 +858,108 @@ func addUpdateInClusterVirtualServices(
 	return nil
 }
 
+func updateClientSidecarWithClusterLocalServices(
+	ctx context.Context,
+	ctxLogger *log.Entry,
+	rc *RemoteController,
+	vs *v1alpha3.VirtualService,
+	sourceCluster string,
+	sourceClusterToEventNsCache map[string]string,
+	addUpdateSidecar addUpdateSidecarFunc) error {
+
+	if !common.IsSidecarCachingEnabled() {
+		ctxLogger.Infof(common.CtxLogFormat, "updateClientSidecarWithClusterLocalServices",
+			vs.Name, vs.Namespace, sourceCluster,
+			"sidecar caching is disabled, skipping")
+		return nil
+	}
+
+	if rc == nil {
+		return fmt.Errorf("remoteController is nil")
+	}
+	if rc.SidecarController == nil {
+		return fmt.Errorf("sidecarController is nil")
+	}
+	if rc.SidecarController.SidecarCache == nil {
+		return fmt.Errorf("sidecarCache is nil")
+	}
+	if vs == nil {
+		return fmt.Errorf("virtualService is nil")
+	}
+	if sourceClusterToEventNsCache == nil {
+		return fmt.Errorf("sourceClusterToEventNsCache is nil")
+	}
+	identityNamespace := sourceClusterToEventNsCache[sourceCluster]
+	if identityNamespace == "" {
+		return fmt.Errorf("identityNamespace is empty for sourceCluster %s", sourceCluster)
+	}
+	newHostToAddToSidecarEgress := fmt.Sprintf("%s/*.svc.cluster.local", identityNamespace)
+
+	exportToNamespaces := vs.Spec.ExportTo
+	if exportToNamespaces == nil || len(exportToNamespaces) == 0 {
+		return fmt.Errorf("exportToNamespaces is nil or empty for virtualService %s", vs.Name)
+	}
+	clientNamepaces := make([]string, 0)
+	for _, namespace := range exportToNamespaces {
+		if namespace == common.GetSyncNamespace() {
+			ctxLogger.Infof(common.CtxLogFormat, "updateClientSidecarWithClusterLocalServices",
+				vs.Name, vs.Namespace, sourceCluster, "virtualservice contains sync namespace, skipping update")
+			return nil
+		}
+		// We skip the self namespace as the sidecar in the identity's namespace
+		// already has ./* in its egress host
+		if namespace == identityNamespace {
+			continue
+		}
+		clientNamepaces = append(clientNamepaces, namespace)
+	}
+	if len(clientNamepaces) == 0 {
+		ctxLogger.Infof(common.CtxLogFormat, "updateClientSidecarWithClusterLocalServices",
+			vs.Name, vs.Namespace, sourceCluster,
+			"no client namespaces found to update sidecar with cluster local services")
+		return nil
+	}
+
+	// For each client namespace, we will update the sidecar with the cluster local services
+	for _, clientNamespace := range clientNamepaces {
+		cachedSidecar := rc.SidecarController.SidecarCache.Get(common.GetWorkloadSidecarName(), clientNamespace)
+		if cachedSidecar == nil {
+			ctxLogger.Infof(common.CtxLogFormat, "updateClientSidecarWithClusterLocalServices",
+				vs.Name, vs.Namespace, sourceCluster,
+				fmt.Sprintf("skipped updating sidecar in namespace %s as it is missing in the cache", clientNamespace))
+			continue
+		}
+		if cachedSidecar.Spec.Egress == nil || len(cachedSidecar.Spec.Egress) == 0 {
+			ctxLogger.Infof(common.CtxLogFormat, "updateClientSidecarWithClusterLocalServices",
+				vs.Name, vs.Namespace, sourceCluster,
+				fmt.Sprintf("skipped updating sidecar in namespace %s as no egress found", clientNamespace))
+			continue
+		}
+		cachedSidecarEgressHosts := cachedSidecar.Spec.Egress[0].Hosts
+		if cachedSidecarEgressHosts == nil || len(cachedSidecarEgressHosts) == 0 {
+			ctxLogger.Infof(common.CtxLogFormat, "updateClientSidecarWithClusterLocalServices",
+				vs.Name, vs.Namespace, sourceCluster,
+				fmt.Sprintf("skipped updating sidecar in namespace %s as no egress hosts found", clientNamespace))
+			continue
+		}
+		lookup := make(map[string]bool)
+		for _, egressHost := range cachedSidecarEgressHosts {
+			lookup[egressHost] = true
+		}
+		if lookup[newHostToAddToSidecarEgress] {
+			continue
+		}
+		newSidecar := copySidecar(cachedSidecar)
+		newSidecar.Spec.Egress[0].Hosts = append(newSidecar.Spec.Egress[0].Hosts, newHostToAddToSidecarEgress)
+		newSidecarConfig := createSidecarSkeleton(newSidecar.Spec, common.GetWorkloadSidecarName(), clientNamespace)
+
+		addUpdateSidecar(ctxLogger, ctx, newSidecarConfig, cachedSidecar, clientNamespace, rc)
+	}
+
+	return nil
+
+}
+
 // shouldPerformDRPinning checks if the DR pinning to remote region is required
 // It checks if the identity is multi-region and if there is a GTP for the identity
 // If the identity is multi-region and has GTP in the NS, it returns true, else false
@@ -1961,7 +2083,8 @@ func addUpdateRoutingDestinationRule(
 			drName, util.IstioSystemNamespace, sourceCluster, "destinationrule created successfully")
 
 		rc.DestinationRuleController.Cache.Put(newDR)
-
+		log.Infof("op=%s type=%v cluster=%s length=%d",
+			"cacheLength", "DestinationRule", sourceCluster, rc.DestinationRuleController.Cache.Len())
 	}
 
 	return nil
@@ -2471,21 +2594,23 @@ func DoDRUpdateForInClusterVSRouting(
 			"", "", cluster, "remoteRegistry is nil")
 		return false
 	}
-	// Check if the incluster VS has valid exportTo namespaces (not sync namespace)
-	hasValidInClusterVS, err := hasInClusterVSWithValidExportToNS(se, remoteRegistry.GetRemoteController(cluster))
-	if err != nil {
-		ctxLogger.Warnf(common.CtxLogFormat, "DoDRUpdateForInClusterVSRouting",
-			identity, "", cluster, fmt.Sprintf("error checking for valid in-cluster VS %v", err))
-		return false
-	}
-	if !hasValidInClusterVS {
-		ctxLogger.Infof(common.CtxLogFormat, "DoDRUpdateForInClusterVSRouting",
-			identity, "", cluster, "skipping DR update as incluter VS does not have valid exportTo namespaces")
-		return false
-	}
-	if isSourceCluster &&
-		DoVSRoutingInClusterForClusterAndIdentity(ctx, ctxLogger, env, cluster, identity, remoteRegistry, performCartographerVSCheck) {
-		return true
+	if isSourceCluster {
+		// Check if the incluster VS has valid exportTo namespaces (not sync namespace)
+		hasValidInClusterVS, err := hasInClusterVSWithValidExportToNS(se, remoteRegistry.GetRemoteController(cluster))
+		if err != nil {
+			ctxLogger.Warnf(common.CtxLogFormat, "DoDRUpdateForInClusterVSRouting",
+				identity, "", cluster, fmt.Sprintf("error checking for valid in-cluster VS %v", err))
+			return false
+		}
+		if !hasValidInClusterVS {
+			ctxLogger.Infof(common.CtxLogFormat, "DoDRUpdateForInClusterVSRouting",
+				identity, "", cluster, "skipping DR update as incluter VS does not have valid exportTo namespaces")
+			return false
+		}
+		if DoVSRoutingInClusterForClusterAndIdentity(
+			ctx, ctxLogger, env, cluster, identity, remoteRegistry, performCartographerVSCheck) {
+			return true
+		}
 	}
 	return false
 }