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

Drop support for docker log format #1216

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

saschagrunert
Copy link
Member

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Dockershim has been removed from Kubernetes so we only need to support the CRI log format.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

None

Does this PR introduce a user-facing change?

Removed support for docker log format in tests since dockershim is not available in Kubernetes any more.

Dockershim has been removed from Kubernetes so we only need to support
the CRI log format.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jul 18, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saschagrunert

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 18, 2023
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 18, 2023
@haircommander
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 18, 2023
@k8s-ci-robot k8s-ci-robot merged commit a498290 into kubernetes-sigs:master Jul 18, 2023
@saschagrunert saschagrunert deleted the docker-logs branch July 18, 2023 14:02
@SergeyKanzhelev
Copy link
Member

Sorry for the late comment. Will this work with CRI-DOCKERD? We are not dropping this from kubelet because of cri-dockerd to protect back compat.

@haircommander
Copy link
Contributor

cri-dockerd should log in cri format IMO

@SergeyKanzhelev
Copy link
Member

Do you know any maintainers of cri-dockerd? @nwneisen I see you are making releases. please advice

@nwneisen
Copy link

@SergeyKanzhelev I recently took over cri-dockerd. The project uses k8s.io/klog/v2 which looks like the output can be configured. The logs I've seen have been in the logfmt format

@haircommander
Copy link
Contributor

@nwneisen in this case we're talking about container logs--how does cri-dockerd write logs to disk for the kubelet to read them?

@nwneisen
Copy link

{"log":"I0719 22:04:51.982483       1 log.go:195] Started HTTP server on port 8080\n","stream":"stderr","time":"2023-07-19T22:04:51.993532538Z"}
{"log":"I0719 22:04:51.982923       1 log.go:195] Started UDP server on port  8081\n","stream":"stderr","time":"2023-07-19T22:04:51.993575341Z"}
{"log":"I0719 22:05:48.595826       1 log.go:195] GET /\n","stream":"stderr","time":"2023-07-19T22:05:48.595996805Z"}
{"log":"I0719 22:05:48.618534       1 log.go:195] GET /\n","stream":"stderr","time":"2023-07-19T22:05:48.618656559Z"}

Container logs are being written to disk in json format. I agree that cri-dockerd should use cri format. I will work on making a change for this.

@saschagrunert
Copy link
Member Author

Thank you @nwneisen! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants