[release-1.26] fix IstioRevisionTag uninstall when revision namespace changes

controllers/istiorevisiontag/istiorevisiontag_controller.go

@@ -110,6 +110,13 @@ func (r *Reconciler) doReconcile(ctx context.Context, tag *v1.IstioRevisionTag)
 		return nil, reconciler.NewValidationError("IstioRevisionTag cannot reference a remote IstioRevision")
 	}
 
+	// if the IstioRevision's namespace changes, we need to completely reinstall the tag
+	if tag.Status.IstiodNamespace != "" && tag.Status.IstiodNamespace != rev.Spec.Namespace {
+		if err := r.uninstallHelmCharts(ctx, tag); err != nil {
+			return nil, err
+		}
+	}
+
 	log.Info("Installing Helm chart")
 	return rev, r.installHelmCharts(ctx, tag, rev)
 }

tests/integration/api/istiorevisiontag_test.go

@@ -43,6 +43,7 @@ var _ = Describe("IstioRevisionTag resource", Ordered, func() {
 		gracePeriod = 5 * time.Second
 	)
 	istio := &v1.Istio{}
+	istio2 := &v1.Istio{}
 	istioKey := client.ObjectKey{Name: istioName}
 	defaultTagKey := client.ObjectKey{Name: defaultTagName}
 	workloadNamespaceKey := client.ObjectKey{Name: workloadNamespace}
@@ -360,6 +361,122 @@ var _ = Describe("IstioRevisionTag resource", Ordered, func() {
 			})
 		})
 	})
+
+	When("Changing the targetRef of a tag to an IstioRevision in another namespace", func() {
+		BeforeAll(func() {
+			Step("Create primary Istio")
+			istio = &v1.Istio{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: istioName,
+				},
+				Spec: v1.IstioSpec{
+					Version:   istioversion.Base,
+					Namespace: istioRevisionTagNamespace,
+					UpdateStrategy: &v1.IstioUpdateStrategy{
+						Type: v1.UpdateStrategyTypeInPlace,
+						InactiveRevisionDeletionGracePeriodSeconds: ptr.Of(int64(gracePeriod.Seconds())),
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, istio)).To(Succeed())
+			Step("Create secondary Istio")
+			istio2 = &v1.Istio{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: istioName + "2",
+				},
+				Spec: v1.IstioSpec{
+					Version:   istioversion.Base,
+					Namespace: workloadNamespace,
+					UpdateStrategy: &v1.IstioUpdateStrategy{
+						Type: v1.UpdateStrategyTypeInPlace,
+						InactiveRevisionDeletionGracePeriodSeconds: ptr.Of(int64(gracePeriod.Seconds())),
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, istio2)).To(Succeed())
+			Step("Create IstioRevisionTag default")
+			tag = &v1.IstioRevisionTag{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: defaultTagName,
+				},
+				Spec: v1.IstioRevisionTagSpec{
+					TargetRef: v1.IstioRevisionTagTargetReference{
+						Kind: "Istio",
+						Name: istioName,
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, tag)).To(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, defaultTagKey, tag)).To(Succeed())
+				g.Expect(tag.Status.IstiodNamespace).To(Equal(istioRevisionTagNamespace))
+			}).Should(Succeed())
+			Step("Switch IstioRevisionTag to secondary istio")
+			tag.Spec.TargetRef.Name = istio2.Name
+			Expect(k8sClient.Update(ctx, tag)).To(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, defaultTagKey, tag)).To(Succeed())
+				g.Expect(tag.Status.IstiodNamespace).To(Equal(workloadNamespace))
+			}).Should(Succeed())
+
+			deleteAllIstiosAndRevisions(ctx)
+			deleteAllIstioRevisionTags(ctx)
+
+			Step("Create conflicting Istio and IstioRevisionTags")
+			istio = &v1.Istio{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: defaultTagName,
+				},
+				Spec: v1.IstioSpec{
+					Version:   istioversion.Base,
+					Namespace: istioRevisionTagNamespace,
+					UpdateStrategy: &v1.IstioUpdateStrategy{
+						Type: v1.UpdateStrategyTypeInPlace,
+						InactiveRevisionDeletionGracePeriodSeconds: ptr.Of(int64(gracePeriod.Seconds())),
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, istio)).To(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, defaultTagKey, istio)).To(Succeed())
+				g.Expect(istio.Status.ObservedGeneration).To(Equal(istio.ObjectMeta.Generation))
+			}).Should(Succeed())
+
+			tag = &v1.IstioRevisionTag{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: defaultTagName,
+				},
+				Spec: v1.IstioRevisionTagSpec{
+					TargetRef: v1.IstioRevisionTagTargetReference{
+						Kind: "Istio",
+						Name: istioName,
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, tag)).To(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, defaultTagKey, tag)).To(Succeed())
+				g.Expect(tag.Status.ObservedGeneration).To(Equal(tag.Generation))
+			}).Should(Succeed())
+			Consistently(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, defaultTagKey, tag)).To(Succeed())
+				g.Expect(tag.Status.GetCondition(v1.IstioRevisionTagConditionReconciled).Status).To(Equal(metav1.ConditionFalse))
+				g.Expect(tag.Status.GetCondition(v1.IstioRevisionTagConditionReconciled).Reason).To(Equal(v1.IstioRevisionTagReasonNameAlreadyExists))
+			}).Should(Succeed())
+		})
+
+		AfterAll(func() {
+			deleteAllIstiosAndRevisions(ctx)
+			deleteAllIstioRevisionTags(ctx)
+		})
+
+		It("can still delete the IstioRevisionTag", func() {
+			Eventually(k8sClient.Delete).WithArguments(ctx, tag).Should(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, defaultTagKey, tag)).To(ReturnNotFoundError())
+			}).Should(Succeed())
+		})
+	})
 })
 
 func deleteAllIstioRevisionTags(ctx context.Context) {