Repository: weaveworks/flagger (go)

Issue:
Support for multiple Istio HTTPRoute
Hello,

I'm trying to set up Flagger in a service (Istio cluster) but I can't find how to handle multiple http routes in the virtual service (as defined here: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPRoute).
I'm using multiple http routes to set specific timeouts/retries per endpoint. Here is an example:

```
spec:
  gateways:
    - istio-system/istio-ilbgateway
    - mesh
  hosts:
    - "{{ .Release.Name }}.{{ .Release.Namespace }}.svc.cluster.local"
  http:
    - match:
      - uri:
          exact: "/api/endpoint-grpc"
      route:
        - destination:
            host: "{{ .Release.Name }}.{{ .Release.Namespace }}.svc.cluster.local"
      timeout: 50ms
      retries:
        attempts: 1
        perTryTimeout: 20ms
        retryOn: "connect-failure,gateway-error,refused-stream,cancelled,deadline-exceeded,unavailable,internal,resource-exhausted"
    - match:
      - uri:
          exact: "/api/endpoint-http"
      route:
        - destination:
            host: "{{ .Release.Name }}.{{ .Release.Namespace }}.svc.cluster.local"
      timeout: 100ms
      retries:
        attempts: 1
        perTryTimeout: 45ms
        retryOn: "connect-failure,gateway-error,refused-stream"
      trafficPolicy:
        connectionPool:
          http:
            maxRetries: 30
    - route:
      - destination:
          host:  {{ .Release.Name }}.{{ .Release.Namespace }}.svc.cluster.local
```

Looking at the CRD, it does not seem possible to do that. Flagger only generates one HTTP route. The match, timeout, retries and trafficPolicy are global to all endpoints.

Did I get that right or did I miss something? If my analysis is correct, is adding this feature something already planned?

Thanks!

Pull request description:
Add support for Istio VirtualService delegation
the Istio provider now supports VIrtualService delegate.
https://istio.io/latest/docs/reference/config/networking/virtual-service/#Delegate
for example, if rootVS and delegateVS are there, the VirtualService which Flagger creates can achieve to behave as delegateVS.
it might related #673 .

for that, the Hosts and Gateways must accept an empty to be applied as a delegateVS.


Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 047423cb..0e2ab403 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -97,6 +97,9 @@ jobs:
       - run: test/e2e-istio-dependencies.sh
       - run: test/e2e-istio-tests.sh
       - run: test/e2e-istio-tests-skip-analysis.sh
+      - run: test/e2e-kubernetes-cleanup.sh
+      - run: test/e2e-istio-dependencies.sh
+      - run: test/e2e-istio-tests-delegate.sh
 
   e2e-gloo-testing:
     machine: true
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b34576ca..158d483b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,7 +2,7 @@
 
 All notable changes to this project are documented in this file.
 
-## 1.2.0 (2020-10-29) 
+## 1.2.0 (2020-09-29) 
 
 Add support for New Relic metrics
 
diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml
index 37311bab..6fd69629 100644
--- a/artifacts/flagger/crd.yaml
+++ b/artifacts/flagger/crd.yaml
@@ -156,6 +156,9 @@ spec:
                   type: array
                   items:
                     type: string
+                delegation:
+                  description: enable behaving as a delegate VirtualService
+                  type: boolean
                 match:
                   description: URI match conditions
                   type: array
diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml
index 37311bab..6fd69629 100644
--- a/charts/flagger/crds/crd.yaml
+++ b/charts/flagger/crds/crd.yaml
@@ -156,6 +156,9 @@ spec:
                   type: array
                   items:
                     type: string
+                delegation:
+                  description: enable behaving as a delegate VirtualService
+                  type: boolean
                 match:
                   description: URI match conditions
                   type: array
diff --git a/docs/gitbook/faq.md b/docs/gitbook/faq.md
index 5deb2cec..081f06ff 100644
--- a/docs/gitbook/faq.md
+++ b/docs/gitbook/faq.md
@@ -554,6 +554,85 @@ spec:
 Flagger works for user facing apps exposed outside the cluster via an ingress gateway
 and for backend HTTP APIs that are accessible only from inside the mesh.
 
