Skip to content

Register logging flags with operator main cmd#12395

Merged
aanm merged 1 commit intomasterfrom
vladu/fix-operator-json-logging
Jul 6, 2020
Merged

Register logging flags with operator main cmd#12395
aanm merged 1 commit intomasterfrom
vladu/fix-operator-json-logging

Conversation

@ungureanuvladvictor
Copy link
Copy Markdown
Member

@ungureanuvladvictor ungureanuvladvictor commented Jul 3, 2020

Signed-off-by: Vlad Ungureanu vladu@palantir.com

Fixes #11133.

In #11133 JSON logging was added in the operator but the flags were never exposed to the command.

If we pass --log-opt flag now to the operator we get:

Error: unknown flag: --log-opt
Flags:
....
unknown flag: --log-opt

Register "log-driver" and "log-opt" flags with the cilium-operator command.

@ungureanuvladvictor ungureanuvladvictor added release-note/bug This PR fixes an issue in a previous release of Cilium. area/operator Impacts the cilium-operator component labels Jul 3, 2020
@ungureanuvladvictor ungureanuvladvictor requested a review from a team as a code owner July 3, 2020 06:31
@ungureanuvladvictor
Copy link
Copy Markdown
Member Author

ungureanuvladvictor commented Jul 3, 2020

test-me-please

tgraf
tgraf approved these changes Jul 3, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tgraf tgraf left a comment

Choose a reason for hiding this comment

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

@ungureanuvladvictor Is this a regression in 1.8?

@ungureanuvladvictor
Copy link
Copy Markdown
Member Author

ungureanuvladvictor commented Jul 3, 2020

@tgraf I think so given the purpose of PR I linked was to enable users to beneft from JSON based logging.

@ungureanuvladvictor
Register logging flags with operator main cmd
2a7e12c 
Signed-off-by: Vlad Ungureanu <vladu@palantir.com>
@ungureanuvladvictor ungureanuvladvictor force-pushed the vladu/fix-operator-json-logging branch from f133068 to 2a7e12c Compare July 3, 2020 17:18
@ungureanuvladvictor
Copy link
Copy Markdown
Member Author

ungureanuvladvictor commented Jul 3, 2020

test-me-please

@ungureanuvladvictor
Copy link
Copy Markdown
Member Author

ungureanuvladvictor commented Jul 3, 2020

@aanm @tgraf fixed the travis failure (forgot to render the command ref).

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 3, 2020

Coverage Status

Coverage increased (+0.05%) to 36.951% when pulling 2a7e12c on vladu/fix-operator-json-logging into f2e181e on master.

@maintainer-s-little-helper maintainer-s-little-helper Bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 3, 2020
@aanm aanm merged commit 54275be into master Jul 6, 2020
@aanm aanm deleted the vladu/fix-operator-json-logging branch July 6, 2020 08:49
@vadorovsky vadorovsky mentioned this pull request Jul 7, 2020
Merged
@aanm aanm mentioned this pull request Nov 25, 2020
Merged
@joestringer joestringer mentioned this pull request Dec 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aanm aanm aanm approved these changes

+1 more reviewer

@tgraf tgraf tgraf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/operator Impacts the cilium-operator component ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ungureanuvladvictor @coveralls @tgraf @aanm @joestringer @qmonnet @vadorovsky