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.

Sign up for GitHub

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

Jump to bottom

‼️ NOTICE: DockerImageAsset and ContainerImage 'file' property breaks docker image building #6954

Closed
iliapolo opened this issue Mar 23, 2020 · 2 comments · Fixed by #6957
Closed

‼️ NOTICE: DockerImageAsset and ContainerImage 'file' property breaks docker image building #6954

iliapolo opened this issue Mar 23, 2020 · 2 comments · Fixed by #6957
Labels
bug This issue is a bug. management/tracking Issues that track a subject or multiple issues

Comments

@iliapolo
Copy link
Contributor

iliapolo commented Mar 23, 2020 •
edited
Loading

Please add your +1 👍 to let us know you have encountered this

Status: IN-PROGRESS

Affected Versions: 1.29.0, 1.30.0.

Overview:

Using the file property in:

ecs.ContainerImage.fromAsset(path.resolve(__dirname, '../../../..'), {
    file: 'services/backend/Dockerfile',
    buildArgs: {
      //
    },
  });

Breaks the docker build because docker fails to find the Dockerfile. This also happens in the DockerImageAsset construct.

We suspect this commit introduced the problem in version 1.29.0.

Complete Error Message:

Workaround:

The current workaround is to revert back to version 1.28.0.

Solution:

Related Issues:

Originally reported here: #6814
@iliapolo iliapolo added bug This issue is a bug. management/tracking Issues that track a subject or multiple issues needs-triage This issue or PR still needs to be triaged. labels Mar 23, 2020
@iliapolo iliapolo mentioned this issue Mar 23, 2020
Closed
@iliapolo iliapolo pinned this issue Mar 23, 2020
@NGL321 NGL321 removed the needs-triage This issue or PR still needs to be triaged. label Mar 23, 2020
@NGL321 NGL321 changed the title DockerImageAsset and ContainerImage 'file' property breaks docker image building ‼️ NOTICE: DockerImageAsset and ContainerImage 'file' property breaks docker image building Mar 23, 2020
@NGL321 NGL321 changed the title ‼️ NOTICE: DockerImageAsset and ContainerImage 'file' property breaks docker image building ‼️ NOTICE: DockerImageAsset and ContainerImage 'file' property breaks docker image building Mar 23, 2020
@NGL321 NGL321 changed the title ‼️ NOTICE: DockerImageAsset and ContainerImage 'file' property breaks docker image building ‼️ NOTICE: DockerImageAsset and ContainerImage 'file' property breaks docker image building Mar 23, 2020
MrArnoldPalmer added a commit to MrArnoldPalmer/aws-cdk that referenced this issue Mar 24, 2020
@MrArnoldPalmer
fix(cdk-assets): Docker Build Context Path
0283d4b 
Prior v1.29.0, the context passed to the docker build command was always
the current working directory. In v1.29.0, this was changed to be the
directory the user passed when instantiating the asset. However, docker
doesn't respect the context passed to the build command when the
`--file` argument is also passed. This broke assets for users passing
the `file` prop when constructing a `DockerImageAsset`.

This is fixed by changing the current working directory of the `docker build`
command and passing that context as '.'. Then docker uses the current working
directory as the context and correctly finds the image definition file from the
`file` prop. An existing integration test in the framework covers this case,
but does not fail unless the stack is deployed fresh since the asset isn't
rebuilt when the hash is not changed.

Fix: aws#6954 aws#6814
@MrArnoldPalmer MrArnoldPalmer mentioned this issue Mar 24, 2020
Merged
MrArnoldPalmer added a commit to MrArnoldPalmer/aws-cdk that referenced this issue Mar 24, 2020
@MrArnoldPalmer
fix(cdk-assets): Docker Build Context Path
ca318e3 
Prior v1.29.0, the context passed to the docker build command was always
the current working directory. In v1.29.0, this was changed to be the
directory the user passed when instantiating the asset. However, docker
doesn't respect the context passed to the build command when the
`--file` argument is also passed. This broke assets for users passing
the `file` prop when constructing a `DockerImageAsset`.

This is fixed by changing the current working directory of the `docker build`
command and passing that context as '.'. Then docker uses the current working
directory as the context and correctly finds the image definition file from the
`file` prop. An existing integration test in the framework covers this case,
but does not fail unless the stack is deployed fresh since the asset isn't
rebuilt when the hash is not changed.

Fix: aws#6954 aws#6814
MrArnoldPalmer added a commit to MrArnoldPalmer/aws-cdk that referenced this issue Mar 24, 2020
@MrArnoldPalmer
fix(cdk-assets): Docker Build Bad Context Path
3d0247b 
Prior v1.29.0, the context passed to the docker build command was always
the current working directory. In v1.29.0, this was changed to be the
directory the user passed when instantiating the asset. However, docker
doesn't respect the context passed to the build command when the
`--file` argument is also passed. This broke assets for users passing
the `file` prop when constructing a `DockerImageAsset`.

This is fixed by changing the current working directory of the `docker build`
command and passing that context as '.'. Then docker uses the current working
directory as the context and correctly finds the image definition file from the
`file` prop. An existing integration test in the framework covers this case,
but does not fail unless the stack is deployed fresh since the asset isn't
rebuilt when the hash is not changed.

Fix: aws#6954 aws#6814
MrArnoldPalmer added a commit to MrArnoldPalmer/aws-cdk that referenced this issue Mar 24, 2020
@MrArnoldPalmer
fix(cdk-assets): Docker Build Bad Context Path
70f984e 
Prior v1.29.0, the context passed to the docker build command was always
the current working directory. In v1.29.0, this was changed to be the
directory the user passed when instantiating the asset. However, docker
doesn't respect the context passed to the build command when the
`--file` argument is also passed. This broke assets for users passing
the `file` prop when constructing a `DockerImageAsset`.

This is fixed by changing the current working directory of the `docker build`
command and passing that context as '.'. Then docker uses the current working
directory as the context and correctly finds the image definition file from the
`file` prop. An existing integration test in the framework covers this case,
but does not fail unless the stack is deployed fresh since the asset isn't
rebuilt when the hash is not changed.

Fix: aws#6954 aws#6814
@mergify mergify bot closed this as completed in #6957 Mar 24, 2020
mergify bot pushed a commit that referenced this issue Mar 24, 2020
@MrArnoldPalmer
fix(cdk-assets): context path not honored by Docker asset build (#6957)
1edd507 
Prior v1.29.0, the context passed to the docker build command was always
the current working directory. In v1.29.0, this was changed to be the
directory the user passed when instantiating the asset. However, docker
doesn't respect the context passed to the build command when the
`--file` argument is also passed. This broke assets for users passing
the `file` prop when constructing a `DockerImageAsset`.

This is fixed by changing the current working directory of the `docker build`
command and passing that context as `.`. Then docker uses the current working
directory as the context and correctly finds the image definition file from the
`file` prop. An existing integration test in the framework covers this case,
but does not fail unless the stack is deployed fresh since the asset isn't
rebuilt when the hash is not changed.

Fix: #6954 #6814
@iliapolo
Copy link
Contributor Author

This was closed because the PR was merged, im reopening because it hasn't been released yet. Once the release happens we can notify that an upgrade will resolve this problem for users.

@iliapolo iliapolo reopened this Mar 24, 2020
shivlaks pushed a commit that referenced this issue Mar 24, 2020
@MrArnoldPalmer @shivlaks
fix(cdk-assets): context path not honored by Docker asset build (#6957)
ea703d0 
Prior v1.29.0, the context passed to the docker build command was always
the current working directory. In v1.29.0, this was changed to be the
directory the user passed when instantiating the asset. However, docker
doesn't respect the context passed to the build command when the
`--file` argument is also passed. This broke assets for users passing
the `file` prop when constructing a `DockerImageAsset`.

This is fixed by changing the current working directory of the `docker build`
command and passing that context as `.`. Then docker uses the current working
directory as the context and correctly finds the image definition file from the
`file` prop. An existing integration test in the framework covers this case,
but does not fail unless the stack is deployed fresh since the asset isn't
rebuilt when the hash is not changed.

Fix: #6954 #6814
@shivlaks
Copy link
Contributor

v1.31.0 has been released.

@shivlaks shivlaks unpinned this issue Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This issue is a bug. management/tracking Issues that track a subject or multiple issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(cdk-assets): context path not honored by Docker asset build MrArnoldPalmer/aws-cdk
3 participants
@iliapolo @NGL321 @shivlaks