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

monitor: Output non-trace messages to stderr #18479

Merged
merged 1 commit into from
Jan 21, 2022
Merged

monitor: Output non-trace messages to stderr #18479

merged 1 commit into from
Jan 21, 2022

Conversation

YutaroHayakawa
Copy link
Member

@YutaroHayakawa YutaroHayakawa commented Jan 14, 2022 •
edited
Loading

When we use JSON output of cilium monitor, it would be useful if we
could filter the output by the tools like JQ. However, since some of the
non-JSON messages are generated through stdout, it ends up to the JSON
parse error. Fix cilium monitor command to generate non-JSON messages
to stderr.

Signed-off-by: Yutaro Hayakawa yutaro.hayakawa@isovalent.com

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

@YutaroHayakawa
monitor: Output non-trace messages to stderr
9fd059a 
When we use JSON output of cilium monitor, it would be useful if we
could filter the output by the tools like JQ. However, since some of the
non-JSON messages are generated through stdout, it ends up to the JSON
parse error. Fix `cilium monitor` command to generate non-JSON messages
to stderr.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa YutaroHayakawa requested a review from a team as a code owner January 14, 2022 07:42
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 14, 2022
@pchaigno pchaigno added needs-backport/1.11 release-note/bug This PR fixes an issue in a previous release of Cilium. labels Jan 14, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 14, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.1 Jan 14, 2022
@pchaigno pchaigno added area/monitor Impacts monitoring, access logging, flow logging, visibility of datapath traffic. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jan 14, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 14, 2022
@pchaigno
Copy link
Member

pchaigno commented Jan 14, 2022 •
edited by maintainer-s-little-helper bot
Loading

/test

Job 'Cilium-PR-K8s-1.21-kernel-5.4' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig Host firewall With VXLAN

Failure Output

FAIL: Failed to reach 10.0.1.202:80 from testclient-host-b4wzd

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.21-kernel-5.4 so I can create a new GitHub issue to track it.

Job 'Cilium-PR-Runtime-net-next' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

RuntimePrivilegedUnitTests Run Tests

Failure Output

FAIL: Failed to run privileged unit tests

If it is a flake, comment /mlh new-flake Cilium-PR-Runtime-net-next so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.22-kernel-4.19' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master

Failure Output

FAIL: terminating containers are not deleted after timeout

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.22-kernel-4.19 so I can create a new GitHub issue to track it.

michi-covalent
michi-covalent approved these changes Jan 15, 2022
View reviewed changes
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

🚢

@pchaigno
Copy link
Member

pchaigno commented Jan 17, 2022 •
edited
Loading

Not sure why the Jenkins tests didn't start 🤔

I had to retrigger them individually.

@joestringer joestringer added this to Needs backport from master in 1.11.2 Jan 18, 2022
@joestringer joestringer removed this from Needs backport from master in 1.11.1 Jan 18, 2022
@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Jan 20, 2022 •
edited by maintainer-s-little-helper bot
Loading

/test

Job 'Cilium-PR-K8s-1.22-kernel-4.19' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master

Failure Output

FAIL: terminating containers are not deleted after timeout

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.22-kernel-4.19 so I can create a new GitHub issue to track it.

@YutaroHayakawa
Copy link
Member Author

/test-1.22-4.19

@YutaroHayakawa
Copy link
Member Author

I mistakenly trigger the CI, but the failing test was a known flake (#18447). So, now let me mark this PR as ready-to-merge.

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 20, 2022
@kkourt kkourt merged commit 97c473f into cilium:master Jan 21, 2022
@kkourt kkourt mentioned this pull request Jan 26, 2022
Merged
@glibsm glibsm added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Jan 30, 2022
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.11 in 1.11.2 Feb 23, 2022
@joestringer joestringer mentioned this pull request Feb 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

Assignees
No one assigned
Labels
area/monitor Impacts monitoring, access logging, flow logging, visibility of datapath traffic. backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. 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
No open projects
1.11.2
Backport done to v1.11
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@YutaroHayakawa @pchaigno @michi-covalent @glibsm @kkourt