Skip to content

Commit

Permalink
Onboard elastic owned ECH clusters to use mget task claiming (#196757)
Browse files Browse the repository at this point in the history
Similar to #196317

In this PR, I'm flipping the mget feature flag to on for all elastic
owned ECH clusters. Elastic owned clusters are determined by looking at
`plugins.cloud?.isElasticStaffOwned`.

## To verify
Observe the PR deployment which doesn't start with `a` or `b` yet is
using the mget claim strategy by logging `Using claim strategy mget` on
startup.

(cherry picked from commit 97f2a90)
  • Loading branch information
mikecote committed Oct 18, 2024
1 parent 21a3625 commit b5b8cc1
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 b5b8cc1

Please sign in to comment.