diff --git a/ray-operator/apis/ray/v1/raycluster_webhook.go b/ray-operator/apis/ray/v1/raycluster_webhook.go deleted file mode 100644 index 6650ef9534f..00000000000 --- a/ray-operator/apis/ray/v1/raycluster_webhook.go +++ /dev/null @@ -1,89 +0,0 @@ -package v1 - -import ( - "regexp" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/validation/field" - ctrl "sigs.k8s.io/controller-runtime" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -// log is for logging in this package. -var ( - rayclusterlog = logf.Log.WithName("raycluster-resource") - nameRegex, _ = regexp.Compile("^[a-z]([-a-z0-9]*[a-z0-9])?$") -) - -func (r *RayCluster) SetupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(r). - Complete() -} - -// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. -//+kubebuilder:webhook:path=/validate-ray-io-v1-raycluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=vraycluster.kb.io,admissionReviewVersions=v1 - -var _ webhook.Validator = &RayCluster{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *RayCluster) ValidateCreate() (admission.Warnings, error) { - rayclusterlog.Info("validate create", "name", r.Name) - return nil, r.validateRayCluster() -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type -func (r *RayCluster) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) { - rayclusterlog.Info("validate update", "name", r.Name) - return nil, r.validateRayCluster() -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type -func (r *RayCluster) ValidateDelete() (admission.Warnings, error) { - rayclusterlog.Info("validate delete", "name", r.Name) - return nil, nil -} - -func (r *RayCluster) validateRayCluster() error { - var allErrs field.ErrorList - - if err := r.validateName(); err != nil { - allErrs = append(allErrs, err) - } - - if err := r.validateWorkerGroups(); err != nil { - allErrs = append(allErrs, err) - } - - if len(allErrs) == 0 { - return nil - } - - return apierrors.NewInvalid( - schema.GroupKind{Group: "ray.io", Kind: "RayCluster"}, - r.Name, allErrs) -} - -func (r *RayCluster) validateName() *field.Error { - if !nameRegex.MatchString(r.Name) { - return field.Invalid(field.NewPath("metadata").Child("name"), r.Name, "name must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')") - } - return nil -} - -func (r *RayCluster) validateWorkerGroups() *field.Error { - workerGroupNames := make(map[string]bool) - - for i, workerGroup := range r.Spec.WorkerGroupSpecs { - if _, ok := workerGroupNames[workerGroup.GroupName]; ok { - return field.Invalid(field.NewPath("spec").Child("workerGroupSpecs").Index(i), workerGroup, "worker group names must be unique") - } - workerGroupNames[workerGroup.GroupName] = true - } - - return nil -} diff --git a/ray-operator/apis/ray/v1/zz_generated.deepcopy.go b/ray-operator/apis/ray/v1/zz_generated.deepcopy.go index 77a202d4058..b9f303aad1c 100644 --- a/ray-operator/apis/ray/v1/zz_generated.deepcopy.go +++ b/ray-operator/apis/ray/v1/zz_generated.deepcopy.go @@ -7,7 +7,7 @@ package v1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" + runtime "k8s.io/apimachinery/pkg/runtime" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. diff --git a/ray-operator/main.go b/ray-operator/main.go index 0446ec49e67..b9d5c1bd6c1 100644 --- a/ray-operator/main.go +++ b/ray-operator/main.go @@ -35,6 +35,7 @@ import ( "github.com/ray-project/kuberay/ray-operator/controllers/ray" "github.com/ray-project/kuberay/ray-operator/controllers/ray/utils" "github.com/ray-project/kuberay/ray-operator/pkg/features" + "github.com/ray-project/kuberay/ray-operator/pkg/webhooks/v1" // +kubebuilder:scaffold:imports ) @@ -241,7 +242,7 @@ func main() { "unable to create controller", "controller", "RayJob") if os.Getenv("ENABLE_WEBHOOKS") == "true" { - exitOnError((&rayv1.RayCluster{}).SetupWebhookWithManager(mgr), + exitOnError(v1.SetupRayClusterWebhookWithManager(mgr), "unable to create webhook", "webhook", "RayCluster") } // +kubebuilder:scaffold:builder diff --git a/ray-operator/pkg/webhooks/v1/raycluster_webhook.go b/ray-operator/pkg/webhooks/v1/raycluster_webhook.go new file mode 100644 index 00000000000..5d1d5409a19 --- /dev/null +++ b/ray-operator/pkg/webhooks/v1/raycluster_webhook.go @@ -0,0 +1,97 @@ +package v1 + +import ( + "context" + "regexp" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1" +) + +// log is for logging in this package. +var ( + rayclusterlog = logf.Log.WithName("raycluster-resource") + nameRegex, _ = regexp.Compile("^[a-z]([-a-z0-9]*[a-z0-9])?$") +) + +// SetupRayClusterWebhookWithManager registers the webhook for RayCluster in the manager. +func SetupRayClusterWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&rayv1.RayCluster{}). + WithValidator(&RayClusterWebhook{}). + Complete() +} + +type RayClusterWebhook struct{} + +// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. +//+kubebuilder:webhook:path=/validate-ray-io-v1-raycluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=vraycluster.kb.io,admissionReviewVersions=v1 + +var _ webhook.CustomValidator = &RayClusterWebhook{} + +// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type +func (w *RayClusterWebhook) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { + rayCluster := obj.(*rayv1.RayCluster) + rayclusterlog.Info("validate create", "name", rayCluster.Name) + return nil, w.validateRayCluster(rayCluster) +} + +// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type +func (w *RayClusterWebhook) ValidateUpdate(_ context.Context, _ runtime.Object, newObj runtime.Object) (admission.Warnings, error) { + rayCluster := newObj.(*rayv1.RayCluster) + rayclusterlog.Info("validate update", "name", rayCluster.Name) + return nil, w.validateRayCluster(rayCluster) +} + +// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type +func (w *RayClusterWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func (w *RayClusterWebhook) validateRayCluster(rayCluster *rayv1.RayCluster) error { + var allErrs field.ErrorList + + if err := w.validateName(rayCluster); err != nil { + allErrs = append(allErrs, err) + } + + if err := w.validateWorkerGroups(rayCluster); err != nil { + allErrs = append(allErrs, err) + } + + if len(allErrs) == 0 { + return nil + } + + return apierrors.NewInvalid( + schema.GroupKind{Group: "ray.io", Kind: "RayCluster"}, + rayCluster.Name, allErrs) +} + +func (w *RayClusterWebhook) validateName(rayCluster *rayv1.RayCluster) *field.Error { + if !nameRegex.MatchString(rayCluster.Name) { + return field.Invalid(field.NewPath("metadata").Child("name"), rayCluster.Name, "name must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')") + } + return nil +} + +func (w *RayClusterWebhook) validateWorkerGroups(rayCluster *rayv1.RayCluster) *field.Error { + workerGroupNames := make(map[string]bool) + + for i, workerGroup := range rayCluster.Spec.WorkerGroupSpecs { + if _, ok := workerGroupNames[workerGroup.GroupName]; ok { + return field.Invalid(field.NewPath("spec").Child("workerGroupSpecs").Index(i), workerGroup, "worker group names must be unique") + } + workerGroupNames[workerGroup.GroupName] = true + } + + return nil +} diff --git a/ray-operator/apis/ray/v1/webhook_suite_test.go b/ray-operator/pkg/webhooks/v1/webhook_suite_test.go similarity index 91% rename from ray-operator/apis/ray/v1/webhook_suite_test.go rename to ray-operator/pkg/webhooks/v1/webhook_suite_test.go index 52f4e10f402..52eba42b4b0 100644 --- a/ray-operator/apis/ray/v1/webhook_suite_test.go +++ b/ray-operator/pkg/webhooks/v1/webhook_suite_test.go @@ -27,6 +27,8 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/webhook" + + rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1" ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to @@ -67,7 +69,7 @@ var _ = BeforeSuite(func() { Expect(cfg).NotTo(BeNil()) scheme := runtime.NewScheme() - err = AddToScheme(scheme) + err = rayv1.AddToScheme(scheme) Expect(err).NotTo(HaveOccurred()) err = admissionv1beta1.AddToScheme(scheme) @@ -95,7 +97,7 @@ var _ = BeforeSuite(func() { }) Expect(err).NotTo(HaveOccurred()) - err = (&RayCluster{}).SetupWebhookWithManager(mgr) + err = SetupRayClusterWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:webhook @@ -126,13 +128,13 @@ var _ = BeforeSuite(func() { var _ = Describe("RayCluster validating webhook", func() { Context("when name is invalid", func() { It("should return error", func() { - rayCluster := RayCluster{ + rayCluster := rayv1.RayCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "invalid.name", }, - Spec: RayClusterSpec{ - HeadGroupSpec: HeadGroupSpec{ + Spec: rayv1.RayClusterSpec{ + HeadGroupSpec: rayv1.HeadGroupSpec{ RayStartParams: map[string]string{"DEADBEEF": "DEADBEEF"}, Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -140,7 +142,7 @@ var _ = Describe("RayCluster validating webhook", func() { }, }, }, - WorkerGroupSpecs: []WorkerGroupSpec{}, + WorkerGroupSpecs: []rayv1.WorkerGroupSpec{}, }, } @@ -153,7 +155,7 @@ var _ = Describe("RayCluster validating webhook", func() { Context("when groupNames are not unique", func() { var name, namespace string - var rayCluster RayCluster + var rayCluster rayv1.RayCluster BeforeEach(func() { namespace = "default" @@ -161,13 +163,13 @@ var _ = Describe("RayCluster validating webhook", func() { }) It("should return error", func() { - rayCluster = RayCluster{ + rayCluster = rayv1.RayCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, - Spec: RayClusterSpec{ - HeadGroupSpec: HeadGroupSpec{ + Spec: rayv1.RayClusterSpec{ + HeadGroupSpec: rayv1.HeadGroupSpec{ RayStartParams: map[string]string{"DEADBEEF": "DEADBEEF"}, Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -175,7 +177,7 @@ var _ = Describe("RayCluster validating webhook", func() { }, }, }, - WorkerGroupSpecs: []WorkerGroupSpec{ + WorkerGroupSpecs: []rayv1.WorkerGroupSpec{ { GroupName: "group1", RayStartParams: map[string]string{"DEADBEEF": "DEADBEEF"},