diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index 0aa8515b..1dcfa51c 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -726,6 +726,8 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin var blueGreenActiveService string var blueGreenPreviewService string + var matchedServices = make(map[string]*WeightedService) + if rolloutStrategy.BlueGreen != nil { // If rollout uses blue green strategy blueGreenActiveService = rolloutStrategy.BlueGreen.ActiveService @@ -739,19 +741,20 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin canaryService = rolloutStrategy.Canary.CanaryService stableService = rolloutStrategy.Canary.StableService - //pick stable service if specified - if len(stableService) > 0 { - istioCanaryWeights[stableService] = 1 - } else { - //pick a service that ends in RolloutStableServiceSuffix if one is available - sName := GetServiceWithSuffixMatch(common.RolloutStableServiceSuffix, cachedServices) - if len(sName) > 0 { - istioCanaryWeights[sName] = 1 - } - } - //calculate canary weights if canary strategy is using Istio traffic management if len(stableService) > 0 && len(canaryService) > 0 && rolloutStrategy.Canary.TrafficRouting != nil && rolloutStrategy.Canary.TrafficRouting.Istio != nil { + + //pick stable service if specified + if len(stableService) > 0 { + istioCanaryWeights[stableService] = 1 + } else { + //pick a service that ends in RolloutStableServiceSuffix if one is available + sName := GetServiceWithSuffixMatch(common.RolloutStableServiceSuffix, cachedServices) + if len(sName) > 0 { + istioCanaryWeights[sName] = 1 + } + } + virtualServiceName := rolloutStrategy.Canary.TrafficRouting.Istio.VirtualService.Name virtualService, err := rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(rollout.Namespace).Get(virtualServiceName, v12.GetOptions{}) @@ -797,11 +800,42 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin log.Warnf("No VirtualService was specified in rollout or the specified VirtualService has NO routes, for rollout with name=%s in namespace=%s and cluster=%s", rollout.Name, rollout.Namespace, rc.ClusterID) } } + for _, service := range cachedServices { + match := common.IsServiceMatch(service.Spec.Selector, rollout.Spec.Selector) + //make sure the service matches the rollout Selector and also has a mesh port in the port spec + if match { + ports := GetMeshPortsForRollout(rc.ClusterID, service, rollout) + if len(ports) > 0 { + if val, ok := istioCanaryWeights[service.Name]; ok { + matchedServices[service.Name] = &WeightedService{Weight: val, Service: service} + } + } + } + } + return matchedServices + } else if len(stableService) > 0 { + for _, service := range cachedServices { + //skip services that are not referenced in the rollout + if service.ObjectMeta.Name != stableService { + log.Infof("Skipping service=%s for rollout=%s in namespace=%s and cluster=%s", service.Name, rollout.Name, rollout.Namespace, rc.ClusterID) + continue + } + + match := common.IsServiceMatch(service.Spec.Selector, rollout.Spec.Selector) + //make sure the service matches the rollout Selector and also has a mesh port in the port spec + if match { + ports := GetMeshPortsForRollout(rc.ClusterID, service, rollout) + if len(ports) > 0 { + if len(istioCanaryWeights) == 0 { + matchedServices[service.Name] = &WeightedService{Weight: 1, Service: service} + return matchedServices + } + } + } + } } } - var matchedServices = make(map[string]*WeightedService) - for _, service := range cachedServices { //skip services that are not referenced in the rollout if len(blueGreenActiveService) > 0 && service.ObjectMeta.Name != blueGreenActiveService && service.ObjectMeta.Name != blueGreenPreviewService { @@ -822,9 +856,6 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin matchedServices[service.Name] = &WeightedService{Weight: 1, Service: service} break } - if val, ok := istioCanaryWeights[service.Name]; ok { - matchedServices[service.Name] = &WeightedService{Weight: val, Service: service} - } } } } diff --git a/admiral/pkg/clusters/handler_test.go b/admiral/pkg/clusters/handler_test.go index ecb05bf0..3ad4065c 100644 --- a/admiral/pkg/clusters/handler_test.go +++ b/admiral/pkg/clusters/handler_test.go @@ -688,24 +688,61 @@ func TestGetServiceForRolloutCanary(t *testing.T) { selectorMap := make(map[string]string) selectorMap["app"] = "test" + ports := []coreV1.ServicePort{{Port: 8080}, {Port: 8081}} service := &coreV1.Service{ ObjectMeta: v12.ObjectMeta{Name: ServiceName, Namespace: Namespace, CreationTimestamp: v12.NewTime(time.Now())}, Spec: coreV1.ServiceSpec{ Selector: selectorMap, + Ports: ports, }, } - port1 := coreV1.ServicePort{ - Port: 8080, + + // namespace1 Services + service1 := &coreV1.Service{ + ObjectMeta: v12.ObjectMeta{Name: "dummy1", Namespace: "namespace1", CreationTimestamp: v12.NewTime(time.Now())}, + Spec: coreV1.ServiceSpec{ + Selector: selectorMap, + Ports: []coreV1.ServicePort{{ + Port: 8080, + Name: "random3", + }, { + Port: 8081, + Name: "random4", + }, + }, + }, } - port2 := coreV1.ServicePort{ - Port: 8081, + // namespace4 Services + service3 := &coreV1.Service{ + ObjectMeta: v12.ObjectMeta{Name: "dummy3", Namespace: "namespace4", CreationTimestamp: v12.NewTime(time.Now())}, + Spec: coreV1.ServiceSpec{ + Selector: selectorMap, + Ports: []coreV1.ServicePort{{ + Port: 8080, + Name: "random3", + }, { + Port: 8081, + Name: "random4", + }, + }, + }, } - ports := []coreV1.ServicePort{port1, port2} - service.Spec.Ports = ports + service4 := &coreV1.Service{ + ObjectMeta: v12.ObjectMeta{Name: "dummy4", Namespace: "namespace4", CreationTimestamp: v12.NewTime(time.Now())}, + Spec: coreV1.ServiceSpec{ + Selector: selectorMap, + Ports: []coreV1.ServicePort{{ + Port: 8081, + Name: "random4", + }, + }, + }, + } + // namespace Services stableService := &coreV1.Service{ ObjectMeta: v12.ObjectMeta{Name: StableServiceName, Namespace: Namespace, CreationTimestamp: v12.NewTime(time.Now().Add(time.Duration(-15)))}, Spec: coreV1.ServiceSpec{ @@ -738,56 +775,13 @@ func TestGetServiceForRolloutCanary(t *testing.T) { }, } - selectorMap1 := make(map[string]string) - selectorMap1["app"] = "test1" - service1 := &coreV1.Service{ - Spec: coreV1.ServiceSpec{ - Selector: selectorMap, - }, - } - service1.Name = "dummy" - service1.Namespace = "namespace1" - port3 := coreV1.ServicePort{ - Port: 8080, - Name: "random3", - } - - port4 := coreV1.ServicePort{ - Port: 8081, - Name: "random4", - } - - ports1 := []coreV1.ServicePort{port3, port4} - service1.Spec.Ports = ports1 - - selectorMap4 := make(map[string]string) - selectorMap4["app"] = "test" - service4 := &coreV1.Service{ - Spec: coreV1.ServiceSpec{ - Selector: selectorMap4, - }, - } - service4.Name = "dummy" - service4.Namespace = "namespace4" - port11 := coreV1.ServicePort{ - Port: 8080, - Name: "random3", - } - - port12 := coreV1.ServicePort{ - Port: 8081, - Name: "random4", - } - - ports11 := []coreV1.ServicePort{port11, port12} - service4.Spec.Ports = ports11 - rcTemp.ServiceController.Cache.Put(service) rcTemp.ServiceController.Cache.Put(service1) + rcTemp.ServiceController.Cache.Put(service3) rcTemp.ServiceController.Cache.Put(service4) rcTemp.ServiceController.Cache.Put(stableService) - rcTemp.ServiceController.Cache.Put(generatedStableService) rcTemp.ServiceController.Cache.Put(canaryService) + rcTemp.ServiceController.Cache.Put(generatedStableService) rcTemp.ServiceController.Cache.Put(latestMatchingService) virtualService := &v1alpha32.VirtualService{ @@ -860,26 +854,19 @@ func TestGetServiceForRolloutCanary(t *testing.T) { canaryRolloutNS4 := argo.Rollout{ Spec: argo.RolloutSpec{Template: coreV1.PodTemplateSpec{ - ObjectMeta: k8sv1.ObjectMeta{Annotations: map[string]string{}}, + ObjectMeta: k8sv1.ObjectMeta{Annotations: map[string]string{common.SidecarEnabledPorts: "8080"}}, }}} matchLabel4 := make(map[string]string) matchLabel4["app"] = "test" - labelSelector4 := v12.LabelSelector{ MatchLabels: matchLabel4, } canaryRolloutNS4.Spec.Selector = &labelSelector4 - canaryRolloutNS4.Namespace = "namespace4" canaryRolloutNS4.Spec.Strategy = argo.RolloutStrategy{ Canary: &argo.CanaryStrategy{}, } - anotationsNS4Map := make(map[string]string) - anotationsNS4Map[common.SidecarEnabledPorts] = "8080" - - canaryRolloutNS4.Spec.Template.Annotations = anotationsNS4Map - canaryRolloutIstioVs := argo.Rollout{ Spec: argo.RolloutSpec{Template: coreV1.PodTemplateSpec{ ObjectMeta: k8sv1.ObjectMeta{Annotations: map[string]string{}}, @@ -931,7 +918,7 @@ func TestGetServiceForRolloutCanary(t *testing.T) { CanaryService: CanaryServiceName, TrafficRouting: &argo.RolloutTrafficRouting{ Istio: &argo.IstioTrafficRouting{ - VirtualService: argo.IstioVirtualService{Name: VS_NAME_3, Routes: []string{"random"}}, + VirtualService: argo.IstioVirtualService{Name: VS_NAME_2, Routes: []string{"random"}}, }, }, }, @@ -989,16 +976,32 @@ func TestGetServiceForRolloutCanary(t *testing.T) { }, } - resultForDummy := map[string]*WeightedService{"dummy": {Weight: 1, Service: service1}} + canaryRolloutWithStableServiceNS4 := argo.Rollout{ + Spec: argo.RolloutSpec{Template: coreV1.PodTemplateSpec{ + ObjectMeta: k8sv1.ObjectMeta{Annotations: map[string]string{common.SidecarEnabledPorts: "8080"}}, + }}} + canaryRolloutWithStableServiceNS4.Spec.Selector = &labelSelector - resultForStableServiceOnly := map[string]*WeightedService{StableServiceName: {Weight: 1, Service: stableService}} + canaryRolloutWithStableServiceNS4.Namespace = "namespace4" + canaryRolloutWithStableServiceNS4.Spec.Strategy = argo.RolloutStrategy{ + Canary: &argo.CanaryStrategy{ + StableService: StableServiceName, + CanaryService: CanaryServiceName, + }, + } + + resultForDummy := map[string]*WeightedService{service3.Name: {Weight: 1, Service: service3}} - resultForEmptyStableServiceOnRollout := map[string]*WeightedService{GeneratedStableServiceName: {Weight: 1, Service: generatedStableService}} + resultForEmptyStableServiceOnRollout := map[string]*WeightedService{LatestMatchingService: {Weight: 1, Service: latestMatchingService}} resultForCanaryWithIstio := map[string]*WeightedService{StableServiceName: {Weight: 80, Service: stableService}, CanaryServiceName: {Weight: 20, Service: canaryService}} - resultForCanaryWithStableService := map[string]*WeightedService{StableServiceName: {Weight: 100, Service: stableService}} + resultForCanaryWithStableService := map[string]*WeightedService{StableServiceName: {Weight: 1, Service: stableService}} + + resultForCanaryWithStableServiceWeight := map[string]*WeightedService{StableServiceName: {Weight: 100, Service: stableService}} + + resultRolloutWithOneServiceHavingMeshPort := map[string]*WeightedService{service3.Name: {Weight: 1, Service: service3}} testCases := []struct { name string @@ -1022,10 +1025,10 @@ func TestGetServiceForRolloutCanary(t *testing.T) { rc: rcTemp, result: resultForEmptyStableServiceOnRollout, }, { - name: "canaryRolloutWithStableService", + name: "canaryRolloutWithIstioVsMimatch", rollout: &canaryRolloutIstioVsMimatch, rc: rcTemp, - result: resultForStableServiceOnly, + result: resultForCanaryWithStableService, }, { name: "canaryRolloutWithIstioVirtualService", rollout: &canaryRolloutIstioVs, @@ -1035,7 +1038,7 @@ func TestGetServiceForRolloutCanary(t *testing.T) { name: "canaryRolloutWithIstioVirtualServiceZeroWeight", rollout: &canaryRolloutIstioVsZeroWeight, rc: rcTemp, - result: resultForCanaryWithStableService, + result: resultForCanaryWithStableServiceWeight, }, { name: "canaryRolloutWithIstioRouteMatch", rollout: &canaryRolloutIstioVsRouteMatch, @@ -1045,21 +1048,25 @@ func TestGetServiceForRolloutCanary(t *testing.T) { name: "canaryRolloutWithIstioRouteMisMatch", rollout: &canaryRolloutIstioVsRouteMisMatch, rc: rcTemp, - result: resultForStableServiceOnly, - }, { + result: resultForCanaryWithStableService, + }, + { name: "canaryRolloutWithStableServiceName", rollout: &canaryRolloutWithStableService, rc: rcTemp, - result: resultForStableServiceOnly, + result: resultForCanaryWithStableService, + }, + { + name: "canaryRolloutWithOneServiceHavingMeshPort", + rollout: &canaryRolloutWithStableServiceNS4, + rc: rcTemp, + result: resultRolloutWithOneServiceHavingMeshPort, }, } //Run the test for every provided case for _, c := range testCases { t.Run(c.name, func(t *testing.T) { - if c.name != "canaryRolloutHappyCase" { - return - } result := getServiceForRollout(c.rc, c.rollout) if len(c.result) == 0 { if result != nil && len(result) != 0 {