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

cmd/observer: add support for agent event sub-type filters #465

Merged
merged 3 commits into from
Jan 14, 2021

Conversation

tklauser
Copy link
Member

The first commit add test coverage for the functionality changed in the successive commits.
The second commit does some small refactoring to avoid duplicating numeric sub-type parsing.
The third commit introduces the actual agent event sub-type filtering. One caveat was that the monitor API (github.com/cilium/cilium/pkg/monitor/api) doesn't provide the sub-type strings a form which would be convenient for CLI arguments (i.e. they contain spaces). Thus, github.com/cilium/cilium/pkg/monitor/api.AgentNotifications is duplicated in cmd/observe in more convenient form. A test is added to ensure the two maps are kept in sync.

See individual commit messages for more details.

Increase test coverage and ensure the changes in the successive
commits won't break existing functionality.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Instead of falling through to the default case, move the default case
out of the switch statement.

Preparatory refactoring for parsing agent event subtype in the next
commit which will require adding an additional case to the switch
statement for which we want to fall back to numeric parsing as well.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Allow to filter by agent event sub-type using "-t agent:<sub-type>".

Unfortunately, the sub-type strings in the monitor API are free form
descriptions and contain uppercase characters and spaces. This makes
them cumbersome and not user-friendly for CLI filter arguments.

Thus, keep our own,  cleaned values for these sub-types and make sure in
TestAgentEventSubTypeMap that they are kept in sync with
monitorAPI.AgentNotifications.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser added ⌨️ area/cli Impacts the command line interface of any command in the repository. release-note/minor This PR introduces functionality that users may find relevant to operating Hubble. labels Jan 14, 2021
@tklauser tklauser requested a review from gandro January 14, 2021 12:53
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Nice. Awesome work with the extended unit tests!

@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 Jan 14, 2021
@tklauser tklauser merged commit c6b7205 into master Jan 14, 2021
@tklauser tklauser deleted the pr/tklauser/agent-event-subtype branch January 14, 2021 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⌨️ area/cli Impacts the command line interface of any command in the repository. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR introduces functionality that users may find relevant to operating Hubble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants