Skip to content

Commit

Permalink
fix: handles ns with recently removed annotation
Browse files Browse the repository at this point in the history
  • Loading branch information
bartoszmajsak committed Dec 6, 2023
1 parent a33d84b commit df13e8d
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 36 deletions.
45 changes: 45 additions & 0 deletions controllers/predicates.go
Original file line number Diff line number Diff line change
@@ -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)
}
64 changes: 64 additions & 0 deletions controllers/predicates_unit_test.go
Original file line number Diff line number Diff line change
@@ -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),
)

})

})
16 changes: 3 additions & 13 deletions controllers/project_mesh_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package controllers

import (
"context"
"regexp"

"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand All @@ -14,7 +13,6 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)

// OpenshiftServiceMeshReconciler holds the controller configuration.
Expand Down Expand Up @@ -51,6 +49,8 @@ func (r *OpenshiftServiceMeshReconciler) Reconcile(ctx context.Context, req ctrl
}

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
}

Expand All @@ -65,17 +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{}, builder.WithPredicates(predicate.NewPredicateFuncs(meshAwareNamespaces))).
For(&v1.Namespace{}, builder.WithPredicates(MeshAwareNamespaces())).
Owns(&maistrav1.ServiceMeshMember{}).
Complete(r)
}

func meshAwareNamespaces(object client.Object) bool {
return !IsReservedNamespace(object.GetName()) && object.GetAnnotations()[AnnotationServiceMesh] != ""
}

var reservedNamespaceRegex = regexp.MustCompile(`^(openshift|istio-system)$|^(kube|openshift)-.*$`)

func IsReservedNamespace(namepace string) bool {
return reservedNamespaceRegex.MatchString(namepace)
}
23 changes: 0 additions & 23 deletions controllers/unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)

})

})

0 comments on commit df13e8d

Please sign in to comment.