Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions packages/kbn-lock-manager/src/lock_manager_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { errors } from '@elastic/elasticsearch';
import { Logger } from '@kbn/logging';
import { v4 as uuid } from 'uuid';
import prettyMilliseconds from 'pretty-ms';
import { once } from 'lodash';
import { duration } from 'moment';
import { ElasticsearchClient } from '@kbn/core/server';
import { LOCKS_CONCRETE_INDEX_NAME, setupLockManagerIndex } from './setup_lock_manager_index';
Expand All @@ -39,10 +38,21 @@ export interface AcquireOptions {
}

// The index assets should only be set up once
let runLockManagerSetupSuccessfully = false;
export const runSetupIndexAssetOnce = async (
esClient: ElasticsearchClient,
logger: Logger
): Promise<void> => {
if (runLockManagerSetupSuccessfully) {
return;
}
await setupLockManagerIndex(esClient, logger);
runLockManagerSetupSuccessfully = true;
};

// For testing purposes, we need to be able to set it up every time
let runSetupIndexAssetOnce = once(setupLockManagerIndex);
export function runSetupIndexAssetEveryTime() {
runSetupIndexAssetOnce = setupLockManagerIndex;
export function rerunSetupIndexAsset() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Observation] Do you think there are any risks of exposing this functionality to downstream dependencies of the lock manager? Guess it was here before, which was going to cache the failures, but now that it's passing through, is this exposing the "reset" functionality downstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't change anything, it's just the new version of runSetIndexAssetEveryTime, but with a new name.

runLockManagerSetupSuccessfully = false;
}

export class LockManager {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
LockManager,
LockDocument,
withLock,
runSetupIndexAssetEveryTime,
rerunSetupIndexAsset,
} from '@kbn/lock-manager/src/lock_manager_client';
import {
LOCKS_COMPONENT_TEMPLATE_NAME,
Expand All @@ -42,9 +42,11 @@ export default function ApiTest({ getService }: DeploymentAgnosticFtrProviderCon
before(async () => {
// delete existing index mappings to ensure we start from a clean state
await deleteLockIndexAssets(es, log);
});

beforeEach(async () => {
// ensure that the index and templates are created
runSetupIndexAssetEveryTime();
rerunSetupIndexAsset();
});

after(async () => {
Expand Down Expand Up @@ -804,6 +806,38 @@ export default function ApiTest({ getService }: DeploymentAgnosticFtrProviderCon
});
});
});

describe('setup robustness', () => {
it('should retry if setup fails the first time', async () => {
const brokenEsClient = {
...es,
cluster: {
...es.cluster,
putComponentTemplate: () => {
throw new Error('Simulated failure on first attempt');
},
},
} as unknown as Client;
const brokenLockManager = new LockManager('test', brokenEsClient, logger);
const workingLockManager = new LockManager('test', es, logger);
let error;
try {
await brokenLockManager.acquire();
} catch (e) {
error = e;
}
expect(error).to.be.an(Error);

// if the the second attempt succeeds, it means it didn't get stuck on the first failure
error = undefined;
try {
await workingLockManager.acquire();
} catch (e) {
error = e;
}
expect(error).to.be(undefined);
});
});
});
}

Expand Down