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.

Sign up for GitHub

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

Jump to bottom

Bug 1852341: fix legacy syslog k8s Metadata #659

Merged
merged 2 commits into from
Aug 20, 2020
Merged

Bug 1852341: fix legacy syslog k8s Metadata #659

merged 2 commits into from
Aug 20, 2020

Conversation

blockloop
Copy link
Contributor

The use_record true setting is trying to use the "level" from the k8s
audit log which is Metadata. I believe we need to relabel k8s audit logs
"level" field to "k8s_audit_level" so that fluentd doesn't try to use it
as the severity and we can retain the original information.

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Aug 12, 2020
@openshift-ci-robot
Copy link

@blockloop: This pull request references Bugzilla bug 1852341, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.6.0) matches configured target release for branch (4.6.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1852341: fix legacy syslog k8s Metadata

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@blockloop
Copy link
Contributor Author

/cc @ewolinetz @periklis @lukas-vlcek

@ewolinetz
Copy link
Contributor

/cc @jcantrill this may be related to something that was brought up in slack earlier regarding the use_record field

@jcantrill
Copy link
Contributor

@ewolinetz i think this needs a template change as well . @lukas-vlcek ?

@jcantrill
Copy link
Contributor

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2020
periklis
periklis reviewed Aug 13, 2020
View reviewed changes
Copy link
Contributor

@periklis periklis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, besides that it needs a fix for the unit tests

@lukas-vlcek
Copy link
Contributor

lukas-vlcek commented Aug 13, 2020 •
edited
Loading

I opened a ticket in data model: ViaQ/elasticsearch-templates#121
This is 4.6 change and is it going to be back-ported?

@lukas-vlcek lukas-vlcek mentioned this pull request Aug 13, 2020
Open
lukas-vlcek
lukas-vlcek approved these changes Aug 13, 2020
View reviewed changes
Copy link
Contributor

@lukas-vlcek lukas-vlcek left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2020
@blockloop
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@blockloop
Copy link
Contributor Author

blockloop commented Aug 17, 2020 •
edited
Loading

/hold waiting on an upstream fix

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. labels Aug 17, 2020
@blockloop
Copy link
Contributor Author

/retest

@blockloop
Bug 1852341: fix legacy syslog k8s Metadata
2bcb086 
The `use_record true` setting is trying to use the "level" from the k8s
audit log which [is Metadata](https://kubernetes.io/docs/tasks/debug-application-cluster/audit/#audit-policy). I believe we need to relabel k8s audit logs
"level" field to "k8s_audit_level" so that fluentd doesn't try to use it
as the severity and we can retain the original information.
@blockloop
fix unit tests
1ec2338
@blockloop
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 19, 2020
@blockloop
Copy link
Contributor Author

/retest

@ewolinetz
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 19, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: blockloop, ewolinetz, jcantrill, lukas-vlcek

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:
  • OWNERS [ewolinetz,jcantrill]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@blockloop
Copy link
Contributor Author

/refresh

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit cb772f9 into openshift:master Aug 20, 2020
@openshift-ci-robot
Copy link

@blockloop: All pull requests linked via external trackers have merged: openshift/cluster-logging-operator#659. Bugzilla bug 1852341 has been moved to the MODIFIED state.

In response to this:

Bug 1852341: fix legacy syslog k8s Metadata

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@blockloop blockloop deleted the bjones/bug-1852341/relabel-level branch August 20, 2020 14:46
@openshift-ci-robot openshift-ci-robot mentioned this pull request Aug 31, 2020
Merged
@openshift-ci-robot openshift-ci-robot mentioned this pull request Sep 11, 2020
Merged
@jcantrill jcantrill mentioned this pull request Oct 1, 2020
Merged
pmoogi-redhat pushed a commit to pmoogi-redhat/cluster-logging-operator that referenced this pull request Apr 26, 2022
@openshift-merge-robot
Merge pull request openshift#659 from blockloop/bjones/bug-1852341/re…
8a21641 
…label-level

Bug 1852341: fix legacy syslog k8s Metadata
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@periklis periklis periklis left review comments

@lukas-vlcek lukas-vlcek lukas-vlcek approved these changes

@ewolinetz ewolinetz Awaiting requested review from ewolinetz

Assignees

@lukas-vlcek lukas-vlcek

@ewolinetz ewolinetz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@blockloop @openshift-ci-robot @ewolinetz @jcantrill @lukas-vlcek @openshift-bot @periklis @openshift-merge-robot