diff --git a/controllers/enable_mesh.go b/controllers/enable_mesh.go index 3365faf..acd0a5f 100644 --- a/controllers/enable_mesh.go +++ b/controllers/enable_mesh.go @@ -81,7 +81,7 @@ func newServiceMeshMember(namespace *v1.Namespace) *maistrav1.ServiceMeshMember controlPlaneName := getControlPlaneName() meshNamespace := getMeshNamespace() - return &maistrav1.ServiceMeshMember{ + smm := &maistrav1.ServiceMeshMember{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: "default", // The name MUST be default, per the maistra docs @@ -94,6 +94,8 @@ func newServiceMeshMember(namespace *v1.Namespace) *maistrav1.ServiceMeshMember }, }, } + + return smm } func compareMeshMembers(m1, m2 maistrav1.ServiceMeshMember) bool { diff --git a/controllers/predicates.go b/controllers/predicates.go new file mode 100644 index 0000000..fa74138 --- /dev/null +++ b/controllers/predicates.go @@ -0,0 +1,45 @@ +package controllers + +import ( + "regexp" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +func MeshAwareNamespaces() predicate.Funcs { + filter := func(object client.Object) bool { + return !IsReservedNamespace(object.GetName()) && object.GetAnnotations()[AnnotationServiceMesh] != "" + } + + return predicate.Funcs{ + CreateFunc: func(createEvent event.CreateEvent) bool { + return filter(createEvent.Object) + }, + UpdateFunc: func(updateEvent event.UpdateEvent) bool { + if annotationRemoved(updateEvent, AnnotationServiceMesh) { + // annotation has been just removed, handle it in reconcile + return true + } + + return filter(updateEvent.ObjectNew) + }, + DeleteFunc: func(deleteEvent event.DeleteEvent) bool { + return filter(deleteEvent.Object) + }, + GenericFunc: func(genericEvent event.GenericEvent) bool { + return filter(genericEvent.Object) + }, + } +} + +func annotationRemoved(e event.UpdateEvent, annotation string) bool { + return e.ObjectOld.GetAnnotations()[annotation] != "" && e.ObjectNew.GetAnnotations()[annotation] == "" +} + +var reservedNamespaceRegex = regexp.MustCompile(`^(openshift|istio-system)$|^(kube|openshift)-.*$`) + +func IsReservedNamespace(namepace string) bool { + return reservedNamespaceRegex.MatchString(namepace) +} diff --git a/controllers/predicates_unit_test.go b/controllers/predicates_unit_test.go new file mode 100644 index 0000000..3843ff5 --- /dev/null +++ b/controllers/predicates_unit_test.go @@ -0,0 +1,64 @@ +package controllers_test + +import ( + "github.com/opendatahub-io/odh-project-controller/controllers" + "github.com/opendatahub-io/odh-project-controller/test/labels" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/event" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Controller predicates functions", Label(labels.Unit), func() { + + When("Checking namespace", func() { + + It("should trigger update event when annotation has been removed", func() { + // given + meshAwareNamespaces := controllers.MeshAwareNamespaces() + + // when + namespace := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns-with-annotation-removed", + }, + } + namespaceWithAnnotation := corev1.Namespace{} + namespace.DeepCopyInto(&namespaceWithAnnotation) + namespaceWithAnnotation.SetAnnotations(map[string]string{ + controllers.AnnotationServiceMesh: "true", + }) + + annotationRemovedEvent := event.UpdateEvent{ + ObjectOld: &namespaceWithAnnotation, + ObjectNew: &namespace, + } + + // then + Expect(meshAwareNamespaces.UpdateFunc(annotationRemovedEvent)).To(BeTrue()) + }) + + DescribeTable("it should not process reserved namespaces", + func(ns string, expected bool) { + Expect(controllers.IsReservedNamespace(ns)).To(Equal(expected)) + }, + Entry("kube-system is reserved namespace", "kube-system", true), + Entry("openshift-build is reserved namespace", "openshift-build", true), + Entry("kube-public is reserved namespace", "kube-public", true), + Entry("openshift-infra is reserved namespace", "openshift-infra", true), + Entry("kube-node-lease is reserved namespace", "kube-node-lease", true), + Entry("openshift is reserved namespace", "openshift", true), + Entry("istio-system is reserved namespace", "istio-system", true), + Entry("openshift-authentication is reserved namespace", "openshift-authentication", true), + Entry("openshift-apiserver is reserved namespace", "openshift-apiserver", true), + Entry("mynamespace is not reserved namespace", "mynamespace", false), + Entry("openshiftmynamespace is not reserved namespace", "openshiftmynamespace", false), + Entry("kubemynamespace is not reserved namespace", "kubemynamespace", false), + Entry("istio-system-openshift is not reserved namespace", "istio-system-openshift ", false), + ) + + }) + +}) diff --git a/controllers/project_mesh_controller.go b/controllers/project_mesh_controller.go index 8a6ea0e..eaff72a 100644 --- a/controllers/project_mesh_controller.go +++ b/controllers/project_mesh_controller.go @@ -2,10 +2,8 @@ package controllers import ( "context" - "regexp" "github.com/go-logr/logr" - "github.com/kuadrant/authorino/api/v1beta1" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" @@ -13,6 +11,7 @@ import ( k8serrs "k8s.io/apimachinery/pkg/util/errors" maistrav1 "maistra.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -49,9 +48,9 @@ func (r *OpenshiftServiceMeshReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, errors.Wrap(err, "failed getting namespace") } - if IsReservedNamespace(namespace.Name) || serviceMeshIsNotEnabled(namespace.ObjectMeta) { - log.Info("Skipped") - + if serviceMeshIsNotEnabled(namespace.ObjectMeta) { + //nolint:godox //reason https://github.com/maistra/odh-project-controller/issues/65 + // TODO handle clean-up here return ctrl.Result{}, nil } @@ -66,14 +65,7 @@ func (r *OpenshiftServiceMeshReconciler) Reconcile(ctx context.Context, req ctrl func (r *OpenshiftServiceMeshReconciler) SetupWithManager(mgr ctrl.Manager) error { //nolint:wrapcheck //reason there is no point in wrapping it return ctrl.NewControllerManagedBy(mgr). - For(&v1.Namespace{}). + For(&v1.Namespace{}, builder.WithPredicates(MeshAwareNamespaces())). Owns(&maistrav1.ServiceMeshMember{}). - Owns(&v1beta1.AuthConfig{}). Complete(r) } - -var reservedNamespaceRegex = regexp.MustCompile(`^(openshift|istio-system)$|^(kube|openshift)-.*$`) - -func IsReservedNamespace(namepace string) bool { - return reservedNamespaceRegex.MatchString(namepace) -} diff --git a/controllers/project_mesh_controller_test.go b/controllers/project_mesh_controller_test.go index dbd7314..690d253 100644 --- a/controllers/project_mesh_controller_test.go +++ b/controllers/project_mesh_controller_test.go @@ -102,6 +102,39 @@ var _ = When("Namespace is created", Label(labels.EnvTest), func() { }) }) + It("should not register it in the mesh if annotation is absent", func() { + // given + testNs = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "almost-meshified-namespace", + Annotations: map[string]string{ + controllers.AnnotationServiceMesh: "false", + }, + }, + } + + // when + Expect(cli.Create(context.Background(), testNs)).To(Succeed()) + + // then + By("ensuring no service mesh member created", func() { + members := &maistrav1.ServiceMeshMemberList{} + + Consistently(func() bool { + if err := cli.List(context.Background(), members, client.InNamespace(testNs.Name)); err != nil { + fmt.Printf("failed ensuring no service mesh member created: %+v\n", err) + + return false + } + + return len(members.Items) == 0 + }). + WithTimeout(timeout). + WithPolling(interval). + Should(BeTrue()) + }) + }) + It("should not register it in the mesh if annotation is absent", func() { // given testNs = &corev1.Namespace{ diff --git a/controllers/unit_test.go b/controllers/unit_test.go index 7e249e3..38abc1d 100644 --- a/controllers/unit_test.go +++ b/controllers/unit_test.go @@ -26,27 +26,4 @@ var _ = Describe("Controller helper functions", Label(labels.Unit), func() { }) - When("Checking namespace", func() { - - DescribeTable("it should not process reserved namespaces", - func(ns string, expected bool) { - Expect(controllers.IsReservedNamespace(ns)).To(Equal(expected)) - }, - Entry("kube-system is reserved namespace", "kube-system", true), - Entry("openshift-build is reserved namespace", "openshift-build", true), - Entry("kube-public is reserved namespace", "kube-public", true), - Entry("openshift-infra is reserved namespace", "openshift-infra", true), - Entry("kube-node-lease is reserved namespace", "kube-node-lease", true), - Entry("openshift is reserved namespace", "openshift", true), - Entry("istio-system is reserved namespace", "istio-system", true), - Entry("openshift-authentication is reserved namespace", "openshift-authentication", true), - Entry("openshift-apiserver is reserved namespace", "openshift-apiserver", true), - Entry("mynamespace is not reserved namespace", "mynamespace", false), - Entry("openshiftmynamespace is not reserved namespace", "openshiftmynamespace", false), - Entry("kubemynamespace is not reserved namespace", "kubemynamespace", false), - Entry("istio-system-openshift is not reserved namespace", "istio-system-openshift ", false), - ) - - }) - })