Skip to content

Commit

Permalink
fix: notebook server validation
Browse files Browse the repository at this point in the history
  • Loading branch information
saffaalvi authored and brendangadd committed Sep 16, 2020
1 parent 0d174a0 commit 1e66633
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 23 deletions.
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 @@ -13,13 +13,17 @@ <h3>
<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>
<mat-error>
{{ showCPUError() }}
</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>
<mat-error>
{{ showMemoryError() }}
</mat-error>
</mat-form-field>
</div>
</div>
81 changes: 79 additions & 2 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,5 @@
import { Component, OnInit, Input } from "@angular/core";
import { FormGroup } from "@angular/forms";
import { FormGroup, AbstractControl, Validators, ValidatorFn, ControlContainer } from "@angular/forms";

@Component({
selector: "app-form-specs",
Expand All @@ -13,5 +13,82 @@ export class FormSpecsComponent implements OnInit {

constructor() {}

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

showCPUError()
{
const cpu = this.parentForm.get('cpu');
const gpus = this.parentForm.get('gpus').get('num').value;

if (cpu.hasError('required')) {
return `Please provide the CPU requirements`;
}
if (cpu.hasError('pattern')) {
return `Invalid character`;
}
if (cpu.hasError('min')) {
return `Can't be less than 0.5 CPU`;
}
if (cpu.hasError('maxCPU')) {
if (gpus=='none') {
return `Can't exceed 15 CPU`;
} else {
return `Can't exceed 5 CPU`;
}
}

}

showMemoryError()
{
const memory = this.parentForm.get('memory');
const gpus = this.parentForm.get('gpus').get('num').value;

if (memory.hasError('required')) {
return "Please provide the RAM requirements";
}
if (memory.hasError('pattern')) {
return `Invalid character`
}
if (memory.hasError('min')) {
return `Can't be less than 1Gi of RAM`
}
if (memory.hasError('maxMemory')) {
if (gpus=='none') {
return `Can't exceed 48Gi of RAM`;
} else {
return `Can't exceed 96Gi of RAM`;
}
}
}

private maxCPUValidator(): ValidatorFn
{
return (control: AbstractControl): { [key: string]: any} | null => {
var max: number;
const gpus = this.parentForm.get('gpus').get('num').value;
gpus == 'none' ? max = 15 : max = 5;
return control.value>max ? { maxCPU: true } : null
}
}

private maxMemoryValidator(): ValidatorFn
{
return (control: AbstractControl): { [key: string]: any} | null => {
var max: number;
const gpus = this.parentForm.get('gpus').get('num').value;
gpus == 'none' ? max = 48 : max = 96;
return control.value>max ? { maxMemory: true } : null
}
}
}


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 1e66633

Please sign in to comment.