-
Notifications
You must be signed in to change notification settings - Fork 4k
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
TarballImageAsset executable
command in ...assets.json
prevents running cdk deploy
outside cloud assembly root directory
#15721
Comments
@chris-leach you will need to specify a path relative to current running script directory. I believe you can do it like this in python: scriptdir=os.path.dirname(os.path.abspath(__file__))
task_def.add_container(
"TestContainer",
image=ContainerImage.from_tarball(scriptdir + "/hello-world.tar"),
) I've created #15750 to improve documentation. |
cdk deploy
cdk deploy
)
cdk deploy
)
Hi @eladb, thanks very much for your reply but I think you've misunderstood the issue. The issue is not that the Python script cannot find the source tarball. I made the contrived example above for the purpose of the issue - apologies if it confused things - in my actual application I do of course use an absolute path to refer to the source tarball (as an aside, In the actual application The
The problem, as mentioned above, is that the executable in ...
"dockerImages": {
"c2a72eb491044b4b079b0430e01e40d428a629b736f0e3c61f2f51170ed20069": {
"source": {
"executable": [
"sh",
"-c",
"docker load -i asset.c2a...069.tar | sed \"s/Loaded image: //g\""
]
},
"destinations": {
...
}
}
}
... This is necessary to an extent, since the intention as I understand it is that people can move the cloud assembly directory around, tar it up to deploy from elsewhere, etc. As mentioned above, though, the way that I hope this clarifies the issue, and would very much appreciate it if you could consider whether any of the suggestions above might improve things. Thanks again for your time. |
executable
command in ...assets.json
prevents running cdk deploy
outside cloud assembly root directory
@chris-leach thanks for clarifying. I will look into it. |
@pgarbe can you take a look? |
@eladb sure, can do |
I can verify that issue. When I created the feature, I used CdkPipeline and it that case it works very well as the workdir in publish-assets stage is within the cdk.out folder (because how the files is moved between the stages within CodePipeline). @chris-leach do you have already an idea how to tackle that? Regarding your other comment: The container image handling fails if the exec command does not return a name of a valid container image. But I agree, there could be a better error handling. |
@pgarbe My initial idea was to change the way If making the cloud assembly root the CWD for all docker executables is indeed unacceptable, a less intrusive change would be to make it available to the executable as an environment variable, so existing executables could remain unchanged, but new ones written by ["sh", "-c", "docker load -i asset.c2a...069.tar | sed \"s/Loaded image: //g\""] to e.g.: ["sh", "-c", "docker load -i $CDK_CLOUD_ASSEMBLY_ROOT/asset.c2a...069.tar | sed \"s/Loaded image: //g\""] What do you think? |
How is that different from the normal "docker build" we are doing and referencing a directory inside the cloud assembly? I don't see a reason for this to be any different or am I missing something. |
The If the asset is build from a Dockerfile, the |
@njlynch @rix0rrr @otaviomacedo it seems like
|
I am okay changing the default I won't have time to work on this myself but I would happily welcome a PR that addresses this. |
Sounds good to me. I can have a look and send a PR. |
…mbly root directory (#16094) Runs the executable command of container assets in the cloud assembly root directory and not the current directory. Could be a breaking change. fixes #15721 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Improve docs to indicate that the path to a tarball should be absolute and not relative. Fixes #15721 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
We were very excited to use the new
TarballImageAsset
class to deploy docker images built by Kaniko in our CI pipeline, but it seems to break deployments unless run in a specific and inconvenient way.Reproduction Steps
Given the following CDK project, where
hello-world.tar
is produced withdocker save hello-world -o hello-world.tar
:app.py
:cdk.json
:Then, run
cdk depoy -vvv
from the project root.What did you expect to happen?
The deployment should succeed, creating an ECS task definition from the docker image tarball.
What actually happened?
Deployment fails with the verbose output shown below.
The root cause appears to be that the executable command (in
cdk.out/TestStack.assets.json
, shown below) is run from the same working directory as thecdk deploy
command - obviously theasset...tar
file is not present when run from the project root.This seems to be confirmed by the fact that when I first run
cdk synth
, and thencd cdk.out; cdk deploy --app .
, the deployment succeeds.Suggestions
It seems very strange to me that the deployment operation is so dependent on the current working directory - I would expect always to be able to run
cdk deploy --app /path/to/cloud/assembly
and have it work.Ideally, asset executable commands would be run from the cloud assembly directory, regardless of where the
cdk deploy
command was run from. If changing this would break backwards compatibility, then a possible solution might be to provide the path to the cloud assembly root to the executable somehow, e.g. as an environment variable. The executable written byTarballImageAsset
could then be something like:In any case there are a couple of other issues I can see that made it very hard to understand what is going wrong here:
docker load
command failing, the exit code of the command overall is 0 because of the pipe tosed
, causing execution to continue past where it should.docker tag
, causing the error seen below. Ideally the output of the executable should be validated as soon as it returns.Thanks for reading, and if you would be willing to accept a PR along the lines of any of these suggestions then let me know.
Environment
Other
Verbose
cdk deploy
output:This is 🐛 Bug Report
The text was updated successfully, but these errors were encountered: