From f42564c5c999a78003584b27cf238cb98eccd676 Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Tue, 7 Jun 2022 14:43:56 +0200 Subject: [PATCH] [controller] Reconciling ServiceURL status from Service Spec --- controllers/limitador_controller.go | 27 +++++++++++++++ controllers/limitador_controller_test.go | 42 ++++++++++++++++++++---- pkg/helpers/helpers.go | 7 ++++ pkg/limitador/k8s_objects.go | 29 ++++++++-------- 4 files changed, 84 insertions(+), 21 deletions(-) diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go index acb9c327..99f8dcd3 100644 --- a/controllers/limitador_controller.go +++ b/controllers/limitador_controller.go @@ -19,6 +19,8 @@ package controllers import ( "context" "fmt" + "github.com/kuadrant/limitador-operator/pkg/helpers" + "strconv" "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" @@ -79,6 +81,11 @@ func (r *LimitadorReconciler) Reconcile(eventCtx context.Context, req ctrl.Reque return ctrl.Result{}, err } + // Reconcile Status + if err = r.reconcileStatus(ctx, limitadorObj); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -89,6 +96,26 @@ func (r *LimitadorReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +func (r *LimitadorReconciler) reconcileStatus(ctx context.Context, limitadorObj *limitadorv1alpha1.Limitador) error { + logger := logr.FromContext(ctx) + // Simple enough now, we could implement a thorough check like with RateLimit with ObservedGeneration in the future + builtServiceUrl := buildServiceUrl(limitadorObj) + if builtServiceUrl != limitadorObj.Status.ServiceURL { + logger.V(1).Info("Updating the Status", "Name", limitadorObj.Name) + limitadorObj.Status.ServiceURL = builtServiceUrl + return r.Client().Status().Update(ctx, limitadorObj) + + } + return nil +} + +func buildServiceUrl(limitadorObj *limitadorv1alpha1.Limitador) string { + return "http://" + + helpers.GetValueOrDefault(*limitadorObj.Spec.Service.Name, limitador.DefaultServiceName).(string) + "." + + limitadorObj.Namespace + ".svc.cluster.local:" + + strconv.Itoa(int(helpers.GetValueOrDefault(*limitadorObj.Spec.Service.Ports.HTTP, limitador.DefaultServiceName).(int32))) +} + func mutateLimitadorDeployment(existingObj, desiredObj client.Object) (bool, error) { existing, ok := existingObj.(*appsv1.Deployment) if !ok { diff --git a/controllers/limitador_controller_test.go b/controllers/limitador_controller_test.go index b35e8ff4..9fdd48b0 100644 --- a/controllers/limitador_controller_test.go +++ b/controllers/limitador_controller_test.go @@ -15,22 +15,32 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" - "github.com/kuadrant/limitador-operator/pkg/limitador" ) var _ = Describe("Limitador controller", func() { const ( - LimitadorNamespace = "default" - LimitadorReplicas = 2 - LimitadorImage = "quay.io/3scale/limitador" - LimitadorVersion = "0.3.0" + LimitadorNamespace = "default" + LimitadorReplicas = 2 + LimitadorImage = "quay.io/3scale/limitador" + LimitadorVersion = "0.3.0" + LimitadorServiceName = "limitador-service" + LimitadorHttpPort = 8000 + LimitadorGttpPort = 8001 timeout = time.Second * 10 interval = time.Millisecond * 250 ) + serviceName := LimitadorServiceName + httpPort := int32(LimitadorHttpPort) + grpcPort := int32(LimitadorGttpPort) + replicas := LimitadorReplicas version := LimitadorVersion + ports := limitadorv1alpha1.Ports{ + GRPC: &grpcPort, + HTTP: &httpPort, + } newLimitador := func() *limitadorv1alpha1.Limitador { // The name can't start with a number. name := "a" + string(uuid.NewUUID()) @@ -47,6 +57,10 @@ var _ = Describe("Limitador controller", func() { Spec: limitadorv1alpha1.LimitadorSpec{ Replicas: &replicas, Version: &version, + Service: limitadorv1alpha1.Service{ + Name: &serviceName, + Ports: ports, + }, }, } } @@ -93,13 +107,27 @@ var _ = Describe("Limitador controller", func() { context.TODO(), types.NamespacedName{ Namespace: LimitadorNamespace, - Name: limitador.ServiceName, // Hardcoded for now + Name: LimitadorServiceName, }, &createdLimitadorService) - return err == nil }, timeout, interval).Should(BeTrue()) }) + It("Should build the correct Status", func() { + // Now checking only the ServiceURL, a more thorough test coming in the future when we have more fields + createdLimitador := limitadorv1alpha1.Limitador{} + Eventually(func() string { + k8sClient.Get( + context.TODO(), + types.NamespacedName{ + Namespace: LimitadorNamespace, + Name: limitadorObj.Name, + }, + &createdLimitador) + return createdLimitador.Status.ServiceURL + }, timeout, interval).Should(Equal("http://limitador-service.default.svc.cluster.local:8000")) + + }) }) Context("Updating a limitador object", func() { diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index bd2c714e..b8ec09f1 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -52,3 +52,10 @@ func FetchEnv(key string, def string) string { return val } + +func GetValueOrDefault(expectedValue interface{}, defaultValue interface{}) interface{} { + if expectedValue != nil { + return expectedValue + } + return defaultValue +} diff --git a/pkg/limitador/k8s_objects.go b/pkg/limitador/k8s_objects.go index d2924b39..eb5907c6 100644 --- a/pkg/limitador/k8s_objects.go +++ b/pkg/limitador/k8s_objects.go @@ -2,6 +2,7 @@ package limitador import ( limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "github.com/kuadrant/limitador-operator/pkg/helpers" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -9,13 +10,13 @@ import ( ) const ( - DefaultVersion = "latest" - DefaultReplicas = 1 - ServiceName = "limitador" - Image = "quay.io/3scale/limitador" - StatusEndpoint = "/status" - ServiceHTTPPort = 8080 - ServiceGRPCPort = 8081 + DefaultVersion = "latest" + DefaultReplicas = 1 + DefaultServiceName = "limitador" + Image = "quay.io/3scale/limitador" + StatusEndpoint = "/status" + DefaultServiceHTTPPort = 8080 + DefaultServiceGRPCPort = 8081 ) func LimitadorService(limitador *limitadorv1alpha1.Limitador) *v1.Service { @@ -25,7 +26,7 @@ func LimitadorService(limitador *limitadorv1alpha1.Limitador) *v1.Service { APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: ServiceName, + Name: helpers.GetValueOrDefault(*limitador.Spec.Service.Name, DefaultServiceName).(string), Namespace: limitador.ObjectMeta.Namespace, // TODO: revisit later. For now assume same. Labels: labels(), OwnerReferences: []metav1.OwnerReference{ownerRefToLimitador(limitador)}, @@ -35,13 +36,13 @@ func LimitadorService(limitador *limitadorv1alpha1.Limitador) *v1.Service { { Name: "http", Protocol: v1.ProtocolTCP, - Port: ServiceHTTPPort, + Port: helpers.GetValueOrDefault(*limitador.Spec.Service.Ports.HTTP, DefaultServiceHTTPPort).(int32), TargetPort: intstr.FromString("http"), }, { Name: "grpc", Protocol: v1.ProtocolTCP, - Port: ServiceGRPCPort, + Port: helpers.GetValueOrDefault(*limitador.Spec.Service.Ports.GRPC, DefaultServiceGRPCPort).(int32), TargetPort: intstr.FromString("grpc"), }, }, @@ -91,12 +92,12 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym Ports: []v1.ContainerPort{ { Name: "http", - ContainerPort: ServiceHTTPPort, + ContainerPort: helpers.GetValueOrDefault(*limitador.Spec.Service.Ports.HTTP, DefaultServiceHTTPPort).(int32), Protocol: v1.ProtocolTCP, }, { Name: "grpc", - ContainerPort: ServiceGRPCPort, + ContainerPort: helpers.GetValueOrDefault(*limitador.Spec.Service.Ports.GRPC, DefaultServiceGRPCPort).(int32), Protocol: v1.ProtocolTCP, }, }, @@ -110,7 +111,7 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym Handler: v1.Handler{ HTTPGet: &v1.HTTPGetAction{ Path: StatusEndpoint, - Port: intstr.FromInt(ServiceHTTPPort), + Port: intstr.FromInt(int(helpers.GetValueOrDefault(*limitador.Spec.Service.Ports.HTTP, DefaultServiceHTTPPort).(int32))), Scheme: v1.URISchemeHTTP, }, }, @@ -124,7 +125,7 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym Handler: v1.Handler{ HTTPGet: &v1.HTTPGetAction{ Path: StatusEndpoint, - Port: intstr.FromInt(ServiceHTTPPort), + Port: intstr.FromInt(int(helpers.GetValueOrDefault(*limitador.Spec.Service.Ports.HTTP, DefaultServiceHTTPPort).(int32))), Scheme: v1.URISchemeHTTP, }, },