Skip to content

Commit

Permalink
make oauth part of config and remove annotation check
Browse files Browse the repository at this point in the history 
Signed-off-by: Kevin <[email protected]>
  • Loading branch information
@KPostOffice @openshift-merge-bot
KPostOffice authored and openshift-merge-bot[bot] committed Apr 3, 2024
1 parent 91f8bf3 commit 4f0c9c1
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 132 deletions.
48 changes: 1 addition & 47 deletions controllers/raycluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"crypto/rand"
"crypto/sha1"
"encoding/base64"
"strconv"

rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"

Expand Down Expand Up @@ -128,52 +127,7 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}

val, ok := cluster.ObjectMeta.Annotations["codeflare.dev/oauth"]
boolVal, err := strconv.ParseBool(val)
if err != nil {
logger.Error(err, "Could not convert codeflare.dev/oauth value to bool", "codeflare.dev/oauth", val)
}
if !ok || err != nil || !boolVal {
logger.Info("Removing all OAuth Objects")
err := r.deleteIfNotExist(
ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: oauthSecretNameFromCluster(&cluster)}, &corev1.Secret{},
)
if err != nil {
logger.Error(err, "Error deleting OAuth Secret, retrying", logRequeueing, strTrue)
return ctrl.Result{RequeueAfter: requeueTime}, nil
}
err = r.deleteIfNotExist(
ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: oauthServiceNameFromCluster(&cluster)}, &corev1.Service{},
)
if err != nil {
logger.Error(err, "Error deleting OAuth Service, retrying", logRequeueing, strTrue)
return ctrl.Result{RequeueAfter: requeueTime}, nil
}
err = r.deleteIfNotExist(
ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: oauthServiceAccountNameFromCluster(&cluster)}, &corev1.ServiceAccount{},
)
if err != nil {
logger.Error(err, "Error deleting OAuth ServiceAccount, retrying", logRequeueing, strTrue)
return ctrl.Result{RequeueAfter: requeueTime}, nil
}
err = r.deleteIfNotExist(
ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: crbNameFromCluster(&cluster)}, &rbacv1.ClusterRoleBinding{},
)
if err != nil {
logger.Error(err, "Error deleting OAuth CRB, retrying", logRequeueing, strTrue)
return ctrl.Result{RequeueAfter: requeueTime}, nil
}
err = r.deleteIfNotExist(
ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: routeNameFromCluster(&cluster)}, &routev1.Route{},
)
if err != nil {
logger.Error(err, "Error deleting OAuth Route, retrying", logRequeueing, strTrue)
return ctrl.Result{RequeueAfter: requeueTime}, nil
}
return ctrl.Result{}, nil
}

_, err = r.routeClient.Routes(cluster.Namespace).Apply(ctx, desiredClusterRoute(&cluster), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
_, err := r.routeClient.Routes(cluster.Namespace).Apply(ctx, desiredClusterRoute(&cluster), metav1.ApplyOptions{FieldManager: controllerName, Force: true})
if err != nil {
logger.Error(err, "Failed to update OAuth Route")
}
Expand Down
132 changes: 48 additions & 84 deletions controllers/raycluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,98 +125,62 @@ var _ = Describe("RayCluster controller", func() {
}, SpecTimeout(time.Second*10)).Should(Equal(true))
})

Context("Cluster has OAuth annotation", func() {
BeforeEach(func() {
By("adding OAuth annotation to RayCluster")
Eventually(func() error {
foundRayCluster := rayv1.RayCluster{}
err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
Expect(err).To(Not(HaveOccurred()))
if foundRayCluster.Annotations == nil {
foundRayCluster.Annotations = map[string]string{"codeflare.dev/oauth": "true"}
} else {
foundRayCluster.Annotations["codeflare.dev/oauth"] = "'true'"
}
return k8sClient.Update(ctx, &foundRayCluster)
}, SpecTimeout(time.Second*10)).Should(Not(HaveOccurred()))
By("waiting for dependent resources to be created")
Eventually(func() error {
foundRayCluster := rayv1.RayCluster{}
err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthSecretNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Secret{})
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthServiceNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Service{})
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &corev1.ServiceAccount{})
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{})
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &routev1.Route{})
if err != nil {
return err
}
return nil
}, SpecTimeout(time.Second*10)).Should(Not(HaveOccurred()))
})

