Add support for gtp with multiple traffic policies (#162)

Fixes #163
Fixes #161

.circleci/config.yml

@@ -49,7 +49,7 @@ jobs:
     environment:
       K8S_VERSION: v1.16.8
       KUBECONFIG: /home/circleci/.kube/config
-      MINIKUBE_VERSION: v1.13.0
+      MINIKUBE_VERSION: v1.16.0
       MINIKUBE_WANTUPDATENOTIFICATION: false
       MINIKUBE_WANTREPORTERRORPROMPT: false
       MINIKUBE_HOME: /home/circleci
@@ -97,12 +97,11 @@ jobs:
             export IS_LOCAL=false
             ./run.sh "1.16.8" "1.6.13" "../out"
       - run:
-          name: Run Integration Test for Istio 1.7.4
+          name: Run Integration Test for Istio 1.7.6
           command:  |
             cd tests
             export IS_LOCAL=false
-            ./run.sh "1.16.8" "1.7.4" "../out"
-
+            ./run.sh "1.16.8" "1.7.6" "../out"
   publish-github-release:
     docker:
     - image: circleci/golang:1.11

Makefile

@@ -54,7 +54,7 @@ dep:
 	$(DEP_PATH)dep ensure -v
 
 setup:
-	$(GOGET) -u github.com/golang/protobuf/protoc-gen-go
+	$(GOGET) -u github.com/golang/protobuf/protoc-gen-go@v1.3.2
 
 
 gen-all: api-gen crd-gen
@@ -66,13 +66,12 @@ install-protoc-mac:
 
 api-gen:
 	#make sure the protobuf matches the generation plugin
-	go install github.com/golang/protobuf/protoc-gen-go
 	$(GOCMD) generate ./...
 	go install k8s.io/code-generator/cmd/deepcopy-gen
 
 crd-gen:
-	go get -d -u -fix k8s.io/code-generator
-	go get -d -u -fix k8s.io/apimachinery
+	go get -d -u -fix k8s.io/code-generator@v0.17.3
+	go get -d -u -fix k8s.io/apimachinery@v0.17.3
 	go get -d -u -fix k8s.io/gengo
 	$(GOPATH)/src/k8s.io/code-generator/generate-groups.sh all "$(ROOT_PACKAGE)/pkg/client" "$(ROOT_PACKAGE)/pkg/apis" "$(CUSTOM_RESOURCE_NAME):$(CUSTOM_RESOURCE_VERSION)"
 
@@ -133,5 +132,7 @@ gen-yaml:
 	kustomize build ./install/sample/overlays/remote > ./out/yaml/remotecluster_sample.yaml
 	cp ./install/sample/sample_dep.yaml ./out/yaml/sample_dep.yaml
 	cp ./install/sample/gtp.yaml ./out/yaml/gtp.yaml
+	cp ./install/sample/gtp_failover.yaml ./out/yaml/gtp_failover.yaml
+	cp ./install/sample/gtp_topology.yaml ./out/yaml/gtp_topology.yaml
 	cp ./install/sample/grpc-client.yaml ./out/yaml/grpc-client.yaml
 	cp ./install/scripts/*.sh ./out/scripts/

admiral/pkg/apis/admiral/model/globalrouting.proto

@@ -17,16 +17,16 @@ option go_package = "model";
 //     identity: my-identity
 //     env: prd
 //   policy:
-//   - dns: prd.accounts.global
+//   - dnsPrefix: prd.accounts.global
 //     lbType: topology
-//   - dns: prd.accounts-us-west2
+//   - dnsPrefix: prd.accounts-us-west2
 //     lbType: failover
 //     target:
 //     - region: us-west2
 //       weight: 90
 //     - region: us-east2
 //       weight: 10
-//   - dns: prd.accounts-us-east2
+//   - dnsPrefix: prd.accounts-us-east2
 //     lbType: failover
 //     target:
 //     - region: us-west2
@@ -52,7 +52,7 @@ message TrafficPolicy {
 
     // REQUIRED: dns that can be used by client.  This name will have the
     // traffic type applied to it
-    string dns = 1;
+    string dns = 1 [deprecated=true];
 
     // REQUIRED: type of global load distrubtion
     LbType lbType = 2;
@@ -70,6 +70,10 @@ message TrafficPolicy {
 
     //weigth of primary and secondary must each 100
     repeated TrafficGroup target = 3;
+
+    //REQUIRED: dnsPrefix that will be prefixed for the service names being generated with this traffic policy
+    //Ex: dnsPrefix = west => generated service name = west.stage.servicename.global
+    string dnsPrefix = 4;
 }
 
 message TrafficGroup {

admiral/pkg/clusters/handler.go

@@ -176,16 +176,25 @@ func makeVirtualService(host string, gateways []string, destination string, port
 		Http:     []*v1alpha32.HTTPRoute{{Route: []*v1alpha32.HTTPRouteDestination{{Destination: &v1alpha32.Destination{Host: destination, Port: &v1alpha32.PortSelector{Number: port}}}}}}}
 }
 
-func getDestinationRule(host string, locality string, gtpWrapper *v1.GlobalTrafficPolicy) *v1alpha32.DestinationRule {
+func getDestinationRule(host string, locality string, gtpTrafficPolicy *model.TrafficPolicy) *v1alpha32.DestinationRule {
 	var dr = &v1alpha32.DestinationRule{}
 	dr.Host = host
 	dr.TrafficPolicy = &v1alpha32.TrafficPolicy{Tls: &v1alpha32.TLSSettings{Mode: v1alpha32.TLSSettings_ISTIO_MUTUAL}}
-	if gtpWrapper != nil {
+	processGtp := true
+	if len(locality) == 0 {
+		log.Warnf(LogErrFormat, "Process", "GlobalTrafficPolicy", host, "", "Skipping gtp processing, locality of the cluster nodes cannot be determined. Is this minikube?")
+		processGtp = false
+	}
+	outlierDetection := &v1alpha32.OutlierDetection{
+		BaseEjectionTime:  &types.Duration{Seconds: 120},
+		ConsecutiveErrors: int32(10),
+		Interval:          &types.Duration{Seconds: 5},
+	}
+	if gtpTrafficPolicy != nil && processGtp {
 		var loadBalancerSettings = &v1alpha32.LoadBalancerSettings{
 			LbPolicy: &v1alpha32.LoadBalancerSettings_Simple{Simple: v1alpha32.LoadBalancerSettings_ROUND_ROBIN},
 		}
-		gtp := gtpWrapper.Spec
-		gtpTrafficPolicy := gtp.Policy[0]
+
 		if len(gtpTrafficPolicy.Target) > 0 {
 			var localityLbSettings = &v1alpha32.LocalityLoadBalancerSetting{}
 
@@ -209,11 +218,7 @@ func getDestinationRule(host string, locality string, gtpWrapper *v1.GlobalTraff
 			dr.TrafficPolicy.LoadBalancer = loadBalancerSettings
 		}
 	}
-	dr.TrafficPolicy.OutlierDetection = &v1alpha32.OutlierDetection{
-		BaseEjectionTime:  &types.Duration{Seconds: 120},
-		ConsecutiveErrors: int32(10),
-		Interval:          &types.Duration{Seconds: 5},
-	}
+	dr.TrafficPolicy.OutlierDetection = outlierDetection
 	return dr
 }
 

admiral/pkg/clusters/handler_test.go

@@ -112,85 +112,72 @@ func TestGetDestinationRule(t *testing.T) {
 		},
 	}
 
-	topologyGTPBody := model.GlobalTrafficPolicy{
-		Policy: []*model.TrafficPolicy{
+	topologyGTPPolicy := &model.TrafficPolicy{
+		LbType: model.TrafficPolicy_TOPOLOGY,
+		Target: []*model.TrafficGroup{
 			{
-				LbType: model.TrafficPolicy_TOPOLOGY,
-				Target: []*model.TrafficGroup{
-					{
-						Region: "us-west-2",
-						Weight: 100,
-					},
-				},
+				Region: "us-west-2",
+				Weight: 100,
 			},
 		},
 	}
 
-	topologyGTP := v1.GlobalTrafficPolicy{
-		Spec: topologyGTPBody,
-	}
-	topologyGTP.Name = "myGTP"
-	topologyGTP.Namespace = "myNS"
-
-	failoverGTPBody := model.GlobalTrafficPolicy{
-		Policy: []*model.TrafficPolicy{
+	failoverGTPPolicy := &model.TrafficPolicy{
+		LbType: model.TrafficPolicy_FAILOVER,
+		Target: []*model.TrafficGroup{
 			{
-				LbType: model.TrafficPolicy_FAILOVER,
-				Target: []*model.TrafficGroup{
-					{
-						Region: "us-west-2",
-						Weight: 100,
-					},
-					{
-						Region: "us-east-2",
-						Weight: 0,
-					},
-				},
+				Region: "us-west-2",
+				Weight: 100,
+			},
+			{
+				Region: "us-east-2",
+				Weight: 0,
 			},
 		},
 	}
 
-	failoverGTP := v1.GlobalTrafficPolicy{
-		Spec: failoverGTPBody,
-	}
-	failoverGTP.Name = "myGTP"
-	failoverGTP.Namespace = "myNS"
-
 	//Struct of test case info. Name is required.
 	testCases := []struct {
 		name            string
 		host            string
 		locality        string
-		gtp             *v1.GlobalTrafficPolicy
+		gtpPolicy             *model.TrafficPolicy
 		destinationRule *v1alpha3.DestinationRule
 	}{
 		{
 			name:            "Should handle a nil GTP",
 			host:            "qa.myservice.global",
 			locality:        "uswest2",
-			gtp:             nil,
+			gtpPolicy:       nil,
+			destinationRule: &noGtpDr,
+		},
+		{
+			name:            "Should return default DR with empty locality",
+			host:            "qa.myservice.global",
+			locality:        "",
+			gtpPolicy:       failoverGTPPolicy,
 			destinationRule: &noGtpDr,
 		},
 		{
 			name:            "Should handle a topology GTP",
 			host:            "qa.myservice.global",
 			locality:        "uswest2",
-			gtp:             &topologyGTP,
+			gtpPolicy:       topologyGTPPolicy,
 			destinationRule: &basicGtpDr,
 		},
 		{
 			name:            "Should handle a failover GTP",
 			host:            "qa.myservice.global",
 			locality:        "uswest2",
-			gtp:             &failoverGTP,
+			gtpPolicy:       failoverGTPPolicy,
 			destinationRule: &failoverGtpDr,
 		},
 	}
 
 	//Run the test for every provided case
 	for _, c := range testCases {
 		t.Run(c.name, func(t *testing.T) {
-			result := getDestinationRule(c.host, c.locality, c.gtp)
+			result := getDestinationRule(c.host, c.locality, c.gtpPolicy)
 			if !cmp.Equal(result, c.destinationRule) {
 				t.Fatalf("DestinationRule Mismatch. Diff: %v", cmp.Diff(result, c.destinationRule))
 			}