Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add validation checks for immutable fields #44

Merged
merged 4 commits into from
Sep 16, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 53 additions & 3 deletions api/v1alpha1/workspace_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package v1alpha1
import (
"context"
"fmt"

"k8s.io/klog/v2"
"reflect"

admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
"knative.dev/pkg/apis"
)

Expand All @@ -18,6 +19,55 @@ func (w *Workspace) SupportedVerbs() []admissionregistrationv1.OperationType {
}

func (w *Workspace) Validate(ctx context.Context) (errs *apis.FieldError) {
klog.InfoS("Validating", "workspace", fmt.Sprintf("%s/%s", w.Namespace, w.Name))

base := apis.GetBaseline(ctx)
if base == nil {
klog.InfoS("Validate creation", "workspace", fmt.Sprintf("%s/%s", w.Namespace, w.Name))
/* TODO
Add the following checks when creating a new workspace:
- For instancetype, check againt a hardcode list with existing GPU skus that we support. For these skus,
validate if the total GPU count is sufficient to run the preset workload if any.
- For other instancetypes, do pattern matches and for N, D series, we let it pass. Otherwise, fail the check.
- For labelSelector, call metav1.LabelSelectorAsMap. If the method returns error, meaning unsupported expressions are found, fail the check.
- For inference, either preset or template has to be set, not all.
- The preset name needs to be supported enum.
- API: change template to be a pointer field, change inference to be a pointer field.
*/
} else {
klog.InfoS("Validate update", "workspace", fmt.Sprintf("%s/%s", w.Namespace, w.Name))
old := base.(*Workspace)
errs = errs.Also(
w.Resource.validateUpdate(&old.Resource).ViaField("resource"),
w.Inference.validateUpdate(&old.Inference).ViaField("inference"),
)
}
return errs
}

func (r *ResourceSpec) validateUpdate(old *ResourceSpec) (errs *apis.FieldError) {
// We disable changing node count for now.
if r.Count != nil && old.Count != nil && *r.Count != *old.Count {
helayoty marked this conversation as resolved.
Show resolved Hide resolved
errs = errs.Also(apis.ErrGeneric("field is immutable", "count"))
}
if r.InstanceType != old.InstanceType {
errs = errs.Also(apis.ErrGeneric("field is immutable", "instanceType"))
}
newLabels, err0 := metav1.LabelSelectorAsMap(r.LabelSelector)
oldLabels, err1 := metav1.LabelSelectorAsMap(old.LabelSelector)
if err0 != nil || err1 != nil {
errs = errs.Also(apis.ErrGeneric("Only allow matchLabels or 'IN' matchExpression", "labelSelector"))
} else {
if !reflect.DeepEqual(newLabels, oldLabels) {
errs = errs.Also(apis.ErrGeneric("field is immutable", "labelSelector"))
}
}
return errs
}

func (i *InferenceSpec) validateUpdate(old *InferenceSpec) (errs *apis.FieldError) {
if i.Preset.Name != old.Preset.Name {
errs = errs.Also(apis.ErrGeneric("field is immutable", "name").ViaField("preset"))
}
//TODO: inference.template can be changed, but cannot be unset.
return errs
}
Loading