Skip to content

Improve the logs#2323

Merged
tejal29 merged 8 commits intoGoogleContainerTools:masterfrom
dgageot:final-logs-2
Jun 28, 2019
Merged

Improve the logs#2323
tejal29 merged 8 commits intoGoogleContainerTools:masterfrom
dgageot:final-logs-2

Conversation

@dgageot
Copy link
Copy Markdown
Contributor

@dgageot dgageot commented Jun 23, 2019

  • Don’t leak go routines
  • Never reprint the log for a given container
  • Simplify code
  • Handle init containers first
  • Notify that container logs ended

@dgageot
Copy link
Copy Markdown
Contributor Author

dgageot commented Jun 23, 2019

@balopat I simplified my PR because it was buggy. I decided to stop printing the container's final state because it's too unreliable. Printing when container logs end is, if think, good enough.

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 23, 2019

Codecov Report

Merging #2323 into master will increase coverage by 0.13%.
The diff coverage is 11.11%.

Impacted Files Coverage Δ
pkg/skaffold/runner/build_deploy.go 51.35% <0%> (-1.43%) ⬇️
pkg/skaffold/kubernetes/log.go 31.25% <10.34%> (+5.64%) ⬆️
pkg/skaffold/runner/dev.go 63.88% <25%> (+0.5%) ⬆️

@dgageot dgageot force-pushed the final-logs-2 branch 6 times, most recently from 81e55cc to dc122a3 Compare June 27, 2019 22:04
dgageot added 8 commits June 28, 2019 17:09
Make sure streamRequest() exits.

Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
But once deploy is done, print the logs up the
just before deploy was started.

Signed-off-by: David Gageot <david@gageot.net>
@tejal29 tejal29 merged commit 734e5e3 into GoogleContainerTools:master Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants