Skip to content

Commit

Permalink
Merge pull request #19 from StatCan/notebook-server-validation
Browse files Browse the repository at this point in the history
Notebook server validation
  • Loading branch information
brendangadd authored Sep 16, 2020
2 parents 0d174a0 + 9cbb66c commit 22bdc13
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 36 deletions.
12 changes: 3 additions & 9 deletions frontend/.prettierrc
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
{
printWidth: 80,
useTabs: false,
tabWidth: 2,
semi: true,
singleQuote: true,
trailingComma: 'all',
bracketSpacing: true,
arrowParens: 'avoid',
proseWrap: 'preserve',
"arrowParens": "avoid",
"bracketSpacing": false,
"trailingComma": "none"
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ <h3>
[pvcs]="pvcs"
[ephemeral]="false"
[defaultStorageClass]="defaultStorageClass"
[sizes]="['4Gi', '8Gi', '16Gi', '32Gi', '64Gi', '128Gi', '256Gi', '512Gi']"
>
</app-volume>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ <h3>
formControlName="name"
#name
/>
<mat-error>{{ showNameError() }}</mat-error>
<mat-error>
{{ showNameError() }}
</mat-error>
</mat-form-field>

<mat-form-field appearance="outline">
Expand Down
22 changes: 15 additions & 7 deletions frontend/src/app/resource-form/form-name/form-name.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ export class FormNameComponent implements OnInit, OnDestroy {
constructor(private k8s: KubernetesService, private ns: NamespaceService) {}

ngOnInit() {
// Add the ExistingName validator to the list if it doesn't already exist
// Add validator for notebook name (existing name, length, lowercase alphanumeric and '-')
this.parentForm
.get("name")
.setValidators([Validators.required, this.existingName()]);

.setValidators([Validators.required, this.existingNameValidator(), Validators.pattern(/^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/), Validators.maxLength(52)]);
// Keep track of the existing Notebooks in the selected Namespace
// Use these names to check if the input name exists
const nsSub = this.ns.getSelectedNamespace().subscribe(ns => {
Expand All @@ -45,18 +45,26 @@ export class FormNameComponent implements OnInit, OnDestroy {

showNameError() {
const nameCtrl = this.parentForm.get("name");


if (nameCtrl.value.length==0) {
return `The Notebook Server's name can't be empty`;
}
if (nameCtrl.hasError("existingName")) {
return `Notebook Server "${nameCtrl.value}" already exists`;
} else {
return "The Notebook Server's name can't be empty";
}
if (nameCtrl.hasError("pattern")) {
return `The Notebook Server's name can only contain lowercase alphanumeric characters or '-' and must start and end with an alphanumeric character`;
}
if (nameCtrl.hasError("maxlength")) {
return `The Notebook Server's name can't be more than 52 characters`;
}
}

private existingName(): ValidatorFn {
private existingNameValidator(): ValidatorFn {
return (control: AbstractControl): { [key: string]: any } => {
const exists = this.notebooks.has(control.value);
return exists ? { existingName: true } : null;
};
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,24 @@ <h3>
<div class="inputs-wrapper">
<mat-form-field appearance="outline">
<mat-label>CPU</mat-label>
<input matInput placeholder="# of CPU Cores" formControlName="cpu" />
<mat-error>Please provide the CPU requirements</mat-error>
<input
matInput
placeholder="# of CPU Cores"
formControlName="cpu"
[errorStateMatcher]="parentErrorKeysErrorStateMatcher('maxCpu')"
/>
<mat-error>{{ cpuErrorMessage() }}</mat-error>
</mat-form-field>

<mat-form-field appearance="outline">
<mat-label>Memory</mat-label>
<input matInput placeholder="Amount of Memory" formControlName="memory" />
<mat-error>Please provide the RAM requirements</mat-error>
<input
matInput
placeholder="Amount of Memory"
formControlName="memory"
[errorStateMatcher]="parentErrorKeysErrorStateMatcher('maxRam')"
/>
<mat-error>{{ memoryErrorMessage() }}</mat-error>
</mat-form-field>
</div>
</div>
107 changes: 103 additions & 4 deletions frontend/src/app/resource-form/form-specs/form-specs.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,41 @@
import { Component, OnInit, Input } from "@angular/core";
import { FormGroup } from "@angular/forms";
import {Component, OnInit, Input} from "@angular/core";
import {
FormGroup,
AbstractControl,
Validators,
ValidatorFn,
ValidationErrors,
FormControl,
FormGroupDirective,
NgForm
} from "@angular/forms";

const MAX_FOR_GPU: ReadonlyMap<number, MaxResourceSpec> = new Map([
[0, {cpu: 15, ram: 96}],
[1, {cpu: 5, ram: 48}]
]);

type MaxResourceSpec = {cpu: number; ram: number};

function resourcesValidator(): ValidatorFn {
return function (control: AbstractControl): ValidationErrors | null {
const gpuNumValue = control.get("gpus").get("num").value;
const gpu = gpuNumValue === "none" ? 0 : parseInt(gpuNumValue, 10) || 0;
const cpu = parseFloat(control.get("cpu").value);
const ram = parseFloat(control.get("memory").value);
const errors = {};

const max = MAX_FOR_GPU.get(gpu);
if (cpu > max.cpu) {
errors["maxCpu"] = {max: max.cpu, gpu};
}
if (ram > max.ram) {
errors["maxRam"] = {max: max.ram, gpu};
}

return Object.entries(errors).length > 0 ? errors : null;
};
}

@Component({
selector: "app-form-specs",
Expand All @@ -11,7 +47,70 @@ export class FormSpecsComponent implements OnInit {
@Input() readonlyCPU: boolean;
@Input() readonlyMemory: boolean;

constructor() {}
ngOnInit() {
this.parentForm
.get("cpu")
.setValidators([
Validators.required,
Validators.pattern(/^[0-9]+([.][0-9]+)?$/),
Validators.min(0.5)
]);
this.parentForm
.get("memory")
.setValidators([
Validators.required,
Validators.pattern(/^[0-9]+([.][0-9]+)?(Gi)$/),
Validators.min(1)
]);
this.parentForm.setValidators(resourcesValidator());
}

parentErrorKeysErrorStateMatcher(keys: string | string[]) {
const arrKeys = ([] as string[]).concat(keys);
return {
isErrorState(
control: FormControl,
form: FormGroupDirective | NgForm
): boolean {
return (
(control.dirty && control.invalid) ||
(form.dirty && arrKeys.some(key => form.hasError(key)))
);
}
};
}

cpuErrorMessage(): string {
let e: any;
const errs = this.parentForm.get("cpu").errors || {};

if (errs.required) return "Specify number of CPUs";
if (errs.pattern) return "Must be a number";
if ((e = errs.min)) return `Specify at least ${e.min} CPUs`;

if (this.parentForm.hasError("maxCpu")) {
e = this.parentForm.errors.maxCpu;
return (
`Can't exceed ${e.max} CPUs` +
(e.gpu > 0 ? ` with ${e.gpu} GPU(s) selected` : "")
);
}
}

memoryErrorMessage(): string {
let e: any;
const errs = this.parentForm.get("memory").errors || {};

if (errs.required || errs.pattern)
return "Specify amount of memory (e.g. 2Gi)";
if ((e = errs.min)) return `Specify at least ${e.min}Gi of memory`;

ngOnInit() {}
if (this.parentForm.hasError("maxRam")) {
e = this.parentForm.errors.maxRam;
return (
`Can't exceed ${e.max}Gi of memory` +
(e.gpu > 0 ? ` with ${e.gpu} GPU(s) selected` : "")
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ <h3>
[ephemeral]="parentForm.value.noWorkspace"
[namespace]="parentForm.value.namespace"
[defaultStorageClass]="defaultStorageClass"
[sizes]="['4Gi', '8Gi', '16Gi', '32Gi']"
>
</app-volume>
</div>
11 changes: 9 additions & 2 deletions frontend/src/app/resource-form/volume/volume.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,20 @@
}}</mat-option>
</mat-select>
</ng-template>
<mat-error>
{{ showNameError() }}
</mat-error>
</mat-form-field>

<!-- Size Input -->
<mat-form-field appearance="outline" id="size">
<mat-label>Size</mat-label>
<input matInput formControlName="size" />
</mat-form-field>
<mat-select formControlName="size">
<mat-option *ngFor="let sizeOptions of sizes" [value]="sizeOptions">{{
sizeOptions
}}</mat-option>
</mat-select>
</mat-form-field>

<!-- Mode Input -->
<mat-form-field appearance="outline" id="mode">
Expand Down
27 changes: 22 additions & 5 deletions frontend/src/app/resource-form/volume/volume.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Component, OnInit, Input, OnDestroy } from "@angular/core";
import { FormGroup } from "@angular/forms";
import { FormGroup, Validators } from "@angular/forms";
import { Volume } from "src/app/utils/types";
import { Subscription } from "rxjs";

Expand All @@ -14,13 +14,14 @@ export class VolumeComponent implements OnInit, OnDestroy {

currentPVC: Volume;
existingPVCs: Set<string> = new Set();

subscriptions = new Subscription();

// ----- @Input Parameters -----
@Input() volume: FormGroup;
@Input() namespace: string;

@Input() sizes: Set<string>;

@Input()
get notebookName() {
return this._notebookName;
Expand Down Expand Up @@ -102,8 +103,11 @@ export class VolumeComponent implements OnInit, OnDestroy {
constructor() {}

ngOnInit() {
// type
this.subscriptions.add(
this.volume
.get("name")
.setValidators([Validators.required, Validators.pattern(/^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$/)]);

this.subscriptions.add(
this.volume.get("type").valueChanges.subscribe((type: string) => {
this.setVolumeType(type);
})
Expand Down Expand Up @@ -158,4 +162,17 @@ export class VolumeComponent implements OnInit, OnDestroy {
this.volume.controls.name.setValue(this.currentVolName);
}
}

showNameError() {
const volumeName = this.volume.get("name");

if (volumeName.hasError("required")) {
return `The volume name can't be empty`
}
if (volumeName.hasError("pattern")) {
return `The volume name can only contain lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character`;
}
}


}
2 changes: 1 addition & 1 deletion frontend/src/app/utils/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function addDataVolume(
value: '{notebook-name}-vol-' + (l + 1),
},
size: {
value: '10Gi',
value: '16Gi',
},
mountPath: {
value: '/home/jovyan/data-vol-' + (l + 1),
Expand Down
6 changes: 3 additions & 3 deletions samples/spawner_ui_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ spawnerFormDefaults:
value: 'workspace-{notebook-name}'
size:
# The Size of the Workspace Volume (in Gi)
value: '10Gi'
value: '4Gi'
mountPath:
# The Path that the Workspace Volume will be mounted
value: /home/jovyan
Expand Down Expand Up @@ -87,7 +87,7 @@ spawnerFormDefaults:
# name:
# value: '{notebook-name}-vol-1'
# size:
# value: '10Gi'
# value: '16Gi'
# class:
# value: standard
# mountPath:
Expand All @@ -102,7 +102,7 @@ spawnerFormDefaults:
# name:
# value: '{notebook-name}-vol-2'
# size:
# value: '10Gi'
# value: '16Gi'
# mountPath:
# value: /home/jovyan/vol-2
# accessModes:
Expand Down

0 comments on commit 22bdc13

Please sign in to comment.