Skip to content

Commit

Permalink
Not validate Trial template resources (kubeflow#1231)
Browse files Browse the repository at this point in the history
  • Loading branch information
andreyvelich committed Jun 24, 2020
1 parent fd1dd0d commit ad3f3b0
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
4 changes: 3 additions & 1 deletion pkg/webhook/v1beta1/experiment/validator/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,9 @@ func validatePatchJob(runSpec *unstructured.Unstructured, job interface{}, jobTy

for _, operation := range runSpecPatchOperations {
// If operation != "remove" some values from trialTemplate were not converted
if operation.Operation != "remove" {
// We can't validate /resources/limits/ because CRDs can have custom k8s resources using defice plugin
// ref https://kubernetes.io/docs/concepts/extend-kubernetes/compute-storage-net/device-plugins/
if operation.Operation != "remove" && !strings.Contains(operation.Path, "/resources/limits/") && !strings.Contains(operation.Path, "/resources/requests/") {
return fmt.Errorf("Unable to convert: %v - %v to %v, converted template: %v", operation.Path, operation.Value, jobType, string(runSpecAfter))
}
}
Expand Down
23 changes: 23 additions & 0 deletions pkg/webhook/v1beta1/experiment/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,23 @@ spec:
t.Errorf("ConvertStringToUnstructured failed: %v", err)
}

notDefaultResourceBatchJob := `apiVersion: batch/v1
kind: Job
spec:
template:
spec:
containers:
- resources:
limits:
nvidia.com/gpu: 1
requests:
nvidia.com/gpu: 1`

notDefaultResourceBatchUnstr, err := util.ConvertStringToUnstructured(notDefaultResourceBatchJob)
if err != nil {
t.Errorf("ConvertStringToUnstructured failed: %v", err)
}

tcs := []struct {
RunSpec *unstructured.Unstructured
Err bool
Expand Down Expand Up @@ -581,6 +598,12 @@ spec:
Err: true,
testDescription: "Trial template has invalid PyTorch Job structure",
},
// Valid case with not default Kubernetes resource (nvidia.com/gpu: 1)
{
RunSpec: notDefaultResourceBatchUnstr,
Err: false,
testDescription: "Valid case with nvidia.com/gpu resource in Trial template",
},
}

for _, tc := range tcs {
Expand Down

0 comments on commit ad3f3b0

Please sign in to comment.