Addressing comments on ingressDomain

Author: ChristianZaccaria

pkg/controllers/raycluster_controller.go

@@ -102,8 +102,8 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	isLocalInteractive := annotationBoolVal(logger, &cluster, "sdk.codeflare.dev/local_interactive")
-	isOpenShift, ingressHost := getClusterType(logger, r.kubeClient, &cluster)
-	ingressDomain := getIngressDomain(&cluster)
+	ingressDomain := cluster.ObjectMeta.Annotations["sdk.codeflare.dev/ingress_domain"]
+	isOpenShift, ingressHost := getClusterType(logger, r.kubeClient, &cluster, ingressDomain)
 
 	if cluster.ObjectMeta.DeletionTimestamp.IsZero() {
 		if !controllerutil.ContainsFinalizer(&cluster, oAuthFinalizer) {
@@ -167,7 +167,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), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
+			_, err := r.routeClient.Routes(cluster.Namespace).Apply(ctx, createRayClientRoute(&cluster, ingressDomain), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
 			if err != nil {
 				logger.Error(err, "Failed to update RayClient Route")
 			}
@@ -182,7 +182,7 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 		if isLocalInteractive && ingressDomain != "" {
 			logger.Info("Creating RayClient Ingress")
-			_, err := r.kubeClient.NetworkingV1().Ingresses(cluster.Namespace).Apply(ctx, createRayClientIngress(&cluster), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
+			_, err := r.kubeClient.NetworkingV1().Ingresses(cluster.Namespace).Apply(ctx, createRayClientIngress(&cluster, ingressDomain), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
 			if err != nil {
 				logger.Error(err, "Failed to update RayClient Ingress")
 			}

pkg/controllers/support.go

@@ -27,12 +27,11 @@ func serviceNameFromCluster(cluster *rayv1.RayCluster) string {
 	return cluster.Name + "-head-svc"
 }
 
-func createRayClientRoute(cluster *rayv1.RayCluster) *routeapply.RouteApplyConfiguration {
-	ingress_domain := cluster.ObjectMeta.Annotations["sdk.codeflare.dev/ingress_domain"]
+func createRayClientRoute(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().
-			WithHost(rayClientNameFromCluster(cluster) + "-" + cluster.Namespace + "." + ingress_domain).
+			WithHost(rayClientNameFromCluster(cluster) + "-" + cluster.Namespace + "." + ingressDomain).
 			WithTo(routeapply.RouteTargetReference().WithKind("Service").WithName(serviceNameFromCluster(cluster)).WithWeight(100)).
 			WithPort(routeapply.RoutePort().WithTargetPort(intstr.FromString("client"))).
 			WithTLS(routeapply.TLSConfig().WithTermination("passthrough")),
@@ -43,8 +42,7 @@ func createRayClientRoute(cluster *rayv1.RayCluster) *routeapply.RouteApplyConfi
 }
 
 // Create an Ingress object for the RayCluster
-func createRayClientIngress(cluster *rayv1.RayCluster) *networkingv1ac.IngressApplyConfiguration {
-	ingress_domain := cluster.ObjectMeta.Annotations["sdk.codeflare.dev/ingress_domain"]
+func createRayClientIngress(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{
@@ -60,7 +58,7 @@ func createRayClientIngress(cluster *rayv1.RayCluster) *networkingv1ac.IngressAp
 		WithSpec(networkingv1ac.IngressSpec().
 			WithIngressClassName("nginx").
 			WithRules(networkingv1ac.IngressRule().
-				WithHost(rayClientNameFromCluster(cluster) + "-" + cluster.Namespace + "." + ingress_domain).
+				WithHost(rayClientNameFromCluster(cluster) + "-" + cluster.Namespace + "." + ingressDomain).
 				WithHTTP(networkingv1ac.HTTPIngressRuleValue().
 					WithPaths(networkingv1ac.HTTPIngressPath().
 						WithPath("/").
@@ -90,7 +88,7 @@ func createIngressApplyConfiguration(cluster *rayv1.RayCluster, ingressHost stri
 			WithUID(types.UID(cluster.UID))).
 		WithSpec(networkingv1ac.IngressSpec().
 			WithRules(networkingv1ac.IngressRule().
-				WithHost(ingressHost). // kind host name or ingress_domain
+				WithHost(ingressHost). // KinD hostname or ingressDomain
 				WithHTTP(networkingv1ac.HTTPIngressRuleValue().
 					WithPaths(networkingv1ac.HTTPIngressPath().
 						WithPath("/").
@@ -129,9 +127,8 @@ func getDiscoveryClient(config *rest.Config) (*discovery.DiscoveryClient, error)
 
 // Check where we are running. We are trying to distinguish here whether
 // this is vanilla kubernetes cluster or Openshift
-func getClusterType(logger logr.Logger, clientset *kubernetes.Clientset, cluster *rayv1.RayCluster) (bool, string) {
+func getClusterType(logger logr.Logger, clientset *kubernetes.Clientset, cluster *rayv1.RayCluster, ingressDomain string) (bool, string) {
 	// The discovery package is used to discover APIs supported by a Kubernetes API server.
-	ingress_domain := cluster.ObjectMeta.Annotations["sdk.codeflare.dev/ingress_domain"]
 	config, err := ctrl.GetConfig()
 	if err == nil && config != nil {
 		dclient, err := getDiscoveryClient(config)
@@ -148,21 +145,21 @@ func getClusterType(logger logr.Logger, clientset *kubernetes.Clientset, cluster
 					}
 				}
 				onKind, _ := isOnKindCluster(clientset)
-				if onKind && ingress_domain == "" {
+				if onKind && ingressDomain == "" {
 					logger.Info("We detected being on a KinD cluster!")
 					return false, "kind"
 				} else {
 					logger.Info("We detected being on Vanilla Kubernetes!")
-					return false, fmt.Sprintf("ray-dashboard-%s-%s.%s", cluster.Name, cluster.Namespace, ingress_domain)
+					return false, fmt.Sprintf("ray-dashboard-%s-%s.%s", cluster.Name, cluster.Namespace, ingressDomain)
 				}
 			}
 		} else {
 			logger.Info("Cannot retrieve a DiscoveryClient, assuming we're on Vanilla Kubernetes")
-			return false, fmt.Sprintf("ray-dashboard-%s-%s.%s", cluster.Name, cluster.Namespace, ingress_domain)
+			return false, fmt.Sprintf("ray-dashboard-%s-%s.%s", cluster.Name, cluster.Namespace, ingressDomain)
 		}
 	} else {
 		logger.Info("Cannot retrieve config, assuming we're on Vanilla Kubernetes")
-		return false, fmt.Sprintf("ray-dashboard-%s-%s.%s", cluster.Name, cluster.Namespace, ingress_domain)
+		return false, fmt.Sprintf("ray-dashboard-%s-%s.%s", cluster.Name, cluster.Namespace, ingressDomain)
 	}
 }
 
@@ -185,10 +182,3 @@ func annotationBoolVal(logger logr.Logger, cluster *rayv1.RayCluster, annotation
 	}
 	return boolVal
 }
-
-func getIngressDomain(cluster *rayv1.RayCluster) string {
-	if ingressDomain, exists := cluster.ObjectMeta.Annotations["sdk.codeflare.dev/ingress_domain"]; exists {
-		return ingressDomain
-	}
-	return ""
-}