Skip to content

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

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

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

Conversation

@KollaAdithya
Copy link
Copy Markdown
Contributor

@KollaAdithya KollaAdithya commented Aug 29, 2023

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
Copy link
Copy Markdown

github-actions Bot commented Aug 29, 2023

🍕 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
Copy link
Copy Markdown

codecov-commenter commented Aug 30, 2023

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
Copy link
Copy Markdown
Contributor

iamhopaul123 commented Aug 31, 2023

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.

Comment thread internal/pkg/stream/ecs.go
Comment thread internal/pkg/stream/ecs.go Outdated
@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
Copy link
Copy Markdown
Contributor

@CaptainCarpensir CaptainCarpensir left a comment

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 Outdated
Comment thread internal/pkg/stream/ecs.go
Comment thread internal/pkg/term/progress/ecs.go
@KollaAdithya KollaAdithya marked this pull request as ready for review September 6, 2023 16:08
Copy link
Copy Markdown
Contributor

@iamhopaul123 iamhopaul123 left a comment

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 Outdated
Copy link
Copy Markdown
Contributor

@Lou1415926 Lou1415926 left a comment

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
Comment thread internal/pkg/term/progress/ecs.go Outdated
Comment thread internal/pkg/term/progress/ecs.go Outdated
Comment thread internal/pkg/term/progress/ecs.go Outdated
Comment thread internal/pkg/term/progress/ecs.go Outdated
Padding: c.padding,
},
&singleLineComponent{
Text: fmt.Sprintf("1. You can run %s to see the logs of the last stopped task.",
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?

Copy link
Copy Markdown
Contributor Author

@KollaAdithya KollaAdithya Sep 8, 2023

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

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants