Skip to content

Commit

Permalink
fix: SE generation for canary rollout strategy (#253) (#151)
Browse files Browse the repository at this point in the history 
Signed-off-by: nbn01 <[email protected]>
  • Loading branch information
nbn01 authored and GitHub Enterprise committed Aug 16, 2022
1 parent d271dfa commit 42e10d3
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 89 deletions.
63 changes: 47 additions & 16 deletions admiral/pkg/clusters/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,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
Expand All @@ -752,19 +754,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{})

Expand Down Expand Up @@ -810,11 +813,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 {
Expand All @@ -835,9 +869,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}
}
}
}
}
Expand Down
153 changes: 80 additions & 73 deletions admiral/pkg/clusters/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,24 +686,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{
Expand Down Expand Up @@ -736,56 +773,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{
Expand Down Expand Up @@ -858,26 +852,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{}},
Expand Down Expand Up @@ -929,7 +916,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"}},
},
},
},
Expand Down Expand Up @@ -987,16 +974,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
Expand All @@ -1020,10 +1023,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,
Expand All @@ -1033,7 +1036,7 @@ func TestGetServiceForRolloutCanary(t *testing.T) {
name: "canaryRolloutWithIstioVirtualServiceZeroWeight",
rollout: &canaryRolloutIstioVsZeroWeight,
rc: rcTemp,
result: resultForCanaryWithStableService,
result: resultForCanaryWithStableServiceWeight,
}, {
name: "canaryRolloutWithIstioRouteMatch",
rollout: &canaryRolloutIstioVsRouteMatch,
Expand All @@ -1043,21 +1046,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 {
Expand Down

0 comments on commit 42e10d3

Please sign in to comment.