Skip to content

Commit

Permalink
chore: simplifies watch (#89)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartoszmajsak authored Dec 6, 2023
1 parent 1cee7e0 commit f136b02
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 37 deletions.
4 changes: 3 additions & 1 deletion controllers/enable_mesh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -94,6 +94,8 @@ func newServiceMeshMember(namespace *v1.Namespace) *maistrav1.ServiceMeshMember
},
},
}

return smm
}

func compareMeshMembers(m1, m2 maistrav1.ServiceMeshMember) bool {
Expand Down
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),
)

})

})
18 changes: 5 additions & 13 deletions controllers/project_mesh_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ 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"
"k8s.io/apimachinery/pkg/runtime"
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"
)

Expand Down Expand Up @@ -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
}

Expand All @@ -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)
}
33 changes: 33 additions & 0 deletions controllers/project_mesh_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
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 f136b02

Please sign in to comment.