Address comments on naming and default values and ctx logger

Author: ChristianZaccaria

go.mod

@@ -3,7 +3,6 @@ module github.com/project-codeflare/codeflare-operator
 go 1.20
 
 require (
-	github.com/go-logr/logr v1.2.4
 	github.com/onsi/ginkgo/v2 v2.11.0
 	github.com/onsi/gomega v1.27.10
 	github.com/openshift/api v0.0.0-20230213134911-7ba313770556
@@ -43,6 +42,7 @@ require (
 	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
 	github.com/felixge/httpsnoop v1.0.3 // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
+	github.com/go-logr/logr v1.2.4 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-logr/zapr v1.2.4 // indirect
 	github.com/go-openapi/jsonpointer v0.19.6 // indirect

pkg/controllers/raycluster_controller.go

@@ -101,7 +101,7 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	isLocalInteractive := annotationBoolVal(logger, &cluster, "sdk.codeflare.dev/local_interactive")
+	isLocalInteractive := annotationBoolVal(ctx, &cluster, "sdk.codeflare.dev/local_interactive", false)
 	ingressDomain := cluster.ObjectMeta.Annotations["sdk.codeflare.dev/ingress_domain"]
 	isOpenShift, ingressHost := getClusterType(ctx, r.kubeClient, &cluster, ingressDomain)
 
@@ -172,7 +172,7 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 		if isLocalInteractive && ingressDomain != "" {
 			logger.Info("Creating RayClient Route")
-			_, err := r.routeClient.Routes(cluster.Namespace).Apply(ctx, createRayClientRoute(&cluster, ingressDomain), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
+			_, err := r.routeClient.Routes(cluster.Namespace).Apply(ctx, desiredRayClientRoute(&cluster, ingressDomain), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
 			if err != nil {
 				logger.Error(err, "Failed to update RayClient Route")
 				return ctrl.Result{RequeueAfter: requeueTime}, err
@@ -181,15 +181,15 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	} else if cluster.Status.State != "suspended" && !r.isRayDashboardOAuthEnabled() && !isOpenShift {
 		logger.Info("Creating Dashboard Ingress")
-		_, err := r.kubeClient.NetworkingV1().Ingresses(cluster.Namespace).Apply(ctx, createIngressApplyConfiguration(&cluster, ingressHost), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
+		_, err := r.kubeClient.NetworkingV1().Ingresses(cluster.Namespace).Apply(ctx, desiredClusterIngress(&cluster, ingressHost), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
 		if err != nil {
 			// This log is info level since errors are not fatal and are expected
 			logger.Info("WARN: Failed to update Dashboard Ingress", "error", err.Error(), logRequeueing, true)
 			return ctrl.Result{RequeueAfter: requeueTime}, err
 		}
 		if isLocalInteractive && ingressDomain != "" {
 			logger.Info("Creating RayClient Ingress")
-			_, err := r.kubeClient.NetworkingV1().Ingresses(cluster.Namespace).Apply(ctx, createRayClientIngress(&cluster, ingressDomain), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
+			_, err := r.kubeClient.NetworkingV1().Ingresses(cluster.Namespace).Apply(ctx, desiredRayClientIngress(&cluster, ingressDomain), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
 			if err != nil {
 				logger.Error(err, "Failed to update RayClient Ingress")
 				return ctrl.Result{RequeueAfter: requeueTime}, err

pkg/controllers/support.go

@@ -6,7 +6,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/go-logr/logr"
 	rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"
 
 	networkingv1 "k8s.io/api/networking/v1"
@@ -27,7 +26,7 @@ func serviceNameFromCluster(cluster *rayv1.RayCluster) string {
 	return cluster.Name + "-head-svc"
 }
 
-func createRayClientRoute(cluster *rayv1.RayCluster, ingressDomain string) *routeapply.RouteApplyConfiguration {
+func desiredRayClientRoute(cluster *rayv1.RayCluster, ingressDomain string) *routeapply.RouteApplyConfiguration {
 	return routeapply.Route(rayClientNameFromCluster(cluster), cluster.Namespace).
 		WithLabels(map[string]string{"ray.io/cluster-name": cluster.Name}).
 		WithSpec(routeapply.RouteSpec().
@@ -42,7 +41,7 @@ func createRayClientRoute(cluster *rayv1.RayCluster, ingressDomain string) *rout
 }
 
 // Create an Ingress object for the RayCluster
-func createRayClientIngress(cluster *rayv1.RayCluster, ingressDomain string) *networkingv1ac.IngressApplyConfiguration {
+func desiredRayClientIngress(cluster *rayv1.RayCluster, ingressDomain string) *networkingv1ac.IngressApplyConfiguration {
 	return networkingv1ac.Ingress(rayClientNameFromCluster(cluster), cluster.Namespace).
 		WithLabels(map[string]string{"ray.io/cluster-name": cluster.Name}).
 		WithAnnotations(map[string]string{
@@ -78,7 +77,7 @@ func createRayClientIngress(cluster *rayv1.RayCluster, ingressDomain string) *ne
 }
 
 // Create an Ingress object for the RayCluster
-func createIngressApplyConfiguration(cluster *rayv1.RayCluster, ingressHost string) *networkingv1ac.IngressApplyConfiguration {
+func desiredClusterIngress(cluster *rayv1.RayCluster, ingressHost string) *networkingv1ac.IngressApplyConfiguration {
 	return networkingv1ac.Ingress(dashboardNameFromCluster(cluster), cluster.Namespace).
 		WithLabels(map[string]string{"ray.io/cluster-name": cluster.Name}).
 		WithOwnerReferences(v1.OwnerReference().
@@ -167,15 +166,16 @@ func (r *RayClusterReconciler) isRayDashboardOAuthEnabled() bool {
 	return true
 }
 
-func annotationBoolVal(logger logr.Logger, cluster *rayv1.RayCluster, annotation string) bool {
+func annotationBoolVal(ctx context.Context, cluster *rayv1.RayCluster, annotation string, defaultValue bool) bool {
+	logger := ctrl.LoggerFrom(ctx)
 	val, exists := cluster.ObjectMeta.Annotations[annotation]
 	if !exists || val == "" {
-		return false
+		return defaultValue
 	}
 	boolVal, err := strconv.ParseBool(val)
 	if err != nil {
 		logger.Error(err, "Could not convert annotation value to bool", "annotation", annotation, "value", val)
-		return false
+		return defaultValue
 	}
 	return boolVal
 }