Skip to content

Commit

Permalink
fix: only validate Kubernetes Job
Browse files Browse the repository at this point in the history
  • Loading branch information
zhixian82 authored and zhouyan committed Dec 2, 2022
1 parent 0d0e77f commit b93eaaa
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 17 deletions.
30 changes: 13 additions & 17 deletions pkg/webhook/v1beta1/experiment/validator/validator.go
Expand Up @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions pkg/webhook/v1beta1/experiment/validator/validator_test.go
Expand Up @@ -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
Expand All @@ -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 {
Expand Down

0 comments on commit b93eaaa

Please sign in to comment.