Implement webhook warnings for the MXJob (kubeflow#2058)

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

manifests/base/webhook/manifests.yaml

@@ -4,6 +4,27 @@ kind: ValidatingWebhookConfiguration
 metadata:
   name: validating-webhook-configuration
 webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-kubeflow-org-v1-mxjob
+  failurePolicy: Fail
+  name: validator.mxjob.training-operator.kubeflow.org
+  rules:
+  - apiGroups:
+    - kubeflow.org
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    - DELETE
+    resources:
+    - mxjobs
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

manifests/base/webhook/patch.yaml

@@ -10,6 +10,9 @@
 - op: replace
   path: /webhooks/3/clientConfig/service/name
   value: training-operator
+- op: replace
+  path: /webhooks/4/clientConfig/service/name
+  value: training-operator
 - op: replace
   path: /metadata/name
   value: validator.training-operator.kubeflow.org

pkg/controller.v1/mxnet/suite_test.go

@@ -16,11 +16,16 @@ package mxnet
 
 import (
 	"context"
+	"crypto/tls"
+	"fmt"
+	"net"
 	"path/filepath"
 	"testing"
+	"time"
 
 	kubeflowv1 "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
 	"github.com/kubeflow/training-operator/pkg/controller.v1/common"
+	mxnetwebhook "github.com/kubeflow/training-operator/pkg/webhooks/mxnet"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -31,6 +36,7 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"volcano.sh/apis/pkg/apis/scheduling/v1beta1"
 	//+kubebuilder:scaffold:imports
 )
@@ -60,6 +66,9 @@ var _ = BeforeSuite(func() {
 	testEnv = &envtest.Environment{
 		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "..", "manifests", "base", "crds")},
 		ErrorIfCRDPathMissing: true,
+		WebhookInstallOptions: envtest.WebhookInstallOptions{
+			Paths: []string{filepath.Join("..", "..", "..", "manifests", "base", "webhook", "manifests.yaml")},
+		},
 	}
 
 	cfg, err := testEnv.Start()
@@ -81,19 +90,34 @@ var _ = BeforeSuite(func() {
 		Metrics: metricsserver.Options{
 			BindAddress: "0",
 		},
+		WebhookServer: webhook.NewServer(
+			webhook.Options{
+				Host:    testEnv.WebhookInstallOptions.LocalServingHost,
+				Port:    testEnv.WebhookInstallOptions.LocalServingPort,
+				CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir,
+			}),
 	})
 	Expect(err).NotTo(HaveOccurred())
 
 	gangSchedulingSetupFunc := common.GenNonGangSchedulerSetupFunc()
 	r := NewReconciler(mgr, gangSchedulingSetupFunc)
 
 	Expect(r.SetupWithManager(mgr, 1)).NotTo(HaveOccurred())
+	Expect(mxnetwebhook.SetupWebhook(mgr)).NotTo(HaveOccurred())
 
 	go func() {
 		defer GinkgoRecover()
 		err = mgr.Start(testCtx)
 		Expect(err).ToNot(HaveOccurred(), "failed to run manager")
 	}()
+
+	dialer := &net.Dialer{Timeout: time.Second}
+	addrPort := fmt.Sprintf("%s:%d", testEnv.WebhookInstallOptions.LocalServingHost, testEnv.WebhookInstallOptions.LocalServingPort)
+	Eventually(func(g Gomega) {
+		conn, err := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true})
+		g.Expect(err).NotTo(HaveOccurred())
+		g.Expect(conn.Close()).NotTo(HaveOccurred())
+	}).Should(Succeed())
 })
 
 var _ = AfterSuite(func() {

pkg/webhooks/mxnet/mxnet_webhook.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2024 The Kubeflow Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package mxnet
+
+import (
+	"context"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/klog/v2"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	trainingoperator "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
+)
+
+type Webhook struct{}
+
+func SetupWebhook(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(&trainingoperator.MXJob{}).
+		WithValidator(&Webhook{}).
+		Complete()
+}
+
+// +kubebuilder:webhook:path=/validate-kubeflow-org-v1-mxjob,mutating=false,failurePolicy=fail,sideEffects=None,groups=kubeflow.org,resources=mxjobs,verbs=create;update;delete,versions=v1,name=validator.mxjob.training-operator.kubeflow.org,admissionReviewVersions=v1
+
+var _ webhook.CustomValidator = &Webhook{}
+
+func (w *Webhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	job := obj.(*trainingoperator.MXJob)
+	log := ctrl.LoggerFrom(ctx).WithName("mxjob-webhook")
+	log.V(5).Info("Validating create", "mxJob", klog.KObj(job))
+	return deprecatedWarning(), nil
+}
+
+func (w *Webhook) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) (admission.Warnings, error) {
+	job := newObj.(*trainingoperator.MXJob)
+	log := ctrl.LoggerFrom(ctx).WithName("mxjob-webhook")
+	log.V(5).Info("Validating update", "mxJob", klog.KObj(job))
+	return deprecatedWarning(), nil
+}
+
+func (w *Webhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	job := obj.(*trainingoperator.MXJob)
+	log := ctrl.LoggerFrom(ctx).WithName("mxjob-webhook")
+	log.V(5).Info("Validating delete", "mxJob", klog.KObj(job))
+	return deprecatedWarning(), nil
+}
+
+func deprecatedWarning() admission.Warnings {
+	return admission.Warnings{
+		fmt.Sprintf("MXJob is deprecated, and the MXJob will be removed in the release v1.9.0.\n" +
+			"Please see https://github.com/kubeflow/training-operator/issues/1996 for more details"),
+	}
+}

pkg/webhooks/webhooks.go

@@ -20,6 +20,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	trainingoperator "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
+	"github.com/kubeflow/training-operator/pkg/webhooks/mxnet"
 	"github.com/kubeflow/training-operator/pkg/webhooks/paddlepaddle"
 	"github.com/kubeflow/training-operator/pkg/webhooks/pytorch"
 	"github.com/kubeflow/training-operator/pkg/webhooks/tensorflow"
@@ -32,7 +33,7 @@ var (
 	SupportedSchemeWebhook = map[string]WebhookSetupFunc{
 		trainingoperator.PyTorchJobKind: pytorch.SetupWebhook,
 		trainingoperator.TFJobKind:      tensorflow.SetupWebhook,
-		trainingoperator.MXJobKind:      scaffold,
+		trainingoperator.MXJobKind:      mxnet.SetupWebhook,
 		trainingoperator.XGBoostJobKind: xgboost.SetupWebhook,
 		trainingoperator.MPIJobKind:     scaffold,
 		trainingoperator.PaddleJobKind:  paddlepaddle.SetupWebhook,