From 13d2059a3d5e97af4fe3a08f3a35862b7c5aa9a6 Mon Sep 17 00:00:00 2001 From: Samruddhi Khandale Date: Thu, 23 Jun 2022 21:55:05 +0000 Subject: [PATCH 1/4] support multiple --image-name params --- src/spec-node/devContainersSpecCLI.ts | 38 +++++++++++++++++---------- src/spec-node/singleContainer.ts | 23 +++++++++------- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index 26acbbddb..d26bbf0f9 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/src/spec-node/devContainersSpecCLI.ts @@ -328,18 +328,24 @@ async function doBuild({ throw new ContainerError({ description: `Dev container config (${uriToFsPath(configFile || getDefaultDevContainerConfigPath(cliHost, workspace!.configFolderPath), cliHost.platform)}) not found.` }); } const { config } = configs; - let imageNameResult = ''; + let imageNameResult = ['']; + + // Support multiple use of `--image-name` + const imageNames = (argImageName && (Array.isArray(argImageName) ? argImageName : [argImageName]) as string[]) || undefined; if (isDockerFileConfig(config)) { // Build the base image and extend with features etc. - const { updatedImageName } = await buildNamedImageAndExtend(params, config, argImageName); - - if (argImageName) { - if (!buildxPush) { - await dockerPtyCLI(params, 'tag', updatedImageName, argImageName); + let { updatedImageName } = await buildNamedImageAndExtend(params, config, imageNames); + updatedImageName = (updatedImageName && (Array.isArray(updatedImageName) ? updatedImageName : [updatedImageName]) as string[]) || ['']; + + if (imageNames) { + if (!buildxPush && typeof updatedImageName === 'string') { + imageNames.forEach(async function (image) { + await dockerPtyCLI(params, 'tag', JSON.stringify(updatedImageName), image); + }); } - imageNameResult = argImageName; + imageNameResult = imageNames; } else { imageNameResult = updatedImageName; } @@ -374,9 +380,11 @@ async function doBuild({ const service = composeConfig.services[config.service]; const originalImageName = service.image || `${projectName}_${config.service}`; - if (argImageName) { - await dockerPtyCLI(params, 'tag', originalImageName, argImageName); - imageNameResult = argImageName; + if (imageNames) { + imageNames.forEach(async function (image) { + await dockerPtyCLI(params, 'tag', originalImageName, image); + }); + imageNameResult = imageNames; } else { imageNameResult = originalImageName; } @@ -388,11 +396,13 @@ async function doBuild({ if (buildxPlatform || buildxPush) { throw new ContainerError({ description: '--platform or --push require dockerfilePath.' }); } - if (argImageName) { - await dockerPtyCLI(params, 'tag', updatedImageName, argImageName); - imageNameResult = argImageName; + if (imageNames) { + imageNames.forEach(async function (image) { + await dockerPtyCLI(params, 'tag', updatedImageName, image); + }); + imageNameResult = imageNames; } else { - imageNameResult = updatedImageName; + imageNameResult = [updatedImageName]; } } diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index 1c0e6477b..a94080b5c 100644 --- a/src/spec-node/singleContainer.ts +++ b/src/spec-node/singleContainer.ts @@ -36,8 +36,9 @@ export async function openDockerfileDevContainer(params: DockerResolverParameter // }; await startExistingContainer(params, idLabels, container); } else { + // check const res = await buildNamedImageAndExtend(params, config); - const updatedImageName = await updateRemoteUserUID(params, config, res.updatedImageName, res.imageDetails, findUserArg(config.runArgs) || config.containerUser); + const updatedImageName = await updateRemoteUserUID(params, config, res.updatedImageName[0], res.imageDetails, findUserArg(config.runArgs) || config.containerUser); // collapsedFeaturesConfig = async () => res.collapsedFeaturesConfig; @@ -103,17 +104,17 @@ async function setupContainer(container: ContainerDetails, params: DockerResolve function getDefaultName(config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig, params: DockerResolverParameters) { return 'image' in config ? config.image : getFolderImageName(params.common); } -export async function buildNamedImageAndExtend(params: DockerResolverParameters, config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig, argImageName?: string) { - const imageName = argImageName ?? getDefaultName(config, params); +export async function buildNamedImageAndExtend(params: DockerResolverParameters, config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig, argImageNames?: string[]) { + const imageNames = argImageNames ?? [getDefaultName(config, params)]; params.common.progress(ResolverProgress.BuildingImage); if (isDockerFileConfig(config)) { - return await buildAndExtendImage(params, config, imageName, params.buildNoCache ?? false); + return await buildAndExtendImage(params, config, imageNames, params.buildNoCache ?? false); } // image-based dev container - extend - return await extendImage(params, config, imageName, 'image' in config); + return await extendImage(params, config, imageNames[0], 'image' in config); } -async function buildAndExtendImage(buildParams: DockerResolverParameters, config: DevContainerFromDockerfileConfig, baseImageName: string, noCache: boolean) { +async function buildAndExtendImage(buildParams: DockerResolverParameters, config: DevContainerFromDockerfileConfig, baseImageNames: string[], noCache: boolean) { const { cliHost, output } = buildParams.common; const dockerfileUri = getDockerfilePath(cliHost, config); const dockerfilePath = await uriToWSLFsPath(dockerfileUri, cliHost); @@ -180,7 +181,11 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config } else { args.push('build'); } - args.push('-f', finalDockerfilePath, '-t', baseImageName); + args.push('-f', finalDockerfilePath); + + baseImageNames.forEach(function(image) { + args.push('-t', image); + }); const target = config.build?.target; if (target) { @@ -223,10 +228,10 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config throw new ContainerError({ description: 'An error occurred building the image.', originalError: err, data: { fileWithError: dockerfilePath } }); } - const imageDetails = () => inspectDockerImage(buildParams, baseImageName, false); + const imageDetails = () => inspectDockerImage(buildParams, baseImageNames[0], false); return { - updatedImageName: baseImageName, + updatedImageName: baseImageNames, collapsedFeaturesConfig: extendImageBuildInfo?.collapsedFeaturesConfig, imageDetails }; From 1b598c36c8282628ae99a16277cba2af603a4894 Mon Sep 17 00:00:00 2001 From: Samruddhi Khandale Date: Fri, 24 Jun 2022 16:15:24 +0000 Subject: [PATCH 2/4] fix tests --- src/spec-node/containerFeatures.ts | 4 ++-- src/spec-node/devContainersSpecCLI.ts | 4 ++-- src/spec-node/singleContainer.ts | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 36844c4a8..285dc4970 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -45,7 +45,7 @@ export async function extendImage(params: DockerResolverParameters, config: DevC if (!extendImageDetails || !extendImageDetails.featureBuildInfo) { // no feature extensions - return return { - updatedImageName: imageName, + updatedImageName: [imageName], collapsedFeaturesConfig: undefined, imageDetails }; @@ -93,7 +93,7 @@ export async function extendImage(params: DockerResolverParameters, config: DevC const infoParams = { ...toExecParameters(params), output: makeLog(output, LogLevel.Info), print: 'continuous' as 'continuous' }; await dockerCLI(infoParams, ...args); } - return { updatedImageName, collapsedFeaturesConfig, imageDetails }; + return { updatedImageName:[updatedImageName], collapsedFeaturesConfig, imageDetails }; } export async function getExtendImageBuildInfo(params: DockerResolverParameters, config: DevContainerConfig, baseName: string, imageUser: string, imageLabelDetails: () => Promise<{ definition: string | undefined; version: string | undefined }>) { diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index d26bbf0f9..71cd9875c 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/src/spec-node/devContainersSpecCLI.ts @@ -398,11 +398,11 @@ async function doBuild({ } if (imageNames) { imageNames.forEach(async function (image) { - await dockerPtyCLI(params, 'tag', updatedImageName, image); + await dockerPtyCLI(params, 'tag', updatedImageName[0], image); }); imageNameResult = imageNames; } else { - imageNameResult = [updatedImageName]; + imageNameResult = updatedImageName; } } diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index a94080b5c..17f91cd53 100644 --- a/src/spec-node/singleContainer.ts +++ b/src/spec-node/singleContainer.ts @@ -36,7 +36,6 @@ export async function openDockerfileDevContainer(params: DockerResolverParameter // }; await startExistingContainer(params, idLabels, container); } else { - // check const res = await buildNamedImageAndExtend(params, config); const updatedImageName = await updateRemoteUserUID(params, config, res.updatedImageName[0], res.imageDetails, findUserArg(config.runArgs) || config.containerUser); From ce490f92dddb4eb7b3b2519fcc5c05309676c06c Mon Sep 17 00:00:00 2001 From: Samruddhi Khandale Date: Fri, 24 Jun 2022 16:33:11 +0000 Subject: [PATCH 3/4] add tests --- src/test/cli.test.ts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/test/cli.test.ts b/src/test/cli.test.ts index 530ff33bb..497ee7926 100644 --- a/src/test/cli.test.ts +++ b/src/test/cli.test.ts @@ -198,6 +198,39 @@ describe('Dev Containers CLI', function () { } assert.equal(success, false, 'expect non-successful call'); }); + + it('should succeed with multiple --image-name parameters when DockerFile is present', async () => { + const testFolder = `${__dirname}/configs/dockerfile-with-features`; + const image1 = 'image-1'; + const image2 = 'image-2'; + const res = await shellExec(`${cli} build --workspace-folder ${testFolder} --image-name ${image1} --image-name ${image2}`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + assert.equal(response.imageName[0], image1); + assert.equal(response.imageName[1], image2); + }); + + it('should succeed with multiple --image-name parameters when dockerComposeFile is present', async () => { + const testFolder = `${__dirname}/configs/compose-Dockerfile-alpine`; + const image1 = 'image-1'; + const image2 = 'image-2'; + const res = await shellExec(`${cli} build --workspace-folder ${testFolder} --image-name ${image1} --image-name ${image2}`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + assert.equal(response.imageName[0], image1); + assert.equal(response.imageName[1], image2); + }); + + it('should succeed with multiple --image-name parameters when image is present', async () => { + const testFolder = `${__dirname}/configs/image`; + const image1 = 'image-1'; + const image2 = 'image-2'; + const res = await shellExec(`${cli} build --workspace-folder ${testFolder} --image-name ${image1} --image-name ${image2}`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + assert.equal(response.imageName[0], image1); + assert.equal(response.imageName[1], image2); + }); }); describe('Command up', () => { From 65044246c2d73176004cebdd6c00dd2cd9483896 Mon Sep 17 00:00:00 2001 From: Samruddhi Khandale Date: Tue, 28 Jun 2022 23:11:58 +0000 Subject: [PATCH 4/4] addressing comments --- src/spec-node/devContainersSpecCLI.ts | 17 +++++------------ src/spec-node/singleContainer.ts | 6 ++---- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index 71cd9875c..63671ca48 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/src/spec-node/devContainersSpecCLI.ts @@ -328,7 +328,7 @@ async function doBuild({ throw new ContainerError({ description: `Dev container config (${uriToFsPath(configFile || getDefaultDevContainerConfigPath(cliHost, workspace!.configFolderPath), cliHost.platform)}) not found.` }); } const { config } = configs; - let imageNameResult = ['']; + let imageNameResult: string[] = ['']; // Support multiple use of `--image-name` const imageNames = (argImageName && (Array.isArray(argImageName) ? argImageName : [argImageName]) as string[]) || undefined; @@ -337,13 +337,10 @@ async function doBuild({ // Build the base image and extend with features etc. let { updatedImageName } = await buildNamedImageAndExtend(params, config, imageNames); - updatedImageName = (updatedImageName && (Array.isArray(updatedImageName) ? updatedImageName : [updatedImageName]) as string[]) || ['']; if (imageNames) { - if (!buildxPush && typeof updatedImageName === 'string') { - imageNames.forEach(async function (image) { - await dockerPtyCLI(params, 'tag', JSON.stringify(updatedImageName), image); - }); + if (!buildxPush) { + await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName))); } imageNameResult = imageNames; } else { @@ -381,9 +378,7 @@ async function doBuild({ const originalImageName = service.image || `${projectName}_${config.service}`; if (imageNames) { - imageNames.forEach(async function (image) { - await dockerPtyCLI(params, 'tag', originalImageName, image); - }); + await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', originalImageName, imageName))); imageNameResult = imageNames; } else { imageNameResult = originalImageName; @@ -397,9 +392,7 @@ async function doBuild({ throw new ContainerError({ description: '--platform or --push require dockerfilePath.' }); } if (imageNames) { - imageNames.forEach(async function (image) { - await dockerPtyCLI(params, 'tag', updatedImageName[0], image); - }); + await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName))); imageNameResult = imageNames; } else { imageNameResult = updatedImageName; diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index 17f91cd53..9d4f6c158 100644 --- a/src/spec-node/singleContainer.ts +++ b/src/spec-node/singleContainer.ts @@ -181,10 +181,8 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config args.push('build'); } args.push('-f', finalDockerfilePath); - - baseImageNames.forEach(function(image) { - args.push('-t', image); - }); + + baseImageNames.map(imageName => args.push('-t', imageName)); const target = config.build?.target; if (target) {