From 845fb137335f41c1158238449c587d942a4741ff Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Tue, 9 Aug 2022 17:12:11 +0200 Subject: [PATCH 1/4] =?UTF-8?q?[limitador=20objs]=C2=A0Creating=20service?= =?UTF-8?q?=20name,=20instead=20of=20using=20`limitador.Name`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/limitador/k8s_objects.go | 4 ++++ pkg/limitador/k8s_objects_test.go | 37 ++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/pkg/limitador/k8s_objects.go b/pkg/limitador/k8s_objects.go index a3e9e702..52899c5c 100644 --- a/pkg/limitador/k8s_objects.go +++ b/pkg/limitador/k8s_objects.go @@ -190,6 +190,10 @@ func LimitsConfigMap(limitador *limitadorv1alpha1.Limitador) (*v1.ConfigMap, err }, nil } +func ServiceName(limitadorObj *limitadorv1alpha1.Limitador) string { + return fmt.Sprintf("limitador-%s", limitadorObj.Name) +} + func labels() map[string]string { return map[string]string{"app": "limitador"} } diff --git a/pkg/limitador/k8s_objects_test.go b/pkg/limitador/k8s_objects_test.go index 91c532e3..df16df1a 100644 --- a/pkg/limitador/k8s_objects_test.go +++ b/pkg/limitador/k8s_objects_test.go @@ -3,7 +3,9 @@ package limitador import ( "testing" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" "gotest.tools/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestConstants(t *testing.T) { @@ -18,4 +20,37 @@ func TestConstants(t *testing.T) { assert.Check(t, "LIMITS_FILE" == LimitadorLimitsFileEnv) } -//TODO: Test individual k8s objects. Extract limitadorObj creation from controller_test +//TODO: Test individual k8s objects. +func newTestLimitadorObj(name, namespace string, limits []limitadorv1alpha1.RateLimit) *limitadorv1alpha1.Limitador { + var ( + replicas = 1 + version = "1.0" + httpPort = int32(8000) + grpcPort = int32(8001) + ) + return &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: limitadorv1alpha1.LimitadorSpec{ + Replicas: &replicas, + Version: &version, + Listener: &limitadorv1alpha1.Listener{ + HTTP: &limitadorv1alpha1.TransportProtocol{Port: &httpPort}, + GRPC: &limitadorv1alpha1.TransportProtocol{Port: &grpcPort}, + }, + Limits: limits, + }, + } + +} + +func TestServiceName(t *testing.T) { + name := ServiceName(newTestLimitadorObj("my-limitador-instance", "default", nil)) + assert.Equal(t, name, "limitador-my-limitador-instance") +} From 440fcddd0b455f83ec14f64fa2117ae2165dd481 Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Tue, 9 Aug 2022 17:13:19 +0200 Subject: [PATCH 2/4] =?UTF-8?q?[controller]=C2=A0Using=20service=20name=20?= =?UTF-8?q?instead=20of=20limitador=20obj=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- controllers/limitador_controller.go | 2 +- controllers/limitador_controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go index c8ea1c8f..23509217 100644 --- a/controllers/limitador_controller.go +++ b/controllers/limitador_controller.go @@ -217,7 +217,7 @@ func updateStatusConditions(currentConditions []metav1.Condition, newCondition m } func buildServiceHost(limitadorObj *limitadorv1alpha1.Limitador) string { - return fmt.Sprintf("%s.%s.svc.cluster.local", limitadorObj.Name, limitadorObj.Namespace) + return fmt.Sprintf("%s.%s.svc.cluster.local", limitador.ServiceName(limitadorObj), limitadorObj.Namespace) } func mutateLimitsConfigMap(existingObj, desiredObj client.Object) (bool, error) { diff --git a/controllers/limitador_controller_test.go b/controllers/limitador_controller_test.go index 5e4c8195..b8095c27 100644 --- a/controllers/limitador_controller_test.go +++ b/controllers/limitador_controller_test.go @@ -183,7 +183,7 @@ var _ = Describe("Limitador controller", func() { &createdLimitador) return createdLimitador.Status.Service }, timeout, interval).Should(Equal(limitadorv1alpha1.LimitadorService{ - Host: limitadorObj.Name + ".default.svc.cluster.local", + Host: "limitador-" + limitadorObj.Name + ".default.svc.cluster.local", Ports: limitadorv1alpha1.Ports{ GRPC: grpcPortNumber, HTTP: httpPortNumber, From ef53302ea72f9515236b060334f7da0fa398d8f0 Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Wed, 10 Aug 2022 12:13:08 +0200 Subject: [PATCH 3/4] [controller] Triggering reconcile watching owned Deployment * Instead of re-queueing when not ready * It simply will change Limitador Status when Deployment obj changes --- controllers/limitador_controller.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go index 23509217..e47e4c92 100644 --- a/controllers/limitador_controller.go +++ b/controllers/limitador_controller.go @@ -27,6 +27,8 @@ import ( "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/source" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" "github.com/kuadrant/limitador-operator/pkg/limitador" @@ -95,12 +97,7 @@ func (r *LimitadorReconciler) Reconcile(eventCtx context.Context, req ctrl.Reque // Reconcile Status if err := r.reconcileStatus(ctx, limitadorObj); err != nil { - switch err.Error() { - case "resource not ready": - return ctrl.Result{Requeue: true}, nil - default: - return ctrl.Result{}, err - } + return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -110,6 +107,10 @@ func (r *LimitadorReconciler) Reconcile(eventCtx context.Context, req ctrl.Reque func (r *LimitadorReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&limitadorv1alpha1.Limitador{}). + Watches( + &source.Kind{Type: &appsv1.Deployment{}}, + &handler.EnqueueRequestForOwner{IsController: true, OwnerType: &limitadorv1alpha1.Limitador{}}, + ). Complete(r) } From 1d5c910097985e6fc6fa0de144fb9d8271dfb817 Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Wed, 10 Aug 2022 12:45:36 +0200 Subject: [PATCH 4/4] =?UTF-8?q?[controller]=C2=A0Returning=20error=20if=20?= =?UTF-8?q?getting=20Lim=20Instance=20fails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Only if error is not `NotFound` --- controllers/limitador_controller.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go index e47e4c92..1ded0499 100644 --- a/controllers/limitador_controller.go +++ b/controllers/limitador_controller.go @@ -117,7 +117,10 @@ func (r *LimitadorReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *LimitadorReconciler) reconcileStatus(ctx context.Context, limitadorObj *limitadorv1alpha1.Limitador) (err error) { logger := logr.FromContext(ctx) - isLimitadorRunning := r.checkLimitadorInstanceIsRunning(ctx, limitadorObj) + isLimitadorRunning, err := r.checkLimitadorInstanceIsRunning(ctx, limitadorObj) + if err != nil { + return err + } changed := updateStatusReady(limitadorObj, isLimitadorRunning) changed = updateStatusService(limitadorObj) || changed @@ -143,7 +146,7 @@ func (r *LimitadorReconciler) reconcileStatus(ctx context.Context, limitadorObj return } -func (r *LimitadorReconciler) checkLimitadorInstanceIsRunning(ctx context.Context, limitadorObj *limitadorv1alpha1.Limitador) bool { +func (r *LimitadorReconciler) checkLimitadorInstanceIsRunning(ctx context.Context, limitadorObj *limitadorv1alpha1.Limitador) (bool, error) { logger := logr.FromContext(ctx) limitadorInstance := &appsv1.Deployment{} limitadorInstanceNamespacedName := client.ObjectKey{ // Its deployment is built after the same name and namespace @@ -152,10 +155,13 @@ func (r *LimitadorReconciler) checkLimitadorInstanceIsRunning(ctx context.Contex } if err := r.Client().Get(ctx, limitadorInstanceNamespacedName, limitadorInstance); err != nil { logger.Error(err, "Failed to get Limitador Instance.") - return false + if errors.IsNotFound(err) { + return false, nil + } + return false, err } - return limitadorInstance.Status.ReadyReplicas >= 1 + return limitadorInstance.Status.ReadyReplicas >= 1, nil } func updateStatusService(limitadorObj *limitadorv1alpha1.Limitador) (changed bool) {