Skip to content
This repository was archived by the owner on Jun 22, 2026. It is now read-only.

chore: Add debug info when an task fails to deploy - #5249

Merged
mergify[bot] merged 16 commits into
aws:mainlinefrom
KollaAdithya:render/stoppedtasks
Sep 19, 2023
Merged

chore: Add debug info when an task fails to deploy#5249
mergify[bot] merged 16 commits into
aws:mainlinefrom
KollaAdithya:render/stoppedtasks

Conversation

@KollaAdithya

@KollaAdithya KollaAdithya commented Aug 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR will display the latest 2 stopped tasks and the reasons in the progress tracker.

  1. In case of Essential container exited
Screenshot 2023-09-05 at 6 15 06 PM
  1. In case of Unable to pull secrets
Screenshot 2023-09-05 at 6 26 34 PM
  1. In case of ELB healthCheck failures
Screenshot 2023-09-05 at 6 55 50 PM

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

@KollaAdithya
KollaAdithya requested a review from a team as a code owner August 29, 2023 16:27
@KollaAdithya
KollaAdithya requested review from huanjani and removed request for a team August 29, 2023 16:27
@github-actions

github-actions Bot commented Aug 29, 2023 •
edited
Loading

Copy link
Copy Markdown

πŸ• Here are the new binary sizes!

Name New size (kiB) size (kiB) Delta (%)
macOS (amd) 51844 51740 +0.20
macOS (arm) 52688 52596 +0.17
linux (amd) 45596 45496 +0.22
linux (arm) 44868 44804 +0.14
windows (amd) 43056 42968 +0.20

@KollaAdithya
KollaAdithya marked this pull request as draft August 29, 2023 17:34
@KollaAdithya
KollaAdithya marked this pull request as ready for review August 30, 2023 21:47
@codecov-commenter

codecov-commenter commented Aug 30, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Patch coverage: 83.49% and project coverage change: -0.07% ⚠️

Comparison is base (d54bff6) 69.84% compared to head (9109bff) 69.78%.
Report is 27 commits behind head on mainline.

Additional details and impacted files 
@@             Coverage Diff              @@
##           mainline    #5249      +/-   ##
============================================
- Coverage     69.84%   69.78%   -0.07%     
============================================
  Files           296      296              
  Lines         44305    44704     +399     
  Branches        286      286              
============================================
+ Hits          30945    31195     +250     
- Misses        11865    11996     +131     
- Partials       1495     1513      +18
Files Changed Coverage Ξ”
internal/pkg/aws/ecs/task.go 87.64% <50.00%> (+2.35%) ⬆️
internal/pkg/term/progress/ecs.go 79.10% <83.09%> (+2.18%) ⬆️
internal/pkg/stream/ecs.go 87.82% <86.66%> (-0.28%) ⬇️

... and 17 files with indirect coverage changes

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

@bvtujo bvtujo added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Aug 31, 2023
@iamhopaul123

iamhopaul123 commented Aug 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

For "In case of Essential container exited" it would be great that we include copilot logs command (copilot svc logs --previous or copilot svc logs --tasks xxxxx) as a shortcut for them to view logs (to debug) or an URL to cloudwatch console for that log stream.

Lou1415926
Lou1415926 reviewed Aug 31, 2023
View reviewed changes
Comment thread internal/pkg/stream/ecs.go Resolved
Comment thread internal/pkg/stream/ecs.go Resolved
@bvtujo bvtujo removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Sep 1, 2023
@KollaAdithya
KollaAdithya marked this pull request as draft September 1, 2023 18:33
CaptainCarpensir
CaptainCarpensir reviewed Sep 1, 2023
View reviewed changes

@CaptainCarpensir CaptainCarpensir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No feedback to give, just a few questions that I'm curious about.

Comment thread internal/pkg/stream/ecs.go Resolved
Comment thread internal/pkg/stream/ecs.go Resolved
Comment thread internal/pkg/term/progress/ecs.go Resolved
@KollaAdithya
KollaAdithya marked this pull request as ready for review September 6, 2023 16:08
iamhopaul123
iamhopaul123 approved these changes Sep 6, 2023
View reviewed changes

@iamhopaul123 iamhopaul123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

Comment thread internal/pkg/term/progress/ecs.go Resolved
Lou1415926
Lou1415926 reviewed Sep 8, 2023
View reviewed changes

@Lou1415926 Lou1415926 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is such a huge improvement on the UX ❀️ !!

Comment thread internal/pkg/aws/ecs/task_test.go Resolved
Comment thread internal/pkg/term/progress/ecs.go Resolved
Comment thread internal/pkg/term/progress/ecs.go Resolved
Comment thread internal/pkg/term/progress/ecs.go Resolved
Comment thread internal/pkg/term/progress/ecs.go Resolved
Comment thread internal/pkg/term/progress/ecs.go
Padding: c.padding,
},
&singleLineComponent{
Text: fmt.Sprintf("1. You can run %s to see the logs of the last stopped task.",

@Lou1415926 Lou1415926 Sep 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick question - should we always suggest this, or should we suggest this only if a stopped reason is "essential container exit" πŸ€” what do you think?

@KollaAdithya KollaAdithya Sep 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this doc , incase of ELB health check failurealso there are suggesting to check for the application logs.

Also there might be a scenario where customers can have print/log statements in the application code, in this case copilot svc logs --previous might be helpful. So I am thinking to include this recommendation for all the cases.

@mergify
mergify Bot merged commit b5e9d5a into aws:mainline Sep 19, 2023
KollaAdithya added a commit to KollaAdithya/copilot-cli that referenced this pull request Oct 18, 2023
@KollaAdithya
chore: Add debug info when an task fails to deploy (aws#5249)
5ac4142 
This PR will display the latest 2 stopped tasks and the reasons in the progress tracker.

1. In case of Essential container exited

<img width="1043" alt="Screenshot 2023-09-05 at 6 15 06 PM" src="https://github.com/aws/copilot-cli/assets/71282729/3cad713f-cead-4414-8cd8-58b00d9100fe">



2. In case of Unable to pull secrets
<img width="1177" alt="Screenshot 2023-09-05 at 6 26 34 PM" src="https://github.com/aws/copilot-cli/assets/71282729/b7122276-9ac9-4341-8202-875e86f27735">


3. In case of ELB healthCheck failures
<img width="1103" alt="Screenshot 2023-09-05 at 6 55 50 PM" src="https://github.com/aws/copilot-cli/assets/71282729/23dbfe88-328d-4be7-848f-796543f08092">




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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@huanjani huanjani Awaiting requested review from huanjani huanjani was automatically assigned from aws/aws-ecs-bdx
3 more reviewers
@iamhopaul123 iamhopaul123 iamhopaul123 approved these changes
@CaptainCarpensir CaptainCarpensir CaptainCarpensir left review comments
@Lou1415926 Lou1415926 Lou1415926 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

Sprint πŸƒβ€β™€οΈ
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@KollaAdithya @codecov-commenter @iamhopaul123 @CaptainCarpensir @Lou1415926 @bvtujo