Skip to content

Commit 6d56f1a

Browse files
opensearch-trigger-bot[bot]github-actions[bot]opensearch-changeset-bot[bot]
authored
[Dynamic Config] Fix bug when attempting dynamic config index creation (#8184) (#8193)
* Fix bug when attempting dynamic config index creation * Changeset file for PR #8184 created/updated --------- (cherry picked from commit 678c644) Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
1 parent 2fc39e8 commit 6d56f1a

File tree

5 files changed

+327
-24
lines changed

5 files changed

+327
-24
lines changed

‎changelogs/fragments/8184.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
fix:
2+
- Fix bug when dynamic config index and alias are checked ([#8184](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8184))

‎src/core/server/config/service/config_store_client/opensearch_config_store.test.ts

Lines changed: 146 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,16 @@
55

66
import { SearchResponse } from '../../../opensearch';
77
import { opensearchClientMock } from '../../../opensearch/client/mocks';
8-
import { DYNAMIC_APP_CONFIG_ALIAS } from '../../utils/constants';
8+
import { DYNAMIC_APP_CONFIG_ALIAS, DYNAMIC_APP_CONFIG_INDEX_PREFIX } from '../../utils/constants';
99
import { OpenSearchConfigStoreClient } from './opensearch_config_store_client';
1010
import { ConfigDocument } from './types';
1111
import _ from 'lodash';
1212
import { ConfigBlob } from '../../types';
13-
import { BulkOperationContainer } from '@opensearch-project/opensearch/api/types';
13+
import {
14+
BulkOperationContainer,
15+
CatIndicesResponse,
16+
IndicesGetAliasResponse,
17+
} from '@opensearch-project/opensearch/api/types';
1418
import { getDynamicConfigIndexName } from '../../utils/utils';
1519

1620
describe('OpenSearchConfigStoreClient', () => {
@@ -32,10 +36,12 @@ describe('OpenSearchConfigStoreClient', () => {
3236
isListConfig: boolean;
3337
configDocuments: ConfigDocument[];
3438
existsAliasResult: boolean;
39+
getAliasIndicesResult: IndicesGetAliasResponse;
40+
catIndicesResult: CatIndicesResponse;
3541
}
3642

3743
/**
38-
* Creates a new OpenSearch client mock complete with a mock for existsAlias() and search() results
44+
* Creates a new OpenSearch client mock complete with a mock for existsAlias(), cat.indices(), and search() results
3945
*
4046
* @param param0
4147
* @returns
@@ -44,6 +50,8 @@ describe('OpenSearchConfigStoreClient', () => {
4450
isListConfig,
4551
configDocuments,
4652
existsAliasResult,
53+
getAliasIndicesResult,
54+
catIndicesResult,
4755
}: OpenSearchClientMockProps) => {
4856
const mockClient = opensearchClientMock.createOpenSearchClient();
4957

@@ -53,6 +61,18 @@ describe('OpenSearchConfigStoreClient', () => {
5361
})
5462
);
5563

64+
mockClient.cat.indices.mockResolvedValue(
65+
opensearchClientMock.createApiResponse<CatIndicesResponse>({
66+
body: catIndicesResult,
67+
})
68+
);
69+
70+
mockClient.indices.getAlias.mockResolvedValue(
71+
opensearchClientMock.createApiResponse<IndicesGetAliasResponse>({
72+
body: getAliasIndicesResult,
73+
})
74+
);
75+
5676
// @ts-expect-error
5777
mockClient.search.mockImplementation((request, options) => {
5878
// Filters out results when the request is for getting/bulk getting configs
@@ -83,6 +103,49 @@ describe('OpenSearchConfigStoreClient', () => {
83103
return mockClient;
84104
};
85105

106+
const noDynamicConfigIndexResults: CatIndicesResponse = [
107+
{
108+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_`,
109+
},
110+
{
111+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_foo`,
112+
},
113+
{
114+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_foo_2`,
115+
},
116+
];
117+
118+
const oneDynamicConfigIndexResult: CatIndicesResponse = [
119+
{
120+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_1`,
121+
},
122+
];
123+
124+
const multipleDynamicConfigIndexResults: CatIndicesResponse = [
125+
{
126+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_2`,
127+
},
128+
{
129+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_4`,
130+
},
131+
{
132+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_800`,
133+
},
134+
];
135+
136+
const validAliasIndicesResponse: IndicesGetAliasResponse = {
137+
[`${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_4`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
138+
};
139+
140+
const multipleAliasIndicesResponse: IndicesGetAliasResponse = {
141+
[`${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_4`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
142+
[`${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_2`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
143+
};
144+
145+
const invalidAliasIndicesResponse: IndicesGetAliasResponse = {
146+
[`.some_random_index_8`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
147+
};
148+
86149
const configDocument: ConfigDocument = {
87150
config_name: 'some_config_name',
88151
config_blob: {
@@ -143,26 +206,88 @@ describe('OpenSearchConfigStoreClient', () => {
143206
it.each([
144207
{
145208
existsAliasResult: false,
209+
catIndicesResult: noDynamicConfigIndexResults,
210+
getAliasIndicesResult: validAliasIndicesResponse,
146211
numCreateCalls: 1,
212+
numUpdateCalls: 0,
213+
errorThrown: false,
214+
},
215+
{
216+
existsAliasResult: false,
217+
catIndicesResult: multipleDynamicConfigIndexResults,
218+
getAliasIndicesResult: validAliasIndicesResponse,
219+
numCreateCalls: 0,
220+
numUpdateCalls: 1,
221+
errorThrown: false,
222+
},
223+
{
224+
existsAliasResult: false,
225+
catIndicesResult: oneDynamicConfigIndexResult,
226+
getAliasIndicesResult: validAliasIndicesResponse,
227+
numCreateCalls: 0,
228+
numUpdateCalls: 1,
229+
errorThrown: false,
147230
},
148231
{
149232
existsAliasResult: true,
233+
catIndicesResult: multipleDynamicConfigIndexResults,
234+
getAliasIndicesResult: {},
150235
numCreateCalls: 0,
236+
numUpdateCalls: 0,
237+
errorThrown: true,
238+
},
239+
{
240+
existsAliasResult: true,
241+
catIndicesResult: multipleDynamicConfigIndexResults,
242+
getAliasIndicesResult: multipleAliasIndicesResponse,
243+
numCreateCalls: 0,
244+
numUpdateCalls: 0,
245+
errorThrown: true,
246+
},
247+
{
248+
existsAliasResult: true,
249+
catIndicesResult: multipleDynamicConfigIndexResults,
250+
getAliasIndicesResult: invalidAliasIndicesResponse,
251+
numCreateCalls: 0,
252+
numUpdateCalls: 0,
253+
errorThrown: true,
254+
},
255+
{
256+
existsAliasResult: true,
257+
catIndicesResult: multipleDynamicConfigIndexResults,
258+
getAliasIndicesResult: validAliasIndicesResponse,
259+
numCreateCalls: 0,
260+
numUpdateCalls: 0,
261+
errorThrown: false,
151262
},
152263
])(
153-
'should create config index $numCreateCalls times when existsAlias() is $existsAliasResult',
154-
async ({ existsAliasResult, numCreateCalls }) => {
264+
'should throw error should be $errorThrown, create() should be called $numCreateCalls times, and update() should be called $numUpdateCalls times',
265+
async ({
266+
existsAliasResult,
267+
catIndicesResult,
268+
getAliasIndicesResult,
269+
numCreateCalls,
270+
numUpdateCalls,
271+
errorThrown,
272+
}) => {
155273
const mockClient = createOpenSearchClientMock({
156274
isListConfig: false,
157275
configDocuments: [],
158276
existsAliasResult,
277+
getAliasIndicesResult,
278+
catIndicesResult,
159279
});
160280
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
161-
await configStoreClient.createDynamicConfigIndex();
281+
282+
if (errorThrown) {
283+
expect(configStoreClient.createDynamicConfigIndex()).rejects.toThrowError();
284+
} else {
285+
await configStoreClient.createDynamicConfigIndex();
286+
}
162287

163288
expect(mockClient.indices.existsAlias).toBeCalled();
164289
expect(mockClient.indices.create).toBeCalledTimes(numCreateCalls);
165-
expect(mockClient.indices.updateAliases).toBeCalledTimes(numCreateCalls);
290+
expect(mockClient.indices.updateAliases).toBeCalledTimes(numUpdateCalls);
166291
}
167292
);
168293
});
@@ -175,6 +300,8 @@ describe('OpenSearchConfigStoreClient', () => {
175300
isListConfig: false,
176301
configDocuments: [configDocument],
177302
existsAliasResult: false,
303+
catIndicesResult: oneDynamicConfigIndexResult,
304+
getAliasIndicesResult: validAliasIndicesResponse,
178305
});
179306
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
180307

@@ -210,6 +337,8 @@ describe('OpenSearchConfigStoreClient', () => {
210337
isListConfig: false,
211338
configDocuments,
212339
existsAliasResult: false,
340+
catIndicesResult: oneDynamicConfigIndexResult,
341+
getAliasIndicesResult: validAliasIndicesResponse,
213342
});
214343
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
215344

@@ -274,6 +403,8 @@ describe('OpenSearchConfigStoreClient', () => {
274403
isListConfig: true,
275404
configDocuments: allConfigDocuments,
276405
existsAliasResult: false,
406+
catIndicesResult: oneDynamicConfigIndexResult,
407+
getAliasIndicesResult: validAliasIndicesResponse,
277408
});
278409
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
279410
const actualMap = await configStoreClient.listConfigs();
@@ -342,6 +473,8 @@ describe('OpenSearchConfigStoreClient', () => {
342473
isListConfig: false,
343474
configDocuments: newConfigDocuments,
344475
existsAliasResult: false,
476+
catIndicesResult: oneDynamicConfigIndexResult,
477+
getAliasIndicesResult: validAliasIndicesResponse,
345478
});
346479
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
347480
await configStoreClient.createConfig({
@@ -540,6 +673,8 @@ describe('OpenSearchConfigStoreClient', () => {
540673
isListConfig: false,
541674
configDocuments: existingConfigs,
542675
existsAliasResult: false,
676+
catIndicesResult: oneDynamicConfigIndexResult,
677+
getAliasIndicesResult: validAliasIndicesResponse,
543678
});
544679
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
545680
await configStoreClient.bulkCreateConfigs({
@@ -565,6 +700,8 @@ describe('OpenSearchConfigStoreClient', () => {
565700
isListConfig: false,
566701
configDocuments: [],
567702
existsAliasResult: false,
703+
catIndicesResult: oneDynamicConfigIndexResult,
704+
getAliasIndicesResult: validAliasIndicesResponse,
568705
});
569706
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
570707
await configStoreClient.deleteConfig({ name: 'some_config_name' });
@@ -604,6 +741,8 @@ describe('OpenSearchConfigStoreClient', () => {
604741
isListConfig: false,
605742
configDocuments: [],
606743
existsAliasResult: false,
744+
catIndicesResult: oneDynamicConfigIndexResult,
745+
getAliasIndicesResult: validAliasIndicesResponse,
607746
});
608747
const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
609748
await configStoreClient.bulkDeleteConfigs({

‎src/core/server/config/service/config_store_client/opensearch_config_store_client.ts

Lines changed: 79 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,15 @@ import {
2020
} from '../../types';
2121
import {
2222
DYNAMIC_APP_CONFIG_ALIAS,
23+
DYNAMIC_APP_CONFIG_INDEX_PREFIX,
2324
DYNAMIC_APP_CONFIG_MAX_RESULT_SIZE,
2425
} from '../../utils/constants';
25-
import { pathToString, getDynamicConfigIndexName } from '../../utils/utils';
26+
import {
27+
pathToString,
28+
getDynamicConfigIndexName,
29+
isDynamicConfigIndex,
30+
extractVersionFromDynamicConfigIndex,
31+
} from '../../utils/utils';
2632
import { ConfigDocument } from './types';
2733

2834
interface ConfigMapEntry {
@@ -48,25 +54,52 @@ export class OpenSearchConfigStoreClient implements IDynamicConfigStoreClient {
4854
* TODO Add migration logic
4955
*/
5056
public async createDynamicConfigIndex() {
51-
const existsResponse = await this.#openSearchClient.indices.existsAlias({
57+
const existsAliasResponse = await this.#openSearchClient.indices.existsAlias({
5258
name: DYNAMIC_APP_CONFIG_ALIAS,
5359
});
54-
if (!existsResponse.body) {
55-
await this.#openSearchClient.indices.create({
56-
index: getDynamicConfigIndexName(1),
57-
});
58-
await this.#openSearchClient.indices.updateAliases({
59-
body: {
60-
actions: [
61-
{
62-
add: {
63-
index: getDynamicConfigIndexName(1),
64-
alias: DYNAMIC_APP_CONFIG_ALIAS,
60+
61+
if (!existsAliasResponse.body) {
62+
const latestVersion = await this.searchLatestConfigIndex();
63+
if (latestVersion < 1) {
64+
await this.#openSearchClient.indices.create({
65+
index: getDynamicConfigIndexName(1),
66+
body: {
67+
aliases: { [DYNAMIC_APP_CONFIG_ALIAS]: {} },
68+
},
69+
});
70+
} else {
71+
await this.#openSearchClient.indices.updateAliases({
72+
body: {
73+
actions: [
74+
{
75+
add: {
76+
index: getDynamicConfigIndexName(latestVersion),
77+
alias: DYNAMIC_APP_CONFIG_ALIAS,
78+
},
6579
},
66-
},
67-
],
68-
},
80+
],
81+
},
82+
});
83+
}
84+
} else {
85+
const results = await this.#openSearchClient.indices.getAlias({
86+
name: DYNAMIC_APP_CONFIG_ALIAS,
6987
});
88+
89+
const indices = Object.keys(results.body);
90+
if (indices.length !== 1) {
91+
throw new Error(
92+
`Alias ${DYNAMIC_APP_CONFIG_ALIAS} is pointing to 0 or multiple indices. Please remove the alias(es) and restart the server`
93+
);
94+
}
95+
const numNonDynamicConfigIndices = indices.filter((index) => !isDynamicConfigIndex(index))
96+
.length;
97+
98+
if (numNonDynamicConfigIndices > 0) {
99+
throw new Error(
100+
`Alias ${DYNAMIC_APP_CONFIG_ALIAS} is pointing to a non dynamic config index. Please remove the alias and restart the server`
101+
);
102+
}
70103
}
71104
}
72105

@@ -342,4 +375,34 @@ export class OpenSearchConfigStoreClient implements IDynamicConfigStoreClient {
342375
},
343376
});
344377
}
378+
379+
/**
380+
* Finds the most updated dynamic config index
381+
*
382+
* @returns the latest version number or 0 if not found
383+
*/
384+
private async searchLatestConfigIndex(): Promise<number> {
385+
const configIndices = await this.#openSearchClient.cat.indices({
386+
index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_*`,
387+
format: 'json',
388+
});
389+
390+
if (configIndices.body.length < 1) {
391+
return 0;
392+
}
393+
394+
const validIndices = configIndices.body
395+
.map((hit) => hit.index?.toString())
396+
.filter((index) => index && isDynamicConfigIndex(index));
397+
398+
return validIndices.length === 0
399+
? 0
400+
: validIndices
401+
.map((configIndex) => {
402+
return configIndex ? extractVersionFromDynamicConfigIndex(configIndex) : 0;
403+
})
404+
.reduce((currentMax, currentNum) => {
405+
return currentMax && currentNum && currentMax > currentNum ? currentMax : currentNum;
406+
});
407+
}
345408
}

0 commit comments

Comments
 (0)