Skip to content

Commit 4124dc5

Browse files
committed
Merge branch 'improvement/CLDSRV-740-healthcheck-S3C' into q/9.0
2 parents a615ab4 + 8dbaa91 commit 4124dc5

File tree

11 files changed

+63
-28
lines changed

11 files changed

+63
-28
lines changed

.github/docker/config.s3c.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
"clusters": 5,
2323
"kmsHideScalityArn": true,
2424
"healthChecks": {
25-
"allowFrom": ["127.0.0.1", "::1"]
25+
"allowFrom": ["127.0.0.1", "::1"],
26+
"enableInternalRoute": true
2627
},
2728
"localCache": {
2829
"host": "localhost",

.github/workflows/tests.yaml

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -532,48 +532,38 @@ jobs:
532532
- name: Setup vault credentials like S3C Integration
533533
run: ./setup-s3c.sh
534534
working-directory: .github/docker
535+
- name: Set config files env variables
536+
run: |-
537+
echo "S3_CONFIG_FILE=${{ github.workspace }}/.github/docker/config.s3c.json" >> $GITHUB_ENV
538+
echo "S3_LOCATION_FILE=${{ github.workspace }}/tests/locationConfig/locationConfigS3C.json" >> $GITHUB_ENV
535539
- name: Run cloudserver-object tests
536-
env:
537-
S3_CONFIG_FILE: ${{ github.workspace }}/.github/docker/config.s3c.json
538-
S3_LOCATION_FILE: ${{ github.workspace }}/tests/locationConfig/locationConfigS3C.json
539540
run: |-
540541
set -o pipefail;
541542
yarn run ft_awssdk_objects_misc | tee /tmp/artifacts/${{ matrix.job-name }}/ft_awssdk_objects_misc.log
542543
- name: Run cloudserver-version tests
543-
env:
544-
S3_CONFIG_FILE: ${{ github.workspace }}/.github/docker/config.s3c.json
545-
S3_LOCATION_FILE: ${{ github.workspace }}/tests/locationConfig/locationConfigS3C.json
546544
run: |-
547545
set -o pipefail;
548546
yarn run ft_awssdk_versioning | tee /tmp/artifacts/${{ matrix.job-name }}/ft_awssdk_versioning.log
549547
- name: Run cloudserver-bucket tests
550-
env:
551-
S3_CONFIG_FILE: ${{ github.workspace }}/.github/docker/config.s3c.json
552-
S3_LOCATION_FILE: ${{ github.workspace }}/tests/locationConfig/locationConfigS3C.json
553548
run: |-
554549
set -o pipefail;
555550
yarn run ft_awssdk_buckets | tee /tmp/artifacts/${{ matrix.job-name }}/ft_awssdk_buckets.log
556551
- name: Run cloudserver-routes (metadata) tests
557-
env:
558-
S3_CONFIG_FILE: ${{ github.workspace }}/.github/docker/config.s3c.json
559-
S3_LOCATION_FILE: ${{ github.workspace }}/tests/locationConfig/locationConfigS3C.json
560552
run: |-
561553
set -o pipefail;
562554
yarn run ft_node_routes | tee /tmp/artifacts/${{ matrix.job-name }}/ft_node_routes.log
563555
- name: Run backbeat route tests
564-
env:
565-
S3_CONFIG_FILE: ${{ github.workspace }}/.github/docker/config.s3c.json
566-
S3_LOCATION_FILE: ${{ github.workspace }}/tests/locationConfig/locationConfigS3C.json
567556
run: |-
568557
set -o pipefail;
569558
yarn run ft_route_backbeat | tee /tmp/artifacts/${{ matrix.job-name }}/ft_route_backbeat.log
570559
- name: Run backbeat tests
571-
env:
572-
S3_CONFIG_FILE: ${{ github.workspace }}/.github/docker/config.s3c.json
573-
S3_LOCATION_FILE: ${{ github.workspace }}/tests/locationConfig/locationConfigS3C.json
574560
run: |-
575561
set -o pipefail;
576562
yarn run ft_backbeat | tee /tmp/artifacts/${{ matrix.job-name }}/ft_backbeat.log
563+
- name: Run healthchecks tests
564+
run: |-
565+
set -o pipefail;
566+
yarn run ft_healthchecks | tee /tmp/artifacts/${{ matrix.job-name }}/ft_healthchecks.log
577567
- name: Teardown CI services
578568
run: docker compose down redis sproxyd metadata-standalone vault-sse-before-migration cloudserver-sse-before-migration
579569
working-directory: .github/docker

lib/Config.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,6 +1548,11 @@ class Config extends EventEmitter {
15481548
this.healthChecks.allowFrom = defaultHealthChecks.allowFrom
15491549
.concat(config.healthChecks.allowFrom);
15501550
}
1551+
/**
1552+
* CLDSRV-740: S3C with nginx s3-frontend needs the healthcheck on the
1553+
* same port as s3 to allow nginx upstream failover based on healthcheck
1554+
*/
1555+
this.healthChecks.enableInternalRoute = config.healthChecks?.enableInternalRoute || false;
15511556

15521557
if (config.certFilePaths) {
15531558
assert(typeof config.certFilePaths === 'object' &&

lib/server.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,10 @@ class S3Server {
209209

210210
switch (req.url) {
211211
case '/live':
212+
healthcheckHandler(clientInfo.clientIP, req, res, log, statsClient, false);
213+
break;
212214
case '/ready':
213-
healthcheckHandler(clientInfo.clientIP, req, res, log, statsClient);
215+
healthcheckHandler(clientInfo.clientIP, req, res, log, statsClient, true);
214216
break;
215217

216218
default:

lib/utilities/healthcheckHandler.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,10 @@ function checkIP(clientIP) {
109109
* @param {object} res - http response object
110110
* @param {object} log - werelogs logger instance
111111
* @param {object} statsClient - StatsClient Instance
112+
* @param {boolean} deep - whether the healthcheck is a deep healthcheck
112113
* @return {undefined}
113114
*/
114-
function healthcheckHandler(clientIP, req, res, log, statsClient) {
115+
function healthcheckHandler(clientIP, req, res, log, statsClient, deep) {
115116
function healthcheckEndHandler(err, results) {
116117
writeResponse(res, err, log, results, error => {
117118
if (error) {
@@ -121,8 +122,6 @@ function healthcheckHandler(clientIP, req, res, log, statsClient) {
121122
});
122123
}
123124

124-
const deep = (req.url === '/ready');
125-
126125
// Attach the apiMethod method to the request, so it can used by monitoring in the server
127126
// eslint-disable-next-line no-param-reassign
128127
req.apiMethod = deep ? 'deepHealthcheck' : 'healthcheck';

lib/utilities/internalHandlers.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ const routeWorkflowEngineOperator =
44
require('../routes/routeWorkflowEngineOperator');
55
const { reportHandler } = require('./reportHandler');
66
const routeVeeam = require('../routes/routeVeeam').routeVeeam;
7+
const { healthcheckHandler } = require('./healthcheckHandler');
8+
const { config } = require('../Config');
79

810
const internalHandlers = {
911
backbeat: routeBackbeat,
@@ -13,6 +15,14 @@ const internalHandlers = {
1315
veeam: routeVeeam,
1416
};
1517

18+
if (config.healthChecks.enableInternalRoute) {
19+
// CLDSRV-740: S3C with the healthcheck on s3 port needs the internal route
20+
internalHandlers.healthcheck = function s3InternalHealthcheckHandler(clientIP, req, res, log, statsClient) {
21+
const deep = req.url === '/_/healthcheck/deep';
22+
return healthcheckHandler(clientIP, req, res, log, statsClient, deep);
23+
};
24+
}
25+
1626
module.exports = {
1727
internalHandlers,
1828
};

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@zenko/cloudserver",
3-
"version": "9.0.25",
3+
"version": "9.0.26",
44
"description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
55
"main": "index.js",
66
"engines": {

tests/functional/healthchecks/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"version": "0.1.0",
44
"description": "Test-healthcheck",
55
"main": "tests.js",
6+
"private": true,
67
"repository": "",
78
"keywords": [
89
"test"

tests/functional/healthchecks/test/checkRoutes.js

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ const async = require('async');
88
const Redis = require('ioredis');
99

1010
const conf = require('../../config.json');
11+
const { config } = require('../../../../lib/Config');
1112

1213
const redis = new Redis({
13-
host: conf.localCache.host,
14-
port: conf.localCache.port,
14+
host: config.localCache.host,
15+
port: config.localCache.port,
1516
// disable offline queue
1617
enableOfflineQueue: false,
1718
});
@@ -23,7 +24,7 @@ const transport = transportStr === 'http' ? http : https;
2324
const options = {
2425
host: conf.ipAddress,
2526
path: '/live',
26-
port: 8002,
27+
port: config.metricsPort || 8002,
2728
};
2829

2930
function checkResult(expectedStatus, res) {
@@ -55,7 +56,7 @@ function makeDummyS3Request(cb) {
5556
const getOptions = deepCopy(options);
5657
getOptions.path = '/foo/bar';
5758
getOptions.method = 'GET';
58-
getOptions.port = 8000;
59+
getOptions.port = config.port || 8000;
5960
getOptions.agent = makeAgent();
6061
const req = transport.request(getOptions);
6162
req.end(() => cb());
@@ -106,6 +107,30 @@ describe('Healthcheck routes', () => {
106107
});
107108
});
108109

110+
if (config.healthChecks.enableInternalRoute) {
111+
describe('Healthcheck s3 port internal routes', () => {
112+
it('should return 200 OK on GET request', done => {
113+
const getOptions = deepCopy(options);
114+
getOptions.method = 'GET';
115+
getOptions.path = '/_/healthcheck';
116+
getOptions.port = config.port || 8000;
117+
getOptions.agent = makeAgent();
118+
const req = transport.request(getOptions, makeChecker(200, done));
119+
req.end();
120+
});
121+
122+
it('should return 200 on deep GET request', done => {
123+
const deepOptions = deepCopy(options);
124+
deepOptions.method = 'GET';
125+
deepOptions.path = '/_/healthcheck/deep';
126+
deepOptions.port = config.port || 8000;
127+
deepOptions.agent = makeAgent();
128+
const req = transport.request(deepOptions, makeChecker(200, done));
129+
req.end();
130+
});
131+
});
132+
}
133+
109134
describe('Healthcheck stats', () => {
110135
const totalReqs = 5;
111136
beforeEach(done => {

tests/functional/raw-node/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"version": "0.1.0",
44
"description": "Test-rawnode",
55
"main": "tests.js",
6+
"private": true,
67
"repository": "",
78
"keywords": [
89
"test"

0 commit comments

Comments
 (0)