Skip to content
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

Add --output option to devcontainer build #166

Merged
merged 16 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/spec-common/injectHeadless.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export interface ResolverParameters {
remoteEnv: Record<string, string>;
buildxPlatform: string | undefined;
buildxPush: boolean;
buildxOutput: string | undefined;
skipFeatureAutoMapping: boolean;
skipPostAttach: boolean;
experimentalImageMetadata: boolean;
Expand Down
3 changes: 3 additions & 0 deletions src/spec-node/devContainers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export interface ProvisionOptions {
omitLoggerHeader?: boolean | undefined;
buildxPlatform: string | undefined;
buildxPush: boolean;
buildxOutput: string | undefined;
skipFeatureAutoMapping: boolean;
skipPostAttach: boolean;
experimentalImageMetadata: boolean;
Expand Down Expand Up @@ -125,6 +126,7 @@ export async function createDockerParams(options: ProvisionOptions, disposables:
remoteEnv,
buildxPlatform: options.buildxPlatform,
buildxPush: options.buildxPush,
buildxOutput: options.buildxOutput,
skipFeatureAutoMapping: options.skipFeatureAutoMapping,
skipPostAttach: options.skipPostAttach,
experimentalImageMetadata: options.experimentalImageMetadata,
Expand Down Expand Up @@ -165,6 +167,7 @@ export async function createDockerParams(options: ProvisionOptions, disposables:
isTTY: process.stdin.isTTY || options.logFormat === 'json',
buildxPlatform: common.buildxPlatform,
buildxPush: common.buildxPush,
buildxOutput: common.buildxOutput,
};
}

Expand Down
19 changes: 18 additions & 1 deletion src/spec-node/devContainersSpecCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ async function provision({
useBuildKit: buildkit,
buildxPlatform: undefined,
buildxPush: false,
buildxOutput: undefined,
skipFeatureAutoMapping,
skipPostAttach,
experimentalImageMetadata,
Expand Down Expand Up @@ -270,6 +271,7 @@ function buildOptions(y: Argv) {
'buildkit': { choices: ['auto' as 'auto', 'never' as 'never'], default: 'auto' as 'auto', description: 'Control whether BuildKit should be used' },
'platform': { type: 'string', description: 'Set target platforms.' },
'push': { type: 'boolean', default: false, description: 'Push to a container registry.' },
'output': { type: 'string', description: 'Overrides the default behavior to load built images into the local docker registry. Valid options are the same ones provided to the --output option of docker buildx build.' },
'skip-feature-auto-mapping': { type: 'boolean', default: false, hidden: true, description: 'Temporary option for testing.' },
'experimental-image-metadata': { type: 'boolean', default: experimentalImageMetadataDefault, hidden: true, description: 'Temporary option for testing.' },
});
Expand Down Expand Up @@ -302,6 +304,7 @@ async function doBuild({
'buildkit': buildkit,
'platform': buildxPlatform,
'push': buildxPush,
'output': buildxOutput,
'skip-feature-auto-mapping': skipFeatureAutoMapping,
'experimental-image-metadata': experimentalImageMetadata,
}: BuildArgs) {
Expand Down Expand Up @@ -343,6 +346,7 @@ async function doBuild({
useBuildKit: buildkit,
buildxPlatform,
buildxPush,
buildxOutput,
skipFeatureAutoMapping,
skipPostAttach: true,
experimentalImageMetadata,
Expand All @@ -363,6 +367,10 @@ async function doBuild({
const { config } = configWithRaw;
let imageNameResult: string[] = [''];

if (buildxOutput && buildxPush) {
throw new ContainerError({ description: '--push true cannot be used with --output.' });
}

// Support multiple use of `--image-name`
const imageNames = (argImageName && (Array.isArray(argImageName) ? argImageName : [argImageName]) as string[]) || undefined;

Expand All @@ -372,7 +380,7 @@ async function doBuild({
let { updatedImageName } = await buildNamedImageAndExtend(params, configWithRaw as SubstitutedConfig<DevContainerFromDockerfileConfig>, imageNames);

if (imageNames) {
if (!buildxPush) {
if (!buildxPush && !buildxOutput) {
await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName)));
}
imageNameResult = imageNames;
Expand All @@ -385,6 +393,10 @@ async function doBuild({
throw new ContainerError({ description: '--platform or --push not supported.' });
}

if (buildxOutput) {
throw new ContainerError({ description: '--output not supported.' });
}

const cwdEnvFile = cliHost.path.join(cliHost.cwd, '.env');
const envFile = Array.isArray(config.dockerComposeFile) && config.dockerComposeFile.length === 0 && await cliHost.isFile(cwdEnvFile) ? cwdEnvFile : undefined;
const composeFiles = await getDockerComposeFilePaths(cliHost, config, cliHost.env, workspaceFolder);
Expand Down Expand Up @@ -425,6 +437,9 @@ async function doBuild({
if (buildxPlatform || buildxPush) {
throw new ContainerError({ description: '--platform or --push require dockerfilePath.' });
}
if (buildxOutput) {
throw new ContainerError({ description: '--output requires dockerfilePath.' });
}
if (imageNames) {
await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName)));
imageNameResult = imageNames;
Expand Down Expand Up @@ -572,6 +587,7 @@ async function doRunUserCommands({
useBuildKit: 'auto',
buildxPlatform: undefined,
buildxPush: false,
buildxOutput: undefined,
skipFeatureAutoMapping,
skipPostAttach,
experimentalImageMetadata,
Expand Down Expand Up @@ -900,6 +916,7 @@ export async function doExec({
buildxPlatform: undefined,
buildxPush: false,
skipFeatureAutoMapping,
buildxOutput: undefined,
skipPostAttach: false,
experimentalImageMetadata,
}, disposables);
Expand Down
1 change: 1 addition & 0 deletions src/spec-node/featuresCLI/testCommandImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ async function generateDockerParams(workspaceFolder: string, args: FeaturesTestC
useBuildKit: 'auto',
buildxPlatform: undefined,
buildxPush: false,
buildxOutput: undefined,
skipFeatureAutoMapping: false,
skipPostAttach: false,
experimentalImageMetadata: false,
Expand Down
1 change: 1 addition & 0 deletions src/spec-node/featuresCLI/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const staticProvisionParams = {
useBuildKit: 'auto' as 'auto',
buildxPlatform: undefined,
buildxPush: false,
buildxOutput: undefined,
skipPostAttach: false,
};

Expand Down
6 changes: 5 additions & 1 deletion src/spec-node/singleContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,11 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config
if (buildParams.buildxPush) {
args.push('--push');
} else {
args.push('--load'); // (short for --output=docker, i.e. load into normal 'docker images' collection)
if (buildParams.buildxOutput) {
args.push('--output', buildParams.buildxOutput);
} else {
args.push('--load'); // (short for --output=docker, i.e. load into normal 'docker images' collection)
}
}
args.push('--build-arg', 'BUILDKIT_INLINE_CACHE=1');
} else {
Expand Down
1 change: 1 addition & 0 deletions src/spec-node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export interface DockerResolverParameters {
isTTY: boolean;
buildxPlatform: string | undefined;
buildxPush: boolean;
buildxOutput: string | undefined;
}

export interface ResolverResult {
Expand Down
29 changes: 29 additions & 0 deletions src/test/cli.build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import * as fs from 'fs';
import * as assert from 'assert';
import * as path from 'path';
import { buildKitOptions, shellExec } from './testUtils';
Expand Down Expand Up @@ -170,5 +171,33 @@ describe('Dev Containers CLI', function () {
assert.equal(response.imageName[0], image1);
assert.equal(response.imageName[1], image2);
});

it('should fail with --push true and --output', async () => {
let success = false;
const testFolder = `${__dirname}/configs/dockerfile-with-target`;
try {
await shellExec(`${cli} build --workspace-folder ${testFolder} --output type=oci,dest=output.tar --push true`);
} catch (error) {
assert.equal(error.error.code, 1, 'Should fail with exit code 1');
const res = JSON.parse(error.stdout);
assert.equal(res.outcome, 'error');
assert.match(res.message, /cannot be used with/);
}
assert.equal(success, false, 'expect non-successful call');
});

it('file /tmp/output.tar should exist when using --output type=oci,dest=/tmp/output.tar', async () => {
natescherer marked this conversation as resolved.
Show resolved Hide resolved
const testFolder = `${__dirname}/configs/dockerfile-with-target`;
const outputPath = `/tmp/output.tar`;
await shellExec(`docker buildx create --name ocitest'`);
natescherer marked this conversation as resolved.
Show resolved Hide resolved
await shellExec(`docker buildx use ocitest'`);
const res = await shellExec(`${cli} build --workspace-folder ${testFolder} --output 'type=oci,dest=${outputPath}'`);
await shellExec(`docker buildx use default'`);
await shellExec(`docker buildx rm ocitest'`);
const response = JSON.parse(res.stdout);
assert.equal(response.outcome, 'success');
assert.equal(fs.existsSync(outputPath), true);
});

natescherer marked this conversation as resolved.
Show resolved Hide resolved
});
});