Skip to content

Commit

Permalink
feat(core): add volumes-from option to docker run command for bundling (
Browse files Browse the repository at this point in the history
aws#22829)

relates to aws#8799 
follow up to stale aws#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*
  • Loading branch information
webratz authored and Brennan Ho committed Jan 20, 2023
1 parent 6abff82 commit c331632
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/@aws-cdk/core/lib/asset-staging.ts
Expand Up @@ -469,6 +469,7 @@ export class AssetStaging extends Construct {
entrypoint: options.entrypoint,
workingDirectory: options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR,
securityOpt: options.securityOpt ?? '',
volumesFrom: options.volumesFrom,
});
}
} catch (err) {
Expand Down
17 changes: 17 additions & 0 deletions packages/@aws-cdk/core/lib/bundling.ts
Expand Up @@ -43,6 +43,13 @@ export interface BundlingOptions {
*/
readonly volumes?: DockerVolume[];

/**
* Where to mount the specified volumes from
* @see https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from
* @default - no containers are specified to mount volumes from
*/
readonly volumesFrom?: string[];

/**
* The environment variables to pass to the Docker container.
*
Expand Down Expand Up @@ -210,6 +217,9 @@ export class BundlingDockerImage {
...options.user
? ['-u', options.user]
: [],
...options.volumesFrom
? flatten(options.volumesFrom.map(v => ['--volumes-from', v]))
: [],
...flatten(volumes.map(v => ['-v', `${v.hostPath}:${v.containerPath}:${isSeLinux() ? 'z,' : ''}${v.consistency ?? DockerVolumeConsistency.DELEGATED}`])),
...flatten(Object.entries(environment).map(([k, v]) => ['--env', `${k}=${v}`])),
...options.workingDirectory
Expand Down Expand Up @@ -441,6 +451,13 @@ export interface DockerRunOptions {
*/
readonly volumes?: DockerVolume[];

/**
* Where to mount the specified volumes from
* @see https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from
* @default - no containers are specified to mount volumes from
*/
readonly volumesFrom?: string[];

/**
* The environment variables to pass to the container.
*
Expand Down
38 changes: 38 additions & 0 deletions packages/@aws-cdk/core/test/bundling.test.ts
Expand Up @@ -438,6 +438,44 @@ describe('bundling', () => {
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('adding user provided docker volume options', () => {
// GIVEN
sinon.stub(process, 'platform').value('darwin');
const spawnSyncStub = sinon.stub(child_process, 'spawnSync').returns({
status: 1,
stderr: Buffer.from('stderr'),
stdout: Buffer.from('stdout'),
pid: 123,
output: ['stdout', 'stderr'],
signal: null,
});
const image = DockerImage.fromRegistry('alpine');

try {
image.run({
command: ['cool', 'command'],
volumesFrom: ['foo', 'bar'],
volumes: [{ hostPath: '/host-path', containerPath: '/container-path' }],
workingDirectory: '/working-directory',
user: 'user:group',
});
} catch (e) {
// We expect this to fail as the test environment will not have the required docker setup for the command to exit successfully
// nevertheless what we want to check here is that the command was built correctly and triggered
};

expect(spawnSyncStub.calledWith('docker', [
'run', '--rm',
'-u', 'user:group',
'--volumes-from', 'foo',
'--volumes-from', 'bar',
'-v', '/host-path:/container-path:delegated',
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('ensure selinux docker mount', () => {
// GIVEN
sinon.stub(process, 'platform').value('linux');
Expand Down

0 comments on commit c331632

Please sign in to comment.