Skip to content

Commit

Permalink
revert changing dest location to /opt/build-features and remerge main
Browse files Browse the repository at this point in the history
  • Loading branch information
joshspicer authored Feb 9, 2023
2 parents c000b10 + 4705707 commit 6687ad4
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 53 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

Notable changes.

## February 2023

### [0.30.0]

- Fix: Merge metadata logic for containerEnv for `devcontainer build`. (https://github.com/devcontainers/cli/pull/392)
- Support querying registries that Accept application/vnd.oci.image.index.v1+json. (https://github.com/devcontainers/cli/pull/393)
- Updates Features cache logic - Incrementally copy features near the layer they're installed. (https://github.com/devcontainers/cli/pull/382)

## January 2023

### [0.29.0]
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@devcontainers/cli",
"description": "Dev Containers CLI",
"version": "0.29.0",
"version": "0.30.0",
"bin": {
"devcontainer": "devcontainer.js"
},
Expand Down
51 changes: 41 additions & 10 deletions src/spec-configuration/containerFeaturesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const DEVCONTAINER_FEATURE_FILE_NAME = 'devcontainer-feature.json';

export type Feature = SchemaFeatureBaseProperties & SchemaFeatureLifecycleHooks & DeprecatedSchemaFeatureProperties & InternalFeatureProperties;

export const CONTAINER_BUILD_FEATURES_DIR = '/opt/build-features';
export const CONTAINER_BUILD_FEATURES_DIR = '/tmp/build-features';

export interface SchemaFeatureLifecycleHooks {
onCreateCommand?: string | string[];
Expand Down Expand Up @@ -247,14 +247,14 @@ export function getContainerFeaturesBaseDockerFile() {
FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize
USER root
COPY --from=dev_containers_feature_content_source {contentSourceRootPath} /opt/build-features/
RUN chmod -R 0777 /opt/build-features
COPY --from=dev_containers_feature_content_source {contentSourceRootPath}/devcontainer-features.builtin.env /tmp/build-features/
RUN chmod -R 0777 /tmp/build-features
FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage
USER root
COPY --from=dev_containers_feature_content_normalize /opt/build-features /opt/build-features
COPY --from=dev_containers_feature_content_normalize /tmp/build-features /tmp/build-features
#{featureLayer}
Expand Down Expand Up @@ -333,32 +333,63 @@ function escapeQuotesForShell(input: string) {
return input.replace(new RegExp(`'`, 'g'), `'\\''`);
}

export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string) {
export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string, useBuildKitBuildContexts = false, contentSourceRootPath = '/tmp/build-features/') {
let result = `RUN \\
echo "_CONTAINER_USER_HOME=$(getent passwd ${containerUser} | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env && \\
echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env
echo "_CONTAINER_USER_HOME=$(getent passwd ${containerUser} | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\
echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env
`;

// Features version 1
const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => x.features[0].consecutiveId);
folders.forEach(folder => {
result += `RUN cd /opt/build-features/${folder} \\
const source = path.posix.join(contentSourceRootPath, folder!);
if (!useBuildKitBuildContexts) {
result += `COPY --chown=root:root --from=dev_containers_feature_content_source ${source} /tmp/build-features/${folder}
RUN chmod -R 0777 /tmp/build-features/${folder} \\
&& cd /tmp/build-features/${folder} \\
&& chmod +x ./install.sh \\
&& ./install.sh
`;
} else {
result += `RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${folder} \\
cp -ar /tmp/build-features-src/${folder} /tmp/build-features/ \\
&& chmod -R 0777 /tmp/build-features/${folder} \\
&& cd /tmp/build-features/${folder} \\
&& chmod +x ./install.sh \\
&& ./install.sh \\
&& rm -rf /tmp/build-features/${folder}
`;
}
});
// Features version 2
featuresConfig.featureSets.filter(y => y.internalVersion === '2').forEach(featureSet => {
featureSet.features.forEach(feature => {
result += generateContainerEnvs(feature);
result += `
RUN cd /opt/build-features/${feature.consecutiveId} \\
const source = path.posix.join(contentSourceRootPath, feature.consecutiveId!);
if (!useBuildKitBuildContexts) {
result += `
COPY --chown=root:root --from=dev_containers_feature_content_source ${source} /tmp/build-features/${feature.consecutiveId}
RUN chmod -R 0777 /tmp/build-features/${feature.consecutiveId} \\
&& cd /tmp/build-features/${feature.consecutiveId} \\
&& chmod +x ./devcontainer-features-install.sh \\
&& ./devcontainer-features-install.sh
`;
} else {
result += `
RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${feature.consecutiveId} \\
cp -ar /tmp/build-features-src/${feature.consecutiveId} /tmp/build-features/ \\
&& chmod -R 0777 /tmp/build-features/${feature.consecutiveId} \\
&& cd /tmp/build-features/${feature.consecutiveId} \\
&& chmod +x ./devcontainer-features-install.sh \\
&& ./devcontainer-features-install.sh \\
&& rm -rf /tmp/build-features/${feature.consecutiveId}
`;
}
});
});
return result;
Expand Down
9 changes: 5 additions & 4 deletions src/spec-node/configContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as jsonc from 'jsonc-parser';

import { openDockerfileDevContainer } from './singleContainer';
import { openDockerComposeDevContainer } from './dockerCompose';
import { ResolverResult, DockerResolverParameters, isDockerFileConfig, runUserCommand, createDocuments, getWorkspaceConfiguration, BindMountConsistency, uriToFsPath, DevContainerAuthority, isDevContainerAuthority, SubstituteConfig, SubstitutedConfig, addSubstitution, envListToObj } from './utils';
import { ResolverResult, DockerResolverParameters, isDockerFileConfig, runUserCommand, createDocuments, getWorkspaceConfiguration, BindMountConsistency, uriToFsPath, DevContainerAuthority, isDevContainerAuthority, SubstituteConfig, SubstitutedConfig, addSubstitution, envListToObj, findContainerAndIdLabels } from './utils';
import { beforeContainerSubstitute, substitute } from '../spec-common/variableSubstitution';
import { ContainerError } from '../spec-common/errors';
import { Workspace, workspaceFromPath, isWorkspacePath } from '../spec-utils/workspaces';
Expand All @@ -21,19 +21,19 @@ import { DevContainerConfig, DevContainerFromDockerComposeConfig, DevContainerFr

export { getWellKnownDevContainerPaths as getPossibleDevContainerPaths } from '../spec-configuration/configurationCommonUtils';

export async function resolve(params: DockerResolverParameters, configFile: URI | undefined, overrideConfigFile: URI | undefined, idLabels: string[], additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>): Promise<ResolverResult> {
export async function resolve(params: DockerResolverParameters, configFile: URI | undefined, overrideConfigFile: URI | undefined, providedIdLabels: string[] | undefined, additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>): Promise<ResolverResult> {
if (configFile && !/\/\.?devcontainer\.json$/.test(configFile.path)) {
throw new Error(`Filename must be devcontainer.json or .devcontainer.json (${uriToFsPath(configFile, params.common.cliHost.platform)}).`);
}
const parsedAuthority = params.parsedAuthority;
if (!parsedAuthority || isDevContainerAuthority(parsedAuthority)) {
return resolveWithLocalFolder(params, parsedAuthority, configFile, overrideConfigFile, idLabels, additionalFeatures);
return resolveWithLocalFolder(params, parsedAuthority, configFile, overrideConfigFile, providedIdLabels, additionalFeatures);
} else {
throw new Error(`Unexpected authority: ${JSON.stringify(parsedAuthority)}`);
}
}

async function resolveWithLocalFolder(params: DockerResolverParameters, parsedAuthority: DevContainerAuthority | undefined, configFile: URI | undefined, overrideConfigFile: URI | undefined, idLabels: string[], additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>): Promise<ResolverResult> {
async function resolveWithLocalFolder(params: DockerResolverParameters, parsedAuthority: DevContainerAuthority | undefined, configFile: URI | undefined, overrideConfigFile: URI | undefined, providedIdLabels: string[] | undefined, additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>): Promise<ResolverResult> {
const { common, workspaceMountConsistencyDefault } = params;
const { cliHost, output } = common;

Expand All @@ -52,6 +52,7 @@ async function resolveWithLocalFolder(params: DockerResolverParameters, parsedAu
throw new ContainerError({ description: `No dev container config and no workspace found.` });
}
}
const idLabels = providedIdLabels || (await findContainerAndIdLabels(params, undefined, providedIdLabels, workspace?.rootFolderPath, configPath?.fsPath, params.removeOnStartup)).idLabels;
const configWithRaw = addSubstitution(configs.config, config => beforeContainerSubstitute(envListToObj(idLabels), config));
const { config } = configWithRaw;

Expand Down
16 changes: 8 additions & 8 deletions src/spec-node/containerFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,13 +283,13 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont
await cliHost.writeFile(envPath, Buffer.from(builtinVariables.join('\n') + '\n'));

// When copying via buildkit, the content is accessed via '.' (i.e. in the context root)
// When copying via temp image, the content is in '/opt/build-features'
const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/opt/build-features/';
// When copying via temp image, the content is in '/tmp/build-features'
const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/tmp/build-features/';
const dockerfile = getContainerFeaturesBaseDockerFile()
.replace('#{nonBuildKitFeatureContentFallback}', useBuildKitBuildContexts ? '' : `FROM ${buildContentImageName} as dev_containers_feature_content_source`)
.replace('{contentSourceRootPath}', contentSourceRootPath)
.replace('#{featureBuildStages}', getFeatureBuildStages(featuresConfig, buildStageScripts, contentSourceRootPath))
.replace('#{featureLayer}', getFeatureLayers(featuresConfig, containerUser, remoteUser))
.replace('#{featureLayer}', getFeatureLayers(featuresConfig, containerUser, remoteUser, useBuildKitBuildContexts, contentSourceRootPath))
.replace('#{containerEnv}', generateContainerEnvs(featuresConfig))
.replace('#{copyFeatureBuildStages}', getCopyFeatureBuildStages(featuresConfig, buildStageScripts))
.replace('#{devcontainerMetadata}', getDevcontainerMetadataLabel(imageMetadata, common.experimentalImageMetadata))
Expand Down Expand Up @@ -349,7 +349,7 @@ ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
if (!useBuildKitBuildContexts) {
const buildContentDockerfile = `
FROM scratch
COPY . /opt/build-features/
COPY . /tmp/build-features/
`;
const buildContentDockerfilePath = cliHost.path.join(dstFolder, 'Dockerfile.buildContent');
await cliHost.writeFile(buildContentDockerfilePath, Buffer.from(buildContentDockerfile));
Expand Down Expand Up @@ -394,9 +394,9 @@ function getFeatureBuildStages(featuresConfig: FeaturesConfig, buildStageScripts
.map((featureSet, i) => featureSet.features
.filter(f => (includeAllConfiguredFeatures || f.included) && f.value && buildStageScripts[i][f.id]?.hasAcquire)
.map(f => `FROM mcr.microsoft.com/vscode/devcontainers/base:0-focal as ${getSourceInfoString(featureSet.sourceInformation)}_${f.id}
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)}
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'common')} ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'common')}
RUN cd ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/acquire`
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)}
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'common')} ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'common')}
RUN cd ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/acquire`
)
)
).join('\n\n');
Expand All @@ -409,7 +409,7 @@ function getCopyFeatureBuildStages(featuresConfig: FeaturesConfig, buildStageScr
.map(f => {
const featurePath = path.posix.join('/usr/local/devcontainer-features', getSourceInfoString(featureSet.sourceInformation), f.id);
return `COPY --from=${getSourceInfoString(featureSet.sourceInformation)}_${f.id} ${featurePath} ${featurePath}${buildStageScripts[i][f.id]?.hasConfigure ? `
RUN cd ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/configure` : ''}`;
RUN cd ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/configure` : ''}`;
})
)
).join('\n\n');
Expand Down
4 changes: 2 additions & 2 deletions src/spec-node/devContainers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ export interface ProvisionOptions {
};
}

export async function launch(options: ProvisionOptions, idLabels: string[], disposables: (() => Promise<unknown> | undefined)[]) {
export async function launch(options: ProvisionOptions, providedIdLabels: string[] | undefined, disposables: (() => Promise<unknown> | undefined)[]) {
const params = await createDockerParams(options, disposables);
const output = params.common.output;
const text = 'Resolving Remote';
const start = output.start(text);

const result = await resolve(params, options.configFile, options.overrideConfigFile, idLabels, options.additionalFeatures ?? {});
const result = await resolve(params, options.configFile, options.overrideConfigFile, providedIdLabels, options.additionalFeatures ?? {});
output.stop(text, start);
const { dockerContainerId, composeProjectName } = result;
return {
Expand Down
Loading

0 comments on commit 6687ad4

Please sign in to comment.