Skip to content

Commit

Permalink
address comments from PR review
Browse files Browse the repository at this point in the history
Signed-off-by: Kevin <[email protected]>
  • Loading branch information
KPostOffice authored and openshift-merge-bot[bot] committed Apr 3, 2024
1 parent 4f0c9c1 commit 9ea041f
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 25 deletions.
36 changes: 14 additions & 22 deletions controllers/raycluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
coreapply "k8s.io/client-go/applyconfigurations/core/v1"
v1 "k8s.io/client-go/applyconfigurations/meta/v1"
Expand All @@ -56,12 +55,9 @@ type RayClusterReconciler struct {
const (
requeueTime = 10
controllerName = "codeflare-raycluster-controller"
oauthAnnotation = "codeflare.dev/oauth=true"
CodeflareOAuthFinalizer = "codeflare.dev/oauth-finalizer"
CodeflareOAuthFinalizer = "openshift.ai/oauth-finalizer"
OAuthServicePort = 443
OAuthServicePortName = "oauth-proxy"
strTrue = "true"
strFalse = "false"
logRequeueing = "requeueing"
)

Expand Down Expand Up @@ -106,24 +102,30 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
controllerutil.AddFinalizer(&cluster, CodeflareOAuthFinalizer)
if err := r.Update(ctx, &cluster); err != nil {
// this log is info level since errors are not fatal and are expected
logger.Info("WARN: Failed to update RayCluster with finalizer", "error", err.Error(), logRequeueing, strTrue)
logger.Info("WARN: Failed to update RayCluster with finalizer", "error", err.Error(), logRequeueing, true)
return ctrl.Result{RequeueAfter: requeueTime}, err
}
}
} else if controllerutil.ContainsFinalizer(&cluster, CodeflareOAuthFinalizer) {
err := r.deleteIfNotExist(
ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: crbNameFromCluster(&cluster)}, &rbacv1.ClusterRoleBinding{},
)
err := client.IgnoreNotFound(r.Client.Delete(
ctx,
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: crbNameFromCluster(&cluster),
},
},
&deleteOptions,
))
if err != nil {
logger.Error(err, "Failed to remove OAuth ClusterRoleBinding.", logRequeueing, strTrue)
logger.Error(err, "Failed to remove OAuth ClusterRoleBinding.", logRequeueing, true)
return ctrl.Result{RequeueAfter: requeueTime}, err
}
controllerutil.RemoveFinalizer(&cluster, CodeflareOAuthFinalizer)
if err := r.Update(ctx, &cluster); err != nil {
logger.Error(err, "Failed to remove finalizer from RayCluster", logRequeueing, strTrue)
logger.Error(err, "Failed to remove finalizer from RayCluster", logRequeueing, true)
return ctrl.Result{RequeueAfter: requeueTime}, err
}
logger.Info("Successfully removed finalizer.", logRequeueing, strFalse)
logger.Info("Successfully removed finalizer.", logRequeueing, false)
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -159,16 +161,6 @@ func crbNameFromCluster(cluster *rayv1.RayCluster) string {
return cluster.Name + "-" + cluster.Namespace + "-auth" // NOTE: potential naming conflicts ie {name: foo, ns: bar-baz} and {name: foo-bar, ns: baz}
}

func (r *RayClusterReconciler) deleteIfNotExist(ctx context.Context, namespacedName types.NamespacedName, obj client.Object) error {
err := r.Client.Get(ctx, namespacedName, obj)
if err != nil && errors.IsNotFound(err) {
return nil
} else if err != nil {
return err
}
return r.Client.Delete(ctx, obj, &deleteOptions)
}

func desiredOAuthClusterRoleBinding(cluster *rayv1.RayCluster) *rbacapply.ClusterRoleBindingApplyConfiguration {
return rbacapply.ClusterRoleBinding(
crbNameFromCluster(cluster)).
Expand Down
6 changes: 4 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ func main() {
MaxScaleoutAllowed: 5,
},
},
RayClusterOAuth: pointer.Bool(true),
KubeRay: &config.KubeRayConfiguration{
RayDashboardOAuthEnabled: pointer.Bool(true),
},
}

kubeConfig, err := ctrl.GetConfig()
Expand Down Expand Up @@ -182,7 +184,7 @@ func main() {
}

v, err := HasAPIResourceForGVK(kubeClient.DiscoveryClient, rayv1.GroupVersion.WithKind("RayCluster"))
if v && *cfg.RayClusterOAuth {
if v && *cfg.KubeRay.RayDashboardOAuthEnabled {
rayClusterController := cfoControllers.RayClusterReconciler{Client: mgr.GetClient(), Scheme: mgr.GetScheme()}
exitOnError(rayClusterController.SetupWithManager(mgr), "Error setting up RayCluster controller")
} else if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ type CodeFlareOperatorConfiguration struct {
// The InstaScale controller configuration
InstaScale *InstaScaleConfiguration `json:"instascale,omitempty"`

RayClusterOAuth *bool `json:"rayClusterOAuth,omitempty"`
KubeRay *KubeRayConfiguration `json:"kuberay,omitempty"`
}

type KubeRayConfiguration struct {
RayDashboardOAuthEnabled *bool `json:"rayDashboardOAuthEnabled,omitempty"`
}

type InstaScaleConfiguration struct {
Expand Down

0 comments on commit 9ea041f

Please sign in to comment.