From 9a353674eae3288ed977a844da145ef14864df5d Mon Sep 17 00:00:00 2001 From: Punakshi Date: Thu, 29 Aug 2024 19:03:16 -0700 Subject: [PATCH 1/9] MESH-MESH-5405-IdentityConfig-Struct-Edit --- admiral/pkg/clusters/configwriter.go | 61 ++++++++-------- admiral/pkg/clusters/configwriter_test.go | 6 +- admiral/pkg/clusters/serviceentry.go | 32 ++++++--- admiral/pkg/clusters/shard_handler.go | 5 +- ...expectedIdentityIdentityConfiguration.json | 5 +- .../identityIdentityConfiguration.json | 5 +- ...eshtestblackholeIdentityConfiguration.json | 18 +++-- ...meshtestinboundsIdentityConfiguration.json | 9 ++- .../testdata/sampleIdentityConfiguration.json | 70 ++++++++++++++----- admiral/pkg/registry/config.go | 21 +++--- .../testdata/sampleIdentityConfiguration.json | 27 ++++--- admiral/pkg/registry/testutils.go | 9 ++- 12 files changed, 176 insertions(+), 92 deletions(-) diff --git a/admiral/pkg/clusters/configwriter.go b/admiral/pkg/clusters/configwriter.go index b753b32b5..f114aeb28 100644 --- a/admiral/pkg/clusters/configwriter.go +++ b/admiral/pkg/clusters/configwriter.go @@ -58,6 +58,9 @@ func (b *ServiceEntryBuilder) BuildServiceEntriesFromIdentityConfig(ctxLogger *l serverCluster := identityConfigCluster.Name for _, identityConfigEnvironment := range identityConfigCluster.Environment { env := identityConfigEnvironment.Name + if len(identityConfigEnvironment.Services) == 0 { + return serviceEntries, fmt.Errorf("there were no services for the asset in namespace %s on cluster %s", identityConfigEnvironment.Namespace, serverCluster) + } var tmpSe *networkingV1Alpha3.ServiceEntry start = time.Now() endpoints, err := getServiceEntryEndpoints(ctxLogger, b.ClientCluster, serverCluster, ingressEndpoints, identityConfigEnvironment) @@ -118,50 +121,52 @@ func getServiceEntryEndpoints( serverCluster string, ingressEndpoints map[string]*networkingV1Alpha3.WorkloadEntry, identityConfigEnvironment *registry.IdentityConfigEnvironment) ([]*networkingV1Alpha3.WorkloadEntry, error) { - if len(identityConfigEnvironment.Services) == 0 { - return nil, fmt.Errorf("there were no services for the asset in namespace %s on cluster %s", identityConfigEnvironment.Namespace, serverCluster) - } var err error endpoint := ingressEndpoints[serverCluster] endpoints := []*networkingV1Alpha3.WorkloadEntry{} tmpEp := endpoint.DeepCopy() - tmpEp.Labels[typeLabel] = identityConfigEnvironment.Type services := []*registry.RegistryServiceConfig{} for _, service := range identityConfigEnvironment.Services { services = append(services, service) } sort.Sort(registry.RegistryServiceConfigSorted(services)) // Deployment won't have weights, so just sort and take the first service to use as the endpoint - if identityConfigEnvironment.Type == common.Deployment { - if clientCluster == serverCluster { - tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - tmpEp.Ports = services[0].Ports - } - endpoints = append(endpoints, tmpEp) - } - // Rollout without weights is treated the same as deployment so sort and take first service - // If any of the services have weights then add them to the list of endpoints - if identityConfigEnvironment.Type == common.Rollout { - for _, service := range services { - if service.Weight > 0 { - weightedEp := tmpEp.DeepCopy() - weightedEp.Weight = uint32(service.Weight) - if clientCluster == serverCluster { - weightedEp.Address = service.Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - weightedEp.Ports = service.Ports - } - endpoints = append(endpoints, weightedEp) - } - } - // If we go through all the services associated with the rollout and none have applicable weights then endpoints is empty - // Treat the rollout like a deployment and sort and take the first service - if len(endpoints) == 0 { + for resourceType, _ := range identityConfigEnvironment.Type { + if resourceType == common.Deployment { if clientCluster == serverCluster { tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() tmpEp.Ports = services[0].Ports } + tmpEp.Labels[typeLabel] = resourceType endpoints = append(endpoints, tmpEp) } + // Rollout without weights is treated the same as deployment so sort and take first service + // If any of the services have weights then add them to the list of endpoints + if resourceType == common.Rollout { + for _, service := range services { + if service.Weight > 0 { + weightedEp := tmpEp.DeepCopy() + weightedEp.Weight = uint32(service.Weight) + if clientCluster == serverCluster { + weightedEp.Address = service.Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + weightedEp.Ports = service.Ports + } + weightedEp.Labels[typeLabel] = resourceType + endpoints = append(endpoints, weightedEp) + } + } + // If we go through all the services associated with the rollout and none have applicable weights then endpoints is empty + // Treat the rollout like a deployment and sort and take the first service + if len(endpoints) == 0 { + if clientCluster == serverCluster { + tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + tmpEp.Ports = services[0].Ports + + } + tmpEp.Labels[typeLabel] = resourceType + endpoints = append(endpoints, tmpEp) + } + } } // TODO: type is rollout, strategy is bluegreen, need a way to know which service is preview/desired, trigger another SE // TODO: type is rollout, strategy is canary, need a way to know which service is stable/root/desired, trigger another SE diff --git a/admiral/pkg/clusters/configwriter_test.go b/admiral/pkg/clusters/configwriter_test.go index fdb7a0dab..01ad4f825 100644 --- a/admiral/pkg/clusters/configwriter_test.go +++ b/admiral/pkg/clusters/configwriter_test.go @@ -106,7 +106,11 @@ func TestGetServiceEntryEndpoints(t *testing.T) { common.InitializeConfig(admiralParams) e2eEnv := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") unweightedDeployment := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") - unweightedDeployment.Type = common.Deployment + unweightedDeployment.Type = map[string]*registry.TypeConfig{ + "deployment": { + Selectors: map[string]string{"app": "app1"}, + }, + } weightedServices := map[string]*registry.RegistryServiceConfig{ "app-1-spk-root-service": { Name: "app-1-spk-root-service", diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index fc93db121..429b7393e 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -45,6 +45,7 @@ type SeDrTuple struct { } const ( + intuitHostSuffix = "intuit" resourceCreatedByAnnotationLabel = "app.kubernetes.io/created-by" resourceCreatedByAnnotationValue = "admiral" resourceCreatedByAnnotationCartographerValue = "cartographer" @@ -54,6 +55,10 @@ const ( gtpManagedByMeshAgent = "mesh-agent" gtpManagerMeshAgentFieldValue = "ewok-mesh-agent" errorCluster = "error-cluster" + bluegreenStrategy = "bluegreen" + canaryStrategy = "canary" + deployToRolloutStrategy = "deployToRollout" + rolloutToDeployStrategy = "rolloutToDeploy" ingressVSGenerationErrorMessage = "skipped generating ingress virtual service on cluster %s due to error %w" ) @@ -213,9 +218,7 @@ func modifyServiceEntryForNewServiceOrPod( Locality: getLocality(rc), IngressEndpoint: ingressEndpoint, IngressPort: strconv.Itoa(port), - Environment: map[string]*registry.IdentityConfigEnvironment{ - env: ®istry.IdentityConfigEnvironment{}, - }, + Environment: map[string]*registry.IdentityConfigEnvironment{}, } // For Deployment <-> Rollout migration @@ -290,7 +293,7 @@ func modifyServiceEntryForNewServiceOrPod( registryConfig.Clusters[clusterId].Environment[env] = ®istry.IdentityConfigEnvironment{ Name: env, Namespace: namespace, - Type: common.Deployment, + Type: map[string]*registry.TypeConfig{common.Deployment: {Selectors: deployment.Spec.Selector.MatchLabels}}, } if common.IsVSBasedRoutingEnabled() { err := generateIngressVirtualServiceForDeployment(deployment, sourceIngressVirtualService) @@ -349,7 +352,7 @@ func modifyServiceEntryForNewServiceOrPod( registryConfig.Clusters[clusterId].Environment[env] = ®istry.IdentityConfigEnvironment{ Name: env, Namespace: namespace, - Type: common.Rollout, + Type: map[string]*registry.TypeConfig{common.Rollout: {Selectors: rollout.Spec.Selector.MatchLabels}}, } if common.IsVSBasedRoutingEnabled() { @@ -432,7 +435,7 @@ func modifyServiceEntryForNewServiceOrPod( } } - //PID: use partitionedIdentity because IdentityDependencyCache is filled using the partitionedIdentity - DONE + // use partitionedIdentity because IdentityDependencyCache is filled using the partitionedIdentity dependents := remoteRegistry.AdmiralCache.IdentityDependencyCache.Get(partitionedIdentity).Copy() // updates CnameDependentClusterCache and CnameDependentClusterNamespaceCache cname = strings.TrimSpace(cname) @@ -589,8 +592,13 @@ func modifyServiceEntryForNewServiceOrPod( registryConfig.Clusters[sourceCluster].Environment[env] = ®istry.IdentityConfigEnvironment{ Name: env, Namespace: namespace, - Type: common.Rollout, - Event: admiral.Delete, // TODO: we need to handle DELETE operations in admiral operator + Type: map[string]*registry.TypeConfig{ + eventResourceType: { + Selectors: serviceInstance[appType[sourceCluster]].Spec.Selector, + }, + }, + Event: admiral.Delete, + //TODO: we need to handle DELETE operations in admiral operator } continue } @@ -623,12 +631,13 @@ func modifyServiceEntryForNewServiceOrPod( cnames, ep, sourceCluster, key) if common.IsAdmiralStateSyncerMode() { registryConfig.Clusters[sourceCluster].Environment[env].Services = map[string]*registry.RegistryServiceConfig{ - blueGreenService.Service.Name: ®istry.RegistryServiceConfig{ + blueGreenService.Service.Name: { Name: blueGreenService.Service.Name, Weight: -1, Ports: GetMeshPortsForRollout(sourceCluster, blueGreenService.Service, sourceRollouts[sourceCluster]), }, } + registryConfig.Clusters[sourceCluster].Environment[env].Type[common.Rollout].Strategy = bluegreenStrategy continue } err := remoteRegistry.ConfigWriter.AddServiceEntriesWithDrToAllCluster( @@ -652,12 +661,13 @@ func modifyServiceEntryForNewServiceOrPod( // use only canary service for fqdn if common.IsAdmiralStateSyncerMode() { registryConfig.Clusters[sourceCluster].Environment[env].Services = map[string]*registry.RegistryServiceConfig{ - canaryService: ®istry.RegistryServiceConfig{ + canaryService: { Name: canaryService, Weight: -1, Ports: meshPorts, }, } + registryConfig.Clusters[sourceCluster].Environment[env].Type[common.Rollout].Strategy = canaryStrategy continue } fqdn := canaryService + common.Sep + serviceInstance[appType[sourceCluster]].Namespace + common.GetLocalDomainSuffix() @@ -727,7 +737,7 @@ func modifyServiceEntryForNewServiceOrPod( // call State Syncer's config syncer for deployment if common.IsAdmiralStateSyncerMode() { registryConfig.Clusters[sourceCluster].Environment[env].Services = map[string]*registry.RegistryServiceConfig{ - localFqdn: ®istry.RegistryServiceConfig{ + localFqdn: { Name: localFqdn, Weight: 0, Ports: meshPorts, diff --git a/admiral/pkg/clusters/shard_handler.go b/admiral/pkg/clusters/shard_handler.go index 10553bbea..7ffca8ef0 100644 --- a/admiral/pkg/clusters/shard_handler.go +++ b/admiral/pkg/clusters/shard_handler.go @@ -193,7 +193,10 @@ func ConsumeIdentityConfigs(ctxLogger *log.Entry, ctx context.Context, configWri // Get any type from the identityConfig for _, cv := range identityConfig.Clusters { for _, ev := range cv.Environment { - ctx = context.WithValue(ctx, common.EventResourceType, ev.Type) + for typeKey := range ev.Type { + ctx = context.WithValue(ctx, common.EventResourceType, typeKey) + break + } break } break diff --git a/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json b/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json index ef742cf7c..460a8d99e 100644 --- a/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json +++ b/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json @@ -13,8 +13,9 @@ "namespace": "foobar-ns", "services": null, "serviceName": "", - "type": "rollout", - "selectors": null, + "type": { + "rollout": null + }, "ports": [ { "number": 80, diff --git a/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json b/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json index ef742cf7c..460a8d99e 100644 --- a/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json +++ b/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json @@ -13,8 +13,9 @@ "namespace": "foobar-ns", "services": null, "serviceName": "", - "type": "rollout", - "selectors": null, + "type": { + "rollout": null + }, "ports": [ { "number": 80, diff --git a/admiral/pkg/clusters/testdata/ppdmeshtestblackholeIdentityConfiguration.json b/admiral/pkg/clusters/testdata/ppdmeshtestblackholeIdentityConfiguration.json index b794c288e..f834db00f 100644 --- a/admiral/pkg/clusters/testdata/ppdmeshtestblackholeIdentityConfiguration.json +++ b/admiral/pkg/clusters/testdata/ppdmeshtestblackholeIdentityConfiguration.json @@ -23,9 +23,12 @@ } } }, - "type": "deployment", - "selectors": { - "app": "blackhole-gw" + "type": { + "deployment": { + "selectors": { + "app": "blackhole-gw" + } + } }, "ports": [ { @@ -130,9 +133,12 @@ } } }, - "type": "deployment", - "selectors": { - "app": "blackhole-gw" + "type": { + "deployment": { + "selectors": { + "app": "blackhole-gw" + } + } }, "ports": [ { diff --git a/admiral/pkg/clusters/testdata/ppdmeshtestinboundsIdentityConfiguration.json b/admiral/pkg/clusters/testdata/ppdmeshtestinboundsIdentityConfiguration.json index bc564a406..ece9e7650 100644 --- a/admiral/pkg/clusters/testdata/ppdmeshtestinboundsIdentityConfiguration.json +++ b/admiral/pkg/clusters/testdata/ppdmeshtestinboundsIdentityConfiguration.json @@ -23,9 +23,12 @@ } } }, - "type": "deployment", - "selectors": { - "app": "inbound-gw" + "type": { + "deployment": { + "selectors": { + "app": "inbound-gw" + } + } }, "ports": [ { diff --git a/admiral/pkg/clusters/testdata/sampleIdentityConfiguration.json b/admiral/pkg/clusters/testdata/sampleIdentityConfiguration.json index 59f6eebb3..b5195d89a 100644 --- a/admiral/pkg/clusters/testdata/sampleIdentityConfiguration.json +++ b/admiral/pkg/clusters/testdata/sampleIdentityConfiguration.json @@ -13,19 +13,41 @@ "prf": { "name": "prf", "namespace": "ns-1-usw2-prf", - "serviceName": "app-1-spk-root-service", "services": { - "app-1-spk-root-service": { + "root": { "name": "app-1-spk-root-service", "weight": -1, "ports": { "http": 8090 } + }, + "stable": { + "name": "app-1-spk-stable-service", + "weight": 25, + "ports": { + "http": 8090 + } + }, + "desired": { + "name": "app-1-spk-desired-service", + "weight": 75, + "ports": { + "http": 8090 + } } }, - "type": "rollout", - "selectors": { - "app": "app-1" + "type": { + "rollout": { + "strategy": "canary", + "selectors": { + "app": "app-1" + } + }, + "deployment": { + "selectors": { + "app": "app-1" + } + } }, "ports": [ { @@ -109,19 +131,28 @@ "e2e": { "name": "e2e", "namespace": "ns-1-usw2-e2e", - "serviceName": "app-1-spk-root-service", + "strategy": "bluegreen", "services": { - "app-1-spk-root-service": { - "name": "app-1-spk-root-service", - "weight": -1, + "preview": { + "name": "app-1-spk-preview-service", + "ports": { + "http": 8090 + } + }, + "desired": { + "name": "app-1-spk-desired-service", "ports": { "http": 8090 } } }, - "type": "rollout", - "selectors": { - "app": "app-1" + "type": { + "rollout": { + "strategy": "canary", + "selectors": { + "app": "app-1" + } + } }, "ports": [ { @@ -205,19 +236,22 @@ "qal": { "name": "qal", "namespace": "ns-1-usw2-qal", - "serviceName": "app-1-spk-root-service", "services": { - "app-1-spk-root-service": { - "name": "app-1-spk-root-service", + "app-1-spk-service": { + "name": "app-1-spk-service", "weight": -1, "ports": { "http": 8090 } } }, - "type": "rollout", - "selectors": { - "app": "app-1" + "type": { + "rollout": { + "strategy": "canary", + "selectors": { + "app": "app-1" + } + } }, "ports": [ { diff --git a/admiral/pkg/registry/config.go b/admiral/pkg/registry/config.go index 6515ea98b..5d75e955d 100644 --- a/admiral/pkg/registry/config.go +++ b/admiral/pkg/registry/config.go @@ -17,12 +17,13 @@ func (config *IdentityConfig) PutClusterConfig(name string, clusterConfig Identi } type IdentityConfigCluster struct { - Name string `json:"name"` - Locality string `json:"locality"` - IngressEndpoint string `json:"ingressEndpoint"` - IngressPort string `json:"ingressPort"` - IngressPortName string `json:"ingressPortName"` - Environment map[string]*IdentityConfigEnvironment `json:"environment"` + Name string `json:"name"` + Locality string `json:"locality"` + IngressEndpoint string `json:"ingressEndpoint"` + IngressPort string `json:"ingressPort"` + IngressPortName string `json:"ingressPortName"` + // env -> rollout/deploy -> IdentityConfigEnvironment + Environment map[string]*IdentityConfigEnvironment `json:"environment"` } func (config *IdentityConfigCluster) PutEnvironment(name string, environmentConfig IdentityConfigEnvironment) error { @@ -45,13 +46,17 @@ type TrafficPolicy struct { ClientConnectionConfig admiralV1Alpha1.ClientConnectionConfig `json:"clientconnectionconfig"` } +type TypeConfig struct { + Strategy string `json:"strategy"` + Selectors map[string]string `json:"selectors"` +} + type IdentityConfigEnvironment struct { Name string `json:"name"` Namespace string `json:"namespace"` Services map[string]*RegistryServiceConfig `json:"services"` ServiceName string `json:"serviceName"` - Type string `json:"type"` - Selectors map[string]string `json:"selectors"` + Type map[string]*TypeConfig `json:"type"` Ports []*networking.ServicePort `json:"ports"` TrafficPolicy TrafficPolicy `json:"trafficPolicy"` Event admiral.EventType `json:"event"` diff --git a/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json b/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json index 59f6eebb3..c0dbf437a 100644 --- a/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json +++ b/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json @@ -23,9 +23,12 @@ } } }, - "type": "rollout", - "selectors": { - "app": "app-1" + "type": { + "rollout": { + "selectors": { + "app": "app1" + } + } }, "ports": [ { @@ -119,9 +122,12 @@ } } }, - "type": "rollout", - "selectors": { - "app": "app-1" + "type": { + "rollout": { + "selectors": { + "app": "app1" + } + } }, "ports": [ { @@ -215,9 +221,12 @@ } } }, - "type": "rollout", - "selectors": { - "app": "app-1" + "type": { + "rollout": { + "selectors": { + "app": "app1" + } + } }, "ports": [ { diff --git a/admiral/pkg/registry/testutils.go b/admiral/pkg/registry/testutils.go index df3fed29f..a1c7963a8 100644 --- a/admiral/pkg/registry/testutils.go +++ b/admiral/pkg/registry/testutils.go @@ -21,9 +21,12 @@ func GetSampleIdentityConfigEnvironment(env string, namespace string, identity s }, }, }, - Type: "rollout", - Selectors: map[string]string{"app": "app-1"}, - Ports: []*networking.ServicePort{{Name: "http", Number: uint32(80), Protocol: "http"}}, + Type: map[string]*TypeConfig{ + "rollout": { + Selectors: map[string]string{"app": "app1"}, + }, + }, + Ports: []*networking.ServicePort{{Name: "http", Number: uint32(80), Protocol: "http"}}, TrafficPolicy: TrafficPolicy{ ClientConnectionConfig: v1alpha1.ClientConnectionConfig{ ObjectMeta: v1.ObjectMeta{ From 552b16cb25f6def5e4c3923480b153fdab990a15 Mon Sep 17 00:00:00 2001 From: Punakshi Date: Thu, 29 Aug 2024 19:14:37 -0700 Subject: [PATCH 2/9] Unit test fix --- .../testdata/expectedIdentityIdentityConfiguration.json | 8 +++++++- .../clusters/testdata/identityIdentityConfiguration.json | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json b/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json index 460a8d99e..86fe30651 100644 --- a/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json +++ b/admiral/pkg/clusters/testdata/expectedIdentityIdentityConfiguration.json @@ -14,7 +14,13 @@ "services": null, "serviceName": "", "type": { - "rollout": null + "rollout": { + "strategy": "", + "selectors": { + "app": "identity", + "identity": "identity" + } + } }, "ports": [ { diff --git a/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json b/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json index 460a8d99e..86fe30651 100644 --- a/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json +++ b/admiral/pkg/clusters/testdata/identityIdentityConfiguration.json @@ -14,7 +14,13 @@ "services": null, "serviceName": "", "type": { - "rollout": null + "rollout": { + "strategy": "", + "selectors": { + "app": "identity", + "identity": "identity" + } + } }, "ports": [ { From 657a3140ab4e1c9d602d8f6712ccd5bb931a1f0a Mon Sep 17 00:00:00 2001 From: Punakshi Date: Sun, 1 Sep 2024 11:33:26 -0700 Subject: [PATCH 3/9] Logic to add SE for bluegreen, canary, dep to rollout migration --- admiral/pkg/clusters/configwriter.go | 150 ++++++++++++++-------- admiral/pkg/clusters/configwriter_test.go | 9 +- 2 files changed, 104 insertions(+), 55 deletions(-) diff --git a/admiral/pkg/clusters/configwriter.go b/admiral/pkg/clusters/configwriter.go index f114aeb28..4bd554088 100644 --- a/admiral/pkg/clusters/configwriter.go +++ b/admiral/pkg/clusters/configwriter.go @@ -15,7 +15,13 @@ import ( networkingV1Alpha3 "istio.io/api/networking/v1alpha3" ) -const typeLabel = "type" +const ( + typeLabel = "type" + previewServiceKey = "preview" + activeServiceKey = "active" + desiredServiceKey = "canary" + rootServiceKey = "root" +) // IstioSEBuilder is an interface to construct Service Entry objects // from IdentityConfig objects. It can construct multiple Service Entries @@ -35,7 +41,7 @@ type ServiceEntryBuilder struct { func (b *ServiceEntryBuilder) BuildServiceEntriesFromIdentityConfig(ctxLogger *logrus.Entry, identityConfig registry.IdentityConfig) ([]*networkingV1Alpha3.ServiceEntry, error) { var ( identity = identityConfig.IdentityName - seMap = map[string]*networkingV1Alpha3.ServiceEntry{} + seMap = map[string]map[string]*networkingV1Alpha3.ServiceEntry{} serviceEntries = []*networkingV1Alpha3.ServiceEntry{} start = time.Now() err error @@ -61,37 +67,58 @@ func (b *ServiceEntryBuilder) BuildServiceEntriesFromIdentityConfig(ctxLogger *l if len(identityConfigEnvironment.Services) == 0 { return serviceEntries, fmt.Errorf("there were no services for the asset in namespace %s on cluster %s", identityConfigEnvironment.Namespace, serverCluster) } - var tmpSe *networkingV1Alpha3.ServiceEntry + start = time.Now() - endpoints, err := getServiceEntryEndpoints(ctxLogger, b.ClientCluster, serverCluster, ingressEndpoints, identityConfigEnvironment) - util.LogElapsedTimeSince("getServiceEntryEndpoint", identity, env, b.ClientCluster, start) - if err != nil { - return serviceEntries, err - } - if se, ok := seMap[env]; !ok { - tmpSe = &networkingV1Alpha3.ServiceEntry{ - Hosts: []string{common.GetCnameVal([]string{env, strings.ToLower(identity), common.GetHostnameSuffix()})}, - Ports: identityConfigEnvironment.Ports, - Location: networkingV1Alpha3.ServiceEntry_MESH_INTERNAL, - Resolution: networkingV1Alpha3.ServiceEntry_DNS, - SubjectAltNames: []string{common.SpiffePrefix + common.GetSANPrefix() + common.Slash + identity}, - Endpoints: endpoints, - ExportTo: dependentNamespaces, + meshHosts := getMeshHosts(identity, identityConfigEnvironment) + for _, host := range meshHosts { + var tmpSe *networkingV1Alpha3.ServiceEntry + endpoints, err := getServiceEntryEndpoints(ctxLogger, b.ClientCluster, serverCluster, host, ingressEndpoints, identityConfigEnvironment) + util.LogElapsedTimeSince("getServiceEntryEndpoint", identity, env, b.ClientCluster, start) + if err != nil { + return serviceEntries, err + } + if se, ok := seMap[env][host]; !ok { + tmpSe = &networkingV1Alpha3.ServiceEntry{ + Hosts: []string{host}, + Ports: identityConfigEnvironment.Ports, + Location: networkingV1Alpha3.ServiceEntry_MESH_INTERNAL, + Resolution: networkingV1Alpha3.ServiceEntry_DNS, + SubjectAltNames: []string{common.SpiffePrefix + common.GetSANPrefix() + common.Slash + identity}, + Endpoints: endpoints, + ExportTo: dependentNamespaces, + } + } else { + tmpSe = se + tmpSe.Endpoints = append(tmpSe.Endpoints, endpoints...) } - } else { - tmpSe = se - tmpSe.Endpoints = append(tmpSe.Endpoints, endpoints...) + sort.Sort(WorkloadEntrySorted(tmpSe.Endpoints)) + seMap[env] = map[string]*networkingV1Alpha3.ServiceEntry{host: tmpSe} } - sort.Sort(WorkloadEntrySorted(tmpSe.Endpoints)) - seMap[env] = tmpSe } } - for _, se := range seMap { - serviceEntries = append(serviceEntries, se) + for _, seForEnv := range seMap { + for _, se := range seForEnv { + serviceEntries = append(serviceEntries, se) + } } return serviceEntries, err } +func getMeshHosts(identity string, identityConfigEnvironment *registry.IdentityConfigEnvironment) []string { + meshHosts := []string{} + meshHosts = append(meshHosts, common.GetCnameVal([]string{identityConfigEnvironment.Name, strings.ToLower(identity), common.GetHostnameSuffix()})) + if identityConfigEnvironment.Type[common.Rollout] != nil { + strategy := identityConfigEnvironment.Type[common.Rollout].Strategy + if strategy == bluegreenStrategy { + meshHosts = append(meshHosts, common.GetCnameVal([]string{previewServiceKey, strings.ToLower(identity), common.GetHostnameSuffix()})) + } + if strategy == canaryStrategy { + meshHosts = append(meshHosts, common.GetCnameVal([]string{canaryStrategy, strings.ToLower(identity), common.GetHostnameSuffix()})) + } + } + return meshHosts +} + // getIngressEndpoints constructs the endpoint of the ingress gateway/remote endpoint for an identity // by reading the information directly from the IdentityConfigCluster. func getIngressEndpoints(clusters map[string]*registry.IdentityConfigCluster) (map[string]*networkingV1Alpha3.WorkloadEntry, error) { @@ -119,6 +146,7 @@ func getServiceEntryEndpoints( ctxLogger *logrus.Entry, clientCluster string, serverCluster string, + host string, ingressEndpoints map[string]*networkingV1Alpha3.WorkloadEntry, identityConfigEnvironment *registry.IdentityConfigEnvironment) ([]*networkingV1Alpha3.WorkloadEntry, error) { var err error @@ -126,51 +154,65 @@ func getServiceEntryEndpoints( endpoints := []*networkingV1Alpha3.WorkloadEntry{} tmpEp := endpoint.DeepCopy() services := []*registry.RegistryServiceConfig{} + servicesMap := identityConfigEnvironment.Services for _, service := range identityConfigEnvironment.Services { services = append(services, service) } sort.Sort(registry.RegistryServiceConfigSorted(services)) // Deployment won't have weights, so just sort and take the first service to use as the endpoint for resourceType, _ := range identityConfigEnvironment.Type { - if resourceType == common.Deployment { - if clientCluster == serverCluster { - tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - tmpEp.Ports = services[0].Ports - } - tmpEp.Labels[typeLabel] = resourceType - endpoints = append(endpoints, tmpEp) - } // Rollout without weights is treated the same as deployment so sort and take first service // If any of the services have weights then add them to the list of endpoints if resourceType == common.Rollout { - for _, service := range services { - if service.Weight > 0 { - weightedEp := tmpEp.DeepCopy() - weightedEp.Weight = uint32(service.Weight) - if clientCluster == serverCluster { - weightedEp.Address = service.Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - weightedEp.Ports = service.Ports + ep := tmpEp.DeepCopy() + if clientCluster == serverCluster { + if identityConfigEnvironment.Type[resourceType].Strategy == canaryStrategy { + if strings.HasPrefix(host, canaryStrategy) { + ep.Address = servicesMap[desiredServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } else { + for _, service := range services { + if service.Weight > 0 { + weightedep := ep.DeepCopy() + weightedep.Ports = services[0].Ports + weightedep.Address = service.Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + weightedep.Weight = uint32(service.Weight) + weightedep.Labels[typeLabel] = resourceType + endpoints = append(endpoints, weightedep) + } + } + } + } else { + if _, ok := servicesMap[rootServiceKey]; ok { + ep.Address = servicesMap[rootServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } else { + ep.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } + + if identityConfigEnvironment.Type[resourceType].Strategy == bluegreenStrategy { + if strings.HasPrefix(host, previewServiceKey) { + ep.Address = servicesMap[previewServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } else { + ep.Address = servicesMap[activeServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } } - weightedEp.Labels[typeLabel] = resourceType - endpoints = append(endpoints, weightedEp) - } - } - // If we go through all the services associated with the rollout and none have applicable weights then endpoints is empty - // Treat the rollout like a deployment and sort and take the first service - if len(endpoints) == 0 { - if clientCluster == serverCluster { - tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - tmpEp.Ports = services[0].Ports + ep.Ports = services[0].Ports + ep.Labels[typeLabel] = resourceType + endpoints = append(endpoints, ep) } - tmpEp.Labels[typeLabel] = resourceType - endpoints = append(endpoints, tmpEp) } } + // If we go through all the services associated with the rollout and none have applicable weights then endpoints is empty + // Treat the rollout like a deployment and sort and take the first service + if len(endpoints) == 0 || resourceType == common.Deployment { + if clientCluster == serverCluster { + tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + tmpEp.Ports = services[0].Ports + } + tmpEp.Labels[typeLabel] = resourceType + endpoints = append(endpoints, tmpEp) + } } - // TODO: type is rollout, strategy is bluegreen, need a way to know which service is preview/desired, trigger another SE - // TODO: type is rollout, strategy is canary, need a way to know which service is stable/root/desired, trigger another SE - // TODO: two types in the environment, deployment to rollout migration return endpoints, err } diff --git a/admiral/pkg/clusters/configwriter_test.go b/admiral/pkg/clusters/configwriter_test.go index 01ad4f825..35aec2333 100644 --- a/admiral/pkg/clusters/configwriter_test.go +++ b/admiral/pkg/clusters/configwriter_test.go @@ -105,6 +105,7 @@ func TestGetServiceEntryEndpoints(t *testing.T) { common.ResetSync() common.InitializeConfig(admiralParams) e2eEnv := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") + host := "e2e.sample.mesh" unweightedDeployment := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") unweightedDeployment.Type = map[string]*registry.TypeConfig{ "deployment": { @@ -135,6 +136,12 @@ func TestGetServiceEntryEndpoints(t *testing.T) { } weightedRollout := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") weightedRollout.Services = weightedServices + weightedRollout.Type = map[string]*registry.TypeConfig{ + "rollout": { + Selectors: map[string]string{"app": "app1"}, + Strategy: canaryStrategy, + }, + } ingressEndpoints := map[string]*networkingV1Alpha3.WorkloadEntry{"cluster1": { Address: "abc-elb.us-west-2.elb.amazonaws.com.", Locality: "us-west-2", @@ -233,7 +240,7 @@ func TestGetServiceEntryEndpoints(t *testing.T) { } for _, c := range testCases { t.Run(c.name, func(t *testing.T) { - seEndpoints, err := getServiceEntryEndpoints(ctxLogger, c.clientCluster, c.serverCluster, c.ingressEndpoints, c.identityConfigEnvironment) + seEndpoints, err := getServiceEntryEndpoints(ctxLogger, c.clientCluster, c.serverCluster, host, c.ingressEndpoints, c.identityConfigEnvironment) if err != nil { t.Errorf("want=nil, got=%v", err) } From 7b9f7ad5d50e9dea6b2d44886fccc4ab2de78acd Mon Sep 17 00:00:00 2001 From: Punakshi Date: Sun, 1 Sep 2024 20:35:56 -0700 Subject: [PATCH 4/9] Logic to add SE for bluegreen, canary, dep to rollout migration --- admiral/pkg/clusters/configwriter.go | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/admiral/pkg/clusters/configwriter.go b/admiral/pkg/clusters/configwriter.go index 4bd554088..ae45e10b2 100644 --- a/admiral/pkg/clusters/configwriter.go +++ b/admiral/pkg/clusters/configwriter.go @@ -19,7 +19,7 @@ const ( typeLabel = "type" previewServiceKey = "preview" activeServiceKey = "active" - desiredServiceKey = "canary" + desiredServiceKey = "desired" rootServiceKey = "root" ) @@ -169,6 +169,9 @@ func getServiceEntryEndpoints( if identityConfigEnvironment.Type[resourceType].Strategy == canaryStrategy { if strings.HasPrefix(host, canaryStrategy) { ep.Address = servicesMap[desiredServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + ep.Ports = services[0].Ports + ep.Labels[typeLabel] = resourceType + endpoints = append(endpoints, ep) } else { for _, service := range services { if service.Weight > 0 { @@ -181,21 +184,12 @@ func getServiceEntryEndpoints( } } } - } else { - if _, ok := servicesMap[rootServiceKey]; ok { - ep.Address = servicesMap[rootServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } else if identityConfigEnvironment.Type[resourceType].Strategy == bluegreenStrategy { + if strings.HasPrefix(host, previewServiceKey) { + ep.Address = servicesMap[previewServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() } else { - ep.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - } - - if identityConfigEnvironment.Type[resourceType].Strategy == bluegreenStrategy { - if strings.HasPrefix(host, previewServiceKey) { - ep.Address = servicesMap[previewServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - } else { - ep.Address = servicesMap[activeServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - } + ep.Address = servicesMap[activeServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() } - ep.Ports = services[0].Ports ep.Labels[typeLabel] = resourceType endpoints = append(endpoints, ep) @@ -206,7 +200,11 @@ func getServiceEntryEndpoints( // Treat the rollout like a deployment and sort and take the first service if len(endpoints) == 0 || resourceType == common.Deployment { if clientCluster == serverCluster { - tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + if _, ok := servicesMap[rootServiceKey]; ok { + tmpEp.Address = servicesMap[rootServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } else { + tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } tmpEp.Ports = services[0].Ports } tmpEp.Labels[typeLabel] = resourceType From bf4c6a2c957d15d7262789fa25b972c9491ebf85 Mon Sep 17 00:00:00 2001 From: Punakshi Date: Sun, 1 Sep 2024 20:48:05 -0700 Subject: [PATCH 5/9] Unit tests for prefixed SE --- admiral/pkg/clusters/configwriter_test.go | 184 ++++++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/admiral/pkg/clusters/configwriter_test.go b/admiral/pkg/clusters/configwriter_test.go index 35aec2333..e154d4535 100644 --- a/admiral/pkg/clusters/configwriter_test.go +++ b/admiral/pkg/clusters/configwriter_test.go @@ -252,6 +252,190 @@ func TestGetServiceEntryEndpoints(t *testing.T) { } } +func TestGetServiceEntryEndpointsForCanaryAndBlueGreen(t *testing.T) { + admiralParams := admiralParamsForConfigWriterTests() + common.ResetSync() + common.InitializeConfig(admiralParams) + host := "e2e.sample.mesh" + canaryRollout := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") + canaryServices := map[string]*registry.RegistryServiceConfig{ + "root": { + Name: "app-1-spk-root-service", + Ports: map[string]uint32{ + "http": 8090, + }, + }, + "desired": { + Name: "app-1-spk-desired-service", + Ports: map[string]uint32{ + "http": 8090, + }, + }, + "stable": { + Name: "app-1-spk-stable-service", + Ports: map[string]uint32{ + "http": 8090, + }, + }, + } + canaryRollout.Services = canaryServices + canaryRollout.Type = map[string]*registry.TypeConfig{ + "rollout": { + Selectors: map[string]string{"app": "app1"}, + Strategy: canaryStrategy, + }, + } + + blueGreenRollout := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") + blueGreenServices := map[string]*registry.RegistryServiceConfig{ + "preview": { + Name: "app-1-spk-preview-service", + Ports: map[string]uint32{ + "http": 8090, + }, + }, + "active": { + Name: "app-1-spk-active-service", + Ports: map[string]uint32{ + "http": 8090, + }, + }, + } + blueGreenRollout.Services = blueGreenServices + blueGreenRollout.Type = map[string]*registry.TypeConfig{ + "rollout": { + Selectors: map[string]string{"app": "app1"}, + Strategy: bluegreenStrategy, + }, + } + ingressEndpoints := map[string]*networkingV1Alpha3.WorkloadEntry{"cluster1": { + Address: "abc-elb.us-west-2.elb.amazonaws.com.", + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(15443)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio"}, + }, "cluster2": { + Address: "def-elb.us-west-2.elb.amazonaws.com.", + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(15443)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio"}, + }} + + localEndpointsWithDesiredService := []*networkingV1Alpha3.WorkloadEntry{{ + Address: "app-1-spk-desired-service.ns-1-usw2-e2e.svc.cluster.local.", + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(8090)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}, + }} + localEndpointsWithPreviewService := []*networkingV1Alpha3.WorkloadEntry{{ + Address: "app-1-spk-preview-service.ns-1-usw2-e2e.svc.cluster.local.", + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(8090)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}, + }} + localEndpointsWithActiveService := []*networkingV1Alpha3.WorkloadEntry{{ + Address: "app-1-spk-active-service.ns-1-usw2-e2e.svc.cluster.local.", + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(8090)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}, + }} + remoteEndpoints := []*networkingV1Alpha3.WorkloadEntry{{ + Address: "def-elb.us-west-2.elb.amazonaws.com.", + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(15443)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}, + }} + ctx := context.Background() + ctxLogger := common.GetCtxLogger(ctx, "sample", "") + testCases := []struct { + name string + identityConfigEnvironment *registry.IdentityConfigEnvironment + ingressEndpoints map[string]*networkingV1Alpha3.WorkloadEntry + clientCluster string + serverCluster string + host string + expectedSEEndpoints []*networkingV1Alpha3.WorkloadEntry + }{ + { + name: "Given an IdentityConfigEnvironment and ingressEndpoint and a desired service, " + + "When the client cluster is the same as the server cluster" + + "Then the constructed endpoint for canary host should be a local endpoint with desired service", + identityConfigEnvironment: canaryRollout, + ingressEndpoints: ingressEndpoints, + clientCluster: "cluster1", + serverCluster: "cluster1", + host: "canary.e2e.sample.mesh", + expectedSEEndpoints: localEndpointsWithDesiredService, + }, + { + name: "Given an IdentityConfigEnvironment and ingressEndpoint and a desired service, " + + "When the client cluster is the different from the server cluster" + + "Then the constructed endpoint for canary host should be a remote endpoint of cluster 2", + identityConfigEnvironment: canaryRollout, + ingressEndpoints: ingressEndpoints, + clientCluster: "cluster1", + serverCluster: "cluster2", + host: host, + expectedSEEndpoints: remoteEndpoints, + }, + { + name: "Given an IdentityConfigEnvironment and ingressEndpoint and a preview and active service for a bluegreen rollout, " + + "When the client cluster is the same as the server cluster" + + "Then the constructed endpoint for preview host should be a local endpoint with preview service", + identityConfigEnvironment: blueGreenRollout, + ingressEndpoints: ingressEndpoints, + clientCluster: "cluster1", + serverCluster: "cluster1", + host: "preview.e2e.sample.mesh", + expectedSEEndpoints: localEndpointsWithPreviewService, + }, + { + name: "Given an IdentityConfigEnvironment and ingressEndpoint and a preview and active service for a bluegreen rollout, " + + "When the client cluster is the same as the server cluster" + + "Then the constructed endpoint for host should be a local endpoint with active service", + identityConfigEnvironment: blueGreenRollout, + ingressEndpoints: ingressEndpoints, + clientCluster: "cluster1", + serverCluster: "cluster1", + host: host, + expectedSEEndpoints: localEndpointsWithActiveService, + }, + { + name: "Given an IdentityConfigEnvironment and ingressEndpoint and a preview and active service for a bluegreen rollout, " + + "When the client cluster is the different from the server cluster" + + "Then the constructed endpoint for host should be a remote endpoint of cluster 2", + identityConfigEnvironment: blueGreenRollout, + ingressEndpoints: ingressEndpoints, + clientCluster: "cluster1", + serverCluster: "cluster2", + host: host, + expectedSEEndpoints: remoteEndpoints, + }, + { + name: "Given an IdentityConfigEnvironment and ingressEndpoint and a preview and active service for a bluegreen rollout, " + + "When the client cluster is the different from the server cluster" + + "Then the constructed endpoint for host should be a remote endpoint of cluster 2", + identityConfigEnvironment: blueGreenRollout, + ingressEndpoints: ingressEndpoints, + clientCluster: "cluster1", + serverCluster: "cluster2", + host: "preview.e2e.sample.mesh", + expectedSEEndpoints: remoteEndpoints, + }, + } + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + seEndpoints, err := getServiceEntryEndpoints(ctxLogger, c.clientCluster, c.serverCluster, c.host, c.ingressEndpoints, c.identityConfigEnvironment) + if err != nil { + t.Errorf("want=nil, got=%v", err) + } + opts := cmpopts.IgnoreUnexported(networkingV1Alpha3.WorkloadEntry{}) + if !cmp.Equal(seEndpoints, c.expectedSEEndpoints, opts) { + t.Errorf("want=%v, got=%v", c.expectedSEEndpoints, seEndpoints) + } + }) + } +} + func TestGetExportTo(t *testing.T) { admiralParams := admiralParamsForConfigWriterTests() common.ResetSync() From ca3b0ebf6596b06c205e413a0c81ee5e82cfa4ad Mon Sep 17 00:00:00 2001 From: Punakshi Date: Mon, 2 Sep 2024 21:46:31 -0700 Subject: [PATCH 6/9] Logic for prefixed SE and migration --- admiral/pkg/clusters/configwriter.go | 82 +++++++++++++++++++--------- admiral/pkg/registry/config.go | 7 ++- 2 files changed, 61 insertions(+), 28 deletions(-) diff --git a/admiral/pkg/clusters/configwriter.go b/admiral/pkg/clusters/configwriter.go index ae45e10b2..735f2057e 100644 --- a/admiral/pkg/clusters/configwriter.go +++ b/admiral/pkg/clusters/configwriter.go @@ -2,6 +2,7 @@ package clusters import ( "fmt" + "reflect" "sort" "strconv" "strings" @@ -151,66 +152,97 @@ func getServiceEntryEndpoints( identityConfigEnvironment *registry.IdentityConfigEnvironment) ([]*networkingV1Alpha3.WorkloadEntry, error) { var err error endpoint := ingressEndpoints[serverCluster] + endpointsMap := map[string]*networkingV1Alpha3.WorkloadEntry{} endpoints := []*networkingV1Alpha3.WorkloadEntry{} tmpEp := endpoint.DeepCopy() - services := []*registry.RegistryServiceConfig{} - servicesMap := identityConfigEnvironment.Services - for _, service := range identityConfigEnvironment.Services { - services = append(services, service) + rolloutServicesMap := map[string]*registry.RegistryServiceConfig{} + deploymentServicesMap := map[string]*registry.RegistryServiceConfig{} + rolloutServices := []*registry.RegistryServiceConfig{} + deploymentServices := []*registry.RegistryServiceConfig{} + endpointFromRollout := false + for resourceType, _ := range identityConfigEnvironment.Type { + for serviceKey, service := range identityConfigEnvironment.Services { + if resourceType == common.Rollout && reflect.DeepEqual(service.Selectors, identityConfigEnvironment.Type[resourceType].Selectors) { + rolloutServicesMap[serviceKey] = service + rolloutServices = append(rolloutServices, service) + } + if resourceType == common.Deployment && reflect.DeepEqual(service.Selectors, identityConfigEnvironment.Type[resourceType].Selectors) { + deploymentServicesMap[serviceKey] = service + deploymentServices = append(deploymentServices, service) + } + } } - sort.Sort(registry.RegistryServiceConfigSorted(services)) + sort.Sort(registry.RegistryServiceConfigSorted(rolloutServices)) + sort.Sort(registry.RegistryServiceConfigSorted(deploymentServices)) // Deployment won't have weights, so just sort and take the first service to use as the endpoint for resourceType, _ := range identityConfigEnvironment.Type { // Rollout without weights is treated the same as deployment so sort and take first service - // If any of the services have weights then add them to the list of endpoints + // If any of the rolloutServicesMap have weights then add them to the list of endpointsMap if resourceType == common.Rollout { ep := tmpEp.DeepCopy() if clientCluster == serverCluster { if identityConfigEnvironment.Type[resourceType].Strategy == canaryStrategy { if strings.HasPrefix(host, canaryStrategy) { - ep.Address = servicesMap[desiredServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - ep.Ports = services[0].Ports + ep.Address = rolloutServicesMap[desiredServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + ep.Ports = rolloutServices[0].Ports ep.Labels[typeLabel] = resourceType - endpoints = append(endpoints, ep) + endpointsMap[ep.Address] = ep + endpointFromRollout = true } else { - for _, service := range services { + for _, service := range rolloutServicesMap { if service.Weight > 0 { weightedep := ep.DeepCopy() - weightedep.Ports = services[0].Ports + weightedep.Ports = rolloutServices[0].Ports weightedep.Address = service.Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() weightedep.Weight = uint32(service.Weight) weightedep.Labels[typeLabel] = resourceType - endpoints = append(endpoints, weightedep) + endpointsMap[weightedep.Address] = weightedep + endpointFromRollout = true } } } } else if identityConfigEnvironment.Type[resourceType].Strategy == bluegreenStrategy { if strings.HasPrefix(host, previewServiceKey) { - ep.Address = servicesMap[previewServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + ep.Address = rolloutServicesMap[previewServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() } else { - ep.Address = servicesMap[activeServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + ep.Address = rolloutServicesMap[activeServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() } - ep.Ports = services[0].Ports + ep.Ports = rolloutServices[0].Ports ep.Labels[typeLabel] = resourceType - endpoints = append(endpoints, ep) + endpointsMap[ep.Address] = ep + endpointFromRollout = true } } } - // If we go through all the services associated with the rollout and none have applicable weights then endpoints is empty + // If we go through all the rolloutServicesMap associated with the rollout and none have applicable weights then endpointsMap is empty // Treat the rollout like a deployment and sort and take the first service - if len(endpoints) == 0 || resourceType == common.Deployment { + if !endpointFromRollout || resourceType == common.Deployment { + tmpEpCopy := tmpEp.DeepCopy() if clientCluster == serverCluster { - if _, ok := servicesMap[rootServiceKey]; ok { - tmpEp.Address = servicesMap[rootServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() - } else { - tmpEp.Address = services[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + if resourceType == common.Rollout { + if _, ok := rolloutServicesMap[rootServiceKey]; ok { + tmpEpCopy.Address = rolloutServicesMap[rootServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } else { + tmpEpCopy.Address = rolloutServices[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + } + tmpEpCopy.Ports = rolloutServices[0].Ports + } + if resourceType == common.Deployment { + tmpEpCopy.Address = deploymentServices[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() + tmpEpCopy.Ports = deploymentServices[0].Ports } - tmpEp.Ports = services[0].Ports } - tmpEp.Labels[typeLabel] = resourceType - endpoints = append(endpoints, tmpEp) + tmpEpCopy.Labels[typeLabel] = resourceType + if _, ok := endpointsMap[tmpEpCopy.Address]; !ok { + endpointsMap[tmpEpCopy.Address] = tmpEpCopy + } } } + + for _, ep := range endpointsMap { + endpoints = append(endpoints, ep) + } + sort.Sort(WorkloadEntrySorted(endpoints)) return endpoints, err } diff --git a/admiral/pkg/registry/config.go b/admiral/pkg/registry/config.go index 5d75e955d..474372c90 100644 --- a/admiral/pkg/registry/config.go +++ b/admiral/pkg/registry/config.go @@ -35,9 +35,10 @@ func (config *IdentityConfigCluster) PutClientAssets(clientAssets []string) erro } type RegistryServiceConfig struct { - Name string `json:"name"` - Weight int `json:"weight"` - Ports map[string]uint32 `json:"ports"` + Name string `json:"name"` + Weight int `json:"weight"` + Ports map[string]uint32 `json:"ports"` + Selectors map[string]string `json:"selectors"` } type TrafficPolicy struct { From c97d693c8f2f30f5eb1feca5022d2b9e3c61d8fe Mon Sep 17 00:00:00 2001 From: Punakshi Date: Mon, 2 Sep 2024 21:49:06 -0700 Subject: [PATCH 7/9] Unit tests prefixed SE and migration --- admiral/pkg/clusters/configwriter_test.go | 141 ++++++++++++++++++++++ admiral/pkg/controller/common/metrics.go | 2 +- admiral/pkg/registry/testutils.go | 136 ++++++++++++++++++++- 3 files changed, 277 insertions(+), 2 deletions(-) diff --git a/admiral/pkg/clusters/configwriter_test.go b/admiral/pkg/clusters/configwriter_test.go index e154d4535..8ec2485db 100644 --- a/admiral/pkg/clusters/configwriter_test.go +++ b/admiral/pkg/clusters/configwriter_test.go @@ -65,6 +65,62 @@ func createMockServiceEntry(env string, identity string, endpointAddress string, return serviceEntry } +func createMockServiceEntryWithTwoEndpoints(env string, identity string, endpointAddress string, endpointPort int, exportTo []string) networkingV1Alpha3.ServiceEntry { + serviceEntry := networkingV1Alpha3.ServiceEntry{ + Hosts: []string{env + "." + strings.ToLower(identity) + ".mesh"}, + Addresses: nil, + Ports: []*networkingV1Alpha3.ServicePort{{Number: uint32(common.DefaultServiceEntryPort), Name: util.Http, Protocol: util.Http}}, + Location: 1, + Resolution: 2, + Endpoints: []*networkingV1Alpha3.WorkloadEntry{{Address: endpointAddress, + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(endpointPort)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}}, + { + Address: "def-elb.us-east-2.elb.amazonaws.com.", + Locality: "us-east-2", + Ports: map[string]uint32{"http": uint32(15443)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}, + }}, + WorkloadSelector: nil, + ExportTo: exportTo, + SubjectAltNames: []string{"spiffe://prefix/" + identity}, + } + return serviceEntry +} + +func createMockServiceEntryWithTwoLocalEndpoints(env string, identity string, endpointAddress1, endpointAddress2 string, endpointPort int, exportTo []string) networkingV1Alpha3.ServiceEntry { + serviceEntry := networkingV1Alpha3.ServiceEntry{ + Hosts: []string{env + "." + strings.ToLower(identity) + ".mesh"}, + Addresses: nil, + Ports: []*networkingV1Alpha3.ServicePort{{Number: uint32(common.DefaultServiceEntryPort), Name: util.Http, Protocol: util.Http}}, + Location: 1, + Resolution: 2, + Endpoints: []*networkingV1Alpha3.WorkloadEntry{ + { + Address: endpointAddress1, + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(endpointPort)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}}, + { + Address: endpointAddress2, + Locality: "us-west-2", + Ports: map[string]uint32{"http": uint32(endpointPort)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "deployment"}}, + { + Address: "def-elb.us-east-2.elb.amazonaws.com.", + Locality: "us-east-2", + Ports: map[string]uint32{"http": uint32(15443)}, + Labels: map[string]string{"security.istio.io/tlsMode": "istio", "type": "rollout"}, + }}, + WorkloadSelector: nil, + ExportTo: exportTo, + SubjectAltNames: []string{"spiffe://prefix/" + identity}, + } + sort.Sort(WorkloadEntrySorted(serviceEntry.Endpoints)) + return serviceEntry +} + func TestGetIngressEndpoints(t *testing.T) { identityConfig := registry.GetSampleIdentityConfig("sample") expectedIngressEndpoints := map[string]*networkingV1Alpha3.WorkloadEntry{ @@ -118,6 +174,7 @@ func TestGetServiceEntryEndpoints(t *testing.T) { Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, "app-1-spk-desired-service": { Name: "app-1-spk-desired-service", @@ -125,6 +182,7 @@ func TestGetServiceEntryEndpoints(t *testing.T) { Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, "app-1-spk-stable-service": { Name: "app-1-spk-stable-service", @@ -132,6 +190,7 @@ func TestGetServiceEntryEndpoints(t *testing.T) { Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, } weightedRollout := registry.GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", "sample") @@ -264,18 +323,21 @@ func TestGetServiceEntryEndpointsForCanaryAndBlueGreen(t *testing.T) { Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, "desired": { Name: "app-1-spk-desired-service", Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, "stable": { Name: "app-1-spk-stable-service", Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, } canaryRollout.Services = canaryServices @@ -293,12 +355,14 @@ func TestGetServiceEntryEndpointsForCanaryAndBlueGreen(t *testing.T) { Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, "active": { Name: "app-1-spk-active-service", Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, } blueGreenRollout.Services = blueGreenServices @@ -564,6 +628,83 @@ func TestBuildServiceEntriesFromIdentityConfig(t *testing.T) { } } +func TestBuildServiceEntriesFromIdentityConfig_MultipleEndpoints(t *testing.T) { + admiralParams := admiralParamsForConfigWriterTests() + common.ResetSync() + common.InitializeConfig(admiralParams) + rr, _ := InitAdmiralOperator(context.Background(), admiralParams) + ctxLogger := common.GetCtxLogger(context.Background(), "test", "") + identityConfig := registry.GetSampleIdentityConfigWithRemoteEndpoints("sample") + expectedLocalServiceEntryPRF := createMockServiceEntryWithTwoEndpoints("prf", "sample", "app-1-spk-root-service.ns-1-usw2-prf.svc.cluster.local.", 8090, []string{"istio-system", "ns-1-usw2-e2e", "ns-1-usw2-prf", "ns-1-usw2-qal"}) + expectedLocalServiceEntryE2E := createMockServiceEntryWithTwoEndpoints("e2e", "sample", "app-1-spk-root-service.ns-1-usw2-e2e.svc.cluster.local.", 8090, []string{"istio-system", "ns-1-usw2-e2e", "ns-1-usw2-prf", "ns-1-usw2-qal"}) + expectedLocalServiceEntryQAL := createMockServiceEntryWithTwoEndpoints("qal", "sample", "app-1-spk-root-service.ns-1-usw2-qal.svc.cluster.local.", 8090, []string{"istio-system", "ns-1-usw2-e2e", "ns-1-usw2-prf", "ns-1-usw2-qal"}) + expectedLocalServiceEntries := []*networkingV1Alpha3.ServiceEntry{&expectedLocalServiceEntryQAL, &expectedLocalServiceEntryPRF, &expectedLocalServiceEntryE2E} + + identityConfigForMigration := registry.GetSampleIdentityConfigWithRolloutAndDeployment("sample") + expectedLocalServiceEntryPRFForMigration := createMockServiceEntryWithTwoLocalEndpoints("prf", "sample", "app-1-spk-root-service.ns-1-usw2-prf.svc.cluster.local.", "app-1-spk-deploy-service.ns-1-usw2-prf.svc.cluster.local.", 8090, []string{"istio-system", "ns-1-usw2-e2e", "ns-1-usw2-prf", "ns-1-usw2-qal"}) + expectedLocalServiceEntryE2EForMigration := createMockServiceEntryWithTwoLocalEndpoints("e2e", "sample", "app-1-spk-root-service.ns-1-usw2-e2e.svc.cluster.local.", "app-1-spk-deploy-service.ns-1-usw2-e2e.svc.cluster.local.", 8090, []string{"istio-system", "ns-1-usw2-e2e", "ns-1-usw2-prf", "ns-1-usw2-qal"}) + expectedLocalServiceEntryQALForMigration := createMockServiceEntryWithTwoLocalEndpoints("qal", "sample", "app-1-spk-root-service.ns-1-usw2-qal.svc.cluster.local.", "app-1-spk-deploy-service.ns-1-usw2-qal.svc.cluster.local.", 8090, []string{"istio-system", "ns-1-usw2-e2e", "ns-1-usw2-prf", "ns-1-usw2-qal"}) + expectedLocalServiceEntriesForMigration := []*networkingV1Alpha3.ServiceEntry{&expectedLocalServiceEntryQALForMigration, &expectedLocalServiceEntryPRFForMigration, &expectedLocalServiceEntryE2EForMigration} + + testCases := []struct { + name string + clientCluster string + event admiral.EventType + identityConfig registry.IdentityConfig + expectedServiceEntries []*networkingV1Alpha3.ServiceEntry + expectedErr bool + }{ + { + name: "Given information to build an se, " + + "When the client cluster is the same as the server cluster" + + "Then the constructed se should have local endpoint and a remote endpoint and istio-system in exportTo", + clientCluster: "cluster1", + event: admiral.Add, + identityConfig: identityConfig, + expectedServiceEntries: expectedLocalServiceEntries, + expectedErr: false, + }, + { + name: "Given information to build an se has a rollout and deployment in the same namespace, " + + "When the client cluster is the same as the server cluster" + + "Then the constructed se should have 2 local endpoints and a remote endpoint and istio-system in exportTo", + clientCluster: "cluster1", + event: admiral.Add, + identityConfig: identityConfigForMigration, + expectedServiceEntries: expectedLocalServiceEntriesForMigration, + expectedErr: false, + }, + } + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + serviceEntryBuilder := ServiceEntryBuilder{ClientCluster: c.clientCluster, RemoteRegistry: rr} + + serviceEntries, err := serviceEntryBuilder.BuildServiceEntriesFromIdentityConfig(ctxLogger, c.identityConfig) + + if err != nil && !c.expectedErr { + t.Errorf("want=%v, \ngot=%v", nil, err) + } + opts := cmpopts.IgnoreUnexported(networkingV1Alpha3.ServiceEntry{}, networkingV1Alpha3.ServicePort{}, networkingV1Alpha3.WorkloadEntry{}) + // sort the service entries by name + sort.Sort(ServiceEntryListSorterByHost(serviceEntries)) + sort.Sort(ServiceEntryListSorterByHost(c.expectedServiceEntries)) + if !cmp.Equal(serviceEntries, c.expectedServiceEntries, opts) { + t.Errorf("want=%v, \ngot=%v", c.expectedServiceEntries, serviceEntries) + } + + }) + } +} + +// run the above test 100 times +func TestBenchmarkBuildServiceEntriesFromIdentityConfig(b *testing.T) { + sum := 1 + for sum < 1000 { + TestBuildServiceEntriesFromIdentityConfig_MultipleEndpoints(b) + sum++ + } +} + type ServiceEntryListSorterByHost []*networkingV1Alpha3.ServiceEntry func (s ServiceEntryListSorterByHost) Len() int { diff --git a/admiral/pkg/controller/common/metrics.go b/admiral/pkg/controller/common/metrics.go index 6da097c87..7948521f3 100644 --- a/admiral/pkg/controller/common/metrics.go +++ b/admiral/pkg/controller/common/metrics.go @@ -15,7 +15,7 @@ func NewGaugeFrom(name string, help string) Gauge { } opts := prometheus.GaugeOpts{Name: name, Help: help} g := prometheus.NewGauge(opts) - prometheus.MustRegister(g) + // prometheus.MustRegister(g) // https://github.com/prometheus/client_golang/issues/716 return &PromGauge{g} } diff --git a/admiral/pkg/registry/testutils.go b/admiral/pkg/registry/testutils.go index a1c7963a8..fb2ec9271 100644 --- a/admiral/pkg/registry/testutils.go +++ b/admiral/pkg/registry/testutils.go @@ -13,12 +13,13 @@ func GetSampleIdentityConfigEnvironment(env string, namespace string, identity s Namespace: namespace, ServiceName: "app-1-spk-root-service", Services: map[string]*RegistryServiceConfig{ - "app-1-spk-root-service": { + "root": { Name: "app-1-spk-root-service", Weight: -1, Ports: map[string]uint32{ "http": 8090, }, + Selectors: map[string]string{"app": "app1"}, }, }, Type: map[string]*TypeConfig{ @@ -119,3 +120,136 @@ func GetSampleIdentityConfig(identity string) IdentityConfig { } return identityConfig } + +func GetSampleIdentityConfigWithRemoteEndpoints(identity string) IdentityConfig { + prfEnv := GetSampleIdentityConfigEnvironment("prf", "ns-1-usw2-prf", identity) + e2eEnv := GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", identity) + qalEnv := GetSampleIdentityConfigEnvironment("qal", "ns-1-usw2-qal", identity) + environments := map[string]*IdentityConfigEnvironment{ + "prf": prfEnv, + "e2e": e2eEnv, + "qal": qalEnv, + } + clientAssets := map[string]string{ + "sample": "sample", + } + cluster := IdentityConfigCluster{ + Name: "cluster1", + Locality: "us-west-2", + IngressEndpoint: "abc-elb.us-west-2.elb.amazonaws.com.", + IngressPort: "15443", + IngressPortName: "http", + Environment: environments, + } + cluster2 := IdentityConfigCluster{ + Name: "cluster2", + Locality: "us-east-2", + IngressEndpoint: "def-elb.us-east-2.elb.amazonaws.com.", + IngressPort: "15443", + IngressPortName: "http", + Environment: environments, + } + identityConfig := IdentityConfig{ + IdentityName: identity, + Clusters: map[string]*IdentityConfigCluster{ + "cluster1": &cluster, + "cluster2": &cluster2, + }, + ClientAssets: clientAssets, + } + return identityConfig +} + +func GetSampleIdentityConfigWithRolloutAndDeployment(identity string) IdentityConfig { + prfEnv := GetSampleIdentityConfigEnvironment("prf", "ns-1-usw2-prf", identity) + e2eEnv := GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", identity) + qalEnv := GetSampleIdentityConfigEnvironment("qal", "ns-1-usw2-qal", identity) + + prfEnv.Type["deployment"] = &TypeConfig{ + Selectors: map[string]string{"deploy-app": "app1"}, + } + prfEnv.Type["rollout"] = &TypeConfig{ + Selectors: map[string]string{"app": "app1"}, + } + + e2eEnv.Type["deployment"] = &TypeConfig{ + Selectors: map[string]string{"deploy-app": "app1"}, + } + e2eEnv.Type["rollout"] = &TypeConfig{ + Selectors: map[string]string{"app": "app1"}, + } + + qalEnv.Type["deployment"] = &TypeConfig{ + Selectors: map[string]string{"deploy-app": "app1"}, + } + qalEnv.Type["rollout"] = &TypeConfig{ + Selectors: map[string]string{"app": "app1"}, + } + + services := map[string]*RegistryServiceConfig{ + "deploysvc": { + Name: "app-1-spk-deploy-service", + Weight: -1, + Ports: map[string]uint32{ + "http": 8090, + }, + Selectors: map[string]string{"deploy-app": "app1"}, + }, + "root": { + Name: "app-1-spk-root-service", + Weight: -1, + Ports: map[string]uint32{ + "http": 8090, + }, + Selectors: map[string]string{"app": "app1"}, + }, + } + + e2eEnv.Services = services + prfEnv.Services = services + qalEnv.Services = services + environments := map[string]*IdentityConfigEnvironment{ + "prf": prfEnv, + "e2e": e2eEnv, + "qal": qalEnv, + } + clientAssets := map[string]string{ + "sample": "sample", + } + cluster := IdentityConfigCluster{ + Name: "cluster1", + Locality: "us-west-2", + IngressEndpoint: "abc-elb.us-west-2.elb.amazonaws.com.", + IngressPort: "15443", + IngressPortName: "http", + Environment: environments, + } + + prfEnv1 := GetSampleIdentityConfigEnvironment("prf", "ns-1-usw2-prf", identity) + e2eEnv1 := GetSampleIdentityConfigEnvironment("e2e", "ns-1-usw2-e2e", identity) + qalEnv1 := GetSampleIdentityConfigEnvironment("qal", "ns-1-usw2-qal", identity) + + environments1 := map[string]*IdentityConfigEnvironment{ + "prf": prfEnv1, + "e2e": e2eEnv1, + "qal": qalEnv1, + } + + cluster2 := IdentityConfigCluster{ + Name: "cluster2", + Locality: "us-east-2", + IngressEndpoint: "def-elb.us-east-2.elb.amazonaws.com.", + IngressPort: "15443", + IngressPortName: "http", + Environment: environments1, + } + identityConfig := IdentityConfig{ + IdentityName: identity, + Clusters: map[string]*IdentityConfigCluster{ + "cluster1": &cluster, + "cluster2": &cluster2, + }, + ClientAssets: clientAssets, + } + return identityConfig +} From 0421531fbd532d9a9e470c486517ac72d3098e53 Mon Sep 17 00:00:00 2001 From: Punakshi Date: Mon, 2 Sep 2024 22:12:16 -0700 Subject: [PATCH 8/9] Unit tests fixes for prefixed SE and migration --- admiral/pkg/clusters/configwriter.go | 6 +++--- .../testdata/sampleIdentityConfiguration.json | 15 ++++++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/admiral/pkg/clusters/configwriter.go b/admiral/pkg/clusters/configwriter.go index 735f2057e..2fc676346 100644 --- a/admiral/pkg/clusters/configwriter.go +++ b/admiral/pkg/clusters/configwriter.go @@ -178,7 +178,7 @@ func getServiceEntryEndpoints( for resourceType, _ := range identityConfigEnvironment.Type { // Rollout without weights is treated the same as deployment so sort and take first service // If any of the rolloutServicesMap have weights then add them to the list of endpointsMap - if resourceType == common.Rollout { + if resourceType == common.Rollout && len(rolloutServicesMap) > 0 { ep := tmpEp.DeepCopy() if clientCluster == serverCluster { if identityConfigEnvironment.Type[resourceType].Strategy == canaryStrategy { @@ -219,7 +219,7 @@ func getServiceEntryEndpoints( if !endpointFromRollout || resourceType == common.Deployment { tmpEpCopy := tmpEp.DeepCopy() if clientCluster == serverCluster { - if resourceType == common.Rollout { + if resourceType == common.Rollout && len(rolloutServicesMap) > 0 { if _, ok := rolloutServicesMap[rootServiceKey]; ok { tmpEpCopy.Address = rolloutServicesMap[rootServiceKey].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() } else { @@ -227,7 +227,7 @@ func getServiceEntryEndpoints( } tmpEpCopy.Ports = rolloutServices[0].Ports } - if resourceType == common.Deployment { + if resourceType == common.Deployment && len(deploymentServicesMap) > 0 { tmpEpCopy.Address = deploymentServices[0].Name + common.Sep + identityConfigEnvironment.Namespace + common.GetLocalDomainSuffix() tmpEpCopy.Ports = deploymentServices[0].Ports } diff --git a/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json b/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json index c0dbf437a..88cd74ef9 100644 --- a/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json +++ b/admiral/pkg/registry/testdata/sampleIdentityConfiguration.json @@ -15,11 +15,14 @@ "namespace": "ns-1-usw2-prf", "serviceName": "app-1-spk-root-service", "services": { - "app-1-spk-root-service": { + "root": { "name": "app-1-spk-root-service", "weight": -1, "ports": { "http": 8090 + }, + "selectors": { + "app": "app1" } } }, @@ -114,11 +117,14 @@ "namespace": "ns-1-usw2-e2e", "serviceName": "app-1-spk-root-service", "services": { - "app-1-spk-root-service": { + "root": { "name": "app-1-spk-root-service", "weight": -1, "ports": { "http": 8090 + }, + "selectors": { + "app": "app1" } } }, @@ -213,11 +219,14 @@ "namespace": "ns-1-usw2-qal", "serviceName": "app-1-spk-root-service", "services": { - "app-1-spk-root-service": { + "root": { "name": "app-1-spk-root-service", "weight": -1, "ports": { "http": 8090 + }, + "selectors": { + "app": "app1" } } }, From 2cbe1f0f1d6695fab9a96ab234cb34a7d8725bf0 Mon Sep 17 00:00:00 2001 From: Punakshi Date: Mon, 2 Sep 2024 22:16:08 -0700 Subject: [PATCH 9/9] Remove redundant code --- admiral/pkg/clusters/configwriter_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/admiral/pkg/clusters/configwriter_test.go b/admiral/pkg/clusters/configwriter_test.go index 8ec2485db..297b9d35b 100644 --- a/admiral/pkg/clusters/configwriter_test.go +++ b/admiral/pkg/clusters/configwriter_test.go @@ -696,15 +696,6 @@ func TestBuildServiceEntriesFromIdentityConfig_MultipleEndpoints(t *testing.T) { } } -// run the above test 100 times -func TestBenchmarkBuildServiceEntriesFromIdentityConfig(b *testing.T) { - sum := 1 - for sum < 1000 { - TestBuildServiceEntriesFromIdentityConfig_MultipleEndpoints(b) - sum++ - } -} - type ServiceEntryListSorterByHost []*networkingV1Alpha3.ServiceEntry func (s ServiceEntryListSorterByHost) Len() int {