From 162f689fd088ea6c391b318e04eb79a695b1f93a Mon Sep 17 00:00:00 2001 From: nbn01 Date: Wed, 16 Mar 2022 18:17:29 +0530 Subject: [PATCH 01/17] Changes to support preview endpoint for bluegreen rollout Signed-off-by: nbn01 --- admiral/cmd/admiral/cmd/root.go | 13 ++-- admiral/pkg/clusters/handler.go | 80 +++++++++++++------------ admiral/pkg/clusters/serviceentry.go | 51 ++++++++++++++-- admiral/pkg/controller/common/config.go | 7 ++- admiral/pkg/controller/common/types.go | 5 +- 5 files changed, 105 insertions(+), 51 deletions(-) diff --git a/admiral/cmd/admiral/cmd/root.go b/admiral/cmd/admiral/cmd/root.go index 1e9254053..70a1b7ed7 100644 --- a/admiral/cmd/admiral/cmd/root.go +++ b/admiral/cmd/admiral/cmd/root.go @@ -4,15 +4,16 @@ import ( "context" "flag" "fmt" + "os" + "os/signal" + "syscall" + "time" + "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/routes" "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/server" "github.com/istio-ecosystem/admiral/admiral/pkg/clusters" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" log "github.com/sirupsen/logrus" - "os" - "os/signal" - "syscall" - "time" "github.com/spf13/cobra" ) @@ -75,7 +76,7 @@ func GetRootCmd(args []string) *cobra.Command { fmt.Sprintf("Set log verbosity, defaults to 'Info'. Must be between %v and %v", int(log.PanicLevel), int(log.TraceLevel))) rootCmd.PersistentFlags().StringVar(¶ms.KubeconfigPath, "kube_config", "", "Use a Kubernetes configuration file instead of in-cluster configuration") - rootCmd.PersistentFlags().BoolVar(¶ms.ArgoRolloutsEnabled, "argo_rollouts", false, + rootCmd.PersistentFlags().BoolVar(¶ms.ArgoRolloutsEnabled, "argo_rollouts", true, "Use argo rollout configurations") rootCmd.PersistentFlags().StringVar(¶ms.ClusterRegistriesNamespace, "secret_namespace", "admiral", "Namespace to monitor for secrets defaults to admiral-secrets") @@ -103,6 +104,8 @@ func GetRootCmd(args []string) *cobra.Command { "The label value, on a namespace, which tells Istio to perform sidecar injection") rootCmd.PersistentFlags().StringVar(¶ms.HostnameSuffix, "hostname_suffix", "global", "The hostname suffix to customize the cname generated by admiral. Default suffix value will be \"global\"") + rootCmd.PersistentFlags().StringVar(¶ms.PreviewHostnamePrefix, "preview_hostname_prefix", "preview", + "The hostname prefix to customize the cname generated by admiral for bluegreen rollout preview service. Default suffix value will be \"preview\"") rootCmd.PersistentFlags().StringVar(¶ms.LabelSet.WorkloadIdentityKey, "workload_identity_key", "identity", "The workload identity key, on deployment which holds identity value used to generate cname by admiral. Default label key will be \"identity\" Admiral will look for a label with this key. If present, that will be used. If not, it will try an annotation (for use cases where an identity is longer than 63 chars)") rootCmd.PersistentFlags().StringVar(¶ms.LabelSet.GlobalTrafficDeploymentLabel, "globaltraffic_deployment_label", "identity", diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index 158eff1f5..4e046ee45 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -3,10 +3,15 @@ package clusters import ( "bytes" "fmt" + "reflect" + "sort" + "strings" + "time" + argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" "github.com/gogo/protobuf/types" "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model" - "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" + v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/util" @@ -16,10 +21,6 @@ import ( k8sAppsV1 "k8s.io/api/apps/v1" k8sV1 "k8s.io/api/core/v1" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" - "reflect" - "sort" - "strings" - "time" ) const ROLLOUT_POD_HASH_LABEL string = "rollouts-pod-template-hash" @@ -45,8 +46,8 @@ type SidecarHandler struct { } type WeightedService struct { - Weight int32 - Service *k8sV1.Service + Weight int32 + Service *k8sV1.Service } func updateIdentityDependencyCache(sourceIdentity string, identityDependencyCache *common.MapOfMaps, dr *v1.Dependency) { @@ -70,9 +71,9 @@ func getDestinationRule(host string, locality string, gtpTrafficPolicy *model.Tr processGtp = false } outlierDetection := &v1alpha32.OutlierDetection{ - BaseEjectionTime: &types.Duration{Seconds: 300}, + BaseEjectionTime: &types.Duration{Seconds: 300}, Consecutive_5XxErrors: &types.UInt32Value{Value: uint32(10)}, - Interval: &types.Duration{Seconds: 60}, + Interval: &types.Duration{Seconds: 60}, } if gtpTrafficPolicy != nil && processGtp { var loadBalancerSettings = &v1alpha32.LoadBalancerSettings{ @@ -108,28 +109,28 @@ func getDestinationRule(host string, locality string, gtpTrafficPolicy *model.Tr func (se *ServiceEntryHandler) Added(obj *v1alpha3.ServiceEntry) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Add", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Add", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } } func (se *ServiceEntryHandler) Updated(obj *v1alpha3.ServiceEntry) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Update", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Update", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } } func (se *ServiceEntryHandler) Deleted(obj *v1alpha3.ServiceEntry) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Delete", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Delete", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } } func (dh *DestinationRuleHandler) Added(obj *v1alpha3.DestinationRule) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Add", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Add", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } handleDestinationRuleEvent(obj, dh, common.Add, common.DestinationRule) @@ -137,7 +138,7 @@ func (dh *DestinationRuleHandler) Added(obj *v1alpha3.DestinationRule) { func (dh *DestinationRuleHandler) Updated(obj *v1alpha3.DestinationRule) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Update", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Update", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } handleDestinationRuleEvent(obj, dh, common.Update, common.DestinationRule) @@ -145,7 +146,7 @@ func (dh *DestinationRuleHandler) Updated(obj *v1alpha3.DestinationRule) { func (dh *DestinationRuleHandler) Deleted(obj *v1alpha3.DestinationRule) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Delete", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Delete", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } handleDestinationRuleEvent(obj, dh, common.Delete, common.DestinationRule) @@ -153,7 +154,7 @@ func (dh *DestinationRuleHandler) Deleted(obj *v1alpha3.DestinationRule) { func (vh *VirtualServiceHandler) Added(obj *v1alpha3.VirtualService) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Add", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Add", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } err := handleVirtualServiceEvent(obj, vh, common.Add, common.VirtualService) @@ -164,7 +165,7 @@ func (vh *VirtualServiceHandler) Added(obj *v1alpha3.VirtualService) { func (vh *VirtualServiceHandler) Updated(obj *v1alpha3.VirtualService) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Update", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Update", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } err := handleVirtualServiceEvent(obj, vh, common.Update, common.VirtualService) @@ -175,7 +176,7 @@ func (vh *VirtualServiceHandler) Updated(obj *v1alpha3.VirtualService) { func (vh *VirtualServiceHandler) Deleted(obj *v1alpha3.VirtualService) { if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { - log.Infof(LogFormat, "Delete", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) + log.Infof(LogFormat, "Delete", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace="+obj.Namespace) return } err := handleVirtualServiceEvent(obj, vh, common.Delete, common.VirtualService) @@ -192,7 +193,7 @@ func (dh *SidecarHandler) Deleted(obj *v1alpha3.Sidecar) {} func IgnoreIstioResource(exportTo []string, annotations map[string]string, namespace string) bool { - if len(annotations) > 0 && annotations[common.AdmiralIgnoreAnnotation] == "true" { + if len(annotations) > 0 && annotations[common.AdmiralIgnoreAnnotation] == "true" { return true } @@ -416,7 +417,7 @@ func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceH } //check if this virtual service is used by Argo rollouts for canary strategy, if so, update the corresponding SE with appropriate weights - if common.GetAdmiralParams().ArgoRolloutsEnabled { + if common.GetAdmiralParams().ArgoRolloutsEnabled { rollouts, err := vh.RemoteRegistry.RemoteControllers[clusterId].RolloutController.RolloutClient.Rollouts(obj.Namespace).List(v12.ListOptions{}) if err != nil { @@ -480,7 +481,7 @@ func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceH } return nil } else { - log.Infof(LogFormat,"Event", "VirtualService", obj.Name, clusterId, "No dependent clusters found") + log.Infof(LogFormat, "Event", "VirtualService", obj.Name, clusterId, "No dependent clusters found") } //copy the VirtualService `as is` if they are not generated by Admiral (not in CnameDependentClusterCache) @@ -510,7 +511,7 @@ func addUpdateVirtualService(obj *v1alpha3.VirtualService, exist *v1alpha3.Virtu if obj.Annotations == nil { obj.Annotations = map[string]string{} } - obj.Annotations["app.kubernetes.io/created-by"] = "admiral" + obj.Annotations["app.kubernetes.io/created-by"] = "admiral" if exist == nil || len(exist.Spec.Hosts) == 0 { obj.Namespace = namespace obj.ResourceVersion = "" @@ -537,20 +538,20 @@ func addUpdateServiceEntry(obj *v1alpha3.ServiceEntry, exist *v1alpha3.ServiceEn if obj.Annotations == nil { obj.Annotations = map[string]string{} } - obj.Annotations["app.kubernetes.io/created-by"] = "admiral" + obj.Annotations["app.kubernetes.io/created-by"] = "admiral" if exist == nil || exist.Spec.Hosts == nil { obj.Namespace = namespace obj.ResourceVersion = "" _, err = rc.ServiceEntryController.IstioClient.NetworkingV1alpha3().ServiceEntries(namespace).Create(obj) op = "Add" - log.Infof(LogFormat + " SE=%s", op, "ServiceEntry", obj.Name, rc.ClusterID, "New SE", obj.Spec.String()) + log.Infof(LogFormat+" SE=%s", op, "ServiceEntry", obj.Name, rc.ClusterID, "New SE", obj.Spec.String()) } else { exist.Labels = obj.Labels exist.Annotations = obj.Annotations op = "Update" skipUpdate, diff := skipDestructiveUpdate(rc, obj, exist) if diff != "" { - log.Infof(LogFormat + " diff=%s", op, "ServiceEntry", obj.Name, rc.ClusterID, "Diff in update", diff) + log.Infof(LogFormat+" diff=%s", op, "ServiceEntry", obj.Name, rc.ClusterID, "Diff in update", diff) } if skipUpdate { log.Infof(LogFormat, op, "ServiceEntry", obj.Name, rc.ClusterID, "Update skipped as it was destructive during Admiral's bootup phase") @@ -573,7 +574,7 @@ func skipDestructiveUpdate(rc *RemoteController, new *v1alpha3.ServiceEntry, old skipDestructive = false destructive, diff := getServiceEntryDiff(new, old) //do not update SEs during bootup phase if they are destructive - if time.Since(rc.StartTime) < (2 * common.GetAdmiralParams().CacheRefreshDuration) && destructive { + if time.Since(rc.StartTime) < (2*common.GetAdmiralParams().CacheRefreshDuration) && destructive { skipDestructive = true } @@ -603,17 +604,17 @@ func getServiceEntryDiff(new *v1alpha3.ServiceEntry, old *v1alpha3.ServiceEntry) found[nEndpoint.Address] = "1" if !reflect.DeepEqual(val, nEndpoint) { destructive = true - buffer.WriteString(fmt.Sprintf(format, "endpoint", "Update", val.String(), nEndpoint.String())) + buffer.WriteString(fmt.Sprintf(format, "endpoint", "Update", val.String(), nEndpoint.String())) } } else { - buffer.WriteString(fmt.Sprintf(format, "endpoint", "Add", "", nEndpoint.String())) + buffer.WriteString(fmt.Sprintf(format, "endpoint", "Add", "", nEndpoint.String())) } } for key := range oldEndpointMap { if _, ok := found[key]; !ok { destructive = true - buffer.WriteString(fmt.Sprintf(format, "endpoint", "Delete", oldEndpointMap[key].String(), "")) + buffer.WriteString(fmt.Sprintf(format, "endpoint", "Delete", oldEndpointMap[key].String(), "")) } } @@ -638,7 +639,7 @@ func addUpdateDestinationRule(obj *v1alpha3.DestinationRule, exist *v1alpha3.Des if obj.Annotations == nil { obj.Annotations = map[string]string{} } - obj.Annotations["app.kubernetes.io/created-by"] = "admiral" + obj.Annotations["app.kubernetes.io/created-by"] = "admiral" if exist == nil || exist.Name == "" || exist.Spec.Host == "" { obj.Namespace = namespace obj.ResourceVersion = "" @@ -744,7 +745,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 always use the active 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 func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[string]*WeightedService { if rollout == nil { @@ -761,14 +762,17 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin return nil } - var canaryService, stableService, virtualServiceRouteName string + var canaryService, stableService, virtualServiceRouteName string var istioCanaryWeights = make(map[string]int32) var blueGreenActiveService string + var blueGreenPreviewService string + if rolloutStrategy.BlueGreen != nil { - // If rollout uses blue green strategy, use the active service + // If rollout uses blue green strategy 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 @@ -827,7 +831,6 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin var matchedServices = make(map[string]*WeightedService) - //if we have more than one matching service we will pick the first one, for this to be deterministic we sort services var servicesInNamespace = cachedService.Service[rollout.Namespace] @@ -842,7 +845,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 { + if len(blueGreenActiveService) > 0 && service.ObjectMeta.Name != blueGreenActiveService && service.ObjectMeta.Name != blueGreenPreviewService { log.Infof("Skipping service=%s for rollout=%s in namespace=%s and cluster=%s", service.Name, rollout.Name, rollout.Namespace, rc.ClusterID) continue } @@ -863,8 +866,11 @@ 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 or using canary with NO istio traffic management, pick the first service that matches - if len(istioCanaryWeights) == 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 { matchedServices[service.Name] = &WeightedService{Weight: 1, Service: service} break } diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index ff7a8c003..e796a71c0 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -3,7 +3,6 @@ package clusters import ( "errors" "fmt" - v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" "math" "math/rand" "reflect" @@ -11,6 +10,8 @@ import ( "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" @@ -61,6 +62,7 @@ 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 @@ -103,6 +105,7 @@ 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 { @@ -131,12 +134,13 @@ 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 + var isBlueGreenStrategy = sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen != nil && sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService != "" + if len(sourceDeployments) > 0 { meshPorts = GetMeshPorts(sourceCluster, serviceInstance, sourceDeployments[sourceCluster]) - } else { + } else if !isBlueGreenStrategy { meshPorts = GetMeshPortsForRollout(sourceCluster, serviceInstance, sourceRollouts[sourceCluster]) } - for key, serviceEntry := range serviceEntries { if len(serviceEntry.Endpoints) == 0 { AddServiceEntriesWithDr(remoteRegistry.AdmiralCache, map[string]string{sourceCluster: sourceCluster}, remoteRegistry.RemoteControllers, @@ -146,8 +150,31 @@ 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 == "" { - // see if we have weighted services (rollouts with canary strategy) - if len(weightedServices) > 1 { + // Update endpoints with locafqdn for active and preview se of bluegreen rollout + if isBlueGreenStrategy { + activeServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.ActiveService + previewServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService + oldPorts := ep.Ports + if previewService, ok := weightedServices[previewServiceName]; strings.HasPrefix(key, common.GetPreviewHostnamePrefix()+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, sourceRollouts[sourceCluster]) + } 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, sourceRollouts[sourceCluster]) + } + 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 { //add one endpoint per each service, may be modify var se = copyServiceEntry(serviceEntry) updateEndpointsForWeightedServices(se, weightedServices, clusterIngress, meshPorts) @@ -168,7 +195,7 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s } for _, val := range dependents.Map() { - remoteRegistry.AdmiralCache.DependencyNamespaceCache.Put(val, serviceInstance.Namespace, localFqdn, map[string]string{cname: "1"}) + remoteRegistry.AdmiralCache.DependencyNamespaceCache.Put(val, serviceInstance.Namespace, localFqdn, cnames) } if common.GetWorkloadSidecarUpdate() == "enabled" { @@ -569,6 +596,18 @@ 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.GetPreviewHostnamePrefix() + common.Sep + common.GetCnameForRollout(destRollout, workloadIdentityKey, common.GetHostnameSuffix()) + previewAddress := getUniqueAddress(admiralCache, previewGlobalFqdn) + if len(previewGlobalFqdn) == 0 || len(previewAddress) == 0 { + return nil + } + generateServiceEntry(event, admiralCache, meshPorts, previewGlobalFqdn, rc, serviceEntries, previewAddress, san) + } + } + tmpSe := generateServiceEntry(event, admiralCache, meshPorts, globalFqdn, rc, serviceEntries, address, san) return tmpSe } diff --git a/admiral/pkg/controller/common/config.go b/admiral/pkg/controller/common/config.go index 8c7f01892..87c4080f0 100644 --- a/admiral/pkg/controller/common/config.go +++ b/admiral/pkg/controller/common/config.go @@ -1,9 +1,10 @@ package common import ( - log "github.com/sirupsen/logrus" "sync" "time" + + log "github.com/sirupsen/logrus" ) var admiralParams = AdmiralParams{ @@ -71,6 +72,10 @@ func GetHostnameSuffix() string { return admiralParams.HostnameSuffix } +func GetPreviewHostnamePrefix() string { + return admiralParams.PreviewHostnamePrefix +} + func GetWorkloadIdentifier() string { return admiralParams.LabelSet.WorkloadIdentityKey } diff --git a/admiral/pkg/controller/common/types.go b/admiral/pkg/controller/common/types.go index c7457ecc9..cf7e508ad 100644 --- a/admiral/pkg/controller/common/types.go +++ b/admiral/pkg/controller/common/types.go @@ -41,6 +41,7 @@ type AdmiralParams struct { LabelSet *LabelSet LogLevel int HostnameSuffix string + PreviewHostnamePrefix string WorkloadSidecarUpdate string WorkloadSidecarName string } @@ -64,8 +65,8 @@ type LabelSet struct { AdmiralIgnoreLabel string WorkloadIdentityKey string //Should always be used for both label and annotation (using label as the primary, and falling back to annotation if the label is not found) GlobalTrafficDeploymentLabel string //label used to tie together deployments and globaltrafficpolicy objects. Configured separately from the identity key because this one _must_ be a label - EnvKey string //key used to group deployments by env. The order would be to use annotation `EnvKey` and then label `EnvKey` and then fallback to label `env` label - GatewayApp string //the value for `app` key that will be used to fetch the loadblancer for cross cluster calls, also referred to as east west gateway + EnvKey string //key used to group deployments by env. The order would be to use annotation `EnvKey` and then label `EnvKey` and then fallback to label `env` label + GatewayApp string //the value for `app` key that will be used to fetch the loadblancer for cross cluster calls, also referred to as east west gateway } func NewSidecarEgressMap() *SidecarEgressMap { From bc09a6d7063e796fb8b4d088656b3acae96adc50 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Wed, 16 Mar 2022 18:30:55 +0530 Subject: [PATCH 02/17] Updated integration test for preview endpoint Signed-off-by: nbn01 --- tests/test2.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test2.sh b/tests/test2.sh index 7bbafbe0a..0806a2b6c 100755 --- a/tests/test2.sh +++ b/tests/test2.sh @@ -9,7 +9,9 @@ dest=$3 #Test output=$(kubectl exec --namespace=$source_ns -it $(kubectl get pod -l "app=$source" --namespace=$source_ns -o jsonpath='{.items[0].metadata.name}') -c $source -- curl -v "http://stage.$dest.global" && echo "") -if [[ "$output" == *"Admiral"* ]]; then +previewOutput=$(kubectl exec --namespace=$source_ns -it $(kubectl get pod -l "app=$source" --namespace=$source_ns -o jsonpath='{.items[0].metadata.name}') -c $source -- curl -v "http://preview.stage.$dest.global" && echo "") + +if [[ "$output" == *"Admiral"* && "$previewOutput" == *"Admiral"* ]]; then echo "PASS" exit 0 else From feed242a8b591ed909b629f92204255acf1f592d Mon Sep 17 00:00:00 2001 From: nbn01 Date: Wed, 16 Mar 2022 21:04:05 +0530 Subject: [PATCH 03/17] Minor check fixes Signed-off-by: nbn01 --- admiral/pkg/clusters/handler.go | 2 +- admiral/pkg/clusters/serviceentry.go | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index 4e046ee45..1d6dd2b07 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -845,7 +845,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 && len(blueGreenPreviewService) > 0 && service.ObjectMeta.Name != blueGreenActiveService && service.ObjectMeta.Name != blueGreenPreviewService { log.Infof("Skipping service=%s for rollout=%s in namespace=%s and cluster=%s", service.Name, rollout.Name, rollout.Namespace, rc.ClusterID) continue } diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index e796a71c0..1f41ef43b 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -134,7 +134,11 @@ 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 - var isBlueGreenStrategy = sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen != nil && sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService != "" + isBlueGreenStrategy := false + + if len(sourceRollouts) > 0 { + isBlueGreenStrategy = sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen != nil && sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService != "" + } if len(sourceDeployments) > 0 { meshPorts = GetMeshPorts(sourceCluster, serviceInstance, sourceDeployments[sourceCluster]) From d72f35a8ab2b2cfe2c7ec608460ecde882471d26 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Wed, 16 Mar 2022 21:12:34 +0530 Subject: [PATCH 04/17] Reverted integration test changes Signed-off-by: nbn01 --- tests/test2.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test2.sh b/tests/test2.sh index 0806a2b6c..7bbafbe0a 100755 --- a/tests/test2.sh +++ b/tests/test2.sh @@ -9,9 +9,7 @@ dest=$3 #Test output=$(kubectl exec --namespace=$source_ns -it $(kubectl get pod -l "app=$source" --namespace=$source_ns -o jsonpath='{.items[0].metadata.name}') -c $source -- curl -v "http://stage.$dest.global" && echo "") -previewOutput=$(kubectl exec --namespace=$source_ns -it $(kubectl get pod -l "app=$source" --namespace=$source_ns -o jsonpath='{.items[0].metadata.name}') -c $source -- curl -v "http://preview.stage.$dest.global" && echo "") - -if [[ "$output" == *"Admiral"* && "$previewOutput" == *"Admiral"* ]]; then +if [[ "$output" == *"Admiral"* ]]; then echo "PASS" exit 0 else From 2f79924c6dbae8696c58348e23c505c83a43ba4f Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 18 Mar 2022 11:40:38 +0530 Subject: [PATCH 05/17] Update unit tests Signed-off-by: nbn01 --- admiral/pkg/clusters/registry_test.go | 36 +-- admiral/pkg/clusters/serviceentry_test.go | 224 ++++++++++++++++--- admiral/pkg/controller/common/common_test.go | 12 +- admiral/pkg/controller/common/config_test.go | 3 + 4 files changed, 217 insertions(+), 58 deletions(-) diff --git a/admiral/pkg/clusters/registry_test.go b/admiral/pkg/clusters/registry_test.go index 56810b522..cf6bcbea0 100644 --- a/admiral/pkg/clusters/registry_test.go +++ b/admiral/pkg/clusters/registry_test.go @@ -2,9 +2,13 @@ package clusters import ( "context" + "strings" + "testing" + "time" + "github.com/google/go-cmp/cmp" depModel "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model" - "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" + v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" "github.com/istio-ecosystem/admiral/admiral/pkg/test" @@ -17,9 +21,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" - "strings" - "testing" - "time" ) func init() { @@ -36,6 +37,7 @@ func init() { SecretResolver: "", WorkloadSidecarUpdate: "enabled", WorkloadSidecarName: "default", + PreviewHostnamePrefix: "preview", } p.LabelSet.WorkloadIdentityKey = "identity" @@ -449,24 +451,24 @@ func TestUpdateCacheController(t *testing.T) { //Struct of test case info. Name is required. testCases := []struct { - name string - oldConfig *rest.Config - newConfig *rest.Config - clusterId string + name string + oldConfig *rest.Config + newConfig *rest.Config + clusterId string shouldRefresh bool }{ { - name: "Should update controller when kubeconfig changes", - oldConfig: originalConfig, - newConfig: changedConfig, - clusterId: "test.cluster", + name: "Should update controller when kubeconfig changes", + oldConfig: originalConfig, + newConfig: changedConfig, + clusterId: "test.cluster", shouldRefresh: true, }, { - name: "Should not update controller when kubeconfig doesn't change", - oldConfig: originalConfig, - newConfig: originalConfig, - clusterId: "test.cluster", + name: "Should not update controller when kubeconfig doesn't change", + oldConfig: originalConfig, + newConfig: originalConfig, + clusterId: "test.cluster", shouldRefresh: false, }, } @@ -476,7 +478,7 @@ func TestUpdateCacheController(t *testing.T) { t.Run(c.name, func(t *testing.T) { hook := logTest.NewGlobal() rr.RemoteControllers[c.clusterId].ApiServer = c.oldConfig.Host - d, err := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, c.oldConfig, time.Second*time.Duration(300)) + d, err := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, c.oldConfig, time.Second*time.Duration(300)) if err != nil { t.Fatalf("Unexpected error creating controller %v", err) } diff --git a/admiral/pkg/clusters/serviceentry_test.go b/admiral/pkg/clusters/serviceentry_test.go index e4d1b5e4d..102e8deae 100644 --- a/admiral/pkg/clusters/serviceentry_test.go +++ b/admiral/pkg/clusters/serviceentry_test.go @@ -3,6 +3,14 @@ package clusters import ( "context" "errors" + "reflect" + "strconv" + "strings" + "sync" + "testing" + "time" + "unicode" + argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" "github.com/google/go-cmp/cmp" "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model" @@ -17,17 +25,10 @@ import ( "istio.io/client-go/pkg/apis/networking/v1alpha3" istiofake "istio.io/client-go/pkg/clientset/versioned/fake" v14 "k8s.io/api/apps/v1" - "k8s.io/api/core/v1" coreV1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" - "reflect" - "strconv" - "strings" - "sync" - "testing" - "time" - "unicode" ) func TestCreateSeWithDrLabels(t *testing.T) { @@ -261,7 +262,6 @@ func TestCreateSeAndDrSetFromGtp(t *testing.T) { seDrSet: map[string]*SeDrTuple{host: nil, common.GetCnameVal([]string{west, host}): nil, strings.ToLower(common.GetCnameVal([]string{eastWithCaps, host})): nil}, }, - } //Run the test for every provided case @@ -994,6 +994,158 @@ func TestCreateServiceEntryForNewServiceOrPodRolloutsUsecase(t *testing.T) { } } +func TestCreateServiceEntryForBlueGreenRolloutsUsecase(t *testing.T) { + + const NAMESPACE = "test-test" + const ACTIVE_SERVICENAME = "serviceNameActive" + const PREVIEW_SERVICENAME = "serviceNamePreview" + const ROLLOUT_POD_HASH_LABEL string = "rollouts-pod-template-hash" + + p := common.AdmiralParams{ + KubeconfigPath: "testdata/fake.config", + PreviewHostnamePrefix: "preview", + } + rr, _ := InitAdmiral(context.Background(), p) + t.Logf(common.GetPreviewHostnamePrefix()) + config := rest.Config{ + Host: "localhost", + } + + d, e := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, &config, time.Second*time.Duration(300)) + + r, e := admiral.NewRolloutsController(make(chan struct{}), &test.MockRolloutHandler{}, &config, time.Second*time.Duration(300)) + v, e := istio.NewVirtualServiceController(make(chan struct{}), &test.MockVirtualServiceHandler{}, &config, time.Second*time.Duration(300)) + + if e != nil { + t.Fail() + } + s, e := admiral.NewServiceController(make(chan struct{}), &test.MockServiceHandler{}, &config, time.Second*time.Duration(300)) + + cacheWithEntry := ServiceEntryAddressStore{ + EntryAddresses: map[string]string{ + "test.test.mesh-se": common.LocalAddressPrefix + ".10.1", + "preview.test.test.mesh-se": common.LocalAddressPrefix + ".10.2", + }, + Addresses: []string{common.LocalAddressPrefix + ".10.1", common.LocalAddressPrefix + ".10.2"}, + } + + fakeIstioClient := istiofake.NewSimpleClientset() + rc := &RemoteController{ + ServiceEntryController: &istio.ServiceEntryController{ + IstioClient: fakeIstioClient, + }, + DestinationRuleController: &istio.DestinationRuleController{ + IstioClient: fakeIstioClient, + }, + NodeController: &admiral.NodeController{ + Locality: &admiral.Locality{ + Region: "us-west-2", + }, + }, + DeploymentController: d, + RolloutController: r, + ServiceController: s, + VirtualServiceController: v, + } + rc.ClusterID = "test.cluster" + rr.RemoteControllers["test.cluster"] = rc + + admiralCache := &AdmiralCache{ + IdentityClusterCache: common.NewMapOfMaps(), + ServiceEntryAddressStore: &cacheWithEntry, + CnameClusterCache: common.NewMapOfMaps(), + CnameIdentityCache: &sync.Map{}, + CnameDependentClusterCache: common.NewMapOfMaps(), + IdentityDependencyCache: common.NewMapOfMaps(), + GlobalTrafficCache: &globalTrafficCache{}, + DependencyNamespaceCache: common.NewSidecarEgressMap(), + SeClusterCache: common.NewMapOfMaps(), + } + rr.AdmiralCache = admiralCache + + rollout := argo.Rollout{} + + rollout.Spec = argo.RolloutSpec{ + Template: coreV1.PodTemplateSpec{ + ObjectMeta: v12.ObjectMeta{ + Labels: map[string]string{"identity": "test"}, + }, + }, + } + + rollout.Namespace = NAMESPACE + rollout.Spec.Strategy = argo.RolloutStrategy{ + BlueGreen: &argo.BlueGreenStrategy{ActiveService: ACTIVE_SERVICENAME, PreviewService: PREVIEW_SERVICENAME}, + } + labelMap := make(map[string]string) + labelMap["identity"] = "test" + + matchLabel4 := make(map[string]string) + matchLabel4["app"] = "test" + + labelSelector4 := v12.LabelSelector{ + MatchLabels: matchLabel4, + } + rollout.Spec.Selector = &labelSelector4 + + r.Cache.UpdateRolloutToClusterCache("bar", &rollout) + + selectorMap := make(map[string]string) + selectorMap["app"] = "test" + selectorMap[ROLLOUT_POD_HASH_LABEL] = "hash" + + port1 := coreV1.ServicePort{ + Port: 8080, + Name: "random1", + } + + port2 := coreV1.ServicePort{ + Port: 8081, + Name: "random2", + } + + ports := []coreV1.ServicePort{port1, port2} + + activeService := &coreV1.Service{ + Spec: coreV1.ServiceSpec{ + Selector: selectorMap, + }, + } + activeService.Name = ACTIVE_SERVICENAME + activeService.Namespace = NAMESPACE + activeService.Spec.Ports = ports + + s.Cache.Put(activeService) + + previewService := &coreV1.Service{ + Spec: coreV1.ServiceSpec{ + Selector: selectorMap, + }, + } + previewService.Name = PREVIEW_SERVICENAME + previewService.Namespace = NAMESPACE + previewService.Spec.Ports = ports + + s.Cache.Put(previewService) + + se := modifyServiceEntryForNewServiceOrPod(admiral.Add, "test", "bar", rr) + + if nil == se { + t.Fatalf("no service entries found") + } + if len(se) != 2 { + t.Fatalf("Expected 2 service entries to be created but found %d", len(se)) + } + serviceEntryResp := se["test.test.mesh"] + if nil == serviceEntryResp { + t.Fatalf("Service entry returned should not be empty") + } + previewServiceEntryResp := se["preview.test.test.mesh"] + if nil == previewServiceEntryResp { + t.Fatalf("Preview Service entry returned should not be empty") + } +} + func TestUpdateEndpointsForWeightedServices(t *testing.T) { t.Parallel() @@ -1001,12 +1153,12 @@ func TestUpdateEndpointsForWeightedServices(t *testing.T) { const CLUSTER_INGRESS_2 = "ingress2.com" const CANARY_SERVICE = "canaryService" const STABLE_SERVICE = "stableService" - const NAMESPACE = "namespace" + const NAMESPACE = "namespace" se := &istionetworkingv1alpha3.ServiceEntry{ - Endpoints: []*istionetworkingv1alpha3.ServiceEntry_Endpoint{ - {Labels: map[string]string{}, Address: CLUSTER_INGRESS_1, Weight: 10, Ports: map[string]uint32{"http" : 15443,}}, - {Labels: map[string]string{}, Address: CLUSTER_INGRESS_2, Weight: 10, Ports: map[string]uint32{"http" : 15443}}, + Endpoints: []*istionetworkingv1alpha3.ServiceEntry_Endpoint{ + {Labels: map[string]string{}, Address: CLUSTER_INGRESS_1, Weight: 10, Ports: map[string]uint32{"http": 15443}}, + {Labels: map[string]string{}, Address: CLUSTER_INGRESS_2, Weight: 10, Ports: map[string]uint32{"http": 15443}}, }, } @@ -1022,47 +1174,47 @@ func TestUpdateEndpointsForWeightedServices(t *testing.T) { } wantedEndpoints := []*istionetworkingv1alpha3.ServiceEntry_Endpoint{ - {Address: CLUSTER_INGRESS_2, Weight: 10, Ports: map[string]uint32{"http" : 15443}}, + {Address: CLUSTER_INGRESS_2, Weight: 10, Ports: map[string]uint32{"http": 15443}}, {Address: STABLE_SERVICE + common.Sep + NAMESPACE + common.DotLocalDomainSuffix, Weight: 90, Ports: meshPorts}, {Address: CANARY_SERVICE + common.Sep + NAMESPACE + common.DotLocalDomainSuffix, Weight: 10, Ports: meshPorts}, } wantedEndpointsZeroWeights := []*istionetworkingv1alpha3.ServiceEntry_Endpoint{ - {Address: CLUSTER_INGRESS_2, Weight: 10, Ports: map[string]uint32{"http" : 15443}}, + {Address: CLUSTER_INGRESS_2, Weight: 10, Ports: map[string]uint32{"http": 15443}}, {Address: STABLE_SERVICE + common.Sep + NAMESPACE + common.DotLocalDomainSuffix, Weight: 100, Ports: meshPorts}, } testCases := []struct { - name string - inputServiceEntry *istionetworkingv1alpha3.ServiceEntry - weightedServices map[string]*WeightedService - clusterIngress string - meshPorts map[string]uint32 - wantedEndpoints []*istionetworkingv1alpha3.ServiceEntry_Endpoint + name string + inputServiceEntry *istionetworkingv1alpha3.ServiceEntry + weightedServices map[string]*WeightedService + clusterIngress string + meshPorts map[string]uint32 + wantedEndpoints []*istionetworkingv1alpha3.ServiceEntry_Endpoint }{ { - name: "should return endpoints with assigned weights", + name: "should return endpoints with assigned weights", inputServiceEntry: copyServiceEntry(se), - weightedServices: weightedServices, - clusterIngress: CLUSTER_INGRESS_1, - meshPorts: meshPorts, - wantedEndpoints: wantedEndpoints, + weightedServices: weightedServices, + clusterIngress: CLUSTER_INGRESS_1, + meshPorts: meshPorts, + wantedEndpoints: wantedEndpoints, }, { - name: "should return endpoints as is", + name: "should return endpoints as is", inputServiceEntry: copyServiceEntry(se), - weightedServices: weightedServices, - clusterIngress: "random", - meshPorts: meshPorts, - wantedEndpoints: copyServiceEntry(se).Endpoints, + weightedServices: weightedServices, + clusterIngress: "random", + meshPorts: meshPorts, + wantedEndpoints: copyServiceEntry(se).Endpoints, }, { - name: "should not return endpoints with zero weight", + name: "should not return endpoints with zero weight", inputServiceEntry: copyServiceEntry(se), - weightedServices: weightedServicesZeroWeight, - clusterIngress: CLUSTER_INGRESS_1, - meshPorts: meshPorts, - wantedEndpoints: wantedEndpointsZeroWeights, + weightedServices: weightedServicesZeroWeight, + clusterIngress: CLUSTER_INGRESS_1, + meshPorts: meshPorts, + wantedEndpoints: wantedEndpointsZeroWeights, }, } diff --git a/admiral/pkg/controller/common/common_test.go b/admiral/pkg/controller/common/common_test.go index 299196a1c..29c19355f 100644 --- a/admiral/pkg/controller/common/common_test.go +++ b/admiral/pkg/controller/common/common_test.go @@ -1,16 +1,17 @@ package common import ( + "reflect" + "strings" + "testing" + "time" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" v12 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" k8sAppsV1 "k8s.io/api/apps/v1" k8sCoreV1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1" - "reflect" - "strings" - "testing" - "time" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var ignoreUnexported = cmpopts.IgnoreUnexported(v12.GlobalTrafficPolicy{}.Status) @@ -29,6 +30,7 @@ func init() { SecretResolver: "", WorkloadSidecarName: "default", WorkloadSidecarUpdate: "disabled", + PreviewHostnamePrefix: "preview", } p.LabelSet.WorkloadIdentityKey = "identity" diff --git a/admiral/pkg/controller/common/config_test.go b/admiral/pkg/controller/common/config_test.go index 0543105a8..c4ab32e64 100644 --- a/admiral/pkg/controller/common/config_test.go +++ b/admiral/pkg/controller/common/config_test.go @@ -50,6 +50,9 @@ func TestConfigManagement(t *testing.T) { if GetHostnameSuffix() != "mesh" { t.Errorf("Hostname suffix mismatch, expected mesh, got %v", GetHostnameSuffix()) } + if GetPreviewHostnamePrefix() != "preview" { + t.Errorf("Prefix for preview hostname mismatch, expected preview, got %v", GetPreviewHostnamePrefix()) + } if GetSyncNamespace() != "ns" { t.Errorf("Sync namespace mismatch, expected ns, got %v", GetSyncNamespace()) } From b08cee15d3a1d737fe4cf8174b0970565dc68fe2 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 18 Mar 2022 11:44:05 +0530 Subject: [PATCH 06/17] Add integration test for preview endpoint of bluegreen rollout Signed-off-by: nbn01 --- install/sample/greeting_preview.yaml | 99 ++++++++++++++++++++++ install/scripts/install_sample_services.sh | 4 + tests/run.sh | 2 + tests/test5.sh | 18 ++++ 4 files changed, 123 insertions(+) create mode 100644 install/sample/greeting_preview.yaml create mode 100644 tests/test5.sh diff --git a/install/sample/greeting_preview.yaml b/install/sample/greeting_preview.yaml new file mode 100644 index 000000000..c9b252157 --- /dev/null +++ b/install/sample/greeting_preview.yaml @@ -0,0 +1,99 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: nginx-conf-preview + namespace: sample-rollout-bluegreen +data: + nginx.conf: | + user nginx; + worker_processes 3; + error_log /var/log/nginx/error.log; + events { + worker_connections 10240; + } + http { + log_format main + 'remote_addr:$remote_addr\t' + 'time_local:$time_local\t' + 'method:$request_method\t' + 'uri:$request_uri\t' + 'host:$host\t' + 'status:$status\t' + 'bytes_sent:$body_bytes_sent\t' + 'referer:$http_referer\t' + 'useragent:$http_user_agent\t' + 'forwardedfor:$http_x_forwarded_for\t' + 'request_time:$request_time'; + access_log /var/log/nginx/access.log main; + server { + listen 80; + server_name _; + location / { + return 200 "Hello World! - Admiral Preview!!"; + + } + } + } +--- +apiVersion: argoproj.io/v1alpha1 +kind: Rollout +metadata: + name: greeting + namespace: sample-rollout-bluegreen + labels: + identity: greeting +spec: + replicas: 1 + selector: + matchLabels: + app: greeting + template: + metadata: + annotations: + admiral.io/env: stage + sidecar.istio.io/inject: "true" + labels: + app: greeting + identity: greeting.bluegreen + spec: + containers: + - image: nginx + name: greeting + ports: + - containerPort: 80 + volumeMounts: + - mountPath: /etc/nginx + name: nginx-conf + readOnly: true + - mountPath: /var/log/nginx + name: log + resources: + requests: + cpu: "10m" + memory: "50Mi" + limits: + cpu: "20m" + memory: "75Mi" + volumes: + - configMap: + items: + - key: nginx.conf + path: nginx.conf + name: nginx-conf-preview + name: nginx-conf + - emptyDir: {} + name: log + strategy: + blueGreen: + # activeService specifies the service to update with the new template hash at time of promotion. + # This field is mandatory for the blueGreen update strategy. + activeService: rollout-bluegreen-active + # previewService specifies the service to update with the new template hash before promotion. + # This allows the preview stack to be reachable without serving production traffic. + # This field is optional. + previewService: rollout-bluegreen-preview + # autoPromotionEnabled disables automated promotion of the new stack by pausing the rollout + # immediately before the promotion. If omitted, the default behavior is to promote the new + # stack as soon as the ReplicaSet are completely ready/available. + # Rollouts can be resumed using: `kubectl argo rollouts resume ROLLOUT` + autoPromotionEnabled: false \ No newline at end of file diff --git a/install/scripts/install_sample_services.sh b/install/scripts/install_sample_services.sh index 0b1bfe121..906c2ee7d 100755 --- a/install/scripts/install_sample_services.sh +++ b/install/scripts/install_sample_services.sh @@ -23,6 +23,9 @@ kubectl apply -f $install_dir/yaml/grpc.yaml kubectl apply -f $install_dir/yaml/sample_dep.yaml +# Update BlueGreen Rollout with new preview release +kubectl apply -f $install_dir/yaml/greeting_preview.yaml + #wait for the deployments to come up kubectl rollout status deployment greeting -n sample kubectl rollout status deployment webapp -n sample @@ -30,6 +33,7 @@ kubectl rollout status deployment webapp -n sample kubectl rollout status deployment webapp -n sample-rollout-bluegreen + #Verify that admiral created service names for 'greeting' service checkse() { identity=$1 diff --git a/tests/run.sh b/tests/run.sh index dd67117d1..13c20a038 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -28,6 +28,8 @@ $install_dir/scripts/install_sample_services.sh $install_dir sleep 10 ./test1.sh "webapp" "sample" "greeting" ./test2.sh "webapp" "sample-rollout-bluegreen" "greeting.bluegreen" +sleep 20 +./test5.sh "webapp" "sample-rollout-bluegreen" "greeting.bluegreen" ./test2.sh "webapp" "sample-rollout-canary" "greeting.canary" #cleanup to fee up the pipeline minkube resources if [[ $IS_LOCAL == "false" ]]; then diff --git a/tests/test5.sh b/tests/test5.sh new file mode 100644 index 000000000..a139746d7 --- /dev/null +++ b/tests/test5.sh @@ -0,0 +1,18 @@ +#!/bin/bash + +[ $# -lt 3 ] && { echo "Usage: $0 " ; exit 1; } + +source=$1 +source_ns=$2 +dest=$3 + +#Test +output=$(kubectl exec --namespace=$source_ns -it $(kubectl get pod -l "app=$source" --namespace=$source_ns -o jsonpath='{.items[0].metadata.name}') -c $source -- curl -v "http://preview.stage.$dest.global" && echo "") + +if [[ "$output" == *"Admiral"* ]]; then + echo "Rollout BlueGreen Preview: PASS" + exit 0 +else + echo "FAIL" . $output + exit 1 +fi From f3b051530a884cb486c72f689d411087e799e3d0 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 18 Mar 2022 12:57:27 +0530 Subject: [PATCH 07/17] Update makefile Signed-off-by: nbn01 --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index b51dd4fba..cd9f41ecc 100644 --- a/Makefile +++ b/Makefile @@ -131,6 +131,7 @@ 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 From e6544d2c89da18a337c54f8a8c009e3237476f17 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 18 Mar 2022 13:25:10 +0530 Subject: [PATCH 08/17] Modified permissions Signed-off-by: nbn01 --- tests/test5.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tests/test5.sh diff --git a/tests/test5.sh b/tests/test5.sh old mode 100644 new mode 100755 From 4f28bb19d545176d32ed2ada7b695dd52078fd6c Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 18 Mar 2022 16:06:11 +0530 Subject: [PATCH 09/17] Reverting argo_rollouts default to false Signed-off-by: nbn01 --- admiral/cmd/admiral/cmd/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admiral/cmd/admiral/cmd/root.go b/admiral/cmd/admiral/cmd/root.go index 70a1b7ed7..0dcff8413 100644 --- a/admiral/cmd/admiral/cmd/root.go +++ b/admiral/cmd/admiral/cmd/root.go @@ -76,7 +76,7 @@ func GetRootCmd(args []string) *cobra.Command { fmt.Sprintf("Set log verbosity, defaults to 'Info'. Must be between %v and %v", int(log.PanicLevel), int(log.TraceLevel))) rootCmd.PersistentFlags().StringVar(¶ms.KubeconfigPath, "kube_config", "", "Use a Kubernetes configuration file instead of in-cluster configuration") - rootCmd.PersistentFlags().BoolVar(¶ms.ArgoRolloutsEnabled, "argo_rollouts", true, + rootCmd.PersistentFlags().BoolVar(¶ms.ArgoRolloutsEnabled, "argo_rollouts", false, "Use argo rollout configurations") rootCmd.PersistentFlags().StringVar(¶ms.ClusterRegistriesNamespace, "secret_namespace", "admiral", "Namespace to monitor for secrets defaults to admiral-secrets") From c00aa5bc3e96477ce26aa34605b9d3c27f5ad08b Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 18 Mar 2022 17:45:53 +0530 Subject: [PATCH 10/17] Updated unit tests to check when preview service not defined Signed-off-by: nbn01 --- admiral/pkg/clusters/handler.go | 2 +- admiral/pkg/clusters/serviceentry.go | 2 +- admiral/pkg/clusters/serviceentry_test.go | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index b710029da..4fab356f7 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -847,7 +847,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 && len(blueGreenPreviewService) > 0 && service.ObjectMeta.Name != blueGreenActiveService && service.ObjectMeta.Name != blueGreenPreviewService { + if len(blueGreenActiveService) > 0 && service.ObjectMeta.Name != blueGreenActiveService && service.ObjectMeta.Name != blueGreenPreviewService { log.Infof("Skipping service=%s for rollout=%s in namespace=%s and cluster=%s", service.Name, rollout.Name, rollout.Namespace, rc.ClusterID) continue } diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index 030fd6e86..a4eee022d 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -137,7 +137,7 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s isBlueGreenStrategy := false if len(sourceRollouts) > 0 { - isBlueGreenStrategy = sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen != nil && sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService != "" + isBlueGreenStrategy = sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen != nil } if len(sourceDeployments) > 0 { diff --git a/admiral/pkg/clusters/serviceentry_test.go b/admiral/pkg/clusters/serviceentry_test.go index 102e8deae..1922bfe98 100644 --- a/admiral/pkg/clusters/serviceentry_test.go +++ b/admiral/pkg/clusters/serviceentry_test.go @@ -1144,6 +1144,22 @@ func TestCreateServiceEntryForBlueGreenRolloutsUsecase(t *testing.T) { if nil == previewServiceEntryResp { t.Fatalf("Preview Service entry returned should not be empty") } + + // When Preview service is not defined in BlueGreen strategy + rollout.Spec.Strategy = argo.RolloutStrategy{ + BlueGreen: &argo.BlueGreenStrategy{ActiveService: ACTIVE_SERVICENAME}, + } + + se = modifyServiceEntryForNewServiceOrPod(admiral.Add, "test", "bar", rr) + + if len(se) != 1 { + t.Fatalf("Expected 1 service entries to be created but found %d", len(se)) + } + serviceEntryResp = se["test.test.mesh"] + + if nil == serviceEntryResp { + t.Fatalf("Service entry returned should not be empty") + } } func TestUpdateEndpointsForWeightedServices(t *testing.T) { From dc5178fa4cd173818652c3ff03c9b4c0fe6c9ae9 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 18 Mar 2022 18:47:34 +0530 Subject: [PATCH 11/17] Improved integration test for bluegreen rollout Signed-off-by: nbn01 --- install/scripts/install_sample_services.sh | 25 +++++++++++++++++++--- tests/run.sh | 3 +-- tests/test5.sh | 2 +- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/install/scripts/install_sample_services.sh b/install/scripts/install_sample_services.sh index 906c2ee7d..7ea2f6ed6 100755 --- a/install/scripts/install_sample_services.sh +++ b/install/scripts/install_sample_services.sh @@ -23,9 +23,6 @@ kubectl apply -f $install_dir/yaml/grpc.yaml kubectl apply -f $install_dir/yaml/sample_dep.yaml -# Update BlueGreen Rollout with new preview release -kubectl apply -f $install_dir/yaml/greeting_preview.yaml - #wait for the deployments to come up kubectl rollout status deployment greeting -n sample kubectl rollout status deployment webapp -n sample @@ -33,6 +30,28 @@ kubectl rollout status deployment webapp -n sample kubectl rollout status deployment webapp -n sample-rollout-bluegreen +checkRolloutStatus() { + rolloutName=$1 + namespace=$2 + status=$(kubectl get rollout -n $2 $1 -o jsonpath="{.status.readyReplicas}") + + if [[ "$status" == "1" ]]; then + return 0 + else + echo "Waiting rollout $1 in $2 namespace is not in Running phase $status" + return 1 + fi +} + +export -f checkRolloutStatus + +timeout 180s bash -c "until checkRolloutStatus greeting sample-rollout-bluegreen ; do sleep 10; done" +if [[ $? -eq 124 ]] + then + exit 1 +fi +# Update BlueGreen Rollout with new preview release +kubectl apply -f $install_dir/yaml/greeting_preview.yaml #Verify that admiral created service names for 'greeting' service checkse() { diff --git a/tests/run.sh b/tests/run.sh index 13c20a038..899d85123 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -28,7 +28,6 @@ $install_dir/scripts/install_sample_services.sh $install_dir sleep 10 ./test1.sh "webapp" "sample" "greeting" ./test2.sh "webapp" "sample-rollout-bluegreen" "greeting.bluegreen" -sleep 20 ./test5.sh "webapp" "sample-rollout-bluegreen" "greeting.bluegreen" ./test2.sh "webapp" "sample-rollout-canary" "greeting.canary" #cleanup to fee up the pipeline minkube resources @@ -41,4 +40,4 @@ fi ./test3.sh "grpc-client" "sample" "grpc-server" $install_dir ./test4.sh "webapp" "sample" -./cleanup.sh $istio_version \ No newline at end of file +# ./cleanup.sh $istio_version \ No newline at end of file diff --git a/tests/test5.sh b/tests/test5.sh index a139746d7..0b9bd0810 100755 --- a/tests/test5.sh +++ b/tests/test5.sh @@ -9,7 +9,7 @@ dest=$3 #Test output=$(kubectl exec --namespace=$source_ns -it $(kubectl get pod -l "app=$source" --namespace=$source_ns -o jsonpath='{.items[0].metadata.name}') -c $source -- curl -v "http://preview.stage.$dest.global" && echo "") -if [[ "$output" == *"Admiral"* ]]; then +if [[ "$output" == *"Admiral Preview"* ]]; then echo "Rollout BlueGreen Preview: PASS" exit 0 else From c7866d4c0c574c9a8f1249951a7b9f3af573cd00 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 25 Mar 2022 09:01:31 +0530 Subject: [PATCH 12/17] Continue with normal flow if preview condition fails Signed-off-by: nbn01 --- admiral/pkg/clusters/serviceentry.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index a4eee022d..93e790544 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -605,10 +605,9 @@ func createServiceEntryForRollout(event admiral.EventType, rc *RemoteController, if _, ok := rolloutServices[destRollout.Spec.Strategy.BlueGreen.PreviewService]; ok { previewGlobalFqdn := common.GetPreviewHostnamePrefix() + common.Sep + common.GetCnameForRollout(destRollout, workloadIdentityKey, common.GetHostnameSuffix()) previewAddress := getUniqueAddress(admiralCache, previewGlobalFqdn) - if len(previewGlobalFqdn) == 0 || len(previewAddress) == 0 { - return nil + if len(previewGlobalFqdn) != 0 && len(previewAddress) != 0 { + generateServiceEntry(event, admiralCache, meshPorts, previewGlobalFqdn, rc, serviceEntries, previewAddress, san) } - generateServiceEntry(event, admiralCache, meshPorts, previewGlobalFqdn, rc, serviceEntries, previewAddress, san) } } From 1f9086a8c506a952e2f0cd9ca4a217a8431c7935 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 25 Mar 2022 09:29:12 +0530 Subject: [PATCH 13/17] Uncomment clean up Signed-off-by: nbn01 --- tests/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run.sh b/tests/run.sh index 899d85123..620159f58 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -40,4 +40,4 @@ fi ./test3.sh "grpc-client" "sample" "grpc-server" $install_dir ./test4.sh "webapp" "sample" -# ./cleanup.sh $istio_version \ No newline at end of file +./cleanup.sh $istio_version \ No newline at end of file From 50c0ce5913980d09775eb579423f2dede4a39e33 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Thu, 31 Mar 2022 14:38:20 +0530 Subject: [PATCH 14/17] make "preview" standard prefix name for bluegreen Signed-off-by: nbn01 --- admiral/cmd/admiral/cmd/root.go | 11 ++-- admiral/pkg/clusters/serviceentry.go | 4 +- admiral/pkg/clusters/serviceentry_test.go | 1 - admiral/pkg/controller/common/common.go | 56 ++++++++++---------- admiral/pkg/controller/common/common_test.go | 12 ++--- admiral/pkg/controller/common/config.go | 7 +-- admiral/pkg/controller/common/config_test.go | 3 -- 7 files changed, 41 insertions(+), 53 deletions(-) diff --git a/admiral/cmd/admiral/cmd/root.go b/admiral/cmd/admiral/cmd/root.go index 0dcff8413..1e9254053 100644 --- a/admiral/cmd/admiral/cmd/root.go +++ b/admiral/cmd/admiral/cmd/root.go @@ -4,16 +4,15 @@ import ( "context" "flag" "fmt" - "os" - "os/signal" - "syscall" - "time" - "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/routes" "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/server" "github.com/istio-ecosystem/admiral/admiral/pkg/clusters" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" log "github.com/sirupsen/logrus" + "os" + "os/signal" + "syscall" + "time" "github.com/spf13/cobra" ) @@ -104,8 +103,6 @@ func GetRootCmd(args []string) *cobra.Command { "The label value, on a namespace, which tells Istio to perform sidecar injection") rootCmd.PersistentFlags().StringVar(¶ms.HostnameSuffix, "hostname_suffix", "global", "The hostname suffix to customize the cname generated by admiral. Default suffix value will be \"global\"") - rootCmd.PersistentFlags().StringVar(¶ms.PreviewHostnamePrefix, "preview_hostname_prefix", "preview", - "The hostname prefix to customize the cname generated by admiral for bluegreen rollout preview service. Default suffix value will be \"preview\"") rootCmd.PersistentFlags().StringVar(¶ms.LabelSet.WorkloadIdentityKey, "workload_identity_key", "identity", "The workload identity key, on deployment which holds identity value used to generate cname by admiral. Default label key will be \"identity\" Admiral will look for a label with this key. If present, that will be used. If not, it will try an annotation (for use cases where an identity is longer than 63 chars)") rootCmd.PersistentFlags().StringVar(¶ms.LabelSet.GlobalTrafficDeploymentLabel, "globaltraffic_deployment_label", "identity", diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index 93e790544..c977e3cf1 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -159,7 +159,7 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s activeServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.ActiveService previewServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService oldPorts := ep.Ports - if previewService, ok := weightedServices[previewServiceName]; strings.HasPrefix(key, common.GetPreviewHostnamePrefix()+common.Sep) && ok { + if previewService, ok := weightedServices[previewServiceName]; strings.HasPrefix(key, common.BlueGreenRolloutPreviewPrefix+common.Sep) && ok { previewServiceInstance := previewService.Service localFqdn := previewServiceInstance.Name + common.Sep + previewServiceInstance.Namespace + common.DotLocalDomainSuffix cnames[localFqdn] = "1" @@ -603,7 +603,7 @@ func createServiceEntryForRollout(event admiral.EventType, rc *RemoteController, 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.GetPreviewHostnamePrefix() + common.Sep + common.GetCnameForRollout(destRollout, workloadIdentityKey, common.GetHostnameSuffix()) + 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) diff --git a/admiral/pkg/clusters/serviceentry_test.go b/admiral/pkg/clusters/serviceentry_test.go index 1922bfe98..cd7ace3d2 100644 --- a/admiral/pkg/clusters/serviceentry_test.go +++ b/admiral/pkg/clusters/serviceentry_test.go @@ -1006,7 +1006,6 @@ func TestCreateServiceEntryForBlueGreenRolloutsUsecase(t *testing.T) { PreviewHostnamePrefix: "preview", } rr, _ := InitAdmiral(context.Background(), p) - t.Logf(common.GetPreviewHostnamePrefix()) config := rest.Config{ Host: "localhost", } diff --git a/admiral/pkg/controller/common/common.go b/admiral/pkg/controller/common/common.go index f0b2ba29a..77aa2e35c 100644 --- a/admiral/pkg/controller/common/common.go +++ b/admiral/pkg/controller/common/common.go @@ -1,37 +1,39 @@ package common import ( - "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" + "sort" + "strings" + + v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" log "github.com/sirupsen/logrus" k8sAppsV1 "k8s.io/api/apps/v1" k8sV1 "k8s.io/api/core/v1" - "sort" - "strings" ) const ( - NamespaceKubeSystem = "kube-system" - NamespaceIstioSystem = "istio-system" - Env = "env" - Http = "http" - Grpc = "grpc" - GrpcWeb = "grpc-web" - Http2 = "http2" - DefaultMtlsPort = 15443 - DefaultServiceEntryPort = 80 - Sep = "." - Dash = "-" - Slash = "/" - DotLocalDomainSuffix = ".svc.cluster.local" - Mesh = "mesh" - MulticlusterIngressGateway = "istio-multicluster-ingressgateway" - LocalAddressPrefix = "240.0" - NodeRegionLabel = "failure-domain.beta.kubernetes.io/region" - SpiffePrefix = "spiffe://" - SidecarEnabledPorts = "traffic.sidecar.istio.io/includeInboundPorts" - Default = "default" - AdmiralIgnoreAnnotation = "admiral.io/ignore" - AdmiralCnameCaseSensitive = "admiral.io/cname-case-sensitive" + NamespaceKubeSystem = "kube-system" + NamespaceIstioSystem = "istio-system" + Env = "env" + Http = "http" + Grpc = "grpc" + GrpcWeb = "grpc-web" + Http2 = "http2" + DefaultMtlsPort = 15443 + DefaultServiceEntryPort = 80 + Sep = "." + Dash = "-" + Slash = "/" + DotLocalDomainSuffix = ".svc.cluster.local" + Mesh = "mesh" + MulticlusterIngressGateway = "istio-multicluster-ingressgateway" + LocalAddressPrefix = "240.0" + NodeRegionLabel = "failure-domain.beta.kubernetes.io/region" + SpiffePrefix = "spiffe://" + SidecarEnabledPorts = "traffic.sidecar.istio.io/includeInboundPorts" + Default = "default" + AdmiralIgnoreAnnotation = "admiral.io/ignore" + AdmiralCnameCaseSensitive = "admiral.io/cname-case-sensitive" + BlueGreenRolloutPreviewPrefix = "preview" ) type Event int @@ -87,7 +89,7 @@ func GetCname(deployment *k8sAppsV1.Deployment, identifier string, nameSuffix st return strings.ToLower(cname) } -func GetCnameVal(vals[] string) string { +func GetCnameVal(vals []string) string { return strings.Join(vals, Sep) } @@ -250,4 +252,4 @@ func GetGtpEnv(gtp *v1.GlobalTrafficPolicy) string { environment = Default } return environment -} \ No newline at end of file +} diff --git a/admiral/pkg/controller/common/common_test.go b/admiral/pkg/controller/common/common_test.go index 29c19355f..299196a1c 100644 --- a/admiral/pkg/controller/common/common_test.go +++ b/admiral/pkg/controller/common/common_test.go @@ -1,17 +1,16 @@ package common import ( - "reflect" - "strings" - "testing" - "time" - "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" v12 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" k8sAppsV1 "k8s.io/api/apps/v1" k8sCoreV1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1" + "reflect" + "strings" + "testing" + "time" ) var ignoreUnexported = cmpopts.IgnoreUnexported(v12.GlobalTrafficPolicy{}.Status) @@ -30,7 +29,6 @@ func init() { SecretResolver: "", WorkloadSidecarName: "default", WorkloadSidecarUpdate: "disabled", - PreviewHostnamePrefix: "preview", } p.LabelSet.WorkloadIdentityKey = "identity" diff --git a/admiral/pkg/controller/common/config.go b/admiral/pkg/controller/common/config.go index 87c4080f0..8c7f01892 100644 --- a/admiral/pkg/controller/common/config.go +++ b/admiral/pkg/controller/common/config.go @@ -1,10 +1,9 @@ package common import ( + log "github.com/sirupsen/logrus" "sync" "time" - - log "github.com/sirupsen/logrus" ) var admiralParams = AdmiralParams{ @@ -72,10 +71,6 @@ func GetHostnameSuffix() string { return admiralParams.HostnameSuffix } -func GetPreviewHostnamePrefix() string { - return admiralParams.PreviewHostnamePrefix -} - func GetWorkloadIdentifier() string { return admiralParams.LabelSet.WorkloadIdentityKey } diff --git a/admiral/pkg/controller/common/config_test.go b/admiral/pkg/controller/common/config_test.go index c4ab32e64..0543105a8 100644 --- a/admiral/pkg/controller/common/config_test.go +++ b/admiral/pkg/controller/common/config_test.go @@ -50,9 +50,6 @@ func TestConfigManagement(t *testing.T) { if GetHostnameSuffix() != "mesh" { t.Errorf("Hostname suffix mismatch, expected mesh, got %v", GetHostnameSuffix()) } - if GetPreviewHostnamePrefix() != "preview" { - t.Errorf("Prefix for preview hostname mismatch, expected preview, got %v", GetPreviewHostnamePrefix()) - } if GetSyncNamespace() != "ns" { t.Errorf("Sync namespace mismatch, expected ns, got %v", GetSyncNamespace()) } From 42f513c1cd8a305054f243a52d8b10149dbd45bf Mon Sep 17 00:00:00 2001 From: nbn01 Date: Thu, 31 Mar 2022 20:02:42 +0530 Subject: [PATCH 15/17] abstract update preview endpoint to method Signed-off-by: nbn01 --- admiral/pkg/clusters/registry_test.go | 36 +++++++++++------------ admiral/pkg/clusters/serviceentry.go | 41 ++++++++++++++++----------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/admiral/pkg/clusters/registry_test.go b/admiral/pkg/clusters/registry_test.go index cf6bcbea0..56810b522 100644 --- a/admiral/pkg/clusters/registry_test.go +++ b/admiral/pkg/clusters/registry_test.go @@ -2,13 +2,9 @@ package clusters import ( "context" - "strings" - "testing" - "time" - "github.com/google/go-cmp/cmp" depModel "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model" - v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" + "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" "github.com/istio-ecosystem/admiral/admiral/pkg/test" @@ -21,6 +17,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "strings" + "testing" + "time" ) func init() { @@ -37,7 +36,6 @@ func init() { SecretResolver: "", WorkloadSidecarUpdate: "enabled", WorkloadSidecarName: "default", - PreviewHostnamePrefix: "preview", } p.LabelSet.WorkloadIdentityKey = "identity" @@ -451,24 +449,24 @@ func TestUpdateCacheController(t *testing.T) { //Struct of test case info. Name is required. testCases := []struct { - name string - oldConfig *rest.Config - newConfig *rest.Config - clusterId string + name string + oldConfig *rest.Config + newConfig *rest.Config + clusterId string shouldRefresh bool }{ { - name: "Should update controller when kubeconfig changes", - oldConfig: originalConfig, - newConfig: changedConfig, - clusterId: "test.cluster", + name: "Should update controller when kubeconfig changes", + oldConfig: originalConfig, + newConfig: changedConfig, + clusterId: "test.cluster", shouldRefresh: true, }, { - name: "Should not update controller when kubeconfig doesn't change", - oldConfig: originalConfig, - newConfig: originalConfig, - clusterId: "test.cluster", + name: "Should not update controller when kubeconfig doesn't change", + oldConfig: originalConfig, + newConfig: originalConfig, + clusterId: "test.cluster", shouldRefresh: false, }, } @@ -478,7 +476,7 @@ func TestUpdateCacheController(t *testing.T) { t.Run(c.name, func(t *testing.T) { hook := logTest.NewGlobal() rr.RemoteControllers[c.clusterId].ApiServer = c.oldConfig.Host - d, err := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, c.oldConfig, time.Second*time.Duration(300)) + d, err := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, c.oldConfig, time.Second*time.Duration(300)) if err != nil { t.Fatalf("Unexpected error creating controller %v", err) } diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index c977e3cf1..97bf7b8cf 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -142,9 +142,8 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s if len(sourceDeployments) > 0 { meshPorts = GetMeshPorts(sourceCluster, serviceInstance, sourceDeployments[sourceCluster]) - } else if !isBlueGreenStrategy { - meshPorts = GetMeshPortsForRollout(sourceCluster, serviceInstance, sourceRollouts[sourceCluster]) } + for key, serviceEntry := range serviceEntries { if len(serviceEntry.Endpoints) == 0 { AddServiceEntriesWithDr(remoteRegistry.AdmiralCache, map[string]string{sourceCluster: sourceCluster}, remoteRegistry.RemoteControllers, @@ -156,22 +155,9 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s if ep.Address == clusterIngress || ep.Address == "" { // Update endpoints with locafqdn for active and preview se of bluegreen rollout if isBlueGreenStrategy { - activeServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.ActiveService - previewServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService oldPorts := ep.Ports - if previewService, ok := weightedServices[previewServiceName]; strings.HasPrefix(key, 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, sourceRollouts[sourceCluster]) - } 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, sourceRollouts[sourceCluster]) - } + 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 @@ -180,6 +166,7 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s // see if we have weighted services (rollouts with canary strategy) } else if len(weightedServices) > 1 { //add one endpoint per each service, may be modify + meshPorts = GetMeshPortsForRollout(sourceCluster, serviceInstance, sourceRollouts[sourceCluster]) var se = copyServiceEntry(serviceEntry) updateEndpointsForWeightedServices(se, weightedServices, clusterIngress, meshPorts) AddServiceEntriesWithDr(remoteRegistry.AdmiralCache, map[string]string{sourceCluster: sourceCluster}, remoteRegistry.RemoteControllers, @@ -209,6 +196,26 @@ 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) From c97ed13fa432df2b5b1225f29f38170fea158ef7 Mon Sep 17 00:00:00 2001 From: nbn01 Date: Fri, 1 Apr 2022 12:33:54 +0530 Subject: [PATCH 16/17] Add unit test Signed-off-by: nbn01 --- admiral/pkg/clusters/serviceentry_test.go | 77 +++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/admiral/pkg/clusters/serviceentry_test.go b/admiral/pkg/clusters/serviceentry_test.go index cd7ace3d2..9a8387a0e 100644 --- a/admiral/pkg/clusters/serviceentry_test.go +++ b/admiral/pkg/clusters/serviceentry_test.go @@ -1161,6 +1161,83 @@ func TestCreateServiceEntryForBlueGreenRolloutsUsecase(t *testing.T) { } } +func TestUpdateEndpointsForBlueGreen(t *testing.T) { + const CLUSTER_INGRESS_1 = "ingress1.com" + const ACTIVE_SERVICE = "activeService" + const PREVIEW_SERVICE = "previewService" + const NAMESPACE = "namespace" + const ACTIVE_MESH_HOST = "qal.example.mesh" + const PREVIEW_MESH_HOST = "preview.qal.example.mesh" + + rollout := argo.Rollout{} + rollout.Spec.Strategy = argo.RolloutStrategy{ + BlueGreen: &argo.BlueGreenStrategy{ + ActiveService: ACTIVE_SERVICE, + PreviewService: PREVIEW_SERVICE, + }, + } + rollout.Spec.Template.Annotations = map[string]string{} + rollout.Spec.Template.Annotations[common.SidecarEnabledPorts] = "8080" + + endpoint := istionetworkingv1alpha3.ServiceEntry_Endpoint{ + Labels: map[string]string{}, Address: CLUSTER_INGRESS_1, Ports: map[string]uint32{"http": 15443}, + } + + meshPorts := map[string]uint32{"http": 8080} + + weightedServices := map[string]*WeightedService{ + ACTIVE_SERVICE: {Service: &v1.Service{ObjectMeta: v12.ObjectMeta{Name: ACTIVE_SERVICE, Namespace: NAMESPACE}}}, + PREVIEW_SERVICE: {Service: &v1.Service{ObjectMeta: v12.ObjectMeta{Name: PREVIEW_SERVICE, Namespace: NAMESPACE}}}, + } + + activeWantedEndpoints := istionetworkingv1alpha3.ServiceEntry_Endpoint{ + Address: ACTIVE_SERVICE + common.Sep + NAMESPACE + common.DotLocalDomainSuffix, Ports: meshPorts, + } + + previewWantedEndpoints := istionetworkingv1alpha3.ServiceEntry_Endpoint{ + Address: PREVIEW_SERVICE + common.Sep + NAMESPACE + common.DotLocalDomainSuffix, Ports: meshPorts, + } + + testCases := []struct { + name string + rollout argo.Rollout + inputEndpoint istionetworkingv1alpha3.ServiceEntry_Endpoint + weightedServices map[string]*WeightedService + clusterIngress string + meshPorts map[string]uint32 + meshHost string + wantedEndpoints istionetworkingv1alpha3.ServiceEntry_Endpoint + }{ + { + name: "should return endpoint with active service address", + rollout: rollout, + inputEndpoint: endpoint, + weightedServices: weightedServices, + meshPorts: meshPorts, + meshHost: ACTIVE_MESH_HOST, + wantedEndpoints: activeWantedEndpoints, + }, + { + name: "should return endpoint with preview service address", + rollout: rollout, + inputEndpoint: endpoint, + weightedServices: weightedServices, + meshPorts: meshPorts, + meshHost: PREVIEW_MESH_HOST, + wantedEndpoints: previewWantedEndpoints, + }, + } + + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + updateEndpointsForBlueGreen(&c.rollout, c.weightedServices, map[string]string{}, &c.inputEndpoint, "test", c.meshHost) + if c.inputEndpoint.Address != c.wantedEndpoints.Address { + t.Errorf("Wanted %s endpoint, got: %s", c.wantedEndpoints.Address, c.inputEndpoint.Address) + } + }) + } +} + func TestUpdateEndpointsForWeightedServices(t *testing.T) { t.Parallel() From 8b489df5ffd3651aba0a57195854621c5167a0ba Mon Sep 17 00:00:00 2001 From: nbn01 Date: Tue, 5 Apr 2022 18:17:45 +0530 Subject: [PATCH 17/17] fix golintci lint to use go1.17.7 Signed-off-by: nbn01 --- .github/workflows/golang-ci-lint.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/golang-ci-lint.yml b/.github/workflows/golang-ci-lint.yml index 01ef830df..0e12e1e15 100644 --- a/.github/workflows/golang-ci-lint.yml +++ b/.github/workflows/golang-ci-lint.yml @@ -13,11 +13,15 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 + - uses: actions/setup-go@v3 + with: + go-version: '1.17.7' - name: golangci-lint uses: golangci/golangci-lint-action@v2 with: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. version: v1.43.0 + skip-go-installation: true # Optional: working directory, useful for monorepos # working-directory: somedir