Merge pull request #34 from Kuadrant/enhance-reconciling-cycle

Enhance status reconciling cycle
Kuadrant · Aug 11, 2022 · e759f15 · e759f15
2 parents 6009e8d + 1d5c910
commit e759f15
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 13 deletions.
diff --git 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,13 +107,20 @@ 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)
 }
 
 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
@@ -142,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
@@ -151,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) {
@@ -217,7 +224,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
@@ -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,

diff --git 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
@@ -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")
+}