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

Remove duplicated configure for cluster logging #28187

Merged
merged 1 commit into from Aug 4, 2020

Conversation

aiwantaozi
Copy link
Contributor

Problem:
PR for collect containerd logs bring in dulicated configure cause dry-run return warning

Solution:
remove duplicated configure

Issue:
#26543

Problem:
PR for collect containerd logs bring in dulicated configure cause dry-run return warning

Solution:
remove duplicated configure
@maggieliu maggieliu requested a review from cbron July 31, 2020 17:08
@cbron
Copy link
Contributor

cbron commented Aug 1, 2020

This removal won’t affect anything because it falls into the multi-format ? Both of those have timezone at the end so what happens if time stamp doesn’t have timezone, does it still match ?

Copy link
Contributor

@cbron cbron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I get it now. We need to do testing on k3s and rke logs to ensure this doesn’t regress.

@aiwantaozi
Copy link
Contributor Author

aiwantaozi commented Aug 4, 2020

Sorry for reply late, docker json-file logs and containerd logs format is different, but the time both field generate with RFC 3339Nano timestamp, related doc. Fluentd use strptime to parse time_format, strptime can handle the timezone.

Here is a similar PR in Kubernetes repo.

@cbron cbron merged commit 3d7c3b3 into rancher:master Aug 4, 2020
@cbron
Copy link
Contributor

cbron commented Aug 4, 2020

Need to backport to 2.4 also

uhhc pushed a commit to uhhc/rancher that referenced this pull request Sep 29, 2020
Remove duplicated configure for cluster logging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants