From 9b10a39402e23cccf7bdc7967d083b11c8a6776e Mon Sep 17 00:00:00 2001 From: Elena Zioga Date: Tue, 24 Jan 2023 14:25:40 +0200 Subject: [PATCH 1/2] backend: Update error message when no logs could be found * Update the message the backend sends to not just expose that logs are not there because 'retain' might not be set, but also because the cluster was scaled down. Signed-off-by: Elena Zioga --- pkg/new-ui/v1beta1/backend.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 1f6b5848702..cc59a1b0a20 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -729,9 +729,9 @@ func fetchMasterPodName(clientset *kubernetes.Clientset, trial *trialsv1beta1.Tr } if len(podList.Items) == 0 { - return "", errors.New(`Logs for the trial could not be found. -Was 'retain: true' specified in the Experiment definition? -An example can be found here: https://github.com/kubeflow/katib/blob/7bf39225f7235ee4ba6cf285ecc2c455c6471234/examples/v1beta1/argo/argo-workflow.yaml#L33`) + return "", errors.New(`Failed to find logs for this Trial. Make sure you've set "spec.trialTemplate.retain" + field to "true" in the Experiment definition. If this error persists then the Pod's logs are not currently + persisted in the cluster.`) } if len(podList.Items) > 1 { return "", errors.New("More than one master replica found") From 340add3bd6bdac577601fd415ca7de6a3c789378 Mon Sep 17 00:00:00 2001 From: Elena Zioga Date: Fri, 13 Jan 2023 11:57:07 +0200 Subject: [PATCH 2/2] frontend: Add LOGS tab in Trial details page In this commit: * Create a distinct LOGS tab, which displays the trial's logs in the Trial details page. * Don't show the backend's error popup for logs, but show the message error in the admonition. Signed-off-by: Elena Zioga --- .../trial-details.component.html | 10 ++++++ .../trial-details.component.spec.ts | 1 + .../trial-details/trial-details.component.ts | 13 +++++++ .../trial-details/trial-details.module.ts | 2 ++ .../trial-logs/trial-logs.component.html | 15 ++++++++ .../trial-logs/trial-logs.component.scss | 9 +++++ .../trial-logs/trial-logs.component.spec.ts | 36 +++++++++++++++++++ .../trial-logs/trial-logs.component.ts | 21 +++++++++++ .../trial-logs/trial-logs.module.ts | 20 +++++++++++ .../src/app/services/backend.service.ts | 21 +++++++++++ 10 files changed, 148 insertions(+) create mode 100644 pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.html create mode 100644 pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.scss create mode 100644 pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.spec.ts create mode 100644 pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.ts create mode 100644 pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.module.ts diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.html b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.html index dda58f3522e..500ed5a6a8a 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.html +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.html @@ -46,6 +46,16 @@ > + + + + + + + diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.spec.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.spec.ts index 53b428f4d35..f3df84b7bb4 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.spec.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.spec.ts @@ -21,6 +21,7 @@ let NamespaceServiceStub: Partial; KWABackendServiceStub = { getTrial: () => of([]), getTrialInfo: () => of(), + getTrialLogs: () => of(), }; NamespaceServiceStub = { diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.ts index 8e3e2406b1b..e6ff7f20f6d 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.component.ts @@ -30,6 +30,8 @@ export class TrialDetailsComponent implements OnInit, OnDestroy { experimentName: string; showTrialGraph: boolean = false; options: {}; + trialLogs: string; + logsRequestError: string; chartData: ChartPoint[] = []; yScaleMax = 0; yScaleMin = 1; @@ -133,6 +135,17 @@ export class TrialDetailsComponent implements OnInit, OnDestroy { } this.pageLoading = false; }); + + this.backendService.getTrialLogs(this.trialName, this.namespace).subscribe( + logs => { + this.trialLogs = logs; + this.logsRequestError = null; + }, + error => { + this.trialLogs = null; + this.logsRequestError = error; + }, + ); } private trialStatus(trial: TrialK8s): StatusEnum { diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.module.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.module.ts index 840a4916f83..2d890e80724 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.module.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-details.module.ts @@ -16,6 +16,7 @@ import { PanelModule, } from 'kubeflow'; import { NgxEchartsModule } from 'ngx-echarts'; +import { TrialLogsModule } from './trial-logs/trial-logs.module'; @NgModule({ declarations: [TrialDetailsComponent], @@ -36,6 +37,7 @@ import { NgxEchartsModule } from 'ngx-echarts'; NgxEchartsModule.forRoot({ echarts: () => import('echarts'), }), + TrialLogsModule, ], exports: [TrialDetailsComponent], }) diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.html b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.html new file mode 100644 index 00000000000..e9b33122d39 --- /dev/null +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.html @@ -0,0 +1,15 @@ + + + + {{ logsRequestError }} + + + + + + diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.scss b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.scss new file mode 100644 index 00000000000..053f94c377f --- /dev/null +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.scss @@ -0,0 +1,9 @@ +lib-panel { + display: block; + margin-top: 1rem; +} + +.logs-viewer { + margin-bottom: 2rem; + margin-top: 1rem; +} diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.spec.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.spec.ts new file mode 100644 index 00000000000..9a018027b10 --- /dev/null +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.spec.ts @@ -0,0 +1,36 @@ +import { CommonModule } from '@angular/common'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { + HeadingSubheadingRowModule, + KubeflowModule, + LogsViewerModule, +} from 'kubeflow'; + +import { TrialLogsComponent } from './trial-logs.component'; + +describe('TrialLogsComponent', () => { + let component: TrialLogsComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TrialLogsComponent], + imports: [ + CommonModule, + KubeflowModule, + HeadingSubheadingRowModule, + LogsViewerModule, + ], + }).compileComponents(); + }); + + beforeEach(() => { + fixture = TestBed.createComponent(TrialLogsComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.ts new file mode 100644 index 00000000000..a81e68a0747 --- /dev/null +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.component.ts @@ -0,0 +1,21 @@ +import { Component, Input } from '@angular/core'; + +@Component({ + selector: 'app-trial-logs', + templateUrl: './trial-logs.component.html', + styleUrls: ['./trial-logs.component.scss'], +}) +export class TrialLogsComponent { + public logs: string[]; + + @Input() logsRequestError: string; + + @Input() + set trialLogs(trialLogs: string) { + if (!trialLogs) { + return; + } + + this.logs = trialLogs.split('\n'); + } +} diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.module.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.module.ts new file mode 100644 index 00000000000..ac570e04ab2 --- /dev/null +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-details/trials-table/trial-details/trial-logs/trial-logs.module.ts @@ -0,0 +1,20 @@ +import { NgModule } from '@angular/core'; +import { CommonModule } from '@angular/common'; +import { TrialLogsComponent } from './trial-logs.component'; +import { + HeadingSubheadingRowModule, + KubeflowModule, + LogsViewerModule, +} from 'kubeflow'; + +@NgModule({ + declarations: [TrialLogsComponent], + imports: [ + CommonModule, + KubeflowModule, + HeadingSubheadingRowModule, + LogsViewerModule, + ], + exports: [TrialLogsComponent], +}) +export class TrialLogsModule {} diff --git a/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts b/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts index 457defc12f2..95e728934bb 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts @@ -102,4 +102,25 @@ export class KWABackendService extends BackendService { .post(url, { postData: exp }) .pipe(catchError(error => this.parseError(error))); } + + getTrialLogs(name: string, namespace: string): Observable { + const url = `/katib/fetch_trial_logs/?trialName=${name}&namespace=${namespace}`; + + return this.http + .get(url) + .pipe(catchError(error => this.handleError(error, false))); + } + + // ---------------------------Error Handling--------------------------------- + + // Override common service's getBackendErrorLog + // in order to properly show the message the backend has sent + public getBackendErrorLog(error: HttpErrorResponse): string { + if (error.error === null) { + return error.message; + } else { + // Show the message the backend has sent + return error.error.log ? error.error.log : error.error; + } + } }