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

Dependency ordering being ignored #2723

Closed
bpottier opened this issue Aug 9, 2021 · 6 comments · Fixed by #2822
Closed

Dependency ordering being ignored #2723

bpottier opened this issue Aug 9, 2021 · 6 comments · Fixed by #2822
Labels
guidance Issue requesting guidance or information about usage

Comments

@bpottier
Copy link

bpottier commented Aug 9, 2021

I'm using version 1.9.0 and am trying to have a dependency condition of "HEALTHY" for my main container on a sidecar container (for a scheduled job). The task definition being generated doesn't include any dependency ordering. Is there something I'm missing?

mainfiest for main container:

depends_on:
  content: HEALTHY

sidecars:
  content:
    image: ...
@iamhopaul123
Copy link
Contributor

Hello @bpottier. It seems like the manifest is not configured properly. Could you refer to this example?

@efekarakus
Copy link
Contributor

Hi @bpottier 👋

Is it possible to modify the manifest like this to see if the dependency works?

image:
  depends_on:
     content: HEALTHY

Thanks!

@efekarakus efekarakus added the guidance Issue requesting guidance or information about usage label Aug 10, 2021
@bpottier
Copy link
Author

Hi @bpottier 👋

Is it possible to modify the manifest like this to see if the dependency works?

image:
  depends_on:
     content: HEALTHY

Thanks!

Sorry about the delay. My manifest was incorrect. I also realized health checks are not supported on sidecars so what I'm trying to accomplish is not possible. Is there already an issue for adding health checks to sidecars?

@efekarakus
Copy link
Contributor

No worries! thanks for getting back to us :D

I also realized health checks are not supported on sidecars so what I'm trying to accomplish is not possible. Is there already an issue for adding health checks to sidecars?

Oh noo you're right the field is not supported at the moment. It should be a small issue for us to fix, would you mind creating the issue for us and we can prioritize it the upcoming sprint (on Monday)?

@bpottier
Copy link
Author

No worries! thanks for getting back to us :D

I also realized health checks are not supported on sidecars so what I'm trying to accomplish is not possible. Is there already an issue for adding health checks to sidecars?

Oh noo you're right the field is not supported at the moment. It should be a small issue for us to fix, would you mind creating the issue for us and we can prioritize it the upcoming sprint (on Monday)?

Will do!

@mergify mergify bot closed this as completed in #2822 Sep 16, 2021
mergify bot pushed a commit that referenced this issue Sep 16, 2021
<!-- Provide summary of changes -->
fixes #2723.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License..
@efekarakus
Copy link
Contributor

This is now released in https://github.com/aws/copilot-cli/releases/tag/v1.11.0! 🎉

thrau pushed a commit to localstack/copilot-cli-local that referenced this issue Dec 9, 2022
<!-- Provide summary of changes -->
fixes aws#2723.
<!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License..
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guidance Issue requesting guidance or information about usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants