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 2 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
2 changes: 2 additions & 0 deletions src/spec-common/injectHeadless.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ export interface ResolverParameters {
buildxPlatform: string | undefined;
buildxPush: boolean;
skipFeatureAutoMapping: boolean;
buildxOutputType: string | undefined;
buildxOutputDest: string | undefined;
}

export interface PostCreate {
Expand Down
6 changes: 6 additions & 0 deletions src/spec-node/devContainers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export interface ProvisionOptions {
buildxPlatform: string | undefined;
buildxPush: boolean;
skipFeatureAutoMapping: boolean;
buildxOutputType: string | undefined;
buildxOutputDest: string | undefined;
}

export async function launch(options: ProvisionOptions, disposables: (() => Promise<unknown> | undefined)[]) {
Expand Down Expand Up @@ -121,6 +123,8 @@ export async function createDockerParams(options: ProvisionOptions, disposables:
buildxPlatform: options.buildxPlatform,
buildxPush: options.buildxPush,
skipFeatureAutoMapping: options.skipFeatureAutoMapping,
buildxOutputType: options.buildxOutputType,
buildxOutputDest: options.buildxOutputDest,
};

const dockerPath = options.dockerPath || 'docker';
Expand Down Expand Up @@ -158,6 +162,8 @@ export async function createDockerParams(options: ProvisionOptions, disposables:
isTTY: process.stdin.isTTY || options.logFormat === 'json',
buildxPlatform: common.buildxPlatform,
buildxPush: common.buildxPush,
buildxOutputType: common.buildxOutputType,
buildxOutputDest: common.buildxOutputDest,
};
}

Expand Down
29 changes: 28 additions & 1 deletion src/spec-node/devContainersSpecCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ async function provision({
buildxPlatform: undefined,
buildxPush: false,
skipFeatureAutoMapping,
buildxOutputType: undefined,
buildxOutputDest: undefined,
};

const result = await doProvision(options);
Expand Down Expand Up @@ -263,6 +265,8 @@ function buildOptions(y: Argv) {
'platform': { type: 'string', description: 'Set target platforms.' },
'push': { type: 'boolean', default: false, description: 'Push to a container registry.' },
'skip-feature-auto-mapping': { type: 'boolean', default: false, hidden: true, description: 'Temporary option for testing.' },
'output-type': { choices: ['local' as 'local', 'tar' as 'tar', 'oci' as 'oci', 'docker' as 'docker', 'image' as 'image', 'registry' as 'registry'], type: 'string', description: 'Overrides the default behavior to load built images into the local docker registry.' },
'output-dest': { type: 'string', description: 'Destination for output files, used in conjunction with --output-type.' },
});
}

Expand Down Expand Up @@ -294,6 +298,8 @@ async function doBuild({
'platform': buildxPlatform,
'push': buildxPush,
'skip-feature-auto-mapping': skipFeatureAutoMapping,
'output-type': buildxOutputType,
'output-dest': buildxOutputDest,
}: BuildArgs) {
const disposables: (() => Promise<unknown> | undefined)[] = [];
const dispose = async () => {
Expand Down Expand Up @@ -334,6 +340,8 @@ async function doBuild({
buildxPlatform,
buildxPush,
skipFeatureAutoMapping,
buildxOutputType,
buildxOutputDest,
}, disposables);

const { common, dockerCLI, dockerComposeCLI } = params;
Expand All @@ -350,6 +358,14 @@ async function doBuild({
const { config } = configs;
let imageNameResult: string[] = [''];

if ((buildxOutputType && !buildxOutputDest) || (!buildxOutputType && buildxOutputDest)) {
throw new ContainerError({ description: '--output-type and --output-dest must be used together.' });
}

if ((buildxOutputType || buildxOutputDest) && (buildxPush)) {
throw new ContainerError({ description: '--push true cannot be used with --output-type or --output-dest.' });
}

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

Expand All @@ -359,7 +375,7 @@ async function doBuild({
let { updatedImageName } = await buildNamedImageAndExtend(params, config, imageNames);

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

if (buildxOutputType || buildxOutputDest) {
throw new ContainerError({ description: '--output-type or --output-dest 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 @@ -411,6 +431,9 @@ async function doBuild({
if (buildxPlatform || buildxPush) {
throw new ContainerError({ description: '--platform or --push require dockerfilePath.' });
}
if (buildxOutputType || buildxOutputDest) {
throw new ContainerError({ description: '--output-type or --output-dest require dockerfilePath.' });
}
if (imageNames) {
await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName)));
imageNameResult = imageNames;
Expand Down Expand Up @@ -555,6 +578,8 @@ async function doRunUserCommands({
buildxPlatform: undefined,
buildxPush: false,
skipFeatureAutoMapping,
buildxOutputType: undefined,
buildxOutputDest: undefined,
}, disposables);

const { common } = params;
Expand Down Expand Up @@ -844,6 +869,8 @@ export async function doExec({
buildxPlatform: undefined,
buildxPush: false,
skipFeatureAutoMapping,
buildxOutputType: undefined,
buildxOutputDest: undefined,
}, disposables);

const { common } = params;
Expand Down
2 changes: 2 additions & 0 deletions src/spec-node/featuresCLI/testCommandImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,5 +459,7 @@ async function generateDockerParams(workspaceFolder: string, args: FeaturesTestC
buildxPlatform: undefined,
buildxPush: false,
skipFeatureAutoMapping: false,
buildxOutputType: undefined,
buildxOutputDest: undefined,
}, disposables);
}
2 changes: 2 additions & 0 deletions src/spec-node/featuresCLI/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export const staticProvisionParams = {
useBuildKit: 'auto' as 'auto',
buildxPlatform: undefined,
buildxPush: false,
buildxOutputType: undefined,
buildxOutputDest: undefined,
};

export const staticExecParams = {
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 @@ -174,7 +174,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.buildxOutputType) {
args.push('--output', `type=${buildParams.buildxOutputType},dest=${buildParams.buildxOutputDest}`);
natescherer marked this conversation as resolved.
Show resolved Hide resolved
} 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
2 changes: 2 additions & 0 deletions src/spec-node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export interface DockerResolverParameters {
isTTY: boolean;
buildxPlatform: string | undefined;
buildxPush: boolean;
buildxOutputType: string | undefined;
buildxOutputDest: string | undefined;
}

export interface ResolverResult {
Expand Down
56 changes: 56 additions & 0 deletions src/test/cli.build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,5 +170,61 @@ describe('Dev Containers CLI', function () {
assert.equal(response.imageName[0], image1);
assert.equal(response.imageName[1], image2);
});

it('should fail with only --output-type and no --output-dest', async () => {
natescherer marked this conversation as resolved.
Show resolved Hide resolved
let success = false;
const testFolder = `${__dirname}/configs/dockerfile-with-target`;
try {
await shellExec(`${cli} build --workspace-folder ${testFolder} --output-type oci --platform linux/amd64`);
} 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, /must be used together/);
}
assert.equal(success, false, 'expect non-successful call');
});

it('should fail with only --output-dest and no --output-type', async () => {
let success = false;
const testFolder = `${__dirname}/configs/dockerfile-with-target`;
try {
await shellExec(`${cli} build --workspace-folder ${testFolder} --output-dest output.tar`);
} 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, /must be used together/);
}
assert.equal(success, false, 'expect non-successful call');
});

it('should fail with --push true and --output-type', async () => {
let success = false;
const testFolder = `${__dirname}/configs/dockerfile-with-target`;
try {
await shellExec(`${cli} build --workspace-folder ${testFolder} --output-type oci --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');
});

natescherer marked this conversation as resolved.
Show resolved Hide resolved
it('should fail with --push true and --output-dest', async () => {
let success = false;
const testFolder = `${__dirname}/configs/dockerfile-with-target`;
try {
await shellExec(`${cli} build --workspace-folder ${testFolder} --output-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');
});
});
});