-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
provider/aws: Support ephemeral volumes in ecs_task_definition #2370
Comments
Even though both the SDK & docs say it should allow an empty parameter, API keeps returning exception:
type Volume struct {
// The path on the host container instance that is presented to the containers
// which access the volume. If this parameter is empty, then the Docker daemon
// assigns a host path for you.
Host *HostVolumeProperties `locationName:"host" type:"structure"` I'm not sure how to address that problem. I will probably have to isolate that piece of code and raise it with AWS. |
Awesome job getting that stuff in! Stoked to try it out. Looking at it from the outside (haven't read all the threads in the PR) it seems odd that the abstraction stopped at volumes, why not have the whole task def in declarative form? Or the entire task def as JSON? |
I was actually thinking about that approach, but then I decided to mimic approach of other tools (e.g. AWS CLI) to make things easily portable - i.e. TD/Service definition in JSON is the building block you can eventually reuse between any tools that share that approach. The API AFAIK doesn't allow updating only certain fields - i.e. we have to send the whole definition to the API anyway, that's just the smallest building block recognised by API. The reason Go SDK crumbled the JSON completely is IMO merely because it's machine-generated code. btw. Terraform doesn't expand IAM definitions either, even though it sometime causes problems. |
True yea good point, not dissimilar to the others – that makes sense with the CLI parity too I didn't notice that they made that split there, seems arbitrary but they must have a reason :D |
See #3810 |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
via @jbrook from #1803 (comment)
Although closed & merged PRs aren't the best place to report issues (new one is almost always better), thanks for bringing this up, @jbrook 👍
It will be necessary to look into the SDK to confirm my theory, but I reckon the solution is just making
host_path
optional and leaving it out from API request in those cases.Either way I feel this one should be fairly easy to fix.
The text was updated successfully, but these errors were encountered: