From eea02c8e341d55a8bc3e262307539be627a00c97 Mon Sep 17 00:00:00 2001 From: Vitaly Pryakhin Date: Thu, 1 Jul 2021 19:20:40 +0300 Subject: [PATCH 1/4] add test cases for _admin/metrics/v2 endpoint in cluster mode --- README_maintainers.md | 8 +- .../js/client/shell/shell-promtool-cluster.js | 202 ++++++++++++++++++ 2 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 tests/js/client/shell/shell-promtool-cluster.js diff --git a/README_maintainers.md b/README_maintainers.md index 208a0098f92c..cc135290ca74 100644 --- a/README_maintainers.md +++ b/README_maintainers.md @@ -211,7 +211,7 @@ Depending on the platform, ArangoDB tries to locate the temporary directory: ### Local Cluster Startup The scripts `scripts/startLocalCluster` helps you to quickly fire up a testing -cluster on your local machine. `scripts/stopLocalCluster` stops it again. +cluster on your local machine. `scripts/shutdownLocalCluster` stops it again. `scripts/startLocalCluster [numDBServers numCoordinators [mode]]` @@ -692,10 +692,14 @@ To locate the suite(s) associated with a specific test file use: ./scripts/unittest find --test tests/js/common/shell/shell-aqlfunctions.js -Run all suite(s) associated with a specific test file: +Run all suite(s) associated with a specific test file in single server mode: ./scripts/unittest auto --test tests/js/common/shell/shell-aqlfunctions.js +Run all suite(s) associated with a specific test file in cluster mode: + + ./scripts/unittest auto --cluster true --test tests/js/common/shell/shell-aqlfunctions.js + Run all C++ based Google Test (gtest) tests using the `arangodbtests` binary: ./scripts/unittest gtest diff --git a/tests/js/client/shell/shell-promtool-cluster.js b/tests/js/client/shell/shell-promtool-cluster.js new file mode 100644 index 000000000000..7fc7dab714b8 --- /dev/null +++ b/tests/js/client/shell/shell-promtool-cluster.js @@ -0,0 +1,202 @@ +/* jshint globalstrict:false, strict:false, maxlen: 200 */ +/* global assertTrue, assertEqual, fail, arango */ + +//////////////////////////////////////////////////////////////////////////////// +/// DISCLAIMER +/// +/// Copyright 2014-2021 ArangoDB GmbH, Cologne, Germany +/// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany +/// +/// 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. +/// +/// Copyright holder is ArangoDB GmbH, Cologne, Germany +/// +/// @author Jan Steemann +//////////////////////////////////////////////////////////////////////////////// + +const jsunity = require('jsunity'); +const internal = require('internal'); +const fs = require('fs'); +const pu = require('@arangodb/testutils/process-utils'); +const console = require('console'); +const request = require("@arangodb/request"); +const expect = require('chai').expect; +const errors = require('@arangodb').errors; +const suspendExternal = internal.suspendExternal; +const continueExternal = internal.continueExternal; + +// name of environment variable +const PATH = 'PROMTOOL_PATH'; + +// detect the path to promtool +let promtoolPath = internal.env[PATH]; +if (!promtoolPath) { + promtoolPath = '.'; +} +promtoolPath = fs.join(promtoolPath, 'promtool' + pu.executableExt); + +const metricsUrlPath = "/_admin/metrics/v2"; +const serverIdPath = "/_admin/server/id"; + +function validateMetrics(metrics) { + let toRemove = []; + try { + // store output of /_admin/metrics/v2 into a temp file + let input = fs.getTempFile(); + fs.writeFileSync(input, metrics); + toRemove.push(input); + + // this is where we will capture the output from promtool + let output = fs.getTempFile(); + toRemove.push(output); + + // build command string. this will be unsafe if input/output + // contain quotation marks and such. but as these parameters are + // under our control this is very unlikely + let command = promtoolPath + ' check metrics < "' + input + '" > "' + output + '" 2>&1'; + // pipe contents of temp file into promtool + let actualRc = internal.executeExternalAndWait('sh', ['-c', command]); + assertTrue(actualRc.hasOwnProperty('exit')); + assertEqual(0, actualRc.exit); + + let promtoolResult = fs.readFileSync(output).toString(); + // no errors found means an empty result file + assertEqual('', promtoolResult); + } finally { + // remove temp files + toRemove.forEach((f) => { + fs.remove(f); + }); + } +} + +function validateMetricsOnServer(server) { + print("Querying server ", server.name); + let res = request.get({ + url: server.url + metricsUrlPath + }); + expect(res).to.be.an.instanceof(request.Response); + expect(res).to.have.property('statusCode', 200); + let body = String(res.body); + validateMetrics(body); +} + +function getServerId(server) { + let res = request.get({ + url: server.url + serverIdPath + }); + return res.json.id; +} + +function validateMetricsViaCoordinator(coordinator, server) { + let serverId = getServerId(server); + let metricsUrl = coordinator.url + metricsUrlPath + "?serverId=" + serverId; + let res = request.get({ url: metricsUrl }); + expect(res).to.be.an.instanceof(request.Response); + expect(res).to.have.property('statusCode', 200); + let body = String(res.body); + validateMetrics(body); +} + +function promtoolClusterSuite() { + 'use strict'; + + if (!internal.env.hasOwnProperty('INSTANCEINFO')) { + throw new Error('env.INSTANCEINFO was not set by caller!'); + } + const instanceinfo = JSON.parse(internal.env.INSTANCEINFO); + let dbServers = instanceinfo.arangods.filter(arangod => arangod.role === "dbserver"); + let agents = instanceinfo.arangods.filter(arangod => arangod.role === "agent"); + let coordinators = instanceinfo.arangods.filter(arangod => arangod.role === "coordinator"); + + return { + testMetricsOnDbServers: function () { + print("Validating metrics from db servers..."); + for (let i = 0; i < dbServers.length; i++) { + validateMetricsOnServer(dbServers[i]); + } + }, + testMetricsOnAgents: function () { + print("Validating metrics from agents..."); + for (let i = 0; i < agents.length; i++) { + validateMetricsOnServer(agents[i]); + } + }, + testMetricsOnCoordinators: function () { + print("Validating metrics coordinators..."); + for (let i = 0; i < coordinators.length; i++) { + validateMetricsOnServer(coordinators[i]); + } + }, + testMetricsViaCoordinator: function () { + //exclude agents, because agents cannot be queried via coordinator + let servers = dbServers.concat(coordinators); + for (let i = 0; i < servers.length; i++) { + validateMetricsViaCoordinator(coordinators[0], servers[i]); + } + }, + testInvalidServerId: function () { + //query metrics from coordinator, supplying invalid server id + let coordinator = coordinators[0]; + let metricsUrl = coordinator.url + metricsUrlPath + "?serverId=" + "invalid-server-id"; + let res = request.get({ url: metricsUrl }); + expect(res).to.be.an.instanceof(request.Response); + expect(res).to.have.property('statusCode', 404); + expect(res.json.errorNum).to.equal(errors.ERROR_HTTP_BAD_PARAMETER.code); + }, + testServerDoesntResponse: function () { + //query metrics from coordinator, supplying id of a server, that is shut down + let dbServer = dbServers[0]; + let serverId = getServerId(dbServer); + assertTrue(suspendExternal(dbServer.pid)); + dbServer.suspended = true; + try { + let metricsUrl = metricsUrlPath + "?serverId=" + serverId; + let res = arango.GET_RAW(metricsUrl); + assertEqual(503, res.code); + //Do not validate response body because errorNum and errorMessage can differ depending on whether there was an open connection + //between coordinator and db server when the later stopped responding. This cannot be reproduced consistently in the test. + // let body = res.parsedBody; + // expect(body.errorNum).to.equal(errors.ERROR_CLUSTER_CONNECTION_LOST.code); + } finally { + assertTrue(continueExternal(dbServer.pid)); + delete dbServer.suspended; + } + } + }; +} + +if (internal.platform === 'linux') { + // this test intentionally only executes on Linux, and only if PROMTOOL_PATH + // is set to the path containing the `promtool` executable. if the PROMTOOL_PATH + // is set, but the executable cannot be found, the test will error out. + // the test also requires `sh` to be a shell that supports input/output redirection, + // and `true` to be an executable that returns exit code 0 (we use sh -c true` as a + // test to check the shell functionality). + if (fs.exists(promtoolPath)) { + let actualRc = internal.executeExternalAndWait('sh', ['-c', 'true']); + if (actualRc.hasOwnProperty('exit') && actualRc.exit === 0) { + jsunity.run(promtoolClusterSuite); + } else { + console.warn('skipping test because no working sh can be found'); + } + } else if (!internal.env.hasOwnProperty(PATH)) { + console.warn('skipping test because promtool is not found. you can set ' + PATH + ' accordingly'); + } else { + fail('promtool not found in PROMTOOL_PATH (' + internal.env[PATH] + ')'); + } +} else { + console.warn('skipping test because we are not on Linux'); +} + +return jsunity.done(); \ No newline at end of file From 9cc5e815c2df9f22605e15eea056b178c2da5850 Mon Sep 17 00:00:00 2001 From: Vitaly Pryakhin Date: Fri, 2 Jul 2021 16:08:11 +0300 Subject: [PATCH 2/4] check that metrics from desired server are returned when requesting via coordinator use 2 coordinator servers for shell_client test suite ensure that server is recovered before going on with other tests --- js/client/modules/@arangodb/testsuites/aql.js | 13 ++- .../js/client/shell/shell-promtool-cluster.js | 96 +++++++++++++++++-- 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/js/client/modules/@arangodb/testsuites/aql.js b/js/client/modules/@arangodb/testsuites/aql.js index ed8d979c0662..76dfa76d96c8 100644 --- a/js/client/modules/@arangodb/testsuites/aql.js +++ b/js/client/modules/@arangodb/testsuites/aql.js @@ -58,6 +58,16 @@ function ensureServers(options, numServers) { return options; } +/// ensure that we have enough coordinators in cluster tests +function ensureCoordinators(options, numServers) { + if (options.cluster && options.coordinators < numServers) { + let localOptions = _.clone(options); + localOptions.coordinators = numServers; + return localOptions; + } + return options; +} + // ////////////////////////////////////////////////////////////////////////////// // / @brief TEST: shell_client // ////////////////////////////////////////////////////////////////////////////// @@ -67,7 +77,8 @@ function shellClient (options) { testCases = tu.splitBuckets(options, testCases); - let opts = ensureServers(options, 3); + var opts = ensureServers(options, 3); + opts = ensureCoordinators(opts, 2); let rc = tu.performTests(opts, testCases, 'shell_client', tu.runInLocalArangosh); options.cleanup = options.cleanup && opts.cleanup; return rc; diff --git a/tests/js/client/shell/shell-promtool-cluster.js b/tests/js/client/shell/shell-promtool-cluster.js index 7fc7dab714b8..0b86ab1c210d 100644 --- a/tests/js/client/shell/shell-promtool-cluster.js +++ b/tests/js/client/shell/shell-promtool-cluster.js @@ -1,5 +1,5 @@ /* jshint globalstrict:false, strict:false, maxlen: 200 */ -/* global assertTrue, assertEqual, fail, arango */ +/* global print, assertNotEqual, assertFalse, assertTrue, assertEqual, fail, arango */ //////////////////////////////////////////////////////////////////////////////// /// DISCLAIMER @@ -47,6 +47,61 @@ promtoolPath = fs.join(promtoolPath, 'promtool' + pu.executableExt); const metricsUrlPath = "/_admin/metrics/v2"; const serverIdPath = "/_admin/server/id"; +const healthUrl = "_admin/cluster/health"; + +function getServerId(server) { + let res = request.get({ + url: server.url + serverIdPath + }); + return res.json.id; +} + +function getServerShortName(server) { + let serverId = getServerId(server); + let clusterHealth = arango.GET_RAW(healthUrl).parsedBody.Health; + let shortName = Object.keys(clusterHealth) + .filter(x => { + return x === serverId; + }) + .map(x => clusterHealth[x]["ShortName"])[0]; + return shortName; +} + +function checkThatServerIsResponsive(server) { + try { + let serverName = getServerShortName(server); + print("Checking if server " + serverName + " is responsive."); + let res = request.get({ + url: server.url + metricsUrlPath + }); + if (res.body.includes(serverName) && res.statusCode === 200) { + print("Server " + serverName + " is OK!"); + return true; + } else { + print("Server " + serverName + " doesn't respond properly to reauests."); + return false; + } + } catch(error){ + return false; + } +} + +function checkThatAllDbServersAreHealthy() { + print("Checking that all DB servers are healthy.") + let clusterHealth = arango.GET_RAW(healthUrl).parsedBody.Health; + let dbServers = Object.keys(clusterHealth) + .filter(x => { + return clusterHealth[x]["Role"] === "DBServer"; + }) + .map(x => clusterHealth[x]); + for(let i = 0; i < dbServers.length; i++) { + print("Server "+ dbServers[i].ShortName + " status is " + dbServers[i]["Status"]) + if(!(dbServers[i]["Status"] === "GOOD")){ + return false; + } + } + return true; +} function validateMetrics(metrics) { let toRemove = []; @@ -80,6 +135,17 @@ function validateMetrics(metrics) { } } +function checkMetricsBelongToServer(metrics, server) { + let serverName = getServerShortName(server); + let positiveRegex = new RegExp('(shortname="(' + serverName + ').*")'); + let negativeRegex = new RegExp('(shortname="(?!' + serverName + ').*")'); + let matchesServerName = metrics.match(positiveRegex); + let matchesAnyOtherName = metrics.match(negativeRegex); + assertNotEqual(null, matchesServerName, "Metrics must contain server name, but they don't"); + assertTrue(matchesServerName.length > 0, "Metrics must contain server name, but they don't"); + assertEqual(null, matchesAnyOtherName, "Metrics must NOT contain other servers' names."); +} + function validateMetricsOnServer(server) { print("Querying server ", server.name); let res = request.get({ @@ -91,13 +157,6 @@ function validateMetricsOnServer(server) { validateMetrics(body); } -function getServerId(server) { - let res = request.get({ - url: server.url + serverIdPath - }); - return res.json.id; -} - function validateMetricsViaCoordinator(coordinator, server) { let serverId = getServerId(server); let metricsUrl = coordinator.url + metricsUrlPath + "?serverId=" + serverId; @@ -106,6 +165,7 @@ function validateMetricsViaCoordinator(coordinator, server) { expect(res).to.have.property('statusCode', 200); let body = String(res.body); validateMetrics(body); + checkMetricsBelongToServer(body, server); } function promtoolClusterSuite() { @@ -133,7 +193,7 @@ function promtoolClusterSuite() { } }, testMetricsOnCoordinators: function () { - print("Validating metrics coordinators..."); + print("Validating metrics from coordinators..."); for (let i = 0; i < coordinators.length; i++) { validateMetricsOnServer(coordinators[i]); } @@ -169,8 +229,24 @@ function promtoolClusterSuite() { // let body = res.parsedBody; // expect(body.errorNum).to.equal(errors.ERROR_CLUSTER_CONNECTION_LOST.code); } finally { + let clusterHealthOk = false; assertTrue(continueExternal(dbServer.pid)); - delete dbServer.suspended; + for (let i = 0; i < 60; i++) { + if (checkThatServerIsResponsive(dbServer)) { + delete dbServer.suspended; + break; + } + internal.sleep(1); + } + for (let i = 0; i < 60; i++) { + if (checkThatAllDbServersAreHealthy()) { + clusterHealthOk = true; + break; + } + internal.sleep(1); + } + assertFalse(dbServer.suspended, "Couldn't recover server after suspension."); + assertTrue(clusterHealthOk, "Some db servers are not ready according to " + healthUrl); } } }; From f826c8a56cba851cf182681517f87c511363bed9 Mon Sep 17 00:00:00 2001 From: Vadim Kondratyev Date: Sat, 31 Jul 2021 00:47:08 +0200 Subject: [PATCH 3/4] Fix jslint --- tests/js/client/shell/shell-promtool-cluster.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/js/client/shell/shell-promtool-cluster.js b/tests/js/client/shell/shell-promtool-cluster.js index 0b86ab1c210d..98e04dd1860f 100644 --- a/tests/js/client/shell/shell-promtool-cluster.js +++ b/tests/js/client/shell/shell-promtool-cluster.js @@ -87,7 +87,7 @@ function checkThatServerIsResponsive(server) { } function checkThatAllDbServersAreHealthy() { - print("Checking that all DB servers are healthy.") + print("Checking that all DB servers are healthy."); let clusterHealth = arango.GET_RAW(healthUrl).parsedBody.Health; let dbServers = Object.keys(clusterHealth) .filter(x => { @@ -95,7 +95,7 @@ function checkThatAllDbServersAreHealthy() { }) .map(x => clusterHealth[x]); for(let i = 0; i < dbServers.length; i++) { - print("Server "+ dbServers[i].ShortName + " status is " + dbServers[i]["Status"]) + print("Server "+ dbServers[i].ShortName + " status is " + dbServers[i]["Status"]); if(!(dbServers[i]["Status"] === "GOOD")){ return false; } @@ -275,4 +275,4 @@ if (internal.platform === 'linux') { console.warn('skipping test because we are not on Linux'); } -return jsunity.done(); \ No newline at end of file +return jsunity.done(); From f8ae8a1000d242968cf152fa509a06b41ebf7eb5 Mon Sep 17 00:00:00 2001 From: Vadim Date: Sat, 31 Jul 2021 23:54:09 +0300 Subject: [PATCH 4/4] Update shell-promtool-cluster.js --- tests/js/client/shell/shell-promtool-cluster.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/js/client/shell/shell-promtool-cluster.js b/tests/js/client/shell/shell-promtool-cluster.js index 98e04dd1860f..dac6670b1e5a 100644 --- a/tests/js/client/shell/shell-promtool-cluster.js +++ b/tests/js/client/shell/shell-promtool-cluster.js @@ -78,7 +78,7 @@ function checkThatServerIsResponsive(server) { print("Server " + serverName + " is OK!"); return true; } else { - print("Server " + serverName + " doesn't respond properly to reauests."); + print("Server " + serverName + " doesn't respond properly to requests."); return false; } } catch(error){