diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index 8735e727c..acdef1443 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -707,6 +707,28 @@ class DockerStackWithCustomFile extends cdk.Stack { } } +class MultipleDockerAssetsStack extends cdk.Stack { + constructor(parent, id, props) { + super(parent, id, props); + + new docker.DockerImageAsset(this, 'image1', { + directory: path.join(__dirname, 'docker-concurrent/image1') + }); + new docker.DockerImageAsset(this, 'image2', { + directory: path.join(__dirname, 'docker-concurrent/image2') + }); + new docker.DockerImageAsset(this, 'image3', { + directory: path.join(__dirname, 'docker-concurrent/image3') + }); + + // Add at least a single resource (WaitConditionHandle), otherwise this stack will never + // be deployed (and its assets never built) + new cdk.CfnResource(this, 'Handle', { + type: 'AWS::CloudFormation::WaitConditionHandle' + }); + } +} + /** * A stack that will never succeed deploying (done in a way that CDK cannot detect but CFN will complain about) */ @@ -921,6 +943,7 @@ switch (stackSet) { new DockerStack(app, `${stackPrefix}-docker`); new DockerInUseStack(app, `${stackPrefix}-docker-in-use`); new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`); + new MultipleDockerAssetsStack(app, `${stackPrefix}-multiple-docker-assets`); new NotificationArnsStack(app, `${stackPrefix}-notification-arns`); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image1/Dockerfile b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image1/Dockerfile new file mode 100644 index 000000000..06f417567 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image1/Dockerfile @@ -0,0 +1,2 @@ +FROM public.ecr.aws/docker/library/alpine:latest +RUN echo "image1" diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image2/Dockerfile b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image2/Dockerfile new file mode 100644 index 000000000..5089609f7 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image2/Dockerfile @@ -0,0 +1,2 @@ +FROM public.ecr.aws/docker/library/alpine:latest +RUN echo "image2" diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image3/Dockerfile b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image3/Dockerfile new file mode 100644 index 000000000..470a54c4a --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/docker-concurrent/image3/Dockerfile @@ -0,0 +1,2 @@ +FROM public.ecr.aws/docker/library/alpine:latest +RUN echo "image3" diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-stack-with-multiple-docker-assets.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-stack-with-multiple-docker-assets.integtest.ts new file mode 100644 index 000000000..0305d45a3 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-stack-with-multiple-docker-assets.integtest.ts @@ -0,0 +1,12 @@ +import { integTest, withDefaultFixture } from '../../../lib'; + +jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime + +integTest( + 'deploy stack with multiple docker assets', + withDefaultFixture(async (fixture) => { + await fixture.cdkDeploy('multiple-docker-assets', { + options: ['--asset-parallelism', '--asset-build-concurrency', '3'], + }); + }), +); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/toolkit-lib-integ-tests/toolkit-deploy-stack-with-multiple-docker-assets.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/toolkit-lib-integ-tests/toolkit-deploy-stack-with-multiple-docker-assets.integtest.ts new file mode 100644 index 000000000..f7f44bc07 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/toolkit-lib-integ-tests/toolkit-deploy-stack-with-multiple-docker-assets.integtest.ts @@ -0,0 +1,27 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import * as toolkit from '@aws-cdk/toolkit-lib'; +import { assemblyFromCdkAppDir, toolkitFromFixture } from './toolkit-helpers'; +import { integTest, withDefaultFixture } from '../../lib'; + +jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime + +integTest( + 'toolkit deploy stack with multiple docker assets', + withDefaultFixture(async (fixture) => { + const tk = toolkitFromFixture(fixture); + + const assembly = await assemblyFromCdkAppDir(tk, fixture); + + const stacks: toolkit.StackSelector = { + strategy: toolkit.StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, + patterns: [fixture.fullStackName('multiple-docker-assets')], + }; + + await tk.deploy(assembly, { + stacks, + assetParallelism: true, + assetBuildConcurrency: 3, + }); + await tk.destroy(assembly, { stacks }); + }), +); diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts index 60a795cc0..eacd92003 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts @@ -214,6 +214,15 @@ export interface DeployOptions extends BaseDeployOptions { */ readonly assetParallelism?: boolean; + /** + * Maximum number of asset builds to run in parallel + * + * This setting only has an effect if `assetParallelism` is set to `true`. + * + * @default 1 + */ + readonly assetBuildConcurrency?: number; + /** * When to build assets * diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 8aa68cd80..6c8680e01 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -823,7 +823,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { const graphConcurrency: Concurrency = { 'stack': concurrency, - 'asset-build': 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds + 'asset-build': (options.assetParallelism ?? true) ? options.assetBuildConcurrency ?? 1 : 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds 'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable }; diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts index ab2a6c76b..09ee0aaa7 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts @@ -1,6 +1,7 @@ import { StackParameters } from '../../lib/actions/deploy'; import type { DeployStackOptions, DeployStackResult } from '../../lib/api/deployments'; import * as deployments from '../../lib/api/deployments'; +import { WorkGraphBuilder } from '../../lib/api/work-graph'; import { Toolkit } from '../../lib/toolkit'; import { builderFixture, cdkOutFixture, disposableCloudAssemblySource, TestIoHost } from '../_helpers'; @@ -161,6 +162,83 @@ IAM Statement Changes forcePublish: true, })); }); + + describe('assetBuildConcurrency', () => { + let buildSpy: jest.SpyInstance; + + afterEach(() => { + buildSpy?.mockRestore(); + }); + + test('is passed when assetParallelism is true', async () => { + const mockWorkGraph = { + doParallel: jest.fn().mockResolvedValue(undefined), + removeUnnecessaryAssets: jest.fn().mockResolvedValue(undefined), + }; + buildSpy = jest.spyOn(WorkGraphBuilder.prototype, 'build').mockReturnValue(mockWorkGraph as any); + + const cx = await builderFixture(toolkit, 'stack-with-asset'); + + await toolkit.deploy(cx, { + assetParallelism: true, + assetBuildConcurrency: 4, + }); + + expect(mockWorkGraph.doParallel).toHaveBeenCalledWith( + expect.objectContaining({ + 'asset-build': 4, + }), + expect.anything(), + ); + }); + + test('is ignored when assetParallelism is false', async () => { + const mockWorkGraph = { + doParallel: jest.fn().mockResolvedValue(undefined), + removeUnnecessaryAssets: jest.fn().mockResolvedValue(undefined), + }; + buildSpy = jest.spyOn(WorkGraphBuilder.prototype, 'build').mockReturnValue(mockWorkGraph as any); + + const cx = await builderFixture(toolkit, 'stack-with-asset'); + + await toolkit.deploy(cx, { + assetParallelism: false, + assetBuildConcurrency: 4, + }); + + expect(mockWorkGraph.doParallel).toHaveBeenCalledWith( + expect.objectContaining({ + 'asset-build': 1, + }), + expect.anything(), + ); + }); + + test.each([ + true, + false, + undefined, + ])('defaults to 1 when assetParallelism=%s and assetBuildConcurrency is not specified', async (assetParallelism) => { + const mockWorkGraph = { + doParallel: jest.fn().mockResolvedValue(undefined), + removeUnnecessaryAssets: jest.fn().mockResolvedValue(undefined), + }; + buildSpy = jest.spyOn(WorkGraphBuilder.prototype, 'build').mockReturnValue(mockWorkGraph as any); + + const cx = await builderFixture(toolkit, 'stack-with-asset'); + + await toolkit.deploy(cx, { + assetParallelism, + }); + + expect(mockWorkGraph.doParallel).toHaveBeenCalledWith( + expect.objectContaining({ + 'asset-build': 1, + }), + expect.anything(), + ); + }); + }); }); describe('deployment results', () => { diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index b22eed419..ca27414b1 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -705,7 +705,7 @@ export class CdkToolkit { const graphConcurrency: Concurrency = { 'stack': concurrency, - 'asset-build': 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds + 'asset-build': (options.assetParallelism ?? true) ? options.assetBuildConcurrency ?? 1 : 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds 'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable }; @@ -1766,6 +1766,15 @@ export interface DeployOptions extends CfnDeployOptions, WatchOptions { */ readonly assetParallelism?: boolean; + /** + * Maximum number of asset builds to run in parallel + * + * This setting only has an effect if `assetParallelism` is set to `true`. + * + * @default 1 + */ + readonly assetBuildConcurrency?: number; + /** * When to build assets * diff --git a/packages/aws-cdk/lib/cli/cli-config.ts b/packages/aws-cdk/lib/cli/cli-config.ts index 162b6eca5..3c14068c0 100644 --- a/packages/aws-cdk/lib/cli/cli-config.ts +++ b/packages/aws-cdk/lib/cli/cli-config.ts @@ -208,6 +208,7 @@ export async function makeConfig(): Promise { }, 'concurrency': { type: 'number', desc: 'Maximum number of simultaneous deployments (dependency permitting) to execute.', default: 1, requiresArg: true }, 'asset-parallelism': { type: 'boolean', desc: 'Whether to build/publish assets in parallel' }, + 'asset-build-concurrency': { type: 'number', desc: 'Maximum number of asset builds to run in parallel', default: 1, requiresArg: true }, 'asset-prebuild': { type: 'boolean', desc: 'Whether to build all assets before deploying the first stack (useful for failing Docker builds)', default: true }, 'ignore-no-stacks': { type: 'boolean', desc: 'Whether to deploy if the app contains no stacks', default: false }, }, diff --git a/packages/aws-cdk/lib/cli/cli-type-registry.json b/packages/aws-cdk/lib/cli/cli-type-registry.json index db494fc7e..eee6d24e3 100644 --- a/packages/aws-cdk/lib/cli/cli-type-registry.json +++ b/packages/aws-cdk/lib/cli/cli-type-registry.json @@ -548,6 +548,12 @@ "type": "boolean", "desc": "Whether to build/publish assets in parallel" }, + "asset-build-concurrency": { + "type": "number", + "desc": "Maximum number of asset builds to run in parallel", + "default": 1, + "requiresArg": true + }, "asset-prebuild": { "type": "boolean", "desc": "Whether to build all assets before deploying the first stack (useful for failing Docker builds)", diff --git a/packages/aws-cdk/lib/cli/cli.ts b/packages/aws-cdk/lib/cli/cli.ts index 4bc38f10f..b34dec51a 100644 --- a/packages/aws-cdk/lib/cli/cli.ts +++ b/packages/aws-cdk/lib/cli/cli.ts @@ -407,6 +407,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise): any { type: 'boolean', desc: 'Whether to build/publish assets in parallel', }) + .option('asset-build-concurrency', { + default: 1, + type: 'number', + desc: 'Maximum number of asset builds to run in parallel', + requiresArg: true, + }) .option('asset-prebuild', { default: true, type: 'boolean', diff --git a/packages/aws-cdk/lib/cli/user-configuration.ts b/packages/aws-cdk/lib/cli/user-configuration.ts index 733f56a83..36b17d333 100644 --- a/packages/aws-cdk/lib/cli/user-configuration.ts +++ b/packages/aws-cdk/lib/cli/user-configuration.ts @@ -321,6 +321,7 @@ export async function commandLineArgumentsToSettings(ioHelper: IoHelper, argv: A rollback: argv.rollback, notices: argv.notices, assetParallelism: argv['asset-parallelism'], + assetBuildConcurrency: argv['asset-build-concurrency'], assetPrebuild: argv['asset-prebuild'], ignoreNoStacks: argv['ignore-no-stacks'], hotswap: { diff --git a/packages/aws-cdk/lib/cli/user-input.ts b/packages/aws-cdk/lib/cli/user-input.ts index 735198270..7cc0667a5 100644 --- a/packages/aws-cdk/lib/cli/user-input.ts +++ b/packages/aws-cdk/lib/cli/user-input.ts @@ -911,6 +911,13 @@ export interface DeployOptions { */ readonly assetParallelism?: boolean; + /** + * Maximum number of asset builds to run in parallel + * + * @default - 1 + */ + readonly assetBuildConcurrency?: number; + /** * Whether to build all assets before deploying the first stack (useful for failing Docker builds) * diff --git a/packages/aws-cdk/lib/legacy/configuration.ts b/packages/aws-cdk/lib/legacy/configuration.ts index 95f31a2ed..dcef0ee87 100644 --- a/packages/aws-cdk/lib/legacy/configuration.ts +++ b/packages/aws-cdk/lib/legacy/configuration.ts @@ -307,6 +307,7 @@ function commandLineArgumentsToSettings(argv: Arguments): Settings { rollback: argv.rollback, notices: argv.notices, assetParallelism: argv['asset-parallelism'], + assetBuildConcurrency: argv['asset-build-concurrency'], assetPrebuild: argv['asset-prebuild'], ignoreNoStacks: argv['ignore-no-stacks'], hotswap: { diff --git a/packages/aws-cdk/test/cli/cdk-toolkit.test.ts b/packages/aws-cdk/test/cli/cdk-toolkit.test.ts index 45c611918..732f1e4b4 100644 --- a/packages/aws-cdk/test/cli/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cli/cdk-toolkit.test.ts @@ -64,7 +64,7 @@ import type { DestroyStackResult } from '@aws-cdk/toolkit-lib/lib/api/deployment import { DescribeStacksCommand, GetTemplateCommand, StackStatus } from '@aws-sdk/client-cloudformation'; import { GetParameterCommand } from '@aws-sdk/client-ssm'; import * as fs from 'fs-extra'; -import type { Template, SdkProvider } from '../../lib/api'; +import { type Template, type SdkProvider, WorkGraphBuilder } from '../../lib/api'; import { Bootstrapper, type BootstrapSource } from '../../lib/api/bootstrap'; import type { DeployStackResult, @@ -338,6 +338,122 @@ describe('deploy', () => { publishEntry.mockRestore(); }); + describe('assetBuildConcurrency', () => { + let buildSpy: jest.SpyInstance; + + afterEach(() => { + buildSpy?.mockRestore(); + }); + + test('is passed when assetParallelism is true', async () => { + cloudExecutable = await MockCloudExecutable.create({ + stacks: [MockStack.MOCK_STACK_WITH_ASSET], + }); + const deployments = new FakeCloudFormation({}); + + const mockWorkGraph = { + doParallel: jest.fn().mockResolvedValue(undefined), + removeUnnecessaryAssets: jest.fn().mockResolvedValue(undefined), + }; + buildSpy = jest.spyOn(WorkGraphBuilder.prototype, 'build').mockReturnValue(mockWorkGraph as any); + + const toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments, + }); + + await toolkit.deploy({ + selector: { patterns: [MockStack.MOCK_STACK_WITH_ASSET.stackName] }, + deploymentMethod: { method: 'change-set' }, + assetParallelism: true, + assetBuildConcurrency: 4, + }); + + expect(mockWorkGraph.doParallel).toHaveBeenCalledWith( + expect.objectContaining({ + 'asset-build': 4, + }), + expect.anything(), + ); + }); + + test('is ignored when assetParallelism is false', async () => { + cloudExecutable = await MockCloudExecutable.create({ + stacks: [MockStack.MOCK_STACK_WITH_ASSET], + }); + const deployments = new FakeCloudFormation({}); + + const mockWorkGraph = { + doParallel: jest.fn().mockResolvedValue(undefined), + removeUnnecessaryAssets: jest.fn().mockResolvedValue(undefined), + }; + buildSpy = jest.spyOn(WorkGraphBuilder.prototype, 'build').mockReturnValue(mockWorkGraph as any); + + const toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments, + }); + + await toolkit.deploy({ + selector: { patterns: [MockStack.MOCK_STACK_WITH_ASSET.stackName] }, + deploymentMethod: { method: 'change-set' }, + assetParallelism: false, + assetBuildConcurrency: 4, + }); + + expect(mockWorkGraph.doParallel).toHaveBeenCalledWith( + expect.objectContaining({ + 'asset-build': 1, + }), + expect.anything(), + ); + }); + + test.each([ + true, + false, + undefined, + ])('defaults to 1 when assetParallelism=%s and assetBuildConcurrency is not specified', async (assetParallelism) => { + cloudExecutable = await MockCloudExecutable.create({ + stacks: [MockStack.MOCK_STACK_WITH_ASSET], + }); + const deployments = new FakeCloudFormation({}); + + const mockWorkGraph = { + doParallel: jest.fn().mockResolvedValue(undefined), + removeUnnecessaryAssets: jest.fn().mockResolvedValue(undefined), + }; + buildSpy = jest.spyOn(WorkGraphBuilder.prototype, 'build').mockReturnValue(mockWorkGraph as any); + + const toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments, + }); + + await toolkit.deploy({ + selector: { patterns: [MockStack.MOCK_STACK_WITH_ASSET.stackName] }, + deploymentMethod: { method: 'change-set' }, + assetParallelism, + }); + + expect(mockWorkGraph.doParallel).toHaveBeenCalledWith( + expect.objectContaining({ + 'asset-build': 1, + }), + expect.anything(), + ); + }); + }); + test('with stacks all stacks specified as wildcard', async () => { // GIVEN const toolkit = defaultToolkitSetup(); diff --git a/packages/aws-cdk/test/cli/cli-arguments.test.ts b/packages/aws-cdk/test/cli/cli-arguments.test.ts index 3c3269361..80dcbe93f 100644 --- a/packages/aws-cdk/test/cli/cli-arguments.test.ts +++ b/packages/aws-cdk/test/cli/cli-arguments.test.ts @@ -41,6 +41,7 @@ describe('yargs', () => { STACKS: undefined, all: false, assetParallelism: undefined, + assetBuildConcurrency: 1, assetPrebuild: true, buildExclude: [], changeSetName: undefined,