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

daemon: Make Hubble Recorder API opt-out #15781

Merged

Conversation

gandro
Copy link
Member

@gandro gandro commented Apr 20, 2021

This commits switches the default value of
--enable-hubble-recorder-api to true. It is important to note that
this change does not mean that the recorder API itself is always
enabled by default. For the Hubble Recorder API to actually be served,
both --enable-hubble and --enable-recorder also need to be enabled
(both of which are false by default).

Previous discussion: #15680 (comment)

@gandro gandro requested review from a team and nathanjsweet April 20, 2021 08:40
@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 Apr 20, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Apr 20, 2021
@gandro gandro added the release-note/misc This PR makes changes that have no direct user impact. label Apr 20, 2021
@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 Apr 20, 2021
@gandro gandro added the area/daemon Impacts operation of the Cilium daemon. label Apr 20, 2021
@gandro
Copy link
Member Author

gandro commented Apr 20, 2021

I don't think this needs a full CI run (as none of the Recorder functionality is currently tested in CI). I have manually tested that it works as expected. If disabled, theHubble CLI will fail with rpc error: code = Unimplemented desc = unknown service recorder.Recorder

@gandro gandro removed the request for review from nathanjsweet April 20, 2021 08:50
This commits switches the default value of
`--enable-hubble-recorder-api` to `true`. It is important to note that
this change does _not_ mean that the recorder API itself is always
enabled by default. For the Hubble Recorder API to actually be served,
both `--enable-hubble` and `--enable-recorder` also need to be enabled
(both of which are `false` by default).

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/hubble-recorder-api-flag-default branch from 97c9a13 to 36c9ba2 Compare April 20, 2021 09:01
@borkmann borkmann added release-blocker/1.10 feature/lb-only Impacts cilium running in lb-only datapath mode sig/hubble Impacts hubble server or relay labels Apr 20, 2021
@gandro gandro added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 20, 2021
@borkmann borkmann merged commit 4a74ba4 into cilium:master Apr 20, 2021
1.10.0 automation moved this from In progress to Done Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. feature/lb-only Impacts cilium running in lb-only datapath mode ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/hubble Impacts hubble server or relay
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants