From 2f2f8b6f0d55e1251a8ef2bdcc15c32e6cfb9b0e Mon Sep 17 00:00:00 2001 From: "Yuan (Bob) Gong" Date: Fri, 20 Mar 2020 17:42:38 +0800 Subject: [PATCH] [UI] Show step pod yaml and events in RunDetails page (#3304) * [UI Server] Pod info handler * [UI] Pod info tab in run details page * Change pod info preview to use yaml editor * Fix namespace * Adds error handling for PodInfo * Adjust to warning message * [UI] Pod events in RunDetails page * Adjust error message * Refactor k8s helper to get rid of in cluster limit * Tests for pod info handler * Tests for pod event list handler * Move pod yaml viewer related components to separate file. * Unit tests for PodYaml component * Fix react unit tests * Fix error message * Address CR comments * Add permission to ui role --- frontend/server/app.test.ts | 141 +++++++++++++-- frontend/server/app.ts | 4 + frontend/server/handlers/gke-metadata.ts | 10 -- frontend/server/handlers/pod-info.ts | 66 +++++++ frontend/server/handlers/pod-logs.ts | 5 - frontend/server/handlers/tensorboard.ts | 15 -- frontend/server/k8s-helper.ts | 145 ++++++++++++--- frontend/server/proxy-middleware.ts | 2 +- frontend/src/TestUtils.tsx | 2 +- frontend/src/components/PodYaml.test.tsx | 168 ++++++++++++++++++ frontend/src/components/PodYaml.tsx | 122 +++++++++++++ frontend/src/lib/Apis.ts | 28 +++ frontend/src/pages/RunDetails.test.tsx | 59 +++--- frontend/src/pages/RunDetails.tsx | 98 ++++++---- .../__snapshots__/RunDetails.test.tsx.snap | 18 ++ .../templates/pipeline.yaml | 6 + .../base/pipeline/ml-pipeline-ui-role.yaml | 6 + 17 files changed, 765 insertions(+), 130 deletions(-) create mode 100644 frontend/server/handlers/pod-info.ts create mode 100644 frontend/src/components/PodYaml.test.tsx create mode 100644 frontend/src/components/PodYaml.tsx diff --git a/frontend/server/app.test.ts b/frontend/server/app.test.ts index 60308e661625..11915f9196b8 100644 --- a/frontend/server/app.test.ts +++ b/frontend/server/app.test.ts @@ -24,16 +24,26 @@ import { Storage as GCSStorage } from '@google-cloud/storage'; import { UIServer } from './app'; import { loadConfigs } from './configs'; import * as minioHelper from './minio-helper'; -import * as k8sHelper from './k8s-helper'; +import { TEST_ONLY as K8S_TEST_EXPORT } from './k8s-helper'; jest.mock('minio'); jest.mock('node-fetch'); jest.mock('@google-cloud/storage'); jest.mock('./minio-helper'); -jest.mock('./k8s-helper'); const mockedFetch: jest.Mock = fetch as any; -const mockedK8sHelper: jest.Mock = k8sHelper as any; + +beforeEach(() => { + const consoleInfoSpy = jest.spyOn(global.console, 'info'); + consoleInfoSpy.mockImplementation(() => null); + const consoleLogSpy = jest.spyOn(global.console, 'log'); + consoleLogSpy.mockImplementation(() => null); +}); + +afterEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); +}); describe('UIServer apis', () => { let app: UIServer; @@ -373,9 +383,6 @@ describe('UIServer apis', () => { describe('/system', () => { describe('/cluster-name', () => { - beforeEach(() => { - mockedK8sHelper.isInCluster = true; - }); it('responds with cluster name data from gke metadata', done => { mockedFetch.mockImplementationOnce((url: string, _opts: any) => url === 'http://metadata/computeMetadata/v1/instance/attributes/cluster-name' @@ -412,9 +419,6 @@ describe('UIServer apis', () => { }); }); describe('/project-id', () => { - beforeEach(() => { - mockedK8sHelper.isInCluster = true; - }); it('responds with project id data from gke metadata', done => { mockedFetch.mockImplementationOnce((url: string, _opts: any) => url === 'http://metadata/computeMetadata/v1/project/project-id' @@ -446,10 +450,121 @@ describe('UIServer apis', () => { }); }); - // TODO: refractor k8s helper module so that api that interact with k8s can be - // mocked and tested. There is currently no way to mock k8s APIs as - // `k8s-helper.isInCluster` is a constant that is generated when the module is - // first loaded. + describe('/k8s/pod', () => { + let request: requests.SuperTest; + beforeEach(() => { + app = new UIServer(loadConfigs(argv, {})); + request = requests(app.start()); + }); + + it('asks for podname if not provided', done => { + request.get('/k8s/pod').expect(422, 'podname argument is required', done); + }); + + it('asks for podnamespace if not provided', done => { + request + .get('/k8s/pod?podname=test-pod') + .expect(422, 'podnamespace argument is required', done); + }); + + it('responds with pod info in JSON', done => { + const readPodSpy = jest.spyOn(K8S_TEST_EXPORT.k8sV1Client, 'readNamespacedPod'); + readPodSpy.mockImplementation(() => + Promise.resolve({ + body: { kind: 'Pod' }, // only body is used + } as any), + ); + request + .get('/k8s/pod?podname=test-pod&podnamespace=test-ns') + .expect(200, '{"kind":"Pod"}', err => { + expect(readPodSpy).toHaveBeenCalledWith('test-pod', 'test-ns'); + done(err); + }); + }); + + it('responds with error when failed to retrieve pod info', done => { + const readPodSpy = jest.spyOn(K8S_TEST_EXPORT.k8sV1Client, 'readNamespacedPod'); + readPodSpy.mockImplementation(() => + Promise.reject({ + body: { + message: 'pod not found', + code: 404, + }, + } as any), + ); + const spyError = jest.spyOn(console, 'error').mockImplementation(() => null); + request + .get('/k8s/pod?podname=test-pod&podnamespace=test-ns') + .expect(500, 'Could not get pod test-pod in namespace test-ns: pod not found', () => { + expect(spyError).toHaveBeenCalledTimes(1); + done(); + }); + }); + }); + + describe('/k8s/pod/events', () => { + let request: requests.SuperTest; + beforeEach(() => { + app = new UIServer(loadConfigs(argv, {})); + request = requests(app.start()); + }); + + it('asks for podname if not provided', done => { + request.get('/k8s/pod/events').expect(422, 'podname argument is required', done); + }); + + it('asks for podnamespace if not provided', done => { + request + .get('/k8s/pod/events?podname=test-pod') + .expect(422, 'podnamespace argument is required', done); + }); + + it('responds with pod info in JSON', done => { + const listEventSpy = jest.spyOn(K8S_TEST_EXPORT.k8sV1Client, 'listNamespacedEvent'); + listEventSpy.mockImplementation(() => + Promise.resolve({ + body: { kind: 'EventList' }, // only body is used + } as any), + ); + request + .get('/k8s/pod/events?podname=test-pod&podnamespace=test-ns') + .expect(200, '{"kind":"EventList"}', err => { + expect(listEventSpy).toHaveBeenCalledWith( + 'test-ns', + undefined, + undefined, + undefined, + 'involvedObject.namespace=test-ns,involvedObject.name=test-pod,involvedObject.kind=Pod', + ); + done(err); + }); + }); + + it('responds with error when failed to retrieve pod info', done => { + const listEventSpy = jest.spyOn(K8S_TEST_EXPORT.k8sV1Client, 'listNamespacedEvent'); + listEventSpy.mockImplementation(() => + Promise.reject({ + body: { + message: 'no events', + code: 404, + }, + } as any), + ); + const spyError = jest.spyOn(console, 'error').mockImplementation(() => null); + request + .get('/k8s/pod/events?podname=test-pod&podnamespace=test-ns') + .expect( + 500, + 'Error when listing pod events for pod "test-pod" in "test-ns" namespace: no events', + err => { + expect(spyError).toHaveBeenCalledTimes(1); + done(err); + }, + ); + }); + }); + + // TODO: Add integration tests for k8s helper related endpoints // describe('/apps/tensorboard', () => { diff --git a/frontend/server/app.ts b/frontend/server/app.ts index 32eb427fa0e5..57f66704db4c 100644 --- a/frontend/server/app.ts +++ b/frontend/server/app.ts @@ -26,6 +26,7 @@ import { deleteTensorboardHandler, } from './handlers/tensorboard'; import { getPodLogsHandler } from './handlers/pod-logs'; +import { podInfoHandler, podEventsHandler } from './handlers/pod-info'; import { getClusterNameHandler, getProjectIdHandler } from './handlers/gke-metadata'; import { getAllowCustomVisualizationsHandler } from './handlers/vis'; import { getIndexHTMLHandler } from './handlers/index-html'; @@ -126,6 +127,9 @@ function createUIServer(options: UIConfigs) { /** Pod logs */ registerHandler(app.get, '/k8s/pod/logs', getPodLogsHandler(options.argo, options.artifacts)); + /** Pod info */ + registerHandler(app.get, '/k8s/pod', podInfoHandler); + registerHandler(app.get, '/k8s/pod/events', podEventsHandler); /** Cluster metadata (GKE only) */ registerHandler(app.get, '/system/cluster-name', getClusterNameHandler(options.gkeMetadata)); diff --git a/frontend/server/handlers/gke-metadata.ts b/frontend/server/handlers/gke-metadata.ts index f9f1e83ce18d..b5a0e1732c35 100644 --- a/frontend/server/handlers/gke-metadata.ts +++ b/frontend/server/handlers/gke-metadata.ts @@ -28,11 +28,6 @@ export const getClusterNameHandler = (options: GkeMetadataConfigs) => { }; const clusterNameHandler: Handler = async (_, res) => { - if (!k8sHelper.isInCluster) { - res.status(500).send('Not running in Kubernetes cluster.'); - return; - } - const response = await fetch( 'http://metadata/computeMetadata/v1/instance/attributes/cluster-name', { headers: { 'Metadata-Flavor': 'Google' } }, @@ -52,11 +47,6 @@ export const getProjectIdHandler = (options: GkeMetadataConfigs) => { }; const projectIdHandler: Handler = async (_, res) => { - if (!k8sHelper.isInCluster) { - res.status(500).send('Not running in Kubernetes cluster.'); - return; - } - const response = await fetch('http://metadata/computeMetadata/v1/project/project-id', { headers: { 'Metadata-Flavor': 'Google' }, }); diff --git a/frontend/server/handlers/pod-info.ts b/frontend/server/handlers/pod-info.ts new file mode 100644 index 000000000000..465ca9c5bffc --- /dev/null +++ b/frontend/server/handlers/pod-info.ts @@ -0,0 +1,66 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { Handler } from 'express'; +import * as k8sHelper from '../k8s-helper'; + +/** + * podInfoHandler retrieves pod info and sends back as JSON format. + */ +export const podInfoHandler: Handler = async (req, res) => { + const { podname, podnamespace } = req.query; + if (!podname) { + // 422 status code "Unprocessable entity", refer to https://stackoverflow.com/a/42171674 + res.status(422).send('podname argument is required'); + return; + } + if (!podnamespace) { + res.status(422).send('podnamespace argument is required'); + return; + } + const podName = decodeURIComponent(podname); + const podNamespace = decodeURIComponent(podnamespace); + + const [pod, err] = await k8sHelper.getPod(podName, podNamespace); + if (err) { + const { message, additionalInfo } = err; + console.error(message, additionalInfo); + res.status(500).send(message); + return; + } + res.status(200).send(JSON.stringify(pod)); +}; + +export const podEventsHandler: Handler = async (req, res) => { + const { podname, podnamespace } = req.query; + if (!podname) { + res.status(422).send('podname argument is required'); + return; + } + if (!podnamespace) { + res.status(422).send('podnamespace argument is required'); + return; + } + const podName = decodeURIComponent(podname); + const podNamespace = decodeURIComponent(podnamespace); + + const [eventList, err] = await k8sHelper.listPodEvents(podName, podNamespace); + if (err) { + const { message, additionalInfo } = err; + console.error(message, additionalInfo); + res.status(500).send(message); + return; + } + res.status(200).send(JSON.stringify(eventList)); +}; diff --git a/frontend/server/handlers/pod-logs.ts b/frontend/server/handlers/pod-logs.ts index 5137d5093857..8a49a9d4d8b6 100644 --- a/frontend/server/handlers/pod-logs.ts +++ b/frontend/server/handlers/pod-logs.ts @@ -61,11 +61,6 @@ export function getPodLogsHandler( ); return async (req, res) => { - if (!k8sHelper.isInCluster) { - res.status(500).send('Cannot talk to Kubernetes master'); - return; - } - if (!req.query.podname) { res.status(404).send('podname argument is required'); return; diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index 187919d41442..0d290845205c 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -20,11 +20,6 @@ import { ViewerTensorboardConfig } from '../configs'; * handler expects a query string `logdir`. */ export const getTensorboardHandler: Handler = async (req, res) => { - if (!k8sHelper.isInCluster) { - res.status(500).send('Cannot talk to Kubernetes master'); - return; - } - if (!req.query.logdir) { res.status(404).send('logdir argument is required'); return; @@ -49,11 +44,6 @@ export const getTensorboardHandler: Handler = async (req, res) => { */ export function getCreateTensorboardHandler(tensorboardConfig: ViewerTensorboardConfig): Handler { return async (req, res) => { - if (!k8sHelper.isInCluster) { - res.status(500).send('Cannot talk to Kubernetes master'); - return; - } - if (!req.query.logdir) { res.status(404).send('logdir argument is required'); return; @@ -87,11 +77,6 @@ export function getCreateTensorboardHandler(tensorboardConfig: ViewerTensorboard * `logdir` in the request. */ export const deleteTensorboardHandler: Handler = async (req, res) => { - if (!k8sHelper.isInCluster) { - res.status(500).send('Cannot talk to Kubernetes master'); - return; - } - if (!req.query.logdir) { res.status(404).send('logdir argument is required'); return; diff --git a/frontend/server/k8s-helper.ts b/frontend/server/k8s-helper.ts index c94e517dedb8..3de970cca058 100644 --- a/frontend/server/k8s-helper.ts +++ b/frontend/server/k8s-helper.ts @@ -12,17 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -// TODO: refractor k8s helper module so that api that interact with k8s can be -// mocked and tested. There is currently no way to mock k8s APIs as -// `k8s-helper.isInCluster` is a constant that is generated when the module is -// first loaded. - -// @ts-ignore import { Core_v1Api, Custom_objectsApi, KubeConfig, V1DeleteOptions, + V1Pod, + V1EventList, } from '@kubernetes/client-node'; import * as crypto from 'crypto-js'; import * as fs from 'fs'; @@ -31,9 +27,7 @@ import { PartialArgoWorkflow } from './workflow-helper'; // If this is running inside a k8s Pod, its namespace should be written at this // path, this is also how we can tell whether we're running in the cluster. const namespaceFilePath = '/var/run/secrets/kubernetes.io/serviceaccount/namespace'; -let namespace = ''; -let k8sV1Client: Core_v1Api | null = null; -let k8sV1CustomObjectClient: Custom_objectsApi | null = null; +let namespace: string | undefined = undefined; // Constants for creating customer resource Viewer. const viewerGroup = 'kubeflow.org'; @@ -52,15 +46,15 @@ export const defaultPodTemplateSpec = { }, }; -export const isInCluster = fs.existsSync(namespaceFilePath); - -if (isInCluster) { +// The file path contains pod namespace when in Kubernetes cluster. +if (fs.existsSync(namespaceFilePath)) { namespace = fs.readFileSync(namespaceFilePath, 'utf-8'); - const kc = new KubeConfig(); - kc.loadFromDefault(); - k8sV1Client = kc.makeApiClient(Core_v1Api); - k8sV1CustomObjectClient = kc.makeApiClient(Custom_objectsApi); } +const kc = new KubeConfig(); +// This loads kubectl config when not in cluster. +kc.loadFromDefault(); +const k8sV1Client = kc.makeApiClient(Core_v1Api); +const k8sV1CustomObjectClient = kc.makeApiClient(Custom_objectsApi); function getNameOfViewerResource(logdir: string): string { // TODO: find some hash function with shorter resulting message. @@ -77,8 +71,8 @@ export async function newTensorboardInstance( tfversion: string, podTemplateSpec: object = defaultPodTemplateSpec, ): Promise { - if (!k8sV1CustomObjectClient) { - throw new Error('Cannot access kubernetes Custom Object API'); + if (!namespace) { + throw new Error(`Cannot get namespace from ${namespaceFilePath}.`); } const currentPod = await getTensorboardInstance(logdir); if (currentPod.podAddress) { @@ -123,8 +117,8 @@ export async function newTensorboardInstance( export async function getTensorboardInstance( logdir: string, ): Promise<{ podAddress: string; tfVersion: string }> { - if (!k8sV1CustomObjectClient) { - throw new Error('Cannot access kubernetes Custom Object API'); + if (!namespace) { + throw new Error(`Cannot get namespace from ${namespaceFilePath}.`); } return await k8sV1CustomObjectClient @@ -168,8 +162,8 @@ export async function getTensorboardInstance( */ export async function deleteTensorboardInstance(logdir: string): Promise { - if (!k8sV1CustomObjectClient) { - throw new Error('Cannot access kubernetes Custom Object API'); + if (!namespace) { + throw new Error(`Cannot get namespace from ${namespaceFilePath}.`); } const currentPod = await getTensorboardInstance(logdir); if (!currentPod.podAddress) { @@ -210,10 +204,13 @@ export function waitForTensorboardInstance(logdir: string, timeout: number): Pro } export function getPodLogs(podName: string, podNamespace?: string): Promise { - if (!k8sV1Client) { - throw new Error('Cannot access kubernetes API'); + podNamespace = podNamespace || namespace; + if (!podNamespace) { + throw new Error( + `podNamespace is not specified and cannot get namespace from ${namespaceFilePath}.`, + ); } - return (k8sV1Client.readNamespacedPodLog(podName, podNamespace || namespace, 'main') as any).then( + return (k8sV1Client.readNamespacedPodLog(podName, podNamespace, 'main') as any).then( (response: any) => (response && response.body ? response.body.toString() : ''), (error: any) => { throw new Error(JSON.stringify(error.body)); @@ -221,13 +218,56 @@ export function getPodLogs(podName: string, podNamespace?: string): Promise { + try { + const { body } = await k8sV1Client.readNamespacedPod(podName, podNamespace); + return [body, undefined]; + } catch (error) { + const { message, additionalInfo } = parseK8sError(error); + const userMessage = `Could not get pod ${podName} in namespace ${podNamespace}: ${message}`; + return [undefined, { message: userMessage, additionalInfo }]; + } +} + +// Golang style result type including an error. +export type Result = [T, undefined] | [undefined, E]; +export async function listPodEvents( + podName: string, + podNamespace: string, +): Promise> { + try { + const { body } = await k8sV1Client.listNamespacedEvent( + podNamespace, + undefined, + undefined, + undefined, + // The following fieldSelector can be found when running + // `kubectl describe -v 8` + // (-v 8) will verbosely print network requests sent by kubectl. + `involvedObject.namespace=${podNamespace},involvedObject.name=${podName},involvedObject.kind=Pod`, + ); + return [body, undefined]; + } catch (error) { + const { message, additionalInfo } = parseK8sError(error); + const userMessage = `Error when listing pod events for pod "${podName}" in "${podNamespace}" namespace: ${message}`; + return [undefined, { message: userMessage, additionalInfo }]; + } +} + /** * Retrieves the argo workflow CRD. * @param workflowName name of the argo workflow */ export async function getArgoWorkflow(workflowName: string): Promise { - if (!k8sV1CustomObjectClient) { - throw new Error('Cannot access kubernetes Custom Object API'); + if (!namespace) { + throw new Error(`Cannot get namespace from ${namespaceFilePath}`); } const res = await k8sV1CustomObjectClient.getNamespacedCustomObject( @@ -254,8 +294,8 @@ export async function getArgoWorkflow(workflowName: string): Promise { proxyPrefix + '*', proxy({ changeOrigin: true, - logLevel: 'debug', + logLevel: process.env.NODE_ENV === 'test' ? 'warn' : 'debug', target: 'http://127.0.0.1', router: (req: any) => { diff --git a/frontend/src/TestUtils.tsx b/frontend/src/TestUtils.tsx index ef893fd8f426..b132803bdcef 100644 --- a/frontend/src/TestUtils.tsx +++ b/frontend/src/TestUtils.tsx @@ -149,6 +149,6 @@ export function diff({ }); } -function formatHTML(html: string): string { +export function formatHTML(html: string): string { return format(html, { parser: 'html' }); } diff --git a/frontend/src/components/PodYaml.test.tsx b/frontend/src/components/PodYaml.test.tsx new file mode 100644 index 000000000000..342003d8bf89 --- /dev/null +++ b/frontend/src/components/PodYaml.test.tsx @@ -0,0 +1,168 @@ +import React, { FC } from 'react'; +import { PodInfo, PodEvents } from './PodYaml'; +import { render, act, fireEvent } from '@testing-library/react'; +import { Apis } from 'src/lib/Apis'; +import TestUtils from 'src/TestUtils'; + +// Original ./Editor uses a complex external editor inside, we use a simple mock +// for testing instead. +jest.mock('./Editor', () => { + return ({ value }: { value: string }) => 
{value}
; +}); + +afterEach(async () => { + jest.resetAllMocks(); + jest.restoreAllMocks(); +}); + +describe('PodInfo', () => { + let podInfoSpy: any; + beforeEach(() => { + podInfoSpy = jest.spyOn(Apis, 'getPodInfo'); + }); + + it('renders Editor with pod yaml', async () => { + podInfoSpy.mockImplementation(() => + Promise.resolve({ + kind: 'Pod', + metadata: { + name: 'test-pod', + }, + }), + ); + const { container } = render(); + // Renders nothing when loading + expect(container).toMatchInlineSnapshot(`
`); + + await act(TestUtils.flushPromises); + expect(container).toMatchInlineSnapshot(` +
+ 
+          kind: Pod
+      metadata:
+        name: test-pod
+
+
+
+ `); + }); + + it('renders pod yaml putting spec to the last section', async () => { + podInfoSpy.mockImplementation(() => + Promise.resolve({ + kind: 'Pod', + spec: { + property: 'value', + }, + status: { + property: 'value2', + }, + }), + ); + const { container } = render(); + await act(TestUtils.flushPromises); + expect(container).toMatchInlineSnapshot(` +
+ 
+          kind: Pod
+      status:
+        property: value2
+      spec:
+        property: value
+
+
+
+ `); + }); + + it('shows a warning banner when request fails', async () => { + podInfoSpy.mockImplementation(() => Promise.reject('Pod not found')); + const { getByText } = render(); + await act(TestUtils.flushPromises); + getByText( + 'Warning: failed to retrieve pod info. Possible reasons include cluster autoscaling, pod preemption or pod cleaned up by time to live configuration', + ); + }); + + it('can be retried when request fails', async () => { + // Network was bad initially + podInfoSpy.mockImplementation(() => Promise.reject('Network failed')); + const { getByText } = render(); + await act(TestUtils.flushPromises); + + // Now network gets healthy + podInfoSpy.mockImplementation(() => + Promise.resolve({ + kind: 'Pod', + }), + ); + const refreshButton = getByText('Refresh'); + fireEvent.click(refreshButton); + await act(TestUtils.flushPromises); + getByText('kind: Pod'); + }); + + it('refreshes automatically when pod name or namespace changes', async () => { + // Now network gets healthy + podInfoSpy.mockImplementation(() => + Promise.resolve({ + metadata: { name: 'pod-1' }, + }), + ); + const { getByText, rerender } = render(); + expect(podInfoSpy).toHaveBeenLastCalledWith('test-pod-1', 'test-ns'); + await act(TestUtils.flushPromises); + getByText(/pod-1/); + + podInfoSpy.mockImplementation(() => + Promise.resolve({ + metadata: { name: 'pod-2' }, + }), + ); + rerender(); + expect(podInfoSpy).toHaveBeenLastCalledWith('test-pod-2', 'test-ns'); + await act(TestUtils.flushPromises); + getByText(/pod-2/); + }); +}); + +// PodEvents is very similar to PodInfo, so we only test different parts here. +describe('PodEvents', () => { + let podEventsSpy: any; + beforeEach(() => { + podEventsSpy = jest.spyOn(Apis, 'getPodEvents'); + }); + + it('renders Editor with pod events yaml', async () => { + podEventsSpy.mockImplementation(() => + Promise.resolve({ + kind: 'EventList', + }), + ); + const { container } = render(); + await act(TestUtils.flushPromises); + expect(container).toMatchInlineSnapshot(` +
+ 
+          kind: EventList
+
+
+
+ `); + }); + + it('shows a warning banner when request fails', async () => { + podEventsSpy.mockImplementation(() => Promise.reject('Pod not found')); + const { getByText } = render(); + await act(TestUtils.flushPromises); + getByText( + 'Warning: failed to retrieve pod events. Possible reasons include cluster autoscaling, pod preemption or pod cleaned up by time to live configuration', + ); + }); +}); diff --git a/frontend/src/components/PodYaml.tsx b/frontend/src/components/PodYaml.tsx new file mode 100644 index 000000000000..03a832911cde --- /dev/null +++ b/frontend/src/components/PodYaml.tsx @@ -0,0 +1,122 @@ +import * as JsYaml from 'js-yaml'; +import React from 'react'; +import { Apis, JSONObject } from 'src/lib/Apis'; +import { serviceErrorToString } from 'src/lib/Utils'; +import Banner from './Banner'; +import Editor from './Editor'; + +async function getPodYaml(name: string, namespace: string): Promise { + const response = await Apis.getPodInfo(name, namespace); + return JsYaml.safeDump(reorderPodJson(response), { skipInvalid: true }); +} +export const PodInfo: React.FC<{ name: string; namespace: string }> = ({ name, namespace }) => { + return ( + + ); +}; + +async function getPodEventsYaml(name: string, namespace: string): Promise { + const response = await Apis.getPodEvents(name, namespace); + return JsYaml.safeDump(response, { skipInvalid: true }); +} +export const PodEvents: React.FC<{ + name: string; + namespace: string; +}> = ({ name, namespace }) => { + return ( + + ); +}; + +const PodYaml: React.FC<{ + name: string; + namespace: string; + errorMessage: string; + getYaml: (name: string, namespace: string) => Promise; +}> = ({ name, namespace, getYaml, errorMessage }) => { + const [yaml, setYaml] = React.useState(undefined); + const [error, setError] = React.useState< + | { + message: string; + additionalInfo: string; + } + | undefined + >(undefined); + const [refreshes, setRefresh] = React.useState(0); + + React.useEffect(() => { + let aborted = false; + async function load() { + setYaml(undefined); + setError(undefined); + try { + const yaml = await getYaml(name, namespace); + if (!aborted) { + setYaml(yaml); + } + } catch (err) { + if (!aborted) { + setError({ + message: errorMessage, + additionalInfo: await serviceErrorToString(err), + }); + } + } + } + load(); + return () => { + aborted = true; + }; + }, [name, namespace, refreshes, getYaml, errorMessage]); // When refreshes change, request is fetched again. + + return ( + <> + {error && ( + setRefresh(refreshes => refreshes + 1)} + /> + )} + {!error && yaml && ( + + )} + + ); +}; + +function reorderPodJson(jsonData: JSONObject): JSONObject { + const orderedData = { ...jsonData }; + if (jsonData.spec) { + // Deleting spec property and add it again moves spec to the last property in the JSON object. + delete orderedData.spec; + orderedData.spec = jsonData.spec; + } + return orderedData; +} + +export const TEST_ONLY = { + PodYaml, +}; diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts index 681fba0f5ac3..b4a15769d684 100644 --- a/frontend/src/lib/Apis.ts +++ b/frontend/src/lib/Apis.ts @@ -40,6 +40,12 @@ export interface BuildInfo { frontendCommitHash?: string; } +// Hack types from https://github.com/microsoft/TypeScript/issues/1897#issuecomment-557057387 +export type JSONPrimitive = string | number | boolean | null; +export type JSONValue = JSONPrimitive | JSONObject | JSONArray; +export type JSONObject = { [member: string]: JSONValue }; +export type JSONArray = JSONValue[]; + let customVisualizationsAllowed: boolean; // For cross browser support, fetch should use 'same-origin' as default. This fixes firefox auth issues. @@ -93,6 +99,28 @@ export class Apis { return this._fetch(query); } + /** + * Get pod info + */ + public static async getPodInfo(podName: string, podNamespace: string): Promise { + const query = `k8s/pod?podname=${encodeURIComponent(podName)}&podnamespace=${encodeURIComponent( + podNamespace, + )}`; + const podInfo = await this._fetch(query); + return JSON.parse(podInfo); + } + + /** + * Get pod events + */ + public static async getPodEvents(podName: string, podNamespace: string): Promise { + const query = `k8s/pod/events?podname=${encodeURIComponent( + podName, + )}&podnamespace=${encodeURIComponent(podNamespace)}`; + const eventList = await this._fetch(query); + return JSON.parse(eventList); + } + public static get basePath(): string { const path = window.location.protocol + '//' + window.location.host + window.location.pathname; // Trim trailing '/' if exists diff --git a/frontend/src/pages/RunDetails.test.tsx b/frontend/src/pages/RunDetails.test.tsx index e0fa210418d0..de8c8caa3e79 100644 --- a/frontend/src/pages/RunDetails.test.tsx +++ b/frontend/src/pages/RunDetails.test.tsx @@ -33,24 +33,22 @@ import { RunDetails, RunDetailsInternalProps } from './RunDetails'; const NODE_DETAILS_SELECTOR = '[data-testid="run-details-node-details"]'; describe('RunDetails', () => { - const updateBannerSpy = jest.fn(); - const updateDialogSpy = jest.fn(); - const updateSnackbarSpy = jest.fn(); - const updateToolbarSpy = jest.fn(); - const historyPushSpy = jest.fn(); - const getRunSpy = jest.spyOn(Apis.runServiceApi, 'getRun'); - const getExperimentSpy = jest.spyOn(Apis.experimentServiceApi, 'getExperiment'); - const isCustomVisualizationsAllowedSpy = jest.spyOn(Apis, 'areCustomVisualizationsAllowed'); - const getPodLogsSpy = jest.spyOn(Apis, 'getPodLogs'); - const pathsParser = jest.spyOn(WorkflowParser, 'loadNodeOutputPaths'); - const pathsWithStepsParser = jest.spyOn(WorkflowParser, 'loadAllOutputPathsWithStepNames'); - const loaderSpy = jest.spyOn(OutputArtifactLoader, 'load'); - const retryRunSpy = jest.spyOn(Apis.runServiceApi, 'retryRun'); - const terminateRunSpy = jest.spyOn(Apis.runServiceApi, 'terminateRun'); - const artifactTypesSpy = jest.spyOn(Api.getInstance().metadataStoreService, 'getArtifactTypes'); - // We mock this because it uses toLocaleDateString, which causes mismatches between local and CI - // test environments - const formatDateStringSpy = jest.spyOn(Utils, 'formatDateString'); + let updateBannerSpy: any; + let updateDialogSpy: any; + let updateSnackbarSpy: any; + let updateToolbarSpy: any; + let historyPushSpy: any; + let getRunSpy: any; + let getExperimentSpy: any; + let isCustomVisualizationsAllowedSpy: any; + let getPodLogsSpy: any; + let pathsParser: any; + let pathsWithStepsParser: any; + let loaderSpy: any; + let retryRunSpy: any; + let terminateRunSpy: any; + let artifactTypesSpy: any; + let formatDateStringSpy: any; let testRun: ApiRunDetail = {}; let tree: ShallowWrapper | ReactWrapper; @@ -72,11 +70,11 @@ describe('RunDetails', () => { }); } - beforeAll(() => jest.spyOn(console, 'error').mockImplementation()); - beforeEach(() => { // The RunDetails page uses timers to periodically refresh jest.useFakeTimers(); + // TODO: mute error only for tests that are expected to have error + jest.spyOn(console, 'error').mockImplementation(() => null); testRun = { pipeline_runtime: { @@ -94,6 +92,25 @@ describe('RunDetails', () => { status: 'Succeeded', }, }; + updateBannerSpy = jest.fn(); + updateDialogSpy = jest.fn(); + updateSnackbarSpy = jest.fn(); + updateToolbarSpy = jest.fn(); + historyPushSpy = jest.fn(); + getRunSpy = jest.spyOn(Apis.runServiceApi, 'getRun'); + getExperimentSpy = jest.spyOn(Apis.experimentServiceApi, 'getExperiment'); + isCustomVisualizationsAllowedSpy = jest.spyOn(Apis, 'areCustomVisualizationsAllowed'); + getPodLogsSpy = jest.spyOn(Apis, 'getPodLogs'); + pathsParser = jest.spyOn(WorkflowParser, 'loadNodeOutputPaths'); + pathsWithStepsParser = jest.spyOn(WorkflowParser, 'loadAllOutputPathsWithStepNames'); + loaderSpy = jest.spyOn(OutputArtifactLoader, 'load'); + retryRunSpy = jest.spyOn(Apis.runServiceApi, 'retryRun'); + terminateRunSpy = jest.spyOn(Apis.runServiceApi, 'terminateRun'); + artifactTypesSpy = jest.spyOn(Api.getInstance().metadataStoreService, 'getArtifactTypes'); + // We mock this because it uses toLocaleDateString, which causes mismatches between local and CI + // test environments + formatDateStringSpy = jest.spyOn(Utils, 'formatDateString'); + getRunSpy.mockImplementation(() => Promise.resolve(testRun)); getExperimentSpy.mockImplementation(() => Promise.resolve({ id: 'some-experiment-id', name: 'some experiment' }), @@ -111,7 +128,6 @@ describe('RunDetails', () => { response.setArtifactTypesList([]); return response; }); - jest.clearAllMocks(); }); afterEach(async () => { @@ -119,6 +135,7 @@ describe('RunDetails', () => { // depends on mocks/spies await tree.unmount(); jest.resetAllMocks(); + jest.restoreAllMocks(); }); it('shows success run status in page title', async () => { diff --git a/frontend/src/pages/RunDetails.tsx b/frontend/src/pages/RunDetails.tsx index 6c7341d60a8d..9d10f6330669 100644 --- a/frontend/src/pages/RunDetails.tsx +++ b/frontend/src/pages/RunDetails.tsx @@ -14,57 +14,58 @@ * limitations under the License. */ +import CircularProgress from '@material-ui/core/CircularProgress'; +import InfoIcon from '@material-ui/icons/InfoOutlined'; +import { flatten } from 'lodash'; import * as React from 'react'; +import { GkeMetadata, GkeMetadataContext } from 'src/lib/GkeMetadata'; +import { classes, stylesheet } from 'typestyle'; +import { + NodePhase as ArgoNodePhase, + NodeStatus, + Workflow, +} from '../../third_party/argo-ui/argo_template'; +import { ApiExperiment } from '../apis/experiment'; +import { ApiRun, RunStorageState } from '../apis/run'; +import { ApiVisualization, ApiVisualizationType } from '../apis/visualization'; +import Hr from '../atoms/Hr'; +import MD2Tabs from '../atoms/MD2Tabs'; +import Separator from '../atoms/Separator'; import Banner, { Mode } from '../components/Banner'; -import Buttons, { ButtonKeys } from '../lib/Buttons'; -import CircularProgress from '@material-ui/core/CircularProgress'; import CompareTable from '../components/CompareTable'; -import CompareUtils from '../lib/CompareUtils'; import DetailsTable from '../components/DetailsTable'; -import MinioArtifactLink from '../components/MinioArtifactLink'; import Graph from '../components/Graph'; -import Hr from '../atoms/Hr'; -import InfoIcon from '@material-ui/icons/InfoOutlined'; import LogViewer from '../components/LogViewer'; -import MD2Tabs from '../atoms/MD2Tabs'; +import MinioArtifactLink from '../components/MinioArtifactLink'; import PlotCard from '../components/PlotCard'; -import RunUtils from '../lib/RunUtils'; -import Separator from '../atoms/Separator'; +import { RoutePage, RouteParams } from '../components/Router'; import SidePanel from '../components/SidePanel'; -import WorkflowParser from '../lib/WorkflowParser'; -import { ApiExperiment } from '../apis/experiment'; -import { ApiRun, RunStorageState } from '../apis/run'; +import { ToolbarProps } from '../components/Toolbar'; +import { HTMLViewerConfig } from '../components/viewers/HTMLViewer'; +import { PlotType, ViewerConfig } from '../components/viewers/Viewer'; +import { componentMap } from '../components/viewers/ViewerContainer'; +import VisualizationCreator, { + VisualizationCreatorConfig, +} from '../components/viewers/VisualizationCreator'; +import { color, commonCss, fonts, fontsize, padding } from '../Css'; import { Apis } from '../lib/Apis'; -import { NodePhase, hasFinished } from '../lib/StatusUtils'; +import Buttons, { ButtonKeys } from '../lib/Buttons'; +import CompareUtils from '../lib/CompareUtils'; import { OutputArtifactLoader } from '../lib/OutputArtifactLoader'; +import RunUtils from '../lib/RunUtils'; import { KeyValue } from '../lib/StaticGraphParser'; -import { Page, PageProps } from './Page'; -import { RoutePage, RouteParams } from '../components/Router'; -import { ToolbarProps } from '../components/Toolbar'; -import { ViewerConfig, PlotType } from '../components/viewers/Viewer'; -import { - Workflow, - NodeStatus, - NodePhase as ArgoNodePhase, -} from '../../third_party/argo-ui/argo_template'; -import { classes, stylesheet } from 'typestyle'; -import { commonCss, padding, color, fonts, fontsize } from '../Css'; -import { componentMap } from '../components/viewers/ViewerContainer'; -import { flatten } from 'lodash'; +import { hasFinished, NodePhase } from '../lib/StatusUtils'; import { + errorToMessage, formatDateString, getRunDurationFromWorkflow, logger, - errorToMessage, serviceErrorToString, } from '../lib/Utils'; +import WorkflowParser from '../lib/WorkflowParser'; +import { Page, PageProps } from './Page'; import { statusToIcon } from './Status'; -import VisualizationCreator, { - VisualizationCreatorConfig, -} from '../components/viewers/VisualizationCreator'; -import { ApiVisualization, ApiVisualizationType } from '../apis/visualization'; -import { HTMLViewerConfig } from '../components/viewers/HTMLViewer'; -import { GkeMetadata, GkeMetadataContext } from 'src/lib/GkeMetadata'; +import { PodEvents, PodInfo } from '../components/PodYaml'; enum SidePaneTab { ARTIFACTS, @@ -72,6 +73,8 @@ enum SidePaneTab { VOLUMES, MANIFEST, LOGS, + POD, + EVENTS, } interface SelectedNodeDetails { @@ -222,6 +225,7 @@ export class RunDetails extends Page { } = this.state; const { projectId, clusterName } = this.props.gkeMetadata; const selectedNodeId = selectedNodeDetails ? selectedNodeDetails.id : ''; + const namespace = workflow?.metadata?.namespace; let stackdriverK8sLogsUrl = ''; if (projectId && clusterName && selectedNodeDetails && selectedNodeDetails.id) { stackdriverK8sLogsUrl = `https://console.cloud.google.com/logs/viewer?project=${projectId}&interval=NO_LIMIT&advancedFilter=resource.type%3D"k8s_container"%0Aresource.labels.cluster_name:"${clusterName}"%0Aresource.labels.pod_name:"${selectedNodeDetails.id}"`; @@ -283,7 +287,15 @@ export class RunDetails extends Page { )}
@@ -357,6 +369,22 @@ export class RunDetails extends Page {
)} + {sidepanelSelectedTab === SidePaneTab.POD && ( +
+ {selectedNodeId && namespace && ( + + )} +
+ )} + + {sidepanelSelectedTab === SidePaneTab.EVENTS && ( +
+ {selectedNodeId && namespace && ( + + )} +
+ )} + {sidepanelSelectedTab === SidePaneTab.LOGS && (
{this.state.logsBannerMessage && ( @@ -638,7 +666,7 @@ export class RunDetails extends Page { }); } catch (err) { await this.showPageError(`Error: failed to retrieve run: ${runId}.`, err); - logger.error('Error loading run:', runId); + logger.error('Error loading run:', runId, err); } // Make sure logs and artifacts in the side panel are refreshed when diff --git a/frontend/src/pages/__snapshots__/RunDetails.test.tsx.snap b/frontend/src/pages/__snapshots__/RunDetails.test.tsx.snap index eefb1e558768..47b2e0051cfd 100644 --- a/frontend/src/pages/__snapshots__/RunDetails.test.tsx.snap +++ b/frontend/src/pages/__snapshots__/RunDetails.test.tsx.snap @@ -570,6 +570,8 @@ exports[`RunDetails logs tab does not load logs if clicked node status is skippe "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -686,6 +688,8 @@ exports[`RunDetails logs tab keeps side pane open and on same tab when logs chan "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -802,6 +806,8 @@ exports[`RunDetails logs tab loads and shows logs in side pane 1`] = ` "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -918,6 +924,8 @@ exports[`RunDetails logs tab switches to logs tab in side pane 1`] = ` "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -1034,6 +1042,8 @@ exports[`RunDetails opens side panel when graph node is clicked 1`] = ` "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -1276,6 +1286,8 @@ exports[`RunDetails shows clicked node message in side panel 1`] = ` "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -1725,6 +1737,8 @@ exports[`RunDetails switches to inputs/outputs tab in side pane 1`] = ` "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -1866,6 +1880,8 @@ exports[`RunDetails switches to manifest tab in side pane 1`] = ` "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> @@ -2015,6 +2031,8 @@ exports[`RunDetails switches to volumes tab in side pane 1`] = ` "Volumes", "Manifest", "Logs", + "Pod", + "Events", ] } /> diff --git a/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/pipeline.yaml b/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/pipeline.yaml index 982b384a28bb..2033db86ed94 100644 --- a/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/pipeline.yaml +++ b/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/pipeline.yaml @@ -163,6 +163,12 @@ rules: - create - get - list + - apiGroups: + - "" + resources: + - events + verbs: + - list - apiGroups: - "" resources: diff --git a/manifests/kustomize/base/pipeline/ml-pipeline-ui-role.yaml b/manifests/kustomize/base/pipeline/ml-pipeline-ui-role.yaml index 1dedfb34e813..4877d7a2ceec 100644 --- a/manifests/kustomize/base/pipeline/ml-pipeline-ui-role.yaml +++ b/manifests/kustomize/base/pipeline/ml-pipeline-ui-role.yaml @@ -14,6 +14,12 @@ rules: - create - get - list +- apiGroups: + - "" + resources: + - events + verbs: + - list - apiGroups: - "" resources: