Skip to content

Commit 32a33d3

Browse files
author
Kerkesni
committed
Merge remote-tracking branch 'origin/bugfix/CLDSRV-665' into w/9.0/bugfix/CLDSRV-665
2 parents 86f9fe7 + 17cab88 commit 32a33d3

5 files changed

Lines changed: 114 additions & 0 deletions

File tree

.github/docker/docker-compose.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ services:
6060
- SCUBA_HEALTHCHECK_FREQUENCY
6161
- S3QUOTA
6262
- QUOTA_ENABLE_INFLIGHTS
63+
- S3_VERSION_ID_ENCODING_TYPE
6364
env_file:
6465
- creds.env
6566
depends_on:

.github/workflows/tests.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ jobs:
344344
S3METADATA: mongodb
345345
S3KMS: file
346346
S3_LOCATION_FILE: /usr/src/app/tests/locationConfig/locationConfigTests.json
347+
S3_VERSION_ID_ENCODING_TYPE: base62
347348
DEFAULT_BUCKET_KEY_FORMAT: v1
348349
METADATA_MAX_CACHED_BUCKETS: 1
349350
MONGODB_IMAGE: ghcr.io/${{ github.repository }}/ci-mongodb:${{ github.sha }}

lib/Config.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,38 @@ class Config extends EventEmitter {
910910
this.replicationGroupId = 'RG001';
911911
}
912912

913+
const instanceId = process.env.CLOUDSERVER_INSTANCE_ID || config.instanceId;
914+
if (instanceId) {
915+
assert(typeof instanceId === 'string',
916+
'bad config: instanceId must be a string');
917+
// versionID generation code will truncate instanceId to 6 characters
918+
// so we enforce this limit here to make the behavior predictable
919+
assert(instanceId.length <= 6,
920+
'bad config: instanceId must be at most 6 characters long');
921+
this.instanceId = instanceId;
922+
} else if (process.env.HOSTNAME) {
923+
// If running in Kubernetes, the pod name is set in the HOSTNAME env var
924+
// and is of the form: cloudserver-7869f99955-nr984
925+
// where:
926+
// - cloudserver is the name of the pod
927+
// - 7869f99955 is a long hash that guarantees uniqueness within the namespace
928+
// - nr984 is a short hash that is unique only within the current ReplicaSet/Deployment
929+
// We use the last part of the pod name as the instanceId, prefixed with 'i' for internal
930+
// or 'e' for external cloudserver.
931+
// This allows having a unique identifier that will be used when generating versionIds
932+
const instanceType = process.env.HOSTNAME.includes('internal') ? 'i' : 'e';
933+
const parts = process.env.HOSTNAME.split('-');
934+
if (parts.length >= 2) {
935+
const shortId = parts[parts.length - 1];
936+
this.instanceId = instanceType + shortId;
937+
} else {
938+
// fallback: generate a random string if format is unexpected
939+
this.instanceId = uuidv4().replace(/-/g, '').slice(0, 6);
940+
}
941+
} else {
942+
this.instanceId = uuidv4().replace(/-/g, '').slice(0, 6);
943+
}
944+
913945
this.replicationEndpoints = [];
914946
if (config.replicationEndpoints) {
915947
const { replicationEndpoints } = config;

lib/metadata/wrapper.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ if (clientName === 'mem') {
3030
params = {
3131
mongodb: config.mongodb,
3232
replicationGroupId: config.replicationGroupId,
33+
instanceId: config.instanceId,
3334
config,
3435
};
3536
} else if (clientName === 'cdmi') {

tests/unit/Config.js

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const assert = require('assert');
22
const fs = require('fs');
33
const sinon = require('sinon');
4+
const path = require('path');
45
const {
56
azureGetStorageAccountName,
67
azureGetLocationCredentials,
@@ -810,4 +811,82 @@ describe('Config', () => {
810811
assert.deepEqual(config.internalListenOn, [{ ip: '127.0.0.1', port: 9000 }]);
811812
});
812813
});
814+
815+
describe('instanceId', () => {
816+
let defaultConfig;
817+
818+
before(() => {
819+
setEnv('S3_CONFIG_FILE', 'tests/unit/testConfigs/allOptsConfig/config.json');
820+
// Import config
821+
const defaultConfigPath = path.join(__dirname, './testConfigs/allOptsConfig/config.json');
822+
defaultConfig = require(defaultConfigPath);
823+
});
824+
825+
afterEach(() => {
826+
sinon.restore();
827+
});
828+
829+
it('should use the instance id set in the config', () => {
830+
// Stub fs.readFileSync to return a specific config.json content that includes instanceId
831+
const originalReadFileSync = fs.readFileSync;
832+
const readFileSyncStub = sinon.stub(fs, 'readFileSync');
833+
// Mock only config.json file
834+
readFileSyncStub
835+
.withArgs(sinon.match(/\/config\.json$/))
836+
.returns(JSON.stringify({ ...defaultConfig, instanceId: 'test' }));
837+
// For all other files, use the original readFileSync
838+
readFileSyncStub
839+
.callsFake((filePath, ...args) => originalReadFileSync(filePath, ...args));
840+
// Create a new ConfigObject instance
841+
const config = new ConfigObject();
842+
assert.strictEqual(config.instanceId, 'test');
843+
});
844+
845+
it('should assert if instanceId is not a string', () => {
846+
// Stub fs.readFileSync to return a specific config.json content that includes instanceId
847+
const originalReadFileSync = fs.readFileSync;
848+
const readFileSyncStub = sinon.stub(fs, 'readFileSync');
849+
// Mock only config.json file
850+
readFileSyncStub
851+
.withArgs(sinon.match(/\/config\.json$/))
852+
.returns(JSON.stringify({ ...defaultConfig, instanceId: 1234 }));
853+
// For all other files, use the original readFileSync
854+
readFileSyncStub
855+
.callsFake((filePath, ...args) => originalReadFileSync(filePath, ...args));
856+
// Create a new ConfigObject instance
857+
assert.throws(() => new ConfigObject());
858+
});
859+
860+
it('should use the instance id set in the env var', () => {
861+
setEnv('CLOUDSERVER_INSTANCE_ID', '123456');
862+
const config = new ConfigObject();
863+
assert.strictEqual(config.instanceId, '123456');
864+
});
865+
866+
it('should assert if instanceId is longer than 6 characters', () => {
867+
setEnv('CLOUDSERVER_INSTANCE_ID', '1234567');
868+
assert.throws(() => new ConfigObject());
869+
});
870+
871+
it('should generate a new instanceId for external cloudserver', () => {
872+
setEnv('HOSTNAME', 'connector-cloudserver-69958b4697-bs5pn');
873+
const config = new ConfigObject();
874+
assert.strictEqual(config.instanceId, 'ebs5pn');
875+
});
876+
877+
it('should generate a new instanceId for internal cloudserver', () => {
878+
setEnv('HOSTNAME', 'internal-cloudserver-54dc76b796-48m2x');
879+
const config = new ConfigObject();
880+
assert.strictEqual(config.instanceId, 'i48m2x');
881+
});
882+
it('should generate a random instanceId if HOSTNAME has an unexpected format', () => {
883+
setEnv('HOSTNAME', 'cldsrv1');
884+
const config = new ConfigObject();
885+
assert.strictEqual(config.instanceId.length, 6);
886+
});
887+
it('should generate a random instanceId if HOSTNAME is not set', () => {
888+
const config = new ConfigObject();
889+
assert.strictEqual(config.instanceId.length, 6);
890+
});
891+
});
813892
});

0 commit comments

Comments
 (0)