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

Add SkaffoldLogEvents for pod updates #6358

Merged
merged 6 commits into from
Aug 5, 2021

Conversation

tejal29
Copy link
Contributor

@tejal29 tejal29 commented Aug 4, 2021

In this PR,
Simple refactors

  • Refactor pkg/skaffold/deploy/resource to pkg/skaffold/kubernetes/status/resource as Resource is used in status check package and represents k8 resource.
 pkg/skaffold/{deploy => kubernetes/status}/resource/deployment.go      |  8 ++++++--
 pkg/skaffold/{deploy => kubernetes/status}/resource/deployment_test.go |  0
 pkg/skaffold/{deploy => kubernetes/status}/resource/logfile.go         |  0
 pkg/skaffold/{deploy => kubernetes/status}/resource/status.go          |  0
 pkg/skaffold/{deploy => kubernetes/status}/resource/status_test.go     |  0
  • Refactor Timestamp to timestamp

Need @marlon-gamez's input on

  • Associate pod status check messages with task id corresponding to pod name.
    Before, the way string was constructed, deployment status string also contained pod status check message.
- deployment/frontend pending
   - pod/frontend-xxx has imge pull error
  • Use normal out writer ie. without WithEventContext

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #6358 (0832920) into main (1e3da21) will decrease coverage by 0.02%.
The diff coverage is 60.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6358      +/-   ##
==========================================
- Coverage   70.48%   70.45%   -0.03%     
==========================================
  Files         498      498              
  Lines       22581    22616      +35     
==========================================
+ Hits        15916    15935      +19     
- Misses       5635     5649      +14     
- Partials     1030     1032       +2     
Impacted Files Coverage Δ
pkg/skaffold/kubernetes/status/resource/logfile.go 84.61% <ø> (ø)
pkg/skaffold/kubernetes/status/resource/status.go 85.71% <ø> (ø)
pkg/skaffold/kubernetes/status/status_check.go 52.40% <ø> (ø)
pkg/skaffold/event/v2/status_check.go 85.45% <50.00%> (-14.55%) ⬇️
.../skaffold/kubernetes/status/resource/deployment.go 85.44% <83.33%> (ø)
pkg/skaffold/output/output.go 86.20% <100.00%> (+3.59%) ⬆️
pkg/skaffold/util/port.go 82.50% <0.00%> (-8.55%) ⬇️
...g/skaffold/kubernetes/portforward/entry_manager.go 89.09% <0.00%> (-2.69%) ⬇️
...affold/kubernetes/portforward/forwarder_manager.go 49.29% <0.00%> (ø)
...g/skaffold/kubernetes/portforward/pod_forwarder.go 86.88% <0.00%> (+0.44%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e3da21...0832920. Read the comment docs.

@tejal29 tejal29 marked this pull request as ready for review August 5, 2021 19:52
@tejal29 tejal29 requested a review from a team as a code owner August 5, 2021 19:52
@tejal29 tejal29 requested a review from MarlonGamez August 5, 2021 19:52
@MarlonGamez MarlonGamez changed the title Add skaffold logs for pod Add SkaffoldLogEvents for pod updates Aug 5, 2021
@MarlonGamez MarlonGamez merged commit 18a8cd8 into GoogleContainerTools:main Aug 5, 2021
@tejal29 tejal29 deleted the add_sk_logs_for_pod branch November 9, 2021 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants