fix: support only tls in virtual services (#2502)

* fix: support only tls in virtual services Signed-off-by: zachaller <[email protected]> * fix typo Signed-off-by: zachaller <[email protected]> * add test file Signed-off-by: zachaller <[email protected]> * add unit test Signed-off-by: zachaller <[email protected]> * cleanup test Signed-off-by: zachaller <[email protected]> * remove comment Signed-off-by: zachaller <[email protected]> * improve comment Signed-off-by: zachaller <[email protected]> * remove /n Signed-off-by: zachaller <[email protected]> * re-ran codegen Signed-off-by: zachaller <[email protected]> * re-run codegen for 2023 Signed-off-by: zachaller <[email protected]> Signed-off-by: zachaller <[email protected]>
argoproj · Jan 4, 2023 · e40c9fe · e40c9fe
1 parent b4bd587
commit e40c9fe
Show file tree

Hide file tree

Showing 7 changed files with 188 additions and 4 deletions.
diff --git a/pkg/apis/rollouts/v1alpha1/generated.pb.go b/pkg/apis/rollouts/v1alpha1/generated.pb.go
diff --git a/pkg/apis/rollouts/v1alpha1/generated.proto b/pkg/apis/rollouts/v1alpha1/generated.proto
diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go
diff --git a/rollout/trafficrouting/istio/istio.go b/rollout/trafficrouting/istio/istio.go
@@ -1499,7 +1499,10 @@ func (r *Reconciler) RemoveManagedRoutes() error {
 			return fmt.Errorf("[RemoveManagedRoutes] failed to get http routes from virtual service: %w", err)
 		}
 		if !found {
-			return fmt.Errorf("[RemoveManagedRoutes] %s: %w", SpecHttpNotFound, err)
+			// This could happen when only TLS routes are defined. We don't need to do anything else and hence we return early
+			// because tls routes do not support header and mirroring which are features that require the use of managed routes.
+			log.Debugf("[RemoveManagedRoutes] %s: not removing any routes", SpecHttpNotFound)
+			return nil
 		}
 
 		managedRoutes := r.rollout.Spec.Strategy.Canary.TrafficRouting.ManagedRoutes

diff --git a/rollout/trafficrouting/istio/istio_test.go b/rollout/trafficrouting/istio/istio_test.go
@@ -2463,6 +2463,34 @@ func TestHttpReconcileMirrorRoute(t *testing.T) {
 
 }
 
+func TestSingleTlsRouteReconcile(t *testing.T) {
+	ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", []v1alpha1.TLSRoute{{
+		Port:     3000,
+		SNIHosts: nil,
+	}})
+
+	obj := unstructuredutil.StrToUnstructuredUnsafe(singleRouteTlsVsvc)
+	client := testutil.NewFakeDynamicClient(obj)
+	vsvcLister, druleLister := getIstioListers(client)
+	r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister)
+	client.ClearActions()
+
+	err := r.SetWeight(30, v1alpha1.WeightDestination{})
+	assert.Nil(t, err)
+	iVirtualService, err := client.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(r.rollout.Namespace).Get(context.TODO(), ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name, metav1.GetOptions{})
+	assert.NoError(t, err)
+	tlsRoutes := extractTlsRoutes(t, iVirtualService)
+	assert.Equal(t, len(tlsRoutes), 1)
+	assert.Equal(t, tlsRoutes[0].Route[0].Weight, int64(70))
+	assert.Equal(t, tlsRoutes[0].Route[1].Weight, int64(30))
+
+	err = r.RemoveManagedRoutes()
+	assert.NoError(t, err)
+
+	_, err = client.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(r.rollout.Namespace).Get(context.TODO(), ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name, metav1.GetOptions{})
+	assert.NoError(t, err)
+}
+
 func TestHttpReconcileMirrorRouteWithExtraFields(t *testing.T) {
 	ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"primary"})
 	obj := unstructuredutil.StrToUnstructuredUnsafe(regularVsvcWithExtra)

diff --git a/test/e2e/istio/istio-host-only-tls-split.yaml b/test/e2e/istio/istio-host-only-tls-split.yaml
@@ -0,0 +1,90 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: istio-host-split-tls-canary
+spec:
+  ports:
+  - port: 80
+    targetPort: http
+    protocol: TCP
+    name: http
+  selector:
+    app: istio-host-split-tls
+
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: istio-host-split-tls-stable
+spec:
+  ports:
+  - port: 80
+    targetPort: http
+    protocol: TCP
+    name: http
+  selector:
+    app: istio-host-split-tls
+
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: VirtualService
+metadata:
+  name: istio-host-split-tls-vsvc
+spec:
+  hosts:
+  - istio-host-split-tls
+  tls:
+  - match:
+    - port: 3000
+      sniHosts:
+      - istio-host-split-tls
+    route:
+    - destination:
+        host: istio-host-split-tls-stable
+      weight: 100
+    - destination:
+        host: istio-host-split-tls-canary
+      weight: 0
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Rollout
+metadata:
+  name: istio-host-split-tls
+spec:
+  strategy:
+    canary:
+      canaryService: istio-host-split-tls-canary
+      stableService: istio-host-split-tls-stable
+      trafficRouting:
+        istio:
+          virtualService: 
+            name: istio-host-split-tls-vsvc
+            routes:
+            - primary
+            tlsRoutes:
+            - port: 3000
+              sniHosts:
+              - istio-host-split-tls
+      steps:
+      - setWeight: 10
+      - pause: {}
+  selector:
+    matchLabels:
+      app: istio-host-split-tls
+  template:
+    metadata:
+      labels:
+        app: istio-host-split-tls
+    spec:
+      containers:
+      - name: istio-host-split-tls
+        image: nginx:1.19-alpine
+        ports:
+        - name: http
+          containerPort: 80
+          protocol: TCP
+        resources:
+          requests:
+            memory: 16Mi
+            cpu: 5m
diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go
@@ -111,6 +111,69 @@ func (s *IstioSuite) TestIstioHostSplit() {
 	}
 }
 
+func (s *IstioSuite) TestIstioHostSplitOnlyTls() {
+
+	tests := []struct {
+		filename string
+	}{
+		{
+			"@istio/istio-host-only-tls-split.yaml",
+		},
+	}
+
+	for _, tc := range tests {
+
+		s.Given().
+			RolloutObjects(tc.filename).
+			When().
+			ApplyManifests().
+			WaitForRolloutStatus("Healthy").
+			Then().
+			Assert(func(t *fixtures.Then) {
+				vsvc := t.GetVirtualService()
+				assert.Equal(s.T(), int64(100), vsvc.Spec.TLS[0].Route[0].Weight)
+				assert.Equal(s.T(), int64(0), vsvc.Spec.TLS[0].Route[1].Weight)
+				desired, stable := t.GetServices()
+				rs1 := t.GetReplicaSetByRevision("1")
+				assert.Equal(s.T(), rs1.Spec.Template.Labels[v1alpha1.DefaultRolloutUniqueLabelKey], desired.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey])
+				assert.Equal(s.T(), rs1.Spec.Template.Labels[v1alpha1.DefaultRolloutUniqueLabelKey], stable.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey])
+			}).
+			When().
+			UpdateSpec().
+			WaitForRolloutStatus("Paused").
+			Then().
+			Assert(func(t *fixtures.Then) {
+				vsvc := t.GetVirtualService()
+				assert.Equal(s.T(), int64(90), vsvc.Spec.TLS[0].Route[0].Weight)
+				assert.Equal(s.T(), int64(10), vsvc.Spec.TLS[0].Route[1].Weight)
+
+				desired, stable := t.GetServices()
+				rs1 := t.GetReplicaSetByRevision("1")
+				rs2 := t.GetReplicaSetByRevision("2")
+				assert.Equal(s.T(), rs2.Spec.Template.Labels[v1alpha1.DefaultRolloutUniqueLabelKey], desired.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey])
+				assert.Equal(s.T(), rs1.Spec.Template.Labels[v1alpha1.DefaultRolloutUniqueLabelKey], stable.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey])
+			}).
+			When().
+			PromoteRollout().
+			WaitForRolloutStatus("Healthy").
+			Sleep(1*time.Second). // stable is currently set first, and then changes made to VirtualServices/DestinationRules
+			Then().
+			Assert(func(t *fixtures.Then) {
+				vsvc := t.GetVirtualService()
+				assert.Equal(s.T(), int64(100), vsvc.Spec.TLS[0].Route[0].Weight)
+				assert.Equal(s.T(), int64(0), vsvc.Spec.TLS[0].Route[1].Weight)
+
+				desired, stable := t.GetServices()
+				rs2 := t.GetReplicaSetByRevision("2")
+				assert.Equal(s.T(), rs2.Spec.Template.Labels[v1alpha1.DefaultRolloutUniqueLabelKey], desired.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey])
+				assert.Equal(s.T(), rs2.Spec.Template.Labels[v1alpha1.DefaultRolloutUniqueLabelKey], stable.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey])
+			}).
+			ExpectRevisionPodCount("1", 1) // don't scale down old replicaset since it will be within scaleDownDelay
+
+		s.TearDownSuite()
+	}
+}
+
 func (s *IstioSuite) TestIstioSubsetSplit() {
 	s.Given().
 		RolloutObjects("@istio/istio-subset-split.yaml").