diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-down.ts b/lambdas/functions/control-plane/src/scale-runners/scale-down.ts index 329ce694d9..1928c770ec 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-down.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-down.ts @@ -367,7 +367,6 @@ export async function scaleDown(): Promise { ...controlPlaneProviderRegistry.capability(computeProviderType, 'scaleDown')(), type: computeProviderType, }; - // first runners marked to be orphan. await terminateOrphan(environment, computeProvider); diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts index d4e3889f19..35dd4daf52 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts @@ -110,7 +110,6 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise { +): ComputeProviderPlugin { const ec2Client = getTracedAWSV3Client(new EC2Client({ region: process.env.AWS_REGION })); const ec2Operations = createEc2RunnerClient(ec2Client).forRequest({ signal: undefined }); return { - type: 'ec2', + type: computeProvider.ec2, capabilities: { pool: () => createEc2PoolCapability(ec2Operations, createStartRunnerConfig), scaleUp: () => createEc2ScaleUpCapability(ec2Operations, createStartRunnerConfig), @@ -26,6 +27,6 @@ export function createEc2ControlPlanePlugin( } export const provider = { - type: 'ec2', + type: computeProvider.ec2, createPlugin: createEc2ControlPlanePlugin, -} satisfies ControlPlaneProviderModule<'ec2'>; +} satisfies ControlPlaneProviderModule; diff --git a/lambdas/libs/compute-providers/aws/ec2/logger.test.ts b/lambdas/libs/compute-providers/aws/ec2/logger.test.ts new file mode 100644 index 0000000000..7c1f99fcc4 --- /dev/null +++ b/lambdas/libs/compute-providers/aws/ec2/logger.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { computeProvider } from '../../provider-types'; +import { createEc2ComputeProviderLogger } from './logger'; + +const loggerMock = vi.hoisted(() => ({ + appendPersistentKeys: vi.fn(), +})); +const createChildLoggerMock = vi.hoisted(() => vi.fn(() => loggerMock)); + +vi.mock('@aws-github-runner/aws-powertools-util', () => ({ + createChildLogger: createChildLoggerMock, +})); + +describe('EC2 compute provider logger', () => { + it('adds the canonical compute provider while preserving the module name', () => { + expect(createEc2ComputeProviderLogger('runners')).toBe(loggerMock); + expect(createChildLoggerMock).toHaveBeenCalledWith('runners'); + expect(loggerMock.appendPersistentKeys).toHaveBeenCalledWith({ + computeProvider: computeProvider.ec2, + }); + }); +}); diff --git a/lambdas/libs/compute-providers/aws/ec2/logger.ts b/lambdas/libs/compute-providers/aws/ec2/logger.ts new file mode 100644 index 0000000000..bff3bc1691 --- /dev/null +++ b/lambdas/libs/compute-providers/aws/ec2/logger.ts @@ -0,0 +1,11 @@ +import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; + +import { computeProvider } from '../../provider-types'; + +export function createEc2ComputeProviderLogger(module: string) { + const logger = createChildLogger(module); + logger.appendPersistentKeys({ + computeProvider: computeProvider.ec2, + }); + return logger; +} diff --git a/lambdas/libs/compute-providers/aws/ec2/src/constants.ts b/lambdas/libs/compute-providers/aws/ec2/src/constants.ts new file mode 100644 index 0000000000..43449c9126 --- /dev/null +++ b/lambdas/libs/compute-providers/aws/ec2/src/constants.ts @@ -0,0 +1,3 @@ +import { computeProvider } from '../../../provider-types'; + +export const EC2_OVERRIDE_LABEL_PREFIX = `ghr-${computeProvider.ec2}-`; diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/dynamic-labels.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/dynamic-labels.ts index 5bbc28a929..e32422335d 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/dynamic-labels.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/dynamic-labels.ts @@ -31,6 +31,7 @@ import { import { InvalidRunnerLabelsError } from '../../../../core'; import { Ec2OverrideConfig } from '../runners.d'; +import { EC2_OVERRIDE_LABEL_PREFIX } from '../constants'; const EC2_OVERRIDE_LIST_VALUE_SEPARATOR = ';'; @@ -130,11 +131,11 @@ export function parseEc2OverrideConfig( labels: string[], defaultBlockDeviceName?: string, ): Ec2OverrideConfig | undefined { - const ec2Labels = labels.filter((l) => l.startsWith('ghr-ec2-')); + const ec2Labels = labels.filter((l) => l.startsWith(EC2_OVERRIDE_LABEL_PREFIX)); const config: Ec2OverrideConfig = {}; for (const label of ec2Labels) { - const [key, ...valueParts] = label.replace('ghr-ec2-', '').split(':'); + const [key, ...valueParts] = label.replace(EC2_OVERRIDE_LABEL_PREFIX, '').split(':'); const value = valueParts.join(':'); if (!value) continue; @@ -355,13 +356,15 @@ function getOrCreateBlockDeviceMapping( } export function shouldLoadLaunchTemplateBlockDeviceName(labels: string[]): boolean { - const blockDeviceNameLabel = 'ghr-ec2-block-device-name:'; + const blockDeviceNameLabel = `${EC2_OVERRIDE_LABEL_PREFIX}block-device-name:`; let hasBlockDeviceOverride = false; let hasBlockDeviceName = false; for (const label of labels) { hasBlockDeviceOverride = - hasBlockDeviceOverride || label.startsWith('ghr-ec2-ebs-') || label.startsWith('ghr-ec2-block-device-'); + hasBlockDeviceOverride || + label.startsWith(`${EC2_OVERRIDE_LABEL_PREFIX}ebs-`) || + label.startsWith(`${EC2_OVERRIDE_LABEL_PREFIX}block-device-`); hasBlockDeviceName = hasBlockDeviceName || (label.startsWith(blockDeviceNameLabel) && label.slice(blockDeviceNameLabel.length) !== ''); diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts index 43ec0aacf9..694bef0ade 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts @@ -1,9 +1,9 @@ -import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; +import { createEc2ComputeProviderLogger } from '../../logger'; import type { CreateStartRunnerConfig, PoolComputeProvider, RunnerInfo, RunnerStatus } from '../../../../core'; import { bootTimeExceeded, type Ec2RunnerResourceOperations } from '../runners'; import { createRunners, loadEc2ProviderConfig } from './runner-creation'; -const logger = createChildLogger('pool'); +const logger = createEc2ComputeProviderLogger('pool'); function countAvailableEc2PoolRunners( ec2runners: RunnerInfo[], diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts index 124b676ae6..8e4d04c945 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts @@ -1,4 +1,4 @@ -import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; +import { createEc2ComputeProviderLogger } from '../../logger'; import type { CreateGitHubRunnerConfig, CreateRunnerResult, @@ -16,7 +16,7 @@ import type { Ec2RunnerResourceOperations } from '../runners'; import type { RunnerInputParameters } from '../runners.d'; import { toControlPlaneCreateRunnerResult } from './create-result'; -const logger = createChildLogger('ec2-runners'); +const logger = createEc2ComputeProviderLogger('ec2-runners'); const RUNNER_LABELS_TAG_KEY = 'ghr:runner_labels'; const RUNNER_LABELS_TAG_VALUE_SEPARATOR = ','; export const EC2_TAG_VALUE_MAX_LENGTH = 256; diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.ts index a27758b574..82d8286150 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.ts @@ -1,9 +1,10 @@ -import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; +import { createEc2ComputeProviderLogger } from '../../logger'; import type { CreateStartRunnerConfig, RunnerLabelResolution, ScaleUpComputeProvider } from '../../../../core'; import yn from 'yn'; import type { Ec2RunnerProvisioningOperations } from '../runners'; import type { Ec2OverrideConfig } from '../runners.d'; +import { EC2_OVERRIDE_LABEL_PREFIX } from '../constants'; import { parseEc2OverrideConfig, shouldLoadLaunchTemplateBlockDeviceName, @@ -12,7 +13,7 @@ import { import { createRunners, loadEc2ProviderConfig } from './runner-creation'; import type { CreateEC2RunnerConfig } from './runner-creation'; -const logger = createChildLogger('ec2-scale-up'); +const logger = createEc2ComputeProviderLogger('ec2-scale-up'); interface Ec2ScaleUpState { ec2OverrideConfig?: Ec2OverrideConfig; @@ -30,9 +31,9 @@ async function resolveEc2ScaleUpRunnerLabels( messageLabels: string[], ): Promise> { const trimmedLabels = messageLabels.map((label) => label.trim()); - const dynamicEC2Labels = trimmedLabels.filter((label) => label.startsWith('ghr-ec2-')); + const dynamicEC2Labels = trimmedLabels.filter((label) => label.startsWith(EC2_OVERRIDE_LABEL_PREFIX)); const nonEc2DynamicLabels = trimmedLabels.filter( - (label) => label.startsWith('ghr-') && !label.startsWith('ghr-ec2-'), + (label) => label.startsWith('ghr-') && !label.startsWith(EC2_OVERRIDE_LABEL_PREFIX), ); const runnerLabels = [...nonEc2DynamicLabels, ...dynamicEC2Labels]; let ec2OverrideConfig: Ec2OverrideConfig | undefined; diff --git a/lambdas/libs/compute-providers/aws/ec2/src/runners.ts b/lambdas/libs/compute-providers/aws/ec2/src/runners.ts index ce7f3de80f..6ae8618e19 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/runners.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/runners.ts @@ -17,7 +17,7 @@ import { TerminateInstancesCommand, _InstanceType, } from '@aws-sdk/client-ec2'; -import { createChildLogger, tracer } from '@aws-github-runner/aws-powertools-util'; +import { tracer } from '@aws-github-runner/aws-powertools-util'; import { getParameter } from '@aws-github-runner/aws-ssm-util'; import moment from 'moment'; @@ -25,8 +25,9 @@ import type { RunnerInfo } from '../../../core'; import { getDefaultBlockDeviceNameFromLaunchTemplate } from './launch-template'; import type { Ec2RunnerCreateResult, Ec2RunnerFailureCode } from './runner-create-result'; import type { Ec2ListRunnerFilters, Ec2OverrideConfig, RunnerInputParameters } from './runners.d'; +import { createEc2ComputeProviderLogger } from '../logger'; -const logger = createChildLogger('runners'); +const logger = createEc2ComputeProviderLogger('runners'); interface Ec2Filter { Name: string; diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts index 8babbadd55..2a43b4e405 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts @@ -1,5 +1,6 @@ import type { AwsDynamicLabelsPolicy, AwsDynamicLabelsValueRule } from '../../../../contracts'; import { violationsAgainstAwsDynamicLabelsPolicy } from '../../../dynamic-labels-policy'; +import { EC2_OVERRIDE_LABEL_PREFIX } from '../constants'; export type Ec2DynamicLabelsValueRule = AwsDynamicLabelsValueRule; @@ -19,5 +20,5 @@ export function violationsAgainstPolicy( labels: string[], policy: Ec2DynamicLabelsPolicy | null | undefined, ): { label: string; reason: string }[] { - return violationsAgainstAwsDynamicLabelsPolicy(labels, policy, 'ghr-ec2-'); + return violationsAgainstAwsDynamicLabelsPolicy(labels, policy, EC2_OVERRIDE_LABEL_PREFIX); } diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts index 5e671da189..9dc1f68380 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts @@ -1,9 +1,9 @@ -import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; +import { createEc2ComputeProviderLogger } from '../../logger'; import type { DynamicLabelProvider, RunnerMatcherConfig } from '../../../../contracts'; import { violationsAgainstPolicy } from './dynamic-labels-policy'; -const logger = createChildLogger('handler'); +const logger = createEc2ComputeProviderLogger('handler'); function resolveEc2DynamicLabelsPolicy(queue: RunnerMatcherConfig) { const hasLegacyEc2DynamicLabelsPolicy = Object.prototype.hasOwnProperty.call( diff --git a/lambdas/libs/compute-providers/aws/ec2/webhook.ts b/lambdas/libs/compute-providers/aws/ec2/webhook.ts index 1357c93f06..ebde24e241 100644 --- a/lambdas/libs/compute-providers/aws/ec2/webhook.ts +++ b/lambdas/libs/compute-providers/aws/ec2/webhook.ts @@ -1,16 +1,20 @@ import type { ComputeProviderPlugin } from '../../core'; +import { computeProvider } from '../../provider-types'; import type { WebhookProviderCapabilities, WebhookProviderModule } from '../../contracts'; import { ec2DynamicLabelProvider } from './src/webhook/dynamic-labels'; -export function createEc2WebhookPlugin(): ComputeProviderPlugin { +export function createEc2WebhookPlugin(): ComputeProviderPlugin< + WebhookProviderCapabilities, + typeof computeProvider.ec2 +> { return { - type: 'ec2', + type: computeProvider.ec2, capabilities: { dynamicLabels: ec2DynamicLabelProvider }, }; } export const provider = { - type: 'ec2', + type: computeProvider.ec2, createPlugin: createEc2WebhookPlugin, -} satisfies WebhookProviderModule<'ec2'>; +} satisfies WebhookProviderModule; diff --git a/lambdas/libs/compute-providers/core/index.test.ts b/lambdas/libs/compute-providers/core/index.test.ts index 8ed95f927e..40fa40f52b 100644 --- a/lambdas/libs/compute-providers/core/index.test.ts +++ b/lambdas/libs/compute-providers/core/index.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it } from 'vitest'; +import { computeProvider } from '../provider-types'; import { createComputeProviderRegistry } from './index'; describe('compute provider registry', () => { const plugin = { - type: 'ec2' as const, + type: computeProvider.ec2, capabilities: { scaleUp: () => 'scale-up', pool: () => 'pool', @@ -13,7 +14,7 @@ describe('compute provider registry', () => { const registry = createComputeProviderRegistry([plugin]); it('resolves capabilities dynamically', () => { - expect(registry.capability('ec2', 'scaleUp')()).toBe('scale-up'); - expect(registry.capability('ec2', 'pool')()).toBe('pool'); + expect(registry.capability(computeProvider.ec2, 'scaleUp')()).toBe('scale-up'); + expect(registry.capability(computeProvider.ec2, 'pool')()).toBe('pool'); }); }); diff --git a/lambdas/libs/compute-providers/provider-types.test.ts b/lambdas/libs/compute-providers/provider-types.test.ts index 9f6f4a981e..edff11a48d 100644 --- a/lambdas/libs/compute-providers/provider-types.test.ts +++ b/lambdas/libs/compute-providers/provider-types.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from 'vitest'; -import { computeProviderTypes, defaultComputeProvider, resolveComputeProviderType } from './provider-types'; +import { + computeProvider, + computeProviderTypes, + defaultComputeProvider, + resolveComputeProviderType, +} from './provider-types'; const defaultProviderInputs = [undefined, '', ' '] as const; const supportedProviderCases = computeProviderTypes.flatMap( @@ -13,6 +18,7 @@ const supportedProviderCases = computeProviderTypes.flatMap( describe('compute provider configuration', () => { it('defines an explicit default provider', () => { + expect(computeProvider.ec2).toBe('ec2'); expect(computeProviderTypes).toContain(defaultComputeProvider); }); }); diff --git a/lambdas/libs/compute-providers/provider-types.ts b/lambdas/libs/compute-providers/provider-types.ts index 64d7be8e5f..ceb2371b04 100644 --- a/lambdas/libs/compute-providers/provider-types.ts +++ b/lambdas/libs/compute-providers/provider-types.ts @@ -1,8 +1,12 @@ -export const computeProviderTypes = ['ec2'] as const; +export const computeProvider = { + ec2: 'ec2', +} as const; + +export const computeProviderTypes = [computeProvider.ec2] as const; export type ComputeProviderType = (typeof computeProviderTypes)[number]; -export const defaultComputeProvider = 'ec2' satisfies ComputeProviderType; +export const defaultComputeProvider = computeProvider.ec2 satisfies ComputeProviderType; export function resolveComputeProviderType(type: unknown): ComputeProviderType { if (type === undefined) return defaultComputeProvider; diff --git a/lambdas/libs/compute-providers/webhook.ts b/lambdas/libs/compute-providers/webhook.ts index 1aa0a7b5e6..930f0440c9 100644 --- a/lambdas/libs/compute-providers/webhook.ts +++ b/lambdas/libs/compute-providers/webhook.ts @@ -29,7 +29,6 @@ export function createDynamicLabelQueueSelector(depend ): DynamicLabelDispatchTarget | undefined => { for (const queue of matches) { const { type: provider, dynamicLabels } = dependencies.resolveProvider(queue); - if (!queue.matcherConfig.enableDynamicLabels) { logger.warn( `Queue ${queue.id} matches non-dynamic labels but does not allow dynamic labels; trying next match`,