From 334c7b7ea74b015f10f3702f9d346d9daa2226bf Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Fri, 15 Feb 2019 22:22:02 +0100 Subject: [PATCH 1/7] feat(toolkit): improve docker build time in CI When running in CI, try to pull the latest image first and use it as cache for the build. CI is detected by the presence of the `CI` environment variable. Closes #1748 --- packages/aws-cdk/lib/api/toolkit-info.ts | 88 ++++++++++++++---------- packages/aws-cdk/lib/docker.ts | 56 ++++++++++++--- 2 files changed, 98 insertions(+), 46 deletions(-) diff --git a/packages/aws-cdk/lib/api/toolkit-info.ts b/packages/aws-cdk/lib/api/toolkit-info.ts index 275c71b16ebd3..68a4aa7852e01 100644 --- a/packages/aws-cdk/lib/api/toolkit-info.ts +++ b/packages/aws-cdk/lib/api/toolkit-info.ts @@ -99,11 +99,11 @@ export class ToolkitInfo { /** * Prepare an ECR repository for uploading to using Docker */ - public async prepareEcrRepository(id: string, imageTag: string): Promise { + public async prepareEcrRepository(assetId: string): Promise { const ecr = await this.props.sdk.ecr(this.props.environment, Mode.ForWriting); - // Create the repository if it doesn't exist yet - const repositoryName = 'cdk/' + id.replace(/[:/]/g, '-').toLowerCase(); + // Repository name based on asset id + const repositoryName = 'cdk/' + assetId.replace(/[:/]/g, '-').toLowerCase(); let repository; try { @@ -115,32 +115,34 @@ export class ToolkitInfo { } if (repository) { - try { - debug(`${repositoryName}: checking for image ${imageTag}`); - await ecr.describeImages({ repositoryName, imageIds: [{ imageTag }] }).promise(); - - // If we got here, the image already exists. Nothing else needs to be done. - return { - alreadyExists: true, - repositoryUri: repository.repositoryUri!, - repositoryName - }; - } catch (e) { - if (e.code !== 'ImageNotFoundException') { throw e; } - } - } else { - debug(`${repositoryName}: creating`); - const response = await ecr.createRepository({ repositoryName }).promise(); - repository = response.repository!; - - // Better put a lifecycle policy on this so as to not cost too much money - await ecr.putLifecyclePolicy({ - repositoryName, - lifecyclePolicyText: JSON.stringify(DEFAULT_REPO_LIFECYCLE) - }).promise(); + return { + repositoryUri: repository.repositoryUri!, + repositoryName + }; } - // The repo exists, image just needs to be uploaded. Get auth to do so. + debug(`${repositoryName}: creating`); + const response = await ecr.createRepository({ repositoryName }).promise(); + repository = response.repository!; + + // Better put a lifecycle policy on this so as to not cost too much money + await ecr.putLifecyclePolicy({ + repositoryName, + lifecyclePolicyText: JSON.stringify(DEFAULT_REPO_LIFECYCLE) + }).promise(); + + return { + repositoryUri: repository.repositoryUri!, + repositoryName + }; + } + + /** + * Get ECR credentials + */ + public async getEcrCredentials(): Promise { + const ecr = await this.props.sdk.ecr(this.props.environment, Mode.ForReading); + debug(`Fetching ECR authorization token`); const authData = (await ecr.getAuthorizationToken({ }).promise()).authorizationData || []; if (authData.length === 0) { @@ -150,28 +152,38 @@ export class ToolkitInfo { const [username, password] = token.split(':'); return { - alreadyExists: false, - repositoryUri: repository.repositoryUri!, - repositoryName, username, password, endpoint: authData[0].proxyEndpoint!, }; } -} -export type EcrRepositoryInfo = CompleteEcrRepositoryInfo | UploadableEcrRepositoryInfo; + /** + * Check if image already exists in ECR repository + */ + public async checkEcrImage(repositoryName: string, imageTag: string): Promise { + const ecr = await this.props.sdk.ecr(this.props.environment, Mode.ForReading); -export interface CompleteEcrRepositoryInfo { - repositoryUri: string; - repositoryName: string; - alreadyExists: true; + try { + debug(`${repositoryName}: checking for image ${imageTag}`); + await ecr.describeImages({ repositoryName, imageIds: [{ imageTag }] }).promise(); + + // If we got here, the image already exists. Nothing else needs to be done. + return true; + } catch (e) { + if (e.code !== 'ImageNotFoundException') { throw e; } + } + + return false; + } } -export interface UploadableEcrRepositoryInfo { +export interface EcrRepositoryInfo { repositoryUri: string; repositoryName: string; - alreadyExists: false; +} + +export interface EcrCredentials { username: string; password: string; endpoint: string; diff --git a/packages/aws-cdk/lib/docker.ts b/packages/aws-cdk/lib/docker.ts index 811ada8baed1b..7a8a43f1bbb65 100644 --- a/packages/aws-cdk/lib/docker.ts +++ b/packages/aws-cdk/lib/docker.ts @@ -18,43 +18,69 @@ import { PleaseHold } from './util/please-hold'; * * As a workaround, we calculate our own digest over parts of the manifest that * are unlikely to change, and tag based on that. + * + * When running in CI, we pull the latest image first and use it as cache for + * the build. CI is detected by the presence of the `CI` environment variable. */ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEntry, toolkitInfo: ToolkitInfo): Promise { debug(' 👑 Preparing Docker image asset:', asset.path); const buildHold = new PleaseHold(` ⌛ Building Docker image for ${asset.path}; this may take a while.`); try { + const ecr = await toolkitInfo.prepareEcrRepository(asset.id); + const latest = `${ecr.repositoryUri}:latest`; + + let loggedIn = false; + + // In CI we try to pull latest first + if (process.env.CI) { + await dockerLogin(toolkitInfo); + loggedIn = true; + + try { + await shell(['docker', 'pull', latest]); + } catch (e) { + debug('Failed to pull latest image from ECR repository'); + } + } + buildHold.start(); - const command = ['docker', + const baseCommand = ['docker', 'build', '--quiet', asset.path]; + const command = process.env.CI + ? [...baseCommand, '--cache-from', latest] // This does not fail if latest is not available + : baseCommand; const imageId = (await shell(command, { quiet: true })).trim(); + buildHold.stop(); const tag = await calculateImageFingerprint(imageId); - debug(` ⌛ Image has tag ${tag}, preparing ECR repository`); - const ecr = await toolkitInfo.prepareEcrRepository(asset.id, tag); + debug(` ⌛ Image has tag ${tag}, checking ECR repository`); + const imageExists = await toolkitInfo.checkEcrImage(ecr.repositoryName, tag); - if (ecr.alreadyExists) { + if (imageExists) { debug(' 👑 Image already uploaded.'); } else { // Login and push debug(` ⌛ Image needs to be uploaded first.`); - await shell(['docker', 'login', - '--username', ecr.username, - '--password', ecr.password, - ecr.endpoint]); + if (!loggedIn) { // We could be already logged in if in CI + await dockerLogin(toolkitInfo); + } const qualifiedImageName = `${ecr.repositoryUri}:${tag}`; + await shell(['docker', 'tag', imageId, qualifiedImageName]); + await shell(['docker', 'tag', imageId, latest]); // Tag with `latest` also // There's no way to make this quiet, so we can't use a PleaseHold. Print a header message. print(` ⌛ Pusing Docker image for ${asset.path}; this may take a while.`); await shell(['docker', 'push', qualifiedImageName]); + await shell(['docker', 'push', latest]); debug(` 👑 Docker image for ${asset.path} pushed.`); } @@ -72,6 +98,17 @@ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEn } } +/** + * Get credentials from ECR and run docker login + */ +async function dockerLogin(toolkitInfo: ToolkitInfo) { + const credentials = await toolkitInfo.getEcrCredentials(); + await shell(['docker', 'login', + '--username', credentials.username, + '--password', credentials.password, + credentials.endpoint]); +} + /** * Calculate image fingerprint. * @@ -95,6 +132,9 @@ async function calculateImageFingerprint(imageId: string) { // Metadata that has no bearing on the image contents delete manifest.Created; + // Parent can change when using --cache-from in CI + delete manifest.Parent; + // We're interested in the image itself, not any running instaces of it delete manifest.Container; delete manifest.ContainerConfig; From 7866319839423b6513280c350232df498c952117 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Mon, 18 Feb 2019 15:51:50 +0100 Subject: [PATCH 2/7] Remove delete manifest.Parent --- packages/aws-cdk/lib/docker.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/aws-cdk/lib/docker.ts b/packages/aws-cdk/lib/docker.ts index 7a8a43f1bbb65..4addf624c83d0 100644 --- a/packages/aws-cdk/lib/docker.ts +++ b/packages/aws-cdk/lib/docker.ts @@ -132,9 +132,6 @@ async function calculateImageFingerprint(imageId: string) { // Metadata that has no bearing on the image contents delete manifest.Created; - // Parent can change when using --cache-from in CI - delete manifest.Parent; - // We're interested in the image itself, not any running instaces of it delete manifest.Container; delete manifest.ContainerConfig; From d9f3cc5fc97c3791c30308ec1808b436668adda6 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Mon, 18 Feb 2019 15:55:08 +0100 Subject: [PATCH 3/7] Add `--ci` option to `deploy` --- packages/aws-cdk/bin/cdk.ts | 8 +++++--- packages/aws-cdk/lib/api/deploy-stack.ts | 3 ++- packages/aws-cdk/lib/assets.ts | 8 ++++---- packages/aws-cdk/lib/docker.ts | 9 ++++++--- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index ff07687969d03..94eda56cf5ad1 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -61,6 +61,7 @@ async function parseCommandLineArguments() { .command('deploy [STACKS..]', 'Deploys the stack(s) named STACKS into your AWS account', yargs => yargs .option('exclusively', { type: 'boolean', alias: 'e', desc: 'only deploy requested stacks, don\'t include dependencies' }) .option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'what security-sensitive changes need manual approval' })) + .option('ci', { type: 'boolean', desc: 'Force CI detection. Use --no-ci to disable CI autodetection.', default: undefined }) .command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs .option('exclusively', { type: 'boolean', alias: 'x', desc: 'only deploy requested stacks, don\'t include dependees' }) .option('force', { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' })) @@ -172,7 +173,7 @@ async function initCommandLine() { return await cliBootstrap(args.ENVIRONMENTS, toolkitStackName, args.roleArn); case 'deploy': - return await cliDeploy(args.STACKS, args.exclusively, toolkitStackName, args.roleArn, configuration.combined.get(['requireApproval'])); + return await cliDeploy(args.STACKS, args.exclusively, toolkitStackName, args.roleArn, configuration.combined.get(['requireApproval']), args.ci); case 'destroy': return await cliDestroy(args.STACKS, args.exclusively, args.force, args.roleArn); @@ -324,7 +325,8 @@ async function initCommandLine() { exclusively: boolean, toolkitStackName: string, roleArn: string | undefined, - requireApproval: RequireApproval) { + requireApproval: RequireApproval, + ci?: boolean) { if (requireApproval === undefined) { requireApproval = RequireApproval.Broadening; } const stacks = await appStacks.selectStacks(stackNames, exclusively ? ExtendedStackSelection.None : ExtendedStackSelection.Upstream); @@ -362,7 +364,7 @@ async function initCommandLine() { } try { - const result = await deployStack({ stack, sdk: aws, toolkitInfo, deployName, roleArn }); + const result = await deployStack({ stack, sdk: aws, toolkitInfo, deployName, roleArn, ci }); const message = result.noOp ? ` ✅ %s (no changes)` : ` ✅ %s`; diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index 8d592b1948739..3ad0c95b8e0aa 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -30,6 +30,7 @@ export interface DeployStackOptions { roleArn?: string; deployName?: string; quiet?: boolean; + ci?: boolean; } const LARGE_TEMPLATE_SIZE_KB = 50; @@ -39,7 +40,7 @@ export async function deployStack(options: DeployStackOptions): Promise { +export async function prepareAssets(stack: SynthesizedStack, toolkitInfo?: ToolkitInfo, ci?: boolean): Promise { const assets = findAssets(stack.metadata); if (assets.length === 0) { return []; @@ -26,13 +26,13 @@ export async function prepareAssets(stack: SynthesizedStack, toolkitInfo?: Toolk for (const asset of assets) { debug(` - ${asset.path} (${asset.packaging})`); - params = params.concat(await prepareAsset(asset, toolkitInfo)); + params = params.concat(await prepareAsset(asset, toolkitInfo, ci)); } return params; } -async function prepareAsset(asset: AssetMetadataEntry, toolkitInfo: ToolkitInfo): Promise { +async function prepareAsset(asset: AssetMetadataEntry, toolkitInfo: ToolkitInfo, ci?: boolean): Promise { debug('Preparing asset', JSON.stringify(asset)); switch (asset.packaging) { case 'zip': @@ -40,7 +40,7 @@ async function prepareAsset(asset: AssetMetadataEntry, toolkitInfo: ToolkitInfo) case 'file': return await prepareFileAsset(asset, toolkitInfo); case 'container-image': - return await prepareContainerAsset(asset, toolkitInfo); + return await prepareContainerAsset(asset, toolkitInfo, ci); default: // tslint:disable-next-line:max-line-length throw new Error(`Unsupported packaging type: ${(asset as any).packaging}. You might need to upgrade your aws-cdk toolkit to support this asset type.`); diff --git a/packages/aws-cdk/lib/docker.ts b/packages/aws-cdk/lib/docker.ts index 4addf624c83d0..c30f0d3984d2b 100644 --- a/packages/aws-cdk/lib/docker.ts +++ b/packages/aws-cdk/lib/docker.ts @@ -20,9 +20,12 @@ import { PleaseHold } from './util/please-hold'; * are unlikely to change, and tag based on that. * * When running in CI, we pull the latest image first and use it as cache for - * the build. CI is detected by the presence of the `CI` environment variable. + * the build. CI is detected by the presence of the `CI` environment variable or + * the `--ci` command line option. */ -export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEntry, toolkitInfo: ToolkitInfo): Promise { +export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEntry, + toolkitInfo: ToolkitInfo, + ci?: boolean): Promise { debug(' 👑 Preparing Docker image asset:', asset.path); const buildHold = new PleaseHold(` ⌛ Building Docker image for ${asset.path}; this may take a while.`); @@ -33,7 +36,7 @@ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEn let loggedIn = false; // In CI we try to pull latest first - if (process.env.CI) { + if (ci === true || (process.env.CI && ci !== false)) { await dockerLogin(toolkitInfo); loggedIn = true; From e2df6f144a02146a4f47063a36f641f4f62db642 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Mon, 18 Feb 2019 16:31:52 +0100 Subject: [PATCH 4/7] Always tag and push latest --- packages/aws-cdk/lib/docker.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/docker.ts b/packages/aws-cdk/lib/docker.ts index c30f0d3984d2b..f549b828a7f6c 100644 --- a/packages/aws-cdk/lib/docker.ts +++ b/packages/aws-cdk/lib/docker.ts @@ -73,20 +73,27 @@ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEn if (!loggedIn) { // We could be already logged in if in CI await dockerLogin(toolkitInfo); + loggedIn = true; } const qualifiedImageName = `${ecr.repositoryUri}:${tag}`; await shell(['docker', 'tag', imageId, qualifiedImageName]); - await shell(['docker', 'tag', imageId, latest]); // Tag with `latest` also // There's no way to make this quiet, so we can't use a PleaseHold. Print a header message. print(` ⌛ Pusing Docker image for ${asset.path}; this may take a while.`); await shell(['docker', 'push', qualifiedImageName]); - await shell(['docker', 'push', latest]); debug(` 👑 Docker image for ${asset.path} pushed.`); } + if (!loggedIn) { // We could be already logged in if in CI or if image did not exist + await dockerLogin(toolkitInfo); + } + + // Always tag and push latest + await shell(['docker', 'tag', imageId, latest]); + await shell(['docker', 'push', latest]); + return [ { ParameterKey: asset.imageNameParameter, ParameterValue: `${ecr.repositoryName}:${tag}` }, ]; From 67947f67eef1c1dd9e92b1b503b1b9c9cd0aa8f6 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Mon, 18 Feb 2019 17:22:40 +0100 Subject: [PATCH 5/7] Add code comment --- packages/aws-cdk/lib/docker.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/docker.ts b/packages/aws-cdk/lib/docker.ts index f549b828a7f6c..8e6548e5304d5 100644 --- a/packages/aws-cdk/lib/docker.ts +++ b/packages/aws-cdk/lib/docker.ts @@ -20,7 +20,12 @@ import { PleaseHold } from './util/please-hold'; * are unlikely to change, and tag based on that. * * When running in CI, we pull the latest image first and use it as cache for - * the build. CI is detected by the presence of the `CI` environment variable or + * the build. Generally pulling will be faster than building, especially for + * Dockerfiles with lots of OS/code packages installation or changes only in + * the bottom layers. When running locally chances are that we already have + * layers cache available. + * + * CI is detected by the presence of the `CI` environment variable or * the `--ci` command line option. */ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEntry, From 5238eb42846e558d66810494a9dabd4ef959ecf2 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 20 Feb 2019 16:27:05 +0100 Subject: [PATCH 6/7] Move CI autodetection to cdk.ts --- packages/aws-cdk/bin/cdk.ts | 2 +- packages/aws-cdk/lib/docker.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 94eda56cf5ad1..80e576dd073f9 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -61,7 +61,7 @@ async function parseCommandLineArguments() { .command('deploy [STACKS..]', 'Deploys the stack(s) named STACKS into your AWS account', yargs => yargs .option('exclusively', { type: 'boolean', alias: 'e', desc: 'only deploy requested stacks, don\'t include dependencies' }) .option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'what security-sensitive changes need manual approval' })) - .option('ci', { type: 'boolean', desc: 'Force CI detection. Use --no-ci to disable CI autodetection.', default: undefined }) + .option('ci', { type: 'boolean', desc: 'Force CI detection. Use --no-ci to disable CI autodetection.', default: process.env.CI !== undefined }) .command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs .option('exclusively', { type: 'boolean', alias: 'x', desc: 'only deploy requested stacks, don\'t include dependees' }) .option('force', { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' })) diff --git a/packages/aws-cdk/lib/docker.ts b/packages/aws-cdk/lib/docker.ts index 8e6548e5304d5..d9a092120e13f 100644 --- a/packages/aws-cdk/lib/docker.ts +++ b/packages/aws-cdk/lib/docker.ts @@ -41,7 +41,7 @@ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEn let loggedIn = false; // In CI we try to pull latest first - if (ci === true || (process.env.CI && ci !== false)) { + if (ci) { await dockerLogin(toolkitInfo); loggedIn = true; @@ -58,7 +58,7 @@ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEn 'build', '--quiet', asset.path]; - const command = process.env.CI + const command = ci ? [...baseCommand, '--cache-from', latest] // This does not fail if latest is not available : baseCommand; const imageId = (await shell(command, { quiet: true })).trim(); From d35682e640a291f619af1f1f7181dee9744de0a1 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 20 Feb 2019 19:11:01 +0100 Subject: [PATCH 7/7] Make ci required in cliDeploy --- packages/aws-cdk/bin/cdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 80e576dd073f9..ca4ff1c8b90e4 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -326,7 +326,7 @@ async function initCommandLine() { toolkitStackName: string, roleArn: string | undefined, requireApproval: RequireApproval, - ci?: boolean) { + ci: boolean) { if (requireApproval === undefined) { requireApproval = RequireApproval.Broadening; } const stacks = await appStacks.selectStacks(stackNames, exclusively ? ExtendedStackSelection.None : ExtendedStackSelection.Upstream);