From f40824f2dfbaf03b1a8b8f7296c8b7900b8fd3b7 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Date: Mon, 27 Sep 2021 09:43:52 -0400 Subject: [PATCH] Fix validation timeout, poll for results --- backend/src/routes/api/validate-isv/index.ts | 18 +- .../routes/api/validate-isv/validateISV.ts | 158 ++++++++++-------- frontend/src/services/validateIsvService.ts | 21 ++- .../src/utilities/useEnableApplication.tsx | 124 +++++++++----- 4 files changed, 211 insertions(+), 110 deletions(-) diff --git a/backend/src/routes/api/validate-isv/index.ts b/backend/src/routes/api/validate-isv/index.ts index b69ca65694..9d937125f6 100644 --- a/backend/src/routes/api/validate-isv/index.ts +++ b/backend/src/routes/api/validate-isv/index.ts @@ -1,7 +1,7 @@ import { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'; import { DEV_MODE } from '../../../utils/constants'; import { addCORSHeader } from '../../../utils/responseUtils'; -import { validateISV } from './validateISV'; +import { getValidateISVResults, validateISV } from './validateISV'; export default async (fastify: FastifyInstance): Promise => { fastify.get('/', async (request: FastifyRequest, reply: FastifyReply) => { @@ -20,4 +20,20 @@ export default async (fastify: FastifyInstance): Promise => { reply.send(res); }); }); + fastify.get('/results', async (request: FastifyRequest, reply: FastifyReply) => { + return getValidateISVResults(fastify, request) + .then((res) => { + if (DEV_MODE) { + addCORSHeader(request, reply); + } + return res; + }) + .catch((res) => { + if (DEV_MODE) { + addCORSHeader(request, reply); + } + fastify.log.error(`Failed to get validation job results: ${res.response?.body?.message}`); + reply.send(res); + }); + }); }; diff --git a/backend/src/routes/api/validate-isv/validateISV.ts b/backend/src/routes/api/validate-isv/validateISV.ts index 145156aca9..05174a0c74 100644 --- a/backend/src/routes/api/validate-isv/validateISV.ts +++ b/backend/src/routes/api/validate-isv/validateISV.ts @@ -5,14 +5,12 @@ import { KubeFastifyInstance, OdhApplication } from '../../../types'; import { getApplicationDef } from '../../../utils/resourceUtils'; import { getApplicationEnabledConfigMap } from '../../../utils/componentUtils'; -const JOB_STATUS_PENDING = 'job-status-pending'; - const doSleep = (timeout: number) => { return new Promise((resolve) => setTimeout(resolve, timeout)); }; const waitOnDeletion = async (reader: () => Promise) => { - const MAX_TRIES = 25; + const MAX_TRIES = 200; let tries = 0; let deleted = false; @@ -27,39 +25,22 @@ const waitOnDeletion = async (reader: () => Promise) => { const waitOnCompletion = async ( fastify: KubeFastifyInstance, - reader: () => Promise<{ valid: boolean; error: string }>, -): Promise<{ valid: boolean; error: string }> => { + reader: () => Promise, +): Promise => { const MAX_TRIES = 120; let tries = 0; let completionStatus; - while (completionStatus === undefined && ++tries < MAX_TRIES) { - await reader() - .then((res) => { - completionStatus = res; - }) - .catch(async (e) => { - if (e.message === JOB_STATUS_PENDING) { - await doSleep(1000); - return; - } - fastify.log.error(`validation job failed: ${e.response?.body?.message ?? e.message}.`); - completionStatus = { - valid: false, - error: e.response?.body?.message ?? e.message, - }; - }); + while (!completionStatus && ++tries < MAX_TRIES) { + completionStatus = await reader(); + if (!completionStatus) { + await doSleep(1000); + } } - if (completionStatus !== undefined) { - return completionStatus; + if (!completionStatus) { + fastify.log.warn('validation job timed out.'); } - - fastify.log.error('validation job timed out.'); - return { - valid: false, - error: 'Validation process timed out. The application may still become enabled.', - }; }; export const createAccessSecret = async ( @@ -94,7 +75,7 @@ export const createAccessSecret = async ( export const runValidation = async ( fastify: KubeFastifyInstance, request: FastifyRequest, -): Promise<{ valid: boolean; error: string }> => { +): Promise<{ complete: boolean; valid: boolean; error: string }> => { const namespace = fastify.kube.namespace; const query = request.query as { [key: string]: string }; const appName = query?.appName; @@ -107,10 +88,7 @@ export const runValidation = async ( const cronjobName = enable?.validationJob; if (!cronjobName) { - return Promise.resolve({ - valid: false, - error: 'Validation job is undefined.', - }); + return { complete: true, valid: false, error: 'Validation job is undefined.' }; } const jobName = `${cronjobName}-job-custom-run`; @@ -127,10 +105,11 @@ export const runValidation = async ( if (!cronJob) { fastify.log.error('The validation job for the application does not exist.'); - return Promise.resolve({ + return { + complete: true, valid: false, error: 'The validation job for the application does not exist.', - }); + }; } const updateCronJobSuspension = async (suspend: boolean) => { @@ -176,52 +155,45 @@ export const runValidation = async ( spec: cronJob.spec.jobTemplate.spec, }; - const { body } = await batchV1Api.createNamespacedJob(namespace, job).catch(() => { + const { body } = await batchV1Api.createNamespacedJob(namespace, job).catch(async () => { fastify.log.error(`failed to create validation job`); // Flag the cronjob as no longer suspended - updateCronJobSuspension(false); + await updateCronJobSuspension(false); return { body: null }; }); if (!body) { // Flag the cronjob as no longer suspended - updateCronJobSuspension(false); + await updateCronJobSuspension(false); fastify.log.error('failed to create validation job'); - return Promise.resolve({ valid: false, error: 'Failed to create validation job.' }); + return { complete: true, valid: false, error: 'Failed to create validation job.' }; } - return await waitOnCompletion(fastify, () => { - return batchV1Api.readNamespacedJobStatus(jobName, namespace).then(async (res) => { - if (res.body.status.succeeded) { - const success = await getApplicationEnabledConfigMap(fastify, appDef); - if (!success) { - fastify.log.warn(`failed attempted validation for ${appName}`); - } - return { - valid: success, - error: success ? '' : 'Error attempting to validate. Please check your entries.', - }; - } - if (res.body.status.failed) { - fastify.log.error('Validation job failed failed to run'); - - return { valid: false, error: 'Validation job failed to run.' }; - } - throw new Error(JOB_STATUS_PENDING); - }); + waitOnCompletion(fastify, async () => { + const res = await batchV1Api.readNamespacedJobStatus(jobName, namespace); + if (res.body.status.succeeded) { + return true; + } + if (res.body.status.failed) { + fastify.log.error('Validation job failed failed to run'); + return true; + } + return false; }).finally(() => { // Flag the cronjob as no longer suspended updateCronJobSuspension(false); }); + + return { complete: false, valid: false, error: null }; }; export const validateISV = async ( fastify: KubeFastifyInstance, request: FastifyRequest, -): Promise<{ valid: boolean; error: string }> => { +): Promise<{ complete: boolean; valid: boolean; error: string }> => { const query = request.query as { [key: string]: string }; const appName = query?.appName; const appDef = getApplicationDef(appName); @@ -236,10 +208,11 @@ export const validateISV = async ( if (!cmName) { fastify.log.error('attempted validation of application with no config map.'); - return Promise.resolve({ + return { + complete: true, valid: false, error: 'The validation config map for the application does not exist.', - }); + }; } const cmBody: V1ConfigMap = { @@ -257,16 +230,69 @@ export const validateISV = async ( .createNamespacedConfigMap(namespace, cmBody) .then(async () => { const success = await getApplicationEnabledConfigMap(fastify, appDef); - if (!success) { - fastify.log.warn(`failed attempted validation for ${appName}`); - } return { + complete: true, valid: success, error: success ? '' : 'Error adding validation flag.', }; }) .catch((e) => { fastify.log.warn(`failed creation of validation configmap: ${e.message}`); - return { valid: false, error: 'Error adding validation flag.' }; + return { complete: true, valid: false, error: 'Error adding validation flag.' }; }); }; + +export const getValidateISVResults = async ( + fastify: KubeFastifyInstance, + request: FastifyRequest, +): Promise<{ complete: boolean; valid: boolean; error: string }> => { + const batchV1Api = fastify.kube.batchV1Api; + const query = request.query as { [key: string]: string }; + const appName = query?.appName; + const appDef = getApplicationDef(appName); + const { enable } = appDef.spec; + const namespace = fastify.kube.namespace; + const cmName = enable?.validationConfigMap; + + if (!cmName) { + fastify.log.error('attempted validation of application with no config map.'); + return { + complete: true, + valid: false, + error: 'The validation config map for the application does not exist.', + }; + } + + // If there are variables associated with enablement, check the job status + if (enable?.variables && Object.keys(enable.variables).length > 0) { + const cronjobName = enable?.validationJob; + if (!cronjobName) { + return { complete: true, valid: false, error: 'Validation job is undefined.' }; + } + const jobName = `${cronjobName}-job-custom-run`; + + try { + const complete = await batchV1Api + .readNamespacedJobStatus(jobName, namespace) + .then(async (res) => { + return res.body.status.succeeded || res.body.status.failed; + }); + if (!complete) { + return { complete: false, valid: false, error: null }; + } + } catch { + return { complete: true, valid: false, error: 'Failed to create validation job.' }; + } + } + + // Check the results config map + const success = await getApplicationEnabledConfigMap(fastify, appDef); + if (!success) { + fastify.log.warn(`failed attempted validation for ${appName}`); + } + return { + complete: true, + valid: success, + error: success ? '' : 'Error attempting to validate. Please check your entries.', + }; +}; diff --git a/frontend/src/services/validateIsvService.ts b/frontend/src/services/validateIsvService.ts index 8107618745..c5cdb83a3b 100644 --- a/frontend/src/services/validateIsvService.ts +++ b/frontend/src/services/validateIsvService.ts @@ -4,7 +4,7 @@ import { getBackendURL } from '../utilities/utils'; export const postValidateIsv = ( appName: string, values: { [key: string]: string }, -): Promise<{ valid: boolean; error: string }> => { +): Promise<{ complete: boolean; valid: boolean; error: string }> => { const url = getBackendURL('/api/validate-isv'); const searchParams = new URLSearchParams(); if (appName) { @@ -21,3 +21,22 @@ export const postValidateIsv = ( throw new Error(e.response.data?.message || e.message); }); }; + +export const getValidationStatus = ( + appName: string, +): Promise<{ complete: boolean; valid: boolean; error: string }> => { + const url = getBackendURL('/api/validate-isv/results'); + const searchParams = new URLSearchParams(); + if (appName) { + searchParams.set('appName', appName); + } + const options = { params: searchParams }; + return axios + .get(url, options) + .then((res) => { + return res.data; + }) + .catch((e) => { + throw new Error(e.response.data?.message || e.message); + }); +}; diff --git a/frontend/src/utilities/useEnableApplication.tsx b/frontend/src/utilities/useEnableApplication.tsx index fbeb580750..e66ef52817 100644 --- a/frontend/src/utilities/useEnableApplication.tsx +++ b/frontend/src/utilities/useEnableApplication.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { useDispatch } from 'react-redux'; -import { postValidateIsv } from '../services/validateIsvService'; +import { getValidationStatus, postValidateIsv } from '../services/validateIsvService'; import { addNotification, forceComponentsUpdate } from '../redux/actions/actions'; export enum EnableApplicationStatus { @@ -16,70 +16,110 @@ export const useEnableApplication = ( appName: string, enableValues: { [key: string]: string }, ): [EnableApplicationStatus, string] => { - const [error, setError] = React.useState(''); - const [status, setStatus] = React.useState(EnableApplicationStatus.IDLE); + const [enableStatus, setEnableStatus] = React.useState<{ + status: EnableApplicationStatus; + error: string; + }>({ status: EnableApplicationStatus.IDLE, error: '' }); const dispatch = useDispatch(); + const dispatchResults = React.useCallback( + (error?: string) => { + if (!error) { + dispatch( + addNotification({ + status: 'success', + title: `${appName} has been added to the Enabled page.`, + timestamp: new Date(), + }), + ); + dispatch(forceComponentsUpdate()); + return; + } + dispatch( + addNotification({ + status: 'danger', + title: `Error attempting to validate ${appName}.`, + message: error, + timestamp: new Date(), + }), + ); + }, + [appName, dispatch], + ); + React.useEffect(() => { if (!doEnable) { - setError(''); - setStatus(EnableApplicationStatus.IDLE); + setEnableStatus({ status: EnableApplicationStatus.IDLE, error: '' }); } }, [doEnable]); + React.useEffect(() => { + let cancelled = false; + let watchHandle; + if (enableStatus.status === EnableApplicationStatus.INPROGRESS) { + const watchStatus = () => { + getValidationStatus(appId) + .then((response) => { + if (!response.complete) { + watchHandle = setTimeout(watchStatus, 10 * 1000); + return; + } + setEnableStatus({ + status: response.valid + ? EnableApplicationStatus.SUCCESS + : EnableApplicationStatus.FAILED, + error: response.valid ? '' : response.error, + }); + dispatchResults(response.valid ? undefined : response.error); + }) + .catch((e) => { + if (!cancelled) { + setEnableStatus({ status: EnableApplicationStatus.FAILED, error: e.message }); + } + dispatchResults(e.message); + }); + }; + watchStatus(); + } + return () => { + cancelled = true; + if (watchHandle) { + clearTimeout(watchHandle); + } + }; + }, [appId, dispatchResults, enableStatus.status]); + React.useEffect(() => { let closed; if (doEnable) { - setStatus(EnableApplicationStatus.INPROGRESS); postValidateIsv(appId, enableValues) .then((response) => { if (!closed) { - if (!response.valid) { - setError(response.error); + if (!response.complete) { + setEnableStatus({ status: EnableApplicationStatus.INPROGRESS, error: '' }); + return; } - setStatus( - response.valid ? EnableApplicationStatus.SUCCESS : EnableApplicationStatus.FAILED, - ); - } - if (response.valid) { - dispatch( - addNotification({ - status: 'success', - title: `${appName} has been added to the Enabled page.`, - timestamp: new Date(), - }), - ); - dispatch(forceComponentsUpdate()); - return; + + setEnableStatus({ + status: response.valid + ? EnableApplicationStatus.SUCCESS + : EnableApplicationStatus.FAILED, + error: response.valid ? '' : response.error, + }); } - dispatch( - addNotification({ - status: 'danger', - title: `Error attempting to validate ${appName}.`, - message: response.error, - timestamp: new Date(), - }), - ); + dispatchResults(response.valid ? undefined : response.error); }) .catch((e) => { if (!closed) { - setError(e.message); - setStatus(EnableApplicationStatus.FAILED); + setEnableStatus({ status: EnableApplicationStatus.FAILED, error: e.m }); } - dispatch( - addNotification({ - status: 'danger', - title: `Error attempting to validate ${appName}.`, - message: e.message, - timestamp: new Date(), - }), - ); + dispatchResults(e.message); }); } return () => { closed = true; }; - }, [appId, appName, dispatch, doEnable, enableValues]); - return [status, error]; + }, [appId, appName, dispatch, dispatchResults, doEnable, enableValues]); + return [enableStatus.status, enableStatus.error]; };