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

hubble: Support --since requests in combination with follow-mode #13324

Merged
merged 1 commit into from Sep 29, 2020

Conversation

gandro
Copy link
Member

@gandro gandro commented Sep 29, 2020

Previously, the observer implementation assumed time range filters on
the request are not compatible with follow-mode. This however is no
longer the case, we can now apply the since filter when rewinding the
ring buffer. This means that if the user specifies a since timestamp,
we first dump all flows newer than since before we enter follow-mode.

Fixes: cilium/hubble#363

Fix issue where Hubble did not properly support `--follow` queries with a `--since` filter

@gandro gandro added release-note/bug This PR fixes an issue in a previous release of Cilium. sig/hubble Impacts hubble server or relay needs-backport/1.8 labels Sep 29, 2020
@gandro gandro requested a review from a team September 29, 2020 13:45
@maintainer-s-little-helper

This comment has been minimized.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Sep 29, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.4 Sep 29, 2020
@gandro gandro force-pushed the pr/gandro/hubble-follow-with-since branch from 166dd04 to 949ecd5 Compare September 29, 2020 13:46
@maintainer-s-little-helper

This comment has been minimized.

@gandro gandro force-pushed the pr/gandro/hubble-follow-with-since branch from 949ecd5 to 7628787 Compare September 29, 2020 13:47
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Sep 29, 2020
Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

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

That's a nice improvement and the code is even more intuitive to read!

@gandro
Copy link
Member Author

gandro commented Sep 29, 2020

test-me-please

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.

💯

@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 Sep 29, 2020
@gandro gandro force-pushed the pr/gandro/hubble-follow-with-since branch from 7628787 to 4abcbc4 Compare September 29, 2020 17:44
@gandro gandro removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 29, 2020
Previously, the observer implementation assumed time range filters on
the request are not compatible with follow-mode. This however is no
longer the case, we can now apply the since filter when rewinding the
ring buffer. This means that if the user specifies a `since` timestamp,
we first dump all flows newer than `since` before we enter follow-mode.

Fixes: cilium/hubble#363

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/hubble-follow-with-since branch from 4abcbc4 to 5277eb8 Compare September 29, 2020 17:47
@gandro
Copy link
Member Author

gandro commented Sep 29, 2020

test-me-please

Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

net-next hit #13062. All other tests have passed and have all code owner approval. Merging.

@christarazi christarazi merged commit 0b8148b into master Sep 29, 2020
@christarazi christarazi deleted the pr/gandro/hubble-follow-with-since branch September 29, 2020 22:20
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.4 Sep 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.8 to Needs backport from master in 1.8.4 Sep 30, 2020
@joestringer joestringer removed this from Needs backport from master in 1.8.4 Sep 30, 2020
@joestringer joestringer added this to Needs backport from master in 1.8.5 Sep 30, 2020
@tklauser tklauser moved this from Needs backport from master to Backport done to v1.8 in 1.8.5 Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/bug This PR fixes an issue in a previous release of Cilium. sig/hubble Impacts hubble server or relay
Projects
No open projects
1.8.5
Backport done to v1.8
Development

Successfully merging this pull request may close these issues.

GetFlows/hubble observe doesn't support both --since and --follow at the same time
7 participants