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

feat(core): add volumes-from option to docker run command for bundling #22829

Merged
merged 30 commits into from Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9a18004
WIP add volumesFrom
webratz Aug 9, 2022
020b825
WIP add volumesFrom
webratz Aug 9, 2022
6bcb4ee
adding tests and usage instruction
webratz Aug 18, 2022
8473d45
disable failing test for now
webratz Sep 13, 2022
abdb9b5
add missing type
webratz Sep 13, 2022
658e1dd
Merge branch 'main' into master
mergify[bot] Oct 2, 2022
a7fc600
Merge branch 'aws:main' into master
webratz Oct 4, 2022
74aae68
cleanup
webratz Oct 4, 2022
2ee951b
re-add adjusted test
webratz Oct 5, 2022
9dbbd18
test variant to allow the call to fail
webratz Oct 5, 2022
1fb4ada
remove failing test
webratz Oct 5, 2022
96c5614
whitespace cleanup
webratz Oct 5, 2022
d6b0bd4
Merge branch 'main' into master
webratz Nov 2, 2022
18073db
adding fixed test
webratz Nov 8, 2022
c287929
Merge branch 'aws:main' into master
webratz Nov 8, 2022
1f5c667
add integration test
webratz Nov 8, 2022
f5e5ea0
Merge branch 'master' of https://github.com/webratz/aws-cdk
webratz Nov 8, 2022
08fd008
check if adding helper container works
webratz Nov 8, 2022
aa528ca
update integration tests
webratz Nov 9, 2022
13c7e8b
test sync spawn of docker container
webratz Nov 9, 2022
bce18f2
Merge branch 'main' into master
webratz Nov 9, 2022
e032d03
Merge branch 'main' into master
webratz Nov 11, 2022
f1d9a16
Merge branch 'main' into master
webratz Nov 11, 2022
5a5d3f7
Apply suggestions from code review
webratz Dec 9, 2022
89bb614
WIP allow multiple volumes to be defined
webratz Dec 9, 2022
2ed997a
add full integration test for all steps
webratz Dec 9, 2022
9754f03
remove of integration tests and additional options
webratz Dec 12, 2022
af81272
remove integration for python
webratz Dec 12, 2022
c637016
remove empty line
webratz Dec 12, 2022
6c21217
Merge branch 'main' into master
webratz Dec 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/README.md
Expand Up @@ -145,6 +145,18 @@ new python.PythonFunction(this, 'function', {
});
```

You can also pass additional options to configure Docker for situations where the docker daemon is not running in the same system as you are bundling from.

```ts
const entry = '/path/to/function';

new python.PythonFunction(this, 'function', {
entry,
runtime: Runtime.PYTHON_3_8,
bundling: { volumesFrom: process.env.HOSTNAME },
});
```

webratz marked this conversation as resolved.
Show resolved Hide resolved
## Custom Bundling with Code Artifact

To use a Code Artifact PyPI repo, the `PIP_INDEX_URL` for bundling the function can be customized (requires AWS CLI in the build environment):
Expand Down
9 changes: 9 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/lib/bundling.ts
Expand Up @@ -35,6 +35,13 @@ export interface BundlingProps extends BundlingOptions {
*/
readonly architecture?: Architecture;

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

/**
* Whether or not the bundling process should be skipped
*
Expand All @@ -59,6 +66,7 @@ export class Bundling implements CdkBundlingOptions {
public readonly image: DockerImage;
public readonly command: string[];
public readonly environment?: { [key: string]: string };
public readonly volumesFrom?: string | undefined;
webratz marked this conversation as resolved.
Show resolved Hide resolved

constructor(props: BundlingProps) {
const {
Expand Down Expand Up @@ -88,6 +96,7 @@ export class Bundling implements CdkBundlingOptions {
});
this.command = ['bash', '-c', chain(bundlingCommands)];
this.environment = props.environment;
this.volumesFrom = props.volumesFrom;
}

private createBundlingCommand(options: BundlingCommandOptions): string[] {
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/lib/types.ts
Expand Up @@ -86,4 +86,11 @@ export interface BundlingOptions {
* @default - Based on `assetHashType`
*/
readonly assetHash?: string;

/**
* Where to mount the specified volumes from
* Docker [volumes-from option](https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from)
* @default - no volumes-from options
*/
readonly volumesFrom?: string;
}
15 changes: 15 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/test/bundling.test.ts
Expand Up @@ -278,6 +278,21 @@ test('Bundling with custom environment vars`', () => {
}));
});

test('Bundling with custom volumes', () => {
const entry = path.join(__dirname, 'lambda-handler');
Bundling.bundle({
entry: entry,
runtime: Runtime.PYTHON_3_7,
volumesFrom: process.env.HOSTNAME,
});

expect(Code.fromAsset).toHaveBeenCalledWith(entry, expect.objectContaining({
bundling: expect.objectContaining({
volumesFrom: process.env.HOSTNAME,
}),
}));
});

test('Do not build docker image when skipping bundling', () => {
const entry = path.join(__dirname, 'lambda-handler');
Bundling.bundle({
Expand Down
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
* Docker [volumes-from option](https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from)
webratz marked this conversation as resolved.
Show resolved Hide resolved
* @default - no volumes-from options
webratz marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly volumesFrom?: string;
webratz marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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
? ['--volumes-from', options.volumesFrom]
: [],
...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
* Docker [volumes-from option](https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from)
* @default - no volumes-from options
*/
readonly volumesFrom?: string;

/**
* The environment variables to pass to the container.
*
Expand Down
37 changes: 37 additions & 0 deletions packages/@aws-cdk/core/test/bundling.test.ts
Expand Up @@ -438,6 +438,43 @@ 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',
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',
'-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