From 255c0ff4380839e95ff69e13369de01030d2ba03 Mon Sep 17 00:00:00 2001 From: Lan Xia Date: Wed, 27 Mar 2024 14:12:10 -0400 Subject: [PATCH] Update build retry logic When querying a build, TRSS will retry 3 times. In the case of an error, the build will move back to the queue if the return code from Jenkins API is not 404. If the return code is 404, x-jenkins header is checked to ensure the error code is from Jenkins (not nginx). In this case, it means the build has an invalid url or is expired. retry function returns 404 and TRSS will stop processing this build. Otherwise, the error will be ignored (network issue) and the build will move back to the queue. return code 404 only if the Jenkins server returns 404 for the build (i.e., invalid url, expired build, etc) - sleep is removed to increase efficiency and reliability. We will check the header in the response to determine the invalid/expired build case. - test API getJenkinsBuildInfo is added - fix log issue related: https://github.com/adoptium/aqa-test-tools/issues/852 Signed-off-by: Lan Xia --- TestResultSummaryService/DataManager.js | 9 +++++- TestResultSummaryService/JenkinsInfo.js | 30 ++++++++++++------- TestResultSummaryService/routes/index.js | 1 + .../routes/test/getJenkinsBuildInfo.js | 14 +++++++++ 4 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 TestResultSummaryService/routes/test/getJenkinsBuildInfo.js diff --git a/TestResultSummaryService/DataManager.js b/TestResultSummaryService/DataManager.js index c044236d..061c1811 100644 --- a/TestResultSummaryService/DataManager.js +++ b/TestResultSummaryService/DataManager.js @@ -111,7 +111,14 @@ class DataManager { } async updateBuild(data) { - logger.verbose('updateBuild', data.buildName, data.buildNum); + if (data.buildUrl) { + logger.verbose('updateBuild', data.buildUrl); + } else if (data.buildName) { + logger.verbose('updateBuild', data.buildName, data.buildNum); + } else { + logger.verbose('updateBuild', data._id); + } + const { _id, buildName, ...newData } = data; const criteria = { _id: new ObjectID(_id) }; const testResults = new TestResultsDB(); diff --git a/TestResultSummaryService/JenkinsInfo.js b/TestResultSummaryService/JenkinsInfo.js index cbd2bf24..bab57e39 100644 --- a/TestResultSummaryService/JenkinsInfo.js +++ b/TestResultSummaryService/JenkinsInfo.js @@ -8,21 +8,31 @@ const options = { request: { timeout: 30000 } }; // Server connection may drop. If timeout, retry. const retry = (fn) => { - const promise = Promise.promisify(fn); return async function () { for (let i = 0; i < 3; i++) { try { - return await promise.apply(null, arguments); + return await new Promise((resolve, reject) => { + fn(...arguments, (err, data) => { + if (err) { + logger.warn(err); + reject(data); + } else { + resolve(data); + } + }); + }); } catch (e) { logger.warn(`Try #${i + 1}: connection issue`, arguments); - logger.warn(e); - logger.warn(`Sleep 2 secs...`); - await Promise.delay(2 * 1000); - if ( - i === 2 && - e.toString().includes('unexpected status code: 404') - ) { - return { code: 404 }; + if (e) { + logger.warn(`status code: ${e.statusCode}`); + logger.warn(`headers: ${JSON.stringify(e.headers)}`); + + // return code 404 only if the Jenkins server returns 404 for the build (i.e., invalid url, expired build, etc) + // x-jenkins header is checked to ensure the error code is from Jenkins (not nginx) + // TRSS will stop processing this build once code 404 is returned. See BuildProcessor for details. + if (e.statusCode === 404 && e.headers['x-jenkins']) { + return { code: e.statusCode }; + } } } } diff --git a/TestResultSummaryService/routes/index.js b/TestResultSummaryService/routes/index.js index 9c3af257..9c01bbbe 100644 --- a/TestResultSummaryService/routes/index.js +++ b/TestResultSummaryService/routes/index.js @@ -26,6 +26,7 @@ app.get('/getData', wrap(require('./getData'))); app.get('/getErrorInOutput', wrap(require('./getErrorInOutput'))); app.get('/getHistoryPerTest', wrap(require('./getHistoryPerTest'))); app.get('/getJenkins', wrap(require('./getJenkins'))); +app.get('/getJenkinsBuildInfo', wrap(require('./test/getJenkinsBuildInfo'))); app.get('/getLastBuildInfo', wrap(require('./getLastBuildInfo'))); app.get('/getLogStream', wrap(require('./getLogStream'))); app.get('/getOutputById', wrap(require('./getOutputById'))); diff --git a/TestResultSummaryService/routes/test/getJenkinsBuildInfo.js b/TestResultSummaryService/routes/test/getJenkinsBuildInfo.js new file mode 100644 index 00000000..f5f1fda4 --- /dev/null +++ b/TestResultSummaryService/routes/test/getJenkinsBuildInfo.js @@ -0,0 +1,14 @@ +const JenkinsInfo = require('../../JenkinsInfo'); + +module.exports = async (req, res) => { + const { url, buildName, buildNum } = req.query; + if (req.query.buildNum) + req.query.buildNum = parseInt(req.query.buildNum, 10); + const jenkinsInfo = new JenkinsInfo(); + try { + const output = await jenkinsInfo.getBuildInfo(url, buildName, buildNum); + res.send({ output }); + } catch (e) { + res.send({ result: e.toString() }); + } +};