Skip to content

Commit

Permalink
[8.x] Onboard elastic owned ECH clusters to use `mget` task…
Browse files Browse the repository at this point in the history
… claiming (#196757) (#196856)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Onboard elastic owned ECH clusters to use `mget` task
claiming (#196757)](#196757)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Mike
Côté","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-18T11:31:32Z","message":"Onboard
elastic owned ECH clusters to use `mget` task claiming
(#196757)\n\nSimilar to
https://github.com/elastic/kibana/pull/196317\r\n\r\nIn this PR, I'm
flipping the mget feature flag to on for all elastic\r\nowned ECH
clusters. Elastic owned clusters are determined by looking
at\r\n`plugins.cloud?.isElasticStaffOwned`.\r\n\r\n## To
verify\r\nObserve the PR deployment which doesn't start with `a` or `b`
yet is\r\nusing the mget claim strategy by logging `Using claim strategy
mget`
on\r\nstartup.","sha":"97f2a9098fb91708250459910820b1b99d40f1c4","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Task
Manager","Team:ResponseOps","v9.0.0","backport:prev-minor","ci:cloud-deploy","v8.16.0","v8.17.0"],"title":"Onboard
elastic owned ECH clusters to use `mget` task
claiming","number":196757,"url":"https://github.com/elastic/kibana/pull/196757","mergeCommit":{"message":"Onboard
elastic owned ECH clusters to use `mget` task claiming
(#196757)\n\nSimilar to
https://github.com/elastic/kibana/pull/196317\r\n\r\nIn this PR, I'm
flipping the mget feature flag to on for all elastic\r\nowned ECH
clusters. Elastic owned clusters are determined by looking
at\r\n`plugins.cloud?.isElasticStaffOwned`.\r\n\r\n## To
verify\r\nObserve the PR deployment which doesn't start with `a` or `b`
yet is\r\nusing the mget claim strategy by logging `Using claim strategy
mget`
on\r\nstartup.","sha":"97f2a9098fb91708250459910820b1b99d40f1c4"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196757","number":196757,"mergeCommit":{"message":"Onboard
elastic owned ECH clusters to use `mget` task claiming
(#196757)\n\nSimilar to
https://github.com/elastic/kibana/pull/196317\r\n\r\nIn this PR, I'm
flipping the mget feature flag to on for all elastic\r\nowned ECH
clusters. Elastic owned clusters are determined by looking
at\r\n`plugins.cloud?.isElasticStaffOwned`.\r\n\r\n## To
verify\r\nObserve the PR deployment which doesn't start with `a` or `b`
yet is\r\nusing the mget claim strategy by logging `Using claim strategy
mget`
on\r\nstartup.","sha":"97f2a9098fb91708250459910820b1b99d40f1c4"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Mike Côté <[email protected]>
  • Loading branch information
kibanamachine and mikecote authored Oct 18, 2024
1 parent 10a9161 commit e32e716
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 47 deletions.
121 changes: 75 additions & 46 deletions x-pack/plugins/task_manager/server/lib/set_claim_strategy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,61 +71,67 @@ describe('setClaimStrategy', () => {
});
for (const isServerless of [true, false]) {
for (const isCloud of [true, false]) {
for (const deploymentId of [undefined, deploymentIdMget, deploymentIdUpdateByQuery]) {
for (const configuredStrategy of [CLAIM_STRATEGY_MGET, CLAIM_STRATEGY_UPDATE_BY_QUERY]) {
test(`should return config as is when claim strategy is already defined: isServerless=${isServerless}, isCloud=${isCloud}, deploymentId=${deploymentId}`, () => {
const config = {
...getConfigWithoutClaimStrategy(),
claim_strategy: configuredStrategy,
};

const returnedConfig = setClaimStrategy({
config,
logger,
isCloud,
isServerless,
deploymentId,
for (const isElasticStaffOwned of [true, false]) {
for (const deploymentId of [undefined, deploymentIdMget, deploymentIdUpdateByQuery]) {
for (const configuredStrategy of [CLAIM_STRATEGY_MGET, CLAIM_STRATEGY_UPDATE_BY_QUERY]) {
test(`should return config as is when claim strategy is already defined: isServerless=${isServerless}, isCloud=${isCloud}, isElasticStaffOwned=${isElasticStaffOwned}, deploymentId=${deploymentId}`, () => {
const config = {
...getConfigWithoutClaimStrategy(),
claim_strategy: configuredStrategy,
};

const returnedConfig = setClaimStrategy({
config,
logger,
isCloud,
isServerless,
isElasticStaffOwned,
deploymentId,
});

expect(returnedConfig).toStrictEqual(config);
if (deploymentId) {
expect(logger.info).toHaveBeenCalledWith(
`Using claim strategy ${configuredStrategy} as configured for deployment ${deploymentId}`
);
} else {
expect(logger.info).toHaveBeenCalledWith(
`Using claim strategy ${configuredStrategy} as configured`
);
}
});

expect(returnedConfig).toStrictEqual(config);
if (deploymentId) {
expect(logger.info).toHaveBeenCalledWith(
`Using claim strategy ${configuredStrategy} as configured for deployment ${deploymentId}`
);
} else {
expect(logger.info).toHaveBeenCalledWith(
`Using claim strategy ${configuredStrategy} as configured`
);
}
});
}
}
}
}
}

for (const isCloud of [true, false]) {
for (const deploymentId of [undefined, deploymentIdMget, deploymentIdUpdateByQuery]) {
test(`should set claim strategy to mget if in serverless: isCloud=${isCloud}, deploymentId=${deploymentId}`, () => {
const config = getConfigWithoutClaimStrategy();
const returnedConfig = setClaimStrategy({
config,
logger,
isCloud,
isServerless: true,
deploymentId,
});
for (const isElasticStaffOwned of [true, false]) {
for (const deploymentId of [undefined, deploymentIdMget, deploymentIdUpdateByQuery]) {
test(`should set claim strategy to mget if in serverless: isCloud=${isCloud}, isElasticStaffOwned=${isElasticStaffOwned}, deploymentId=${deploymentId}`, () => {
const config = getConfigWithoutClaimStrategy();
const returnedConfig = setClaimStrategy({
config,
logger,
isCloud,
isServerless: true,
isElasticStaffOwned,
deploymentId,
});

expect(returnedConfig.claim_strategy).toBe(CLAIM_STRATEGY_MGET);
expect(returnedConfig.poll_interval).toBe(MGET_DEFAULT_POLL_INTERVAL);
expect(returnedConfig.claim_strategy).toBe(CLAIM_STRATEGY_MGET);
expect(returnedConfig.poll_interval).toBe(MGET_DEFAULT_POLL_INTERVAL);

if (deploymentId) {
expect(logger.info).toHaveBeenCalledWith(
`Setting claim strategy to mget for serverless deployment ${deploymentId}`
);
} else {
expect(logger.info).toHaveBeenCalledWith(`Setting claim strategy to mget`);
}
});
if (deploymentId) {
expect(logger.info).toHaveBeenCalledWith(
`Setting claim strategy to mget for serverless deployment ${deploymentId}`
);
} else {
expect(logger.info).toHaveBeenCalledWith(`Setting claim strategy to mget`);
}
});
}
}
}

Expand All @@ -135,6 +141,7 @@ describe('setClaimStrategy', () => {
config,
logger,
isCloud: false,
isElasticStaffOwned: false,
isServerless: false,
});

Expand All @@ -150,6 +157,7 @@ describe('setClaimStrategy', () => {
config,
logger,
isCloud: true,
isElasticStaffOwned: false,
isServerless: false,
});

Expand All @@ -165,6 +173,7 @@ describe('setClaimStrategy', () => {
config,
logger,
isCloud: true,
isElasticStaffOwned: false,
isServerless: false,
deploymentId: deploymentIdUpdateByQuery,
});
Expand All @@ -177,12 +186,32 @@ describe('setClaimStrategy', () => {
);
});

test(`should set claim strategy to mget if cloud, deploymentId does not start with a or b, not serverless and isElasticStaffOwned is true`, () => {
const config = getConfigWithoutClaimStrategy();
const returnedConfig = setClaimStrategy({
config,
logger,
isCloud: true,
isElasticStaffOwned: true,
isServerless: false,
deploymentId: deploymentIdUpdateByQuery,
});

expect(returnedConfig.claim_strategy).toBe(CLAIM_STRATEGY_MGET);
expect(returnedConfig.poll_interval).toBe(MGET_DEFAULT_POLL_INTERVAL);

expect(logger.info).toHaveBeenCalledWith(
`Setting claim strategy to mget for deployment ${deploymentIdUpdateByQuery}`
);
});

test(`should set claim strategy to mget if cloud and not serverless and deploymentId starts with a or b`, () => {
const config = getConfigWithoutClaimStrategy();
const returnedConfig = setClaimStrategy({
config,
logger,
isCloud: true,
isElasticStaffOwned: false,
isServerless: false,
deploymentId: deploymentIdMget,
});
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/task_manager/server/lib/set_claim_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface SetClaimStrategyOpts {
deploymentId?: string;
isServerless: boolean;
isCloud: boolean;
isElasticStaffOwned: boolean;
logger: Logger;
}

Expand Down Expand Up @@ -50,7 +51,10 @@ export function setClaimStrategy(opts: SetClaimStrategyOpts): TaskManagerConfig
let defaultToMget = false;

if (opts.isCloud && !opts.isServerless && opts.deploymentId) {
defaultToMget = opts.deploymentId.startsWith('a') || opts.deploymentId.startsWith('b');
defaultToMget =
opts.deploymentId.startsWith('a') ||
opts.deploymentId.startsWith('b') ||
opts.isElasticStaffOwned;
if (defaultToMget) {
opts.logger.info(`Setting claim strategy to mget for deployment ${opts.deploymentId}`);
} else {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/task_manager/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export class TaskManagerPlugin
deploymentId: plugins.cloud?.deploymentId,
isServerless: this.initContext.env.packageInfo.buildFlavor === 'serverless',
isCloud: plugins.cloud?.isCloudEnabled ?? false,
isElasticStaffOwned: plugins.cloud?.isElasticStaffOwned ?? false,
logger: this.logger,
});

Expand Down

0 comments on commit e32e716

Please sign in to comment.