+If `Delegation` is enabled, Flagger would generate Istio VirtualService without hosts and gateway,
+making the service compatible with Istio delegation.
+
+```yaml
+apiVersion: flagger.app/v1beta1
+kind: Canary
+metadata:
+  name: backend
+  namespace: test
+spec:
+  service:
+    delegation: true
+    port: 9898
+  targetRef:
+    apiVersion: v1
+    kind: Deployment
+    name: podinfo
+  analysis:
+    interval: 15s
+    threshold: 15
+    maxWeight: 30
+    stepWeight: 10
+```
+
+Based on the above spec, Flagger will create the following virtual service:
+
+```yaml
+apiVersion: networking.istio.io/v1alpha3
+kind: VirtualService
+metadata:
+  name: backend
+  namespace: test
+  ownerReferences:
+  - apiVersion: flagger.app/v1beta1
+    blockOwnerDeletion: true
+    controller: true
+    kind: Canary
+    name: backend
+    uid: 58562662-5e10-4512-b269-2b789c1b30fe
+spec:
+  http:
+  - route:
+    - destination:
+        host: podinfo-primary
+      weight: 100
+    - destination:
+        host: podinfo-canary
+      weight: 0
+```
+
+Therefore, The following virtual service forward the traffic to `/podinfo` by the above delegate VirtualService.
+
+```yaml
+apiVersion: networking.istio.io/v1alpha3
+kind: VirtualService
+metadata:
+  name: frontend
+  namespace: test
+spec:
+  gateways:
+    - public-gateway.istio-system.svc.cluster.local
+    - mesh
+  hosts:
+    - frontend.example.com
+    - frontend
+  http:
+  - match:
+    - uri:
+        prefix: /podinfo
+    rewrite:
+      uri: /
+    delegate:
+      name: backend
+      namespace: test
+```
+
+Note that pilot env `PILOT_ENABLE_VIRTUAL_SERVICE_DELEGATE` must also be set.
+(For the use of Istio Delegation, you can refer to the documentation of [Virtual Service](https://istio.io/latest/docs/reference/config/networking/virtual-service/#Delegate) and [pilot environment variables](https://istio.io/latest/docs/reference/commands/pilot-discovery/#envvars).)
+
 ### Istio Ingress Gateway
 
 **How can I expose multiple canaries on the same external domain?**
diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml
index 37311bab..6fd69629 100644
--- a/kustomize/base/flagger/crd.yaml
+++ b/kustomize/base/flagger/crd.yaml
@@ -156,6 +156,9 @@ spec:
                   type: array
                   items:
                     type: string
+                delegation:
+                  description: enable behaving as a delegate VirtualService
+                  type: boolean
                 match:
                   description: URI match conditions
                   type: array
diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go
index 3451e796..12cd6052 100644
--- a/pkg/apis/flagger/v1beta1/canary.go
+++ b/pkg/apis/flagger/v1beta1/canary.go
@@ -137,6 +137,12 @@ type CanaryService struct {
 	// +optional
 	Hosts []string `json:"hosts,omitempty"`
 
+	// If enabled, Flagger would generate Istio VirtualServices without hosts and gateway,
+	// making the service compatible with Istio delegation. Note that pilot env
+	// `PILOT_ENABLE_VIRTUAL_SERVICE_DELEGATE` must also be set.
+	// +optional
+	Delegation bool `json:"delegation,omitempty"`
+
 	// TrafficPolicy attached to the generated Istio destination rules
 	// +optional
 	TrafficPolicy *istiov1alpha3.TrafficPolicy `json:"trafficPolicy,omitempty"`
diff --git a/pkg/router/istio.go b/pkg/router/istio.go
index bb15c4b6..93cc1139 100644
--- a/pkg/router/istio.go
+++ b/pkg/router/istio.go
@@ -98,6 +98,13 @@ func (ir *IstioRouter) reconcileDestinationRule(canary *flaggerv1.Canary, name s
 func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error {
 	apexName, primaryName, canaryName := canary.GetServiceNames()
 
+	if canary.Spec.Service.Delegation {
+		if len(canary.Spec.Service.Hosts) > 0 || len(canary.Spec.Service.Gateways) > 0 {
+			// delegate VirtualService cannot have hosts and gateways.
+			return fmt.Errorf("VirtualService %s.%s cannot have hosts and gateways when delegation enabled", apexName, canary.Namespace)
+		}
+	}
+
 	// set hosts and add the ClusterIP service host if it doesn't exists
 	hosts := canary.Spec.Service.Hosts
 	var hasServiceHost bool
@@ -132,6 +139,12 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error {
 		makeDestination(canary, canaryName, 0),
 	}
 
+	if canary.Spec.Service.Delegation {
+		// delegate VirtualService requires the hosts and gateway empty.
+		hosts = []string{}
+		gateways = []string{}
+	}
+
 	newSpec := istiov1alpha3.VirtualServiceSpec{
 		Hosts:    hosts,
 		Gateways: gateways,


Test patch:
diff --git a/pkg/router/istio_test.go b/pkg/router/istio_test.go
index b3457ad1..08b3b7cc 100644
--- a/pkg/router/istio_test.go
+++ b/pkg/router/istio_test.go
@@ -333,6 +333,53 @@ func TestIstioRouter_GatewayPort(t *testing.T) {
 	assert.Equal(t, uint32(mocks.canary.Spec.Service.Port), port)
 }
 
+func TestIstioRouter_Delegate(t *testing.T) {
+	t.Run("ok", func(t *testing.T) {
+		mocks := newFixture(nil)
+		mocks.canary.Spec.Service.Hosts = []string{}
+		mocks.canary.Spec.Service.Gateways = []string{}
+		mocks.canary.Spec.Service.Delegation = true
+
+		router := &IstioRouter{
+			logger:        mocks.logger,
+			flaggerClient: mocks.flaggerClient,
+			istioClient:   mocks.meshClient,
+			kubeClient:    mocks.kubeClient,
+		}
+
+		err := router.Reconcile(mocks.canary)
+		require.NoError(t, err)
+
+		vs, err := mocks.meshClient.NetworkingV1alpha3().VirtualServices("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
+		require.NoError(t, err)
+
+		assert.Equal(t, 0, len(vs.Spec.Hosts))
+		assert.Equal(t, 0, len(vs.Spec.Gateways))
+	})
+
+	t.Run("invalid", func(t *testing.T) {
+		mocks := newFixture(nil)
+		if len(mocks.canary.Spec.Service.Gateways) == 0 {
+			// in this case, the gateways or hosts should not be not empty because it requires to cause an error.
+			mocks.canary.Spec.Service.Gateways = []string{
+				"public-gateway.istio",
+				"mesh",
+			}
+		}
+		mocks.canary.Spec.Service.Delegation = true
+
+		router := &IstioRouter{
+			logger:        mocks.logger,
+			flaggerClient: mocks.flaggerClient,
+			istioClient:   mocks.meshClient,
+			kubeClient:    mocks.kubeClient,
+		}
+
+		err := router.Reconcile(mocks.canary)
+		require.Error(t, err)
+	})
+}
+
 func TestIstioRouter_Finalize(t *testing.T) {
 	mocks := newFixture(nil)
 	router := &IstioRouter{
diff --git a/test/e2e-istio-tests-delegate.sh b/test/e2e-istio-tests-delegate.sh
new file mode 100755
index 00000000..9f934c15
--- /dev/null
+++ b/test/e2e-istio-tests-delegate.sh
@@ -0,0 +1,150 @@
+#!/usr/bin/env bash
+
+# This script runs e2e tests for when the canary delegation is enabled
+# Prerequisites: Kubernetes Kind and Istio
+
+set -o errexit
+
+echo '>>> Set pilot env to enable virtual service delegate'
+kubectl -n istio-system set env deploy istiod PILOT_ENABLE_VIRTUAL_SERVICE_DELEGATE=true
+kubectl -n istio-system rollout status deploy istiod
+
+echo '>>> Initialising Gateway'
+cat <<EOF | kubectl apply -f -
+apiVersion: networking.istio.io/v1alpha3
+kind: Gateway
+metadata:
+  name: istio-ingressgateway
+  namespace: istio-system
+spec:
+  selector:
+    app: istio-ingressgateway
+    istio: ingressgateway
+  servers:
+  - port:
+      number: 80
+      name: http
+      protocol: HTTP
+    hosts:
+      - "*"
+EOF
+
+echo '>>> Initialising root virtual service'
+cat <<EOF | kubectl apply -f -
+apiVersion: networking.istio.io/v1alpha3
+kind: VirtualService
+metadata:
+  name: root-vs
+  namespace: test
+spec:
+  gateways:
+    - istio-system/istio-ingressgateway
+  hosts:
+    - "*"
+  http:
+    - match:
+        - uri:
+            prefix: "/podinfo"
+      rewrite:
+        uri: "/"
+      delegate:
+        name: podinfo
+        namespace: test
+EOF
+
+echo '>>> Initialising canary for delegate'
+cat <<EOF | kubectl apply -f -
+apiVersion: flagger.app/v1beta1
+kind: Canary
+metadata:
+  name: podinfo
+  namespace: test
+spec:
+  targetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: podinfo
+  progressDeadlineSeconds: 60
+  service:
+    port: 80
+    targetPort: 9898
+    portDiscovery: true
+    delegation: true
+  skipAnalysis: true
+  analysis:
+    interval: 15s
+    threshold: 15
+    maxWeight: 30
+    stepWeight: 10
+    webhooks:
+      - name: load-test
+        url: http://flagger-loadtester.test/
+        timeout: 5s
+        metadata:
+          type: cmd
+          cmd: "hey -z 10m -q 10 -c 2 http://istio-ingressgateway.istio-system/podinfo"
+          logCmdOutput: "true"
+EOF
+
+echo '>>> Waiting for primary to be ready'
+retries=50
+count=0
+ok=false
+until ${ok}; do
+    kubectl -n test get canary/podinfo | grep 'Initialized' && ok=true || ok=false
+    sleep 5
+    count=$(($count + 1))
+    if [[ ${count} -eq ${retries} ]]; then
+        kubectl -n istio-system logs deployment/flagger
+        echo "No more retries left"
+        exit 1
+    fi
+done
+
+echo '✔ Canary initialization test passed'
+
+echo '>>> Triggering canary deployment'
+kubectl -n test set image deployment/podinfo podinfod=stefanprodan/podinfo:3.1.1
+
+echo '>>> Waiting for canary promotion'
+retries=50
+count=0
+ok=false
+until ${ok}; do
+    kubectl -n test describe deployment/podinfo-primary | grep '3.1.1' && ok=true || ok=false
+    sleep 10
+    kubectl -n istio-system logs deployment/flagger --tail 1
+    count=$(($count + 1))
+    if [[ ${count} -eq ${retries} ]]; then
+        kubectl -n test describe deployment/podinfo
+        kubectl -n test describe deployment/podinfo-primary
+        kubectl -n istio-system logs deployment/flagger
+        echo "No more retries left"
+        exit 1
+    fi
+done
+
+echo '>>> Waiting for canary finalization'
+retries=50
+count=0
+ok=false
+until ${ok}; do
+    kubectl -n test get canary/podinfo | grep 'Succeeded' && ok=true || ok=false
+    sleep 5
+    count=$(($count + 1))
+    if [[ ${count} -eq ${retries} ]]; then
+        kubectl -n istio-system logs deployment/flagger
+        echo "No more retries left"
+        exit 1
+    fi
+done
+
+echo '>>> Set pilot env to disable virtual service delegate'
+kubectl -n istio-system set env deploy istiod PILOT_ENABLE_VIRTUAL_SERVICE_DELEGATE=false
+kubectl -n istio-system rollout status deploy istiod
+
+echo '✔ Canary promotion test passed'
+
+if [[ "$1" = "canary" ]]; then
+  exit 0
+fi
diff --git a/test/e2e-istio.sh b/test/e2e-istio.sh
index 20c1c5c7..c32ec1fb 100755
--- a/test/e2e-istio.sh
+++ b/test/e2e-istio.sh
@@ -2,7 +2,7 @@
 
 set -o errexit
 
-ISTIO_VER="1.6.7"
+ISTIO_VER="1.7.3"
 REPO_ROOT=$(git rev-parse --show-toplevel)
 
 echo ">>> Downloading Istio ${ISTIO_VER}"
@@ -10,8 +10,11 @@ cd ${REPO_ROOT}/bin && \
 curl -L https://istio.io/downloadIstio | ISTIO_VERSION=${ISTIO_VER} sh -
 
 echo ">>> Installing Istio ${ISTIO_VER}"
-${REPO_ROOT}/bin/istio-${ISTIO_VER}/bin/istioctl manifest apply --set profile=default
+${REPO_ROOT}/bin/istio-${ISTIO_VER}/bin/istioctl manifest install --set profile=default \
+  --set values.pilot.resources.requests.cpu=100m \
+  --set values.pilot.resources.requests.memory=100Mi
 
+kubectl apply -f https://raw.githubusercontent.com/istio/istio/release-1.7/samples/addons/prometheus.yaml
 kubectl -n istio-system rollout status deployment/prometheus
 
 kubectl -n istio-system get all