MESH-1787 Add debug logging for events and generation (istio-ecosyste…

…m#104) * MESH-1026 - Trigger pipeline * Revert "MESH-1709 support bluegreen rollout preview (istio-ecosystem#103)" This reverts commit 7d6991e. * Revert "MESH-1709 support bluegreen rollout preview (istio-ecosystem#101)" This reverts commit acca3f7. * MESH-1787 - Undo preview endpoint generation and add informer logs
itsLucario · Apr 11, 2022 · 93391ee · 93391ee
1 parent 7d6991e
commit 93391ee
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 529 deletions.
diff --git a/Makefile b/Makefile
@@ -161,7 +161,6 @@ gen-yaml:
 	kustomize build ./install/sample/overlays/rollout-bluegreen > ./out/yaml/sample-greeting-rollout-bluegreen.yaml
 	kustomize build ./install/sample/overlays/remote > ./out/yaml/remotecluster_sample.yaml
 	cp ./install/sample/sample_dep.yaml ./out/yaml/sample_dep.yaml
-	cp ./install/sample/greeting_preview.yaml ./out/yaml/greeting_preview.yaml
 	cp ./install/sample/gtp.yaml ./out/yaml/gtp.yaml
 	cp ./install/sample/gtp_failover.yaml ./out/yaml/gtp_failover.yaml
 	cp ./install/sample/gtp_topology.yaml ./out/yaml/gtp_topology.yaml

diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go
@@ -760,7 +760,7 @@ func copyEndpoint(e *v1alpha32.ServiceEntry_Endpoint) *v1alpha32.ServiceEntry_En
 
 // A rollout can use one of 2 stratergies :-
 // 1. Canary strategy - which can use a virtual service to manage the weights associated with a stable and canary service. Admiral created endpoints in service entries will use the weights assigned in the Virtual Service
-// 2. Blue green strategy- this contains 2 service instances in a namespace, an active service and a preview service. Admiral will use repective service to create active and preview endpoints
+// 2. Blue green strategy- this contains 2 service instances in a namespace, an active service and a preview service. Admiral will always use the active service
 func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[string]*WeightedService {
 
 	if rollout == nil {
@@ -788,12 +788,9 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin
 	var istioCanaryWeights = make(map[string]int32)
 
 	var blueGreenActiveService string
-	var blueGreenPreviewService string
-
 	if rolloutStrategy.BlueGreen != nil {
-		// If rollout uses blue green strategy
+		// If rollout uses blue green strategy, use the active service
 		blueGreenActiveService = rolloutStrategy.BlueGreen.ActiveService
-		blueGreenPreviewService = rolloutStrategy.BlueGreen.PreviewService
 	} else if rolloutStrategy.Canary != nil && rolloutStrategy.Canary.TrafficRouting != nil && rolloutStrategy.Canary.TrafficRouting.Istio != nil {
 		canaryService = rolloutStrategy.Canary.CanaryService
 		stableService = rolloutStrategy.Canary.StableService
@@ -866,7 +863,7 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin
 		var service = servicesInNamespace[s]
 		var match = true
 		//skip services that are not referenced in the rollout
-		if len(blueGreenActiveService) > 0 && service.ObjectMeta.Name != blueGreenActiveService && service.ObjectMeta.Name != blueGreenPreviewService {
+		if len(blueGreenActiveService) > 0 && service.ObjectMeta.Name != blueGreenActiveService {
 			log.Infof("Skipping service=%s for rollout=%s in namespace=%s and cluster=%s", service.Name, rollout.Name, rollout.Namespace, rc.ClusterID)
 			continue
 		}
@@ -887,11 +884,8 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin
 		if match {
 			ports := GetMeshPortsForRollout(rc.ClusterID, service, rollout)
 			if len(ports) > 0 {
-				// if the strategy is bluegreen return matched services
-				// else if using canary with NO istio traffic management, pick the first service that matches
-				if rolloutStrategy.BlueGreen != nil {
-					matchedServices[service.Name] = &WeightedService{Weight: 1, Service: service}
-				} else if len(istioCanaryWeights) == 0 {
+				//if the strategy is bluegreen or using canary with NO istio traffic management, pick the first service that matches
+				if len(istioCanaryWeights) == 0 {
 					matchedServices[service.Name] = &WeightedService{Weight: 1, Service: service}
 					break
 				}

diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go
@@ -3,15 +3,14 @@ package clusters
 import (
 	"errors"
 	"fmt"
+	v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
 	"math"
 	"math/rand"
 	"reflect"
 	"strconv"
 	"strings"
 	"time"
 
-	v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
-
 	argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
@@ -62,7 +61,6 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s
 	var serviceEntries = make(map[string]*networking.ServiceEntry)
 
 	var cname string
-	cnames := make(map[string]string)
 	var serviceInstance *k8sV1.Service
 	var weightedServices map[string]*WeightedService
 	var rollout *admiral.RolloutClusterEntry
@@ -113,7 +111,6 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s
 			localMeshPorts := GetMeshPortsForRollout(rc.ClusterID, serviceInstance, rolloutInstance)
 
 			cname = common.GetCnameForRollout(rolloutInstance, common.GetWorkloadIdentifier(), common.GetHostnameSuffix())
-			cnames[cname] = "1"
 			sourceRollouts[rc.ClusterID] = rolloutInstance
 			createServiceEntryForRollout(event, rc, remoteRegistry.AdmiralCache, localMeshPorts, rolloutInstance, serviceEntries)
 		} else {
@@ -141,12 +138,6 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s
 		localFqdn := serviceInstance.Name + common.Sep + serviceInstance.Namespace + common.DotLocalDomainSuffix
 		rc := remoteRegistry.RemoteControllers[sourceCluster]
 		var meshPorts map[string]uint32
-		isBlueGreenStrategy := false
-
-		if len(sourceRollouts) > 0 {
-			isBlueGreenStrategy = sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen != nil
-		}
-
 		if len(sourceDeployments) > 0 {
 			meshPorts = GetMeshPorts(sourceCluster, serviceInstance, sourceDeployments[sourceCluster])
 		} else {
@@ -162,18 +153,8 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s
 			for _, ep := range serviceEntry.Endpoints {
 				//replace istio ingress-gateway address with local fqdn, note that ingress-gateway can be empty (not provisoned, or is not up)
 				if ep.Address == clusterIngress || ep.Address == "" {
-					// Update endpoints with locafqdn for active and preview se of bluegreen rollout
-					if isBlueGreenStrategy {
-						oldPorts := ep.Ports
-						updateEndpointsForBlueGreen(sourceRollouts[sourceCluster], weightedServices, cnames, ep, sourceCluster, key)
-
-						AddServiceEntriesWithDr(remoteRegistry.AdmiralCache, map[string]string{sourceCluster: sourceCluster}, remoteRegistry.RemoteControllers,
-							map[string]*networking.ServiceEntry{key: serviceEntry})
-						//swap it back to use for next iteration
-						ep.Address = clusterIngress
-						ep.Ports = oldPorts
-						// see if we have weighted services (rollouts with canary strategy)
-					} else if len(weightedServices) > 1 {
+					// see if we have weighted services (rollouts with canary strategy)
+					if len(weightedServices) > 1 {
 						//add one endpoint per each service, may be modify
 						var se = copyServiceEntry(serviceEntry)
 						updateEndpointsForWeightedServices(se, weightedServices, clusterIngress, meshPorts)
@@ -194,7 +175,7 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s
 		}
 
 		for _, val := range dependents {
-			remoteRegistry.AdmiralCache.DependencyNamespaceCache.Put(val, serviceInstance.Namespace, localFqdn, cnames)
+			remoteRegistry.AdmiralCache.DependencyNamespaceCache.Put(val, serviceInstance.Namespace, localFqdn, map[string]string{cname: "1"})
 		}
 
 		if common.GetWorkloadSidecarUpdate() == "enabled" {
@@ -204,26 +185,6 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s
 	return serviceEntries
 }
 
-func updateEndpointsForBlueGreen(rollout *argo.Rollout, weightedServices map[string]*WeightedService, cnames map[string]string,
-	ep *networking.ServiceEntry_Endpoint, sourceCluster string, meshHost string) {
-	activeServiceName := rollout.Spec.Strategy.BlueGreen.ActiveService
-	previewServiceName := rollout.Spec.Strategy.BlueGreen.PreviewService
-
-	if previewService, ok := weightedServices[previewServiceName]; strings.HasPrefix(meshHost, common.BlueGreenRolloutPreviewPrefix+common.Sep) && ok {
-		previewServiceInstance := previewService.Service
-		localFqdn := previewServiceInstance.Name + common.Sep + previewServiceInstance.Namespace + common.DotLocalDomainSuffix
-		cnames[localFqdn] = "1"
-		ep.Address = localFqdn
-		ep.Ports = GetMeshPortsForRollout(sourceCluster, previewServiceInstance, rollout)
-	} else if activeService, ok := weightedServices[activeServiceName]; ok {
-		activeServiceInstance := activeService.Service
-		localFqdn := activeServiceInstance.Name + common.Sep + activeServiceInstance.Namespace + common.DotLocalDomainSuffix
-		cnames[localFqdn] = "1"
-		ep.Address = localFqdn
-		ep.Ports = GetMeshPortsForRollout(sourceCluster, activeServiceInstance, rollout)
-	}
-}
-
 //update endpoints for Argo rollouts specific Service Entries to account for traffic splitting (Canary strategy)
 func updateEndpointsForWeightedServices(serviceEntry *networking.ServiceEntry, weightedServices map[string]*WeightedService, clusterIngress string, meshPorts map[string]uint32) {
 	var endpoints = make([]*networking.ServiceEntry_Endpoint, 0)
@@ -623,17 +584,6 @@ func createServiceEntryForRollout(event admiral.EventType, rc *RemoteController,
 
 	san := getSanForRollout(destRollout, workloadIdentityKey)
 
-	if destRollout.Spec.Strategy.BlueGreen != nil && destRollout.Spec.Strategy.BlueGreen.PreviewService != "" {
-		rolloutServices := getServiceForRollout(rc, destRollout)
-		if _, ok := rolloutServices[destRollout.Spec.Strategy.BlueGreen.PreviewService]; ok {
-			previewGlobalFqdn := common.BlueGreenRolloutPreviewPrefix + common.Sep + common.GetCnameForRollout(destRollout, workloadIdentityKey, common.GetHostnameSuffix())
-			previewAddress := getUniqueAddress(admiralCache, previewGlobalFqdn)
-			if len(previewGlobalFqdn) != 0 && len(previewAddress) != 0 {
-				generateServiceEntry(event, admiralCache, meshPorts, previewGlobalFqdn, rc, serviceEntries, previewAddress, san)
-			}
-		}
-	}
-
 	tmpSe := generateServiceEntry(event, admiralCache, meshPorts, globalFqdn, rc, serviceEntries, address, san)
 	return tmpSe
 }