Skip to content

Commit 813c2f1

Browse files
authored
feat(core): add volumes-from option to docker run command for bundling (#22829)
relates to #8799 follow up to stale #21660 ## Describe the feature Ability to add [--volumes-from](https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from) flag when bundling assets with docker. This enabled people using Docker in Docker to use CDKs bundling functionality, which is currently not possible. ## Use Case CICD systems often run within a docker container already. Many systems mount the ` /var/run/docker.sock` from the host system into the CICD container. When running bundling within such a container it currently breaks, as docker assume the path is from the host system, not within the CICD container. The options allows to mount the data from any other container. Very often it will be the current one which can be used by using the `HOSTNAME` environment variable ## Proposed Solution Add optional property to [DockerRunOptions](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.DockerRunOptions.html) and [BundlingOptions](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.BundlingOptions.html) that would translate into --volumes-from {user provided option} This change would not reflect in any CloudFormation changes, but only with the docker commands performed when bundling. Due to using the `--volumes-from` option, docker will instead of trying to find the path on the host (where it does not exist) try to use the volume that is created by the container C1 that is actually running the CDK. With that it is able to access the files from CDK and can continue the build. ![Docker volumes from](https://user-images.githubusercontent.com/2162832/193787498-de03c66c-7bce-458b-9776-7ba421b9d929.jpg) The following plain docker steps show how this works from the docker side, and why we need to adjust the `--volumes-from` parameter. ```sh docker volume create builds docker run -v /var/run/docker.sock:/var/run/docker.sock -v builds:/builds -it docker ``` Now within the just created docker container, run the following commands. ```sh echo "testfile" > /builds/my-share-file.txt docker run --rm --name DinDContainer --volumes-from="${HOSTNAME}" ubuntu bash -c "ls -hla /builds" ``` We see that the second container C2 (here `DinDContainer`) has the same files available as the container C1. ## Alternative solutions I'm not aware of alternative solutions for this docker in docker use cases, besides of not relying on docker at all, which is out of scope for this MR. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? I ran it, but it seems not to have generated something, i might need some guidance there. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent b66b0ca commit 813c2f1

File tree

3 files changed

+56
-0
lines changed

3 files changed

+56
-0
lines changed

packages/@aws-cdk/core/lib/asset-staging.ts

+1
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ export class AssetStaging extends Construct {
469469
entrypoint: options.entrypoint,
470470
workingDirectory: options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR,
471471
securityOpt: options.securityOpt ?? '',
472+
volumesFrom: options.volumesFrom,
472473
});
473474
}
474475
} catch (err) {

packages/@aws-cdk/core/lib/bundling.ts

+17
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ export interface BundlingOptions {
4343
*/
4444
readonly volumes?: DockerVolume[];
4545

46+
/**
47+
* Where to mount the specified volumes from
48+
* @see https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from
49+
* @default - no containers are specified to mount volumes from
50+
*/
51+
readonly volumesFrom?: string[];
52+
4653
/**
4754
* The environment variables to pass to the Docker container.
4855
*
@@ -210,6 +217,9 @@ export class BundlingDockerImage {
210217
...options.user
211218
? ['-u', options.user]
212219
: [],
220+
...options.volumesFrom
221+
? flatten(options.volumesFrom.map(v => ['--volumes-from', v]))
222+
: [],
213223
...flatten(volumes.map(v => ['-v', `${v.hostPath}:${v.containerPath}:${isSeLinux() ? 'z,' : ''}${v.consistency ?? DockerVolumeConsistency.DELEGATED}`])),
214224
...flatten(Object.entries(environment).map(([k, v]) => ['--env', `${k}=${v}`])),
215225
...options.workingDirectory
@@ -441,6 +451,13 @@ export interface DockerRunOptions {
441451
*/
442452
readonly volumes?: DockerVolume[];
443453

454+
/**
455+
* Where to mount the specified volumes from
456+
* @see https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from
457+
* @default - no containers are specified to mount volumes from
458+
*/
459+
readonly volumesFrom?: string[];
460+
444461
/**
445462
* The environment variables to pass to the container.
446463
*

packages/@aws-cdk/core/test/bundling.test.ts

+38
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,44 @@ describe('bundling', () => {
438438
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
439439
});
440440

441+
test('adding user provided docker volume options', () => {
442+
// GIVEN
443+
sinon.stub(process, 'platform').value('darwin');
444+
const spawnSyncStub = sinon.stub(child_process, 'spawnSync').returns({
445+
status: 1,
446+
stderr: Buffer.from('stderr'),
447+
stdout: Buffer.from('stdout'),
448+
pid: 123,
449+
output: ['stdout', 'stderr'],
450+
signal: null,
451+
});
452+
const image = DockerImage.fromRegistry('alpine');
453+
454+
try {
455+
image.run({
456+
command: ['cool', 'command'],
457+
volumesFrom: ['foo', 'bar'],
458+
volumes: [{ hostPath: '/host-path', containerPath: '/container-path' }],
459+
workingDirectory: '/working-directory',
460+
user: 'user:group',
461+
});
462+
} catch (e) {
463+
// We expect this to fail as the test environment will not have the required docker setup for the command to exit successfully
464+
// nevertheless what we want to check here is that the command was built correctly and triggered
465+
};
466+
467+
expect(spawnSyncStub.calledWith('docker', [
468+
'run', '--rm',
469+
'-u', 'user:group',
470+
'--volumes-from', 'foo',
471+
'--volumes-from', 'bar',
472+
'-v', '/host-path:/container-path:delegated',
473+
'-w', '/working-directory',
474+
'alpine',
475+
'cool', 'command',
476+
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
477+
});
478+
441479
test('ensure selinux docker mount', () => {
442480
// GIVEN
443481
sinon.stub(process, 'platform').value('linux');

0 commit comments

Comments
 (0)