It("should set owner references for all resources", func() {
It("should create all oauth resources", func() {
Eventually(func() error {
foundRayCluster := rayv1.RayCluster{}
err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
Expect(err).ToNot(HaveOccurred())
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthSecretNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Secret{})
Expect(err).To(Not(HaveOccurred()))
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthServiceNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Service{})
Expect(err).To(Not(HaveOccurred()))
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &corev1.ServiceAccount{})
Expect(err).To(Not(HaveOccurred()))
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{})
Expect(err).To(Not(HaveOccurred()))
if err != nil {
return err
}
err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &routev1.Route{})
Expect(err).To(Not(HaveOccurred()))
})
if err != nil {
return err
}
return nil
}, SpecTimeout(time.Second*10)).Should(Not(HaveOccurred()))
})

It("should delete OAuth resources when annotation is removed", func() {
foundRayCluster := rayv1.RayCluster{}
err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
Expect(err).To(Not(HaveOccurred()))
delete(foundRayCluster.Annotations, "codeflare.dev/oauth")
err = k8sClient.Update(ctx, &foundRayCluster)
Expect(err).To(Not(HaveOccurred()))
Eventually(func() bool {
return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: oauthSecretNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Secret{}))
}, SpecTimeout(time.Second*10)).Should(Equal(true))
Eventually(func() bool {
return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: oauthServiceNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Service{}))
}, SpecTimeout(time.Second*10)).Should(Equal(true))
Eventually(func() bool {
return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &corev1.ServiceAccount{}))
}, SpecTimeout(time.Second*10)).Should(Equal(true))
Eventually(func() bool {
return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{}))
}, SpecTimeout(time.Second*10)).Should(Equal(true))
})

It("should remove CRB when the RayCluster is deleted", func() {
foundRayCluster := rayv1.RayCluster{}
err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
Expect(err).To(Not(HaveOccurred()))
err = k8sClient.Delete(ctx, &foundRayCluster)
Expect(err).To(Not(HaveOccurred()))
Eventually(func() bool {
return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{}))
}, SpecTimeout(time.Second*10)).Should(Equal(true))
})
It("should set owner references for all resources", func() {
foundRayCluster := rayv1.RayCluster{}
err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
Expect(err).ToNot(HaveOccurred())
err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthSecretNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Secret{})
Expect(err).To(Not(HaveOccurred()))
err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthServiceNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Service{})
Expect(err).To(Not(HaveOccurred()))
err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &corev1.ServiceAccount{})
Expect(err).To(Not(HaveOccurred()))
err = k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{})
Expect(err).To(Not(HaveOccurred()))
err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &routev1.Route{})
Expect(err).To(Not(HaveOccurred()))
})

It("should remove CRB when the RayCluster is deleted", func() {
foundRayCluster := rayv1.RayCluster{}
err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
Expect(err).To(Not(HaveOccurred()))
err = k8sClient.Delete(ctx, &foundRayCluster)
Expect(err).To(Not(HaveOccurred()))
Eventually(func() bool {
return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{}))
}, SpecTimeout(time.Second*10)).Should(Equal(true))
})
})
})
4 changes: 3 additions & 1 deletion main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func main() {
MaxScaleoutAllowed: 5,
},
},
RayClusterOAuth: pointer.Bool(true),
}

kubeConfig, err := ctrl.GetConfig()
Expand Down Expand Up @@ -180,7 +181,8 @@ func main() {
exitOnError(instaScaleController.SetupWithManager(context.Background(), mgr), "Error setting up InstaScale controller")
}

if v, err := HasAPIResourceForGVK(kubeClient.DiscoveryClient, rayv1.GroupVersion.WithKind("RayCluster")); v {
v, err := HasAPIResourceForGVK(kubeClient.DiscoveryClient, rayv1.GroupVersion.WithKind("RayCluster"))
if v && *cfg.RayClusterOAuth {
rayClusterController := cfoControllers.RayClusterReconciler{Client: mgr.GetClient(), Scheme: mgr.GetScheme()}
exitOnError(rayClusterController.SetupWithManager(mgr), "Error setting up RayCluster controller")
} else if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ type CodeFlareOperatorConfiguration struct {

// The InstaScale controller configuration
InstaScale *InstaScaleConfiguration `json:"instascale,omitempty"`

RayClusterOAuth *bool `json:"rayClusterOAuth,omitempty"`
}

type InstaScaleConfiguration struct {
Expand Down

0 comments on commit 4f0c9c1

Please sign in to comment.