-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(toolkit): improve docker build time in CI #1776
Changes from 5 commits
334c7b7
7866319
d9f3cc5
e2df6f1
67947f6
5238eb4
d35682e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,38 +18,71 @@ 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. 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, toolkitInfo: ToolkitInfo): Promise<CloudFormation.Parameter[]> { | ||
export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEntry, | ||
toolkitInfo: ToolkitInfo, | ||
ci?: boolean): Promise<CloudFormation.Parameter[]> { | ||
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 (ci === true || (process.env.CI && ci !== false)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't do the CI autodetection here, move it up to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. I have a script that recreates the 'cdk --help' part of the tools section whenever we have a new release. |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to use the variable here. |
||
? [...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); | ||
loggedIn = true; | ||
} | ||
|
||
const qualifiedImageName = `${ecr.repositoryUri}:${tag}`; | ||
|
||
await shell(['docker', 'tag', imageId, qualifiedImageName]); | ||
|
||
// There's no way to make this quiet, so we can't use a PleaseHold. Print a header message. | ||
|
@@ -58,6 +91,14 @@ export async function prepareContainerAsset(asset: ContainerImageAssetMetadataEn | |
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}` }, | ||
]; | ||
|
@@ -72,6 +113,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. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rix0rrr The description makes no reference to docker/asset preparation. I thought that this option could be used elsewhere in the future. What about the documentation for this new option? I see that https://github.com/awsdocs/aws-cdk-user-guide/blob/master/doc_source/tools.md is not up to date...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems good. @Doug-AWS will update the tools section once we release.
Please follow this by something like:
To make sure that afterwards
argv.ci
is well-defined. Saves having to redo the CI-autodetection logic in every place where it's used. Configuring:To force the default value to either
true
orfalse
might also work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tools is updated. I should have a new public build of the guide by EOW.