Ensure control plane is deployed immediately when the target namespac…

…e is created

Signed-off-by: Marko Lukša <mluksa@redhat.com>

controllers/istiocni/istiocni_controller.go

@@ -44,7 +44,6 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	"istio.io/istio/pkg/log"
 	"istio.io/istio/pkg/ptr"
 )
 
@@ -107,6 +106,7 @@ func (r *Reconciler) Finalize(ctx context.Context, cni *v1alpha1.IstioCNI) error
 }
 
 func (r *Reconciler) doReconcile(ctx context.Context, cni *v1alpha1.IstioCNI) error {
+	log := logf.FromContext(ctx)
 	if err := r.validateIstioCNI(ctx, cni); err != nil {
 		return err
 	}

controllers/istiorevision/istiorevision_controller.go

@@ -50,7 +50,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	networkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
-	"istio.io/istio/pkg/log"
 	"istio.io/istio/pkg/ptr"
 )
 
@@ -110,6 +109,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, rev *v1alpha1.IstioRevision)
 }
 
 func (r *Reconciler) doReconcile(ctx context.Context, rev *v1alpha1.IstioRevision) error {
+	log := logf.FromContext(ctx)
 	if err := r.validateIstioRevision(ctx, rev); err != nil {
 		return err
 	}
@@ -187,8 +187,12 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	// ownedResourceHandler handles resources that are owned by the IstioRevision CR
 	ownedResourceHandler := handler.EnqueueRequestForOwner(r.Scheme, r.RESTMapper(), &v1alpha1.IstioRevision{}, handler.OnlyControllerOwner())
 
-	// nsHandler handles namespaces that reference the IstioRevision CR via the istio.io/rev or istio-injection labels.
-	// The handler triggers the reconciliation of the referenced IstioRevision CR so that its InUse condition is updated.
+	// nsHandler triggers reconciliation in two cases:
+	// - when a namespace that is referenced in IstioRevision.spec.namespace is
+	//   created, so that the control plane is installed immediately.
+	// - when a namespace that references the IstioRevision CR via the istio.io/rev
+	//   or istio-injection labels is updated, so that the InUse condition of
+	//   the IstioRevision CR is updated.
 	nsHandler := handler.EnqueueRequestsFromMapFunc(r.mapNamespaceToReconcileRequest)
 
 	// podHandler handles pods that reference the IstioRevision CR via the istio.io/rev or sidecar.istio.io/inject labels.
@@ -433,11 +437,27 @@ func istiodDeploymentKey(rev *v1alpha1.IstioRevision) client.ObjectKey {
 }
 
 func (r *Reconciler) mapNamespaceToReconcileRequest(ctx context.Context, ns client.Object) []reconcile.Request {
+	log := logf.FromContext(ctx)
+	var requests []reconcile.Request
+
+	// Check if any IstioRevision references this namespace in .spec.namespace
+	revList := v1alpha1.IstioRevisionList{}
+	if err := r.Client.List(ctx, &revList); err != nil {
+		log.Error(err, "failed to list IstioRevisions")
+		return nil
+	}
+	for _, rev := range revList.Items {
+		if rev.Spec.Namespace == ns.GetName() {
+			requests = append(requests, reconcile.Request{NamespacedName: types.NamespacedName{Name: rev.Name}})
+		}
+	}
+
+	// Check if the namespace references an IstioRevision in its labels
 	revision := getReferencedRevisionFromNamespace(ns.GetLabels())
 	if revision != "" {
-		return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: revision}}}
+		requests = append(requests, reconcile.Request{NamespacedName: types.NamespacedName{Name: revision}})
 	}
-	return nil
+	return requests
 }
 
 func (r *Reconciler) mapPodToReconcileRequest(ctx context.Context, pod client.Object) []reconcile.Request {

pkg/kube/key.go

@@ -0,0 +1,27 @@
+// Copyright Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package kube
+
+import "sigs.k8s.io/controller-runtime/pkg/client"
+
+// key returns the client.ObjectKey for the given name and namespace. If no namespace is provided, it returns a key cluster scoped
+func Key(name string, namespace ...string) client.ObjectKey {
+	if len(namespace) > 1 {
+		panic("you can only provide one namespace")
+	} else if len(namespace) == 1 {
+		return client.ObjectKey{Name: name, Namespace: namespace[0]}
+	}
+	return client.ObjectKey{Name: name}
+}

tests/e2e/controlplane/control_plane_test.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
+	"github.com/istio-ecosystem/sail-operator/pkg/kube"
 	. "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo"
 	"github.com/istio-ecosystem/sail-operator/pkg/test/util/supportedversion"
 	common "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common"
@@ -58,7 +59,7 @@ var _ = Describe("Control Plane Installation", Ordered, func() {
 		Expect(helm.Install("sail-operator", filepath.Join(baseDir, "chart"), "--namespace "+namespace, "--set=image="+image, extraArg)).
 			To(Succeed(), "Operator failed to be deployed")
 
-		Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(deploymentName, namespace), &appsv1.Deployment{}).
+		Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(deploymentName, namespace), &appsv1.Deployment{}).
 			Should(HaveCondition(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Error getting Istio CRD")
 		Success("Operator is deployed in the namespace and Running")
 	})
@@ -78,12 +79,12 @@ metadata:
 				Success("IstioCNI created")
 
 				cni := &v1alpha1.IstioCNI{}
-				Expect(cl.Get(ctx, common.Key("default"), cni)).To(Succeed())
+				Expect(cl.Get(ctx, kube.Key("default"), cni)).To(Succeed())
 				Expect(cni.Spec.Version).To(Equal(supportedversion.Default))
 				Expect(cni.Spec.Namespace).To(Equal("istio-cni"))
 
 				Expect(cl.Delete(ctx, cni)).To(Succeed())
-				Eventually(cl.Get).WithArguments(ctx, common.Key("default"), cni).Should(ReturnNotFoundError())
+				Eventually(cl.Get).WithArguments(ctx, kube.Key("default"), cni).Should(ReturnNotFoundError())
 			},
 		)
 
@@ -102,14 +103,14 @@ metadata:
 				Success("Istio created")
 
 				istio := &v1alpha1.Istio{}
-				Expect(cl.Get(ctx, common.Key("default"), istio)).To(Succeed())
+				Expect(cl.Get(ctx, kube.Key("default"), istio)).To(Succeed())
 				Expect(istio.Spec.Version).To(Equal(supportedversion.Default))
 				Expect(istio.Spec.Namespace).To(Equal("istio-system"))
 				Expect(istio.Spec.UpdateStrategy).ToNot(BeNil())
 				Expect(istio.Spec.UpdateStrategy.Type).To(Equal(v1alpha1.UpdateStrategyTypeInPlace))
 
 				Expect(cl.Delete(ctx, istio)).To(Succeed())
-				Eventually(cl.Get).WithArguments(ctx, common.Key("default"), istio).Should(ReturnNotFoundError())
+				Eventually(cl.Get).WithArguments(ctx, kube.Key("default"), istio).Should(ReturnNotFoundError())
 			},
 		)
 	})
@@ -144,21 +145,21 @@ spec:
 					It("deploys the CNI DaemonSet", func(ctx SpecContext) {
 						Eventually(func(g Gomega) {
 							daemonset := &appsv1.DaemonSet{}
-							g.Expect(cl.Get(ctx, common.Key("istio-cni-node", istioCniNamespace), daemonset)).To(Succeed(), "Error getting IstioCNI DaemonSet")
+							g.Expect(cl.Get(ctx, kube.Key("istio-cni-node", istioCniNamespace), daemonset)).To(Succeed(), "Error getting IstioCNI DaemonSet")
 							g.Expect(daemonset.Status.NumberAvailable).
 								To(Equal(daemonset.Status.CurrentNumberScheduled), "CNI DaemonSet Pods not Available; expected numberAvailable to be equal to currentNumberScheduled")
 						}).Should(Succeed(), "CNI DaemonSet Pods are not Available")
 						Success("CNI DaemonSet is deployed in the namespace and Running")
 					})
 
 					It("updates the status to Reconciled", func(ctx SpecContext) {
-						Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(istioCniName), &v1alpha1.IstioCNI{}).
+						Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioCniName), &v1alpha1.IstioCNI{}).
 							Should(HaveCondition(v1alpha1.IstioCNIConditionReconciled, metav1.ConditionTrue), "IstioCNI is not Reconciled; unexpected Condition")
 						Success("IstioCNI is Reconciled")
 					})
 
 					It("updates the status to Ready", func(ctx SpecContext) {
-						Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(istioCniName), &v1alpha1.IstioCNI{}).
+						Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioCniName), &v1alpha1.IstioCNI{}).
 							Should(HaveCondition(v1alpha1.IstioCNIConditionReady, metav1.ConditionTrue), "IstioCNI is not Ready; unexpected Condition")
 						Success("IstioCNI is Ready")
 					})
@@ -188,19 +189,19 @@ spec:
 					})
 
 					It("updates the Istio CR status to Reconciled", func(ctx SpecContext) {
-						Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(istioName), &v1alpha1.Istio{}).
+						Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1alpha1.Istio{}).
 							Should(HaveCondition(v1alpha1.IstioConditionReconciled, metav1.ConditionTrue), "Istio is not Reconciled; unexpected Condition")
 						Success("Istio CR is Reconciled")
 					})
 
 					It("updates the Istio CR status to Ready", func(ctx SpecContext) {
-						Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(istioName), &v1alpha1.Istio{}).
+						Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1alpha1.Istio{}).
 							Should(HaveCondition(v1alpha1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready; unexpected Condition")
 						Success("Istio CR is Ready")
 					})
 
 					It("deploys istiod", func(ctx SpecContext) {
-						Eventually(common.GetObject).WithArguments(ctx, cl, common.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}).
+						Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}).
 							Should(HaveCondition(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available; unexpected Condition")
 						Expect(getVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version")
 						Success("Istiod is deployed in the namespace and Running")
@@ -220,7 +221,7 @@ spec:
 					})
 
 					It("removes everything from the namespace", func(ctx SpecContext) {
-						Eventually(cl.Get).WithArguments(ctx, common.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}).
+						Eventually(cl.Get).WithArguments(ctx, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}).
 							Should(ReturnNotFoundError(), "Istiod should not exist anymore")
 						common.CheckNamespaceEmpty(ctx, cl, controlPlaneNamespace)
 						Success("Namespace is empty")
@@ -235,7 +236,7 @@ spec:
 
 					It("removes everything from the CNI namespace", func(ctx SpecContext) {
 						daemonset := &appsv1.DaemonSet{}
-						Eventually(cl.Get).WithArguments(ctx, common.Key("istio-cni-node", istioCniNamespace), daemonset).
+						Eventually(cl.Get).WithArguments(ctx, kube.Key("istio-cni-node", istioCniNamespace), daemonset).
 							Should(ReturnNotFoundError(), "IstioCNI DaemonSet should not exist anymore")
 						common.CheckNamespaceEmpty(ctx, cl, istioCniNamespace)
 						Success("CNI namespace is empty")

tests/e2e/operator/operator_install_test.go

@@ -20,6 +20,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/istio-ecosystem/sail-operator/pkg/kube"
 	. "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo"
 	common "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common"
 	. "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega"
@@ -84,21 +85,21 @@ var _ = Describe("Operator", Ordered, func() {
 
 		It("updates the CRDs status to Established", func(ctx SpecContext) {
 			for _, crdName := range sailCRDs {
-				Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(crdName), &apiextensionsv1.CustomResourceDefinition{}).
+				Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(crdName), &apiextensionsv1.CustomResourceDefinition{}).
 					Should(HaveCondition(apiextensionsv1.Established, metav1.ConditionTrue), "Error getting Istio CRD")
 			}
 			Success("CRDs are Established")
 		})
 
 		Specify("istio crd is present", func(ctx SpecContext) {
 			// When the operator runs in OCP cluster, the CRD is created but not available at the moment
-			Eventually(cl.Get).WithArguments(ctx, common.Key("istios.operator.istio.io"), &apiextensionsv1.CustomResourceDefinition{}).
+			Eventually(cl.Get).WithArguments(ctx, kube.Key("istios.operator.istio.io"), &apiextensionsv1.CustomResourceDefinition{}).
 				Should(Succeed(), "Error getting Istio CRD")
 			Success("Istio CRD is present")
 		})
 
 		It("starts successfully", func(ctx SpecContext) {
-			Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(deploymentName, namespace), &appsv1.Deployment{}).
+			Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(deploymentName, namespace), &appsv1.Deployment{}).
 				Should(HaveCondition(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Error getting Istio CRD")
 		})
 

tests/e2e/util/client/client.go

@@ -15,7 +15,6 @@
 package client
 
 import (
-	"flag"
 	"fmt"
 	"log"
 	"os"
@@ -29,11 +28,8 @@ import (
 
 // getConfig returns the configuration of the kubernetes go-client
 func getConfig() (*rest.Config, error) {
-	kubeconfig := flag.String("kubeconfig", os.Getenv("KUBECONFIG"), "(optional) absolute path to the kubeconfig file")
-	flag.Parse()
-
 	// use the current context in kubeconfig
-	config, err := clientcmd.BuildConfigFromFlags("", *kubeconfig)
+	config, err := clientcmd.BuildConfigFromFlags("", os.Getenv("KUBECONFIG"))
 	if err != nil {
 		return nil, fmt.Errorf("error building config: %w", err)
 	}

tests/e2e/util/common/e2e_utils.go

@@ -41,16 +41,6 @@ var (
 	istioCniNamespace     = env.Get("ISTIOCNI_NAMESPACE", "istio-cni")
 )
 
-// key returns the client.ObjectKey for the given name and namespace. If no namespace is provided, it returns a key cluster scoped
-func Key(name string, namespace ...string) client.ObjectKey {
-	if len(namespace) > 1 {
-		panic("you can only provide one namespace")
-	} else if len(namespace) == 1 {
-		return client.ObjectKey{Name: name, Namespace: namespace[0]}
-	}
-	return client.ObjectKey{Name: name}
-}
-
 // getObject returns the object with the given key
 func GetObject(ctx context.Context, cl client.Client, key client.ObjectKey, obj client.Object) (client.Object, error) {
 	err := cl.Get(ctx, key, obj)

tests/integration/api/istiorevision_test.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
+	"github.com/istio-ecosystem/sail-operator/pkg/kube"
 	. "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo"
 	"github.com/istio-ecosystem/sail-operator/pkg/test/util/supportedversion"
 	. "github.com/onsi/ginkgo/v2"
@@ -158,6 +159,71 @@ var _ = Describe("IstioRevision resource", Ordered, func() {
 		})
 	})
 
+	Describe("reconciles immediately after target namespace is created", func() {
+		BeforeAll(func() {
+			Step("Creating the IstioRevision resource without the namespace")
+			rev = &v1alpha1.IstioRevision{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: revName,
+				},
+				Spec: v1alpha1.IstioRevisionSpec{
+					Version:   supportedversion.Default,
+					Namespace: "nonexistent-namespace",
+					Values: &v1alpha1.Values{
+						Revision: revName,
+						Global: &v1alpha1.GlobalConfig{
+							IstioNamespace: "nonexistent-namespace",
+						},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, rev)).To(Succeed())
+		})
+
+		AfterAll(func() {
+			Expect(k8sClient.Delete(ctx, rev)).To(Succeed())
+			Eventually(k8sClient.Get).WithArguments(ctx, kube.Key(revName), rev).Should(ReturnNotFoundError())
+		})
+
+		It("indicates in the status that the namespace doesn't exist", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, revKey, rev)).To(Succeed())
+				g.Expect(rev.Status.ObservedGeneration).To(Equal(rev.ObjectMeta.Generation))
+
+				reconciled := rev.Status.GetCondition(v1alpha1.IstioRevisionConditionReconciled)
+				g.Expect(reconciled.Status).To(Equal(metav1.ConditionFalse))
+				g.Expect(reconciled.Reason).To(Equal(v1alpha1.IstioRevisionReasonReconcileError))
+				g.Expect(reconciled.Message).To(ContainSubstring(`namespace "nonexistent-namespace" doesn't exist`))
+			}).Should(Succeed())
+		})
+
+		When("the namespace is created", func() {
+			BeforeAll(func() {
+				ns := &corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "nonexistent-namespace",
+					},
+				}
+				Expect(k8sClient.Create(ctx, ns)).To(Succeed())
+			})
+
+			It("reconciles immediately", func() {
+				Step("Checking if istiod is deployed immediately")
+				istiod := &appsv1.Deployment{}
+				istiodKey := client.ObjectKey{Name: "istiod-" + revName, Namespace: "nonexistent-namespace"}
+				Eventually(k8sClient.Get).WithArguments(ctx, istiodKey, istiod).WithTimeout(10 * time.Second).Should(Succeed())
+
+				Step("Checking if the status is updated")
+				Eventually(func(g Gomega) {
+					g.Expect(k8sClient.Get(ctx, revKey, rev)).To(Succeed())
+					g.Expect(rev.Status.ObservedGeneration).To(Equal(rev.ObjectMeta.Generation))
+					reconciled := rev.Status.GetCondition(v1alpha1.IstioRevisionConditionReconciled)
+					g.Expect(reconciled.Status).To(Equal(metav1.ConditionTrue))
+				}).Should(Succeed())
+			})
+		})
+	})
+
 	It("successfully reconciles the resource", func() {
 		Step("Creating the custom resource")
 		rev = &v1alpha1.IstioRevision{