From b93eaaa4bcaff3edad1f29aecd3966ec4555730d Mon Sep 17 00:00:00 2001
From: zhixian <zhy0882@mail.ustc.edu.cn>
Date: Mon, 21 Nov 2022 10:56:38 +0800
Subject: [PATCH] fix: only validate Kubernetes Job

---
 .../v1beta1/experiment/validator/validator.go | 30 ++++++++-----------
 .../experiment/validator/validator_test.go    | 19 ++++++++++++
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/pkg/webhook/v1beta1/experiment/validator/validator.go b/pkg/webhook/v1beta1/experiment/validator/validator.go
index aeaf911904a..8da0c3e3ff9 100644
--- a/pkg/webhook/v1beta1/experiment/validator/validator.go
+++ b/pkg/webhook/v1beta1/experiment/validator/validator.go
@@ -376,26 +376,22 @@ func (g *DefaultValidator) validateTrialTemplate(instance *experimentsv1beta1.Ex
 func (g *DefaultValidator) validateTrialJob(runSpec *unstructured.Unstructured) error {
 	gvk := runSpec.GroupVersionKind()
 
-	// Validate only Job
-	switch gvk.Kind {
-	case consts.JobKindJob:
-		batchJob := batchv1.Job{}
-
-		// Validate that RunSpec can be converted to Batch Job
-		err := runtime.DefaultUnstructuredConverter.FromUnstructured(runSpec.Object, &batchJob)
-		if err != nil {
-			return fmt.Errorf("unable to convert spec.TrialTemplate: %v to %v: %v", runSpec.Object, gvk.Kind, err)
-		}
+	// Validate only Kubernetes Job
+	if gvk.GroupVersion() != batchv1.SchemeGroupVersion || gvk.Kind != consts.JobKindJob {
+		return nil
+	}
 
-		// Try to patch runSpec to Batch Job
-		// TODO (andreyvelich): Do we want to remove it completely ?
-		err = validatePatchJob(runSpec, batchJob, gvk.Kind)
-		if err != nil {
-			return err
-		}
+	batchJob := batchv1.Job{}
+
+	// Validate that RunSpec can be converted to Batch Job
+	err := runtime.DefaultUnstructuredConverter.FromUnstructured(runSpec.Object, &batchJob)
+	if err != nil {
+		return fmt.Errorf("unable to convert spec.TrialTemplate: %v to %v: %v", runSpec.Object, gvk.Kind, err)
 	}
 
-	return nil
+	// Try to patch runSpec to Batch Job
+	// TODO (andreyvelich): Do we want to remove it completely ?
+	return validatePatchJob(runSpec, batchJob, gvk.Kind)
 }
 
 func validatePatchJob(runSpec *unstructured.Unstructured, job interface{}, jobType string) error {
diff --git a/pkg/webhook/v1beta1/experiment/validator/validator_test.go b/pkg/webhook/v1beta1/experiment/validator/validator_test.go
index 916ef599a96..10bd66aa0c5 100644
--- a/pkg/webhook/v1beta1/experiment/validator/validator_test.go
+++ b/pkg/webhook/v1beta1/experiment/validator/validator_test.go
@@ -809,6 +809,19 @@ spec:
 		t.Errorf("ConvertStringToUnstructured failed: %v", err)
 	}
 
+	notKubernetesBatchJob := `apiVersion: test/v1
+kind: Job
+spec:
+  template:
+    spec:
+      containers:
+      - name: container`
+
+	notKubernetesBatchJobUnstr, err := util.ConvertStringToUnstructured(notKubernetesBatchJob)
+	if err != nil {
+		t.Errorf("ConvertStringToUnstructured failed: %v", err)
+	}
+
 	tcs := []struct {
 		RunSpec         *unstructured.Unstructured
 		Err             bool
@@ -835,6 +848,12 @@ spec:
 			Err:             false,
 			testDescription: "Valid case with nvidia.com/gpu resource in Trial template",
 		},
+		// Not kubernetes batch job
+		{
+			RunSpec:         notKubernetesBatchJobUnstr,
+			Err:             false,
+			testDescription: "Only validate Kuernetes Job",
+		},
 	}
 
 	for _, tc := range tcs {