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
Copy jsonlog from docker/docker locally #89013
Copy jsonlog from docker/docker locally #89013
Conversation
/test pull-kubernetes-e2e-kind |
Keeping me eyes on this re how it impacts #87746 I ended up just deleting |
@mattjmcnaughton sure please do. |
/priority important-longterm |
@@ -165,12 +164,32 @@ func parseCRILog(log []byte, msg *logMessage) error { | |||
return nil | |||
} | |||
|
|||
// jsonLog is a log message, typically a single entry from a given log stream. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note the type this comes from so people don't think we can arbitrarily rename these fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, adding a note.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even stronger phrasing, as in "this deserializes the json log format returned by docker; do not modify this struct's fields or json serialization"
a few nits, lgtm otherwise. glad to see these start to go |
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
438f7fb
to
825f99c
Compare
/assign @liggitt @derekwaynecarr @yujuhong |
/test pull-kubernetes-e2e-kind-ipv6 |
🎉 🙌 @dims mind pinging me when this is merged - I'll then rebase my |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dims, liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@mattjmcnaughton this is merged 🎉 |
On Thu, Mar 19, 2020 at 12:50:40PM -0700, Davanum Srinivas wrote:
@mattjmcnaughton this is merged 🎉
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#89013 (comment)
Hooray! Will try and find some time today to rebase :)
|
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Allows us to drop
docker/docker/daemon/logger/jsonfilelog/jsonlog
dependencyWhich issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: