8000 daemon: fix hubble can't be disabled by devodev · Pull Request #37587 · cilium/cilium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

daemon: fix hubble can't be disabled #37587

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
Merged
merged 1 commit into from
Feb 13, 2025
Merged

daemon: fix hubble can't be disabled #37587

merged 1 commit into from
Feb 13, 2025

Conversation

devodev
Copy link
Contributor
@devodev devodev commented Feb 12, 2025 •
edited by rolinh
Loading

As part of #35206, we refactored Hubble as a Hive cell, and we inadvertently changed the default EnableHubble to true.

Since the helm chart defaults to not setting EnableHubble at all if the option is false in the values file, it is currently not possible to disable Hubble.

This takes care of setting back the default value of the flag to false.

This PR needs to be backported to 1.17.

Before:

func InitGlobalFlags(cmd *cobra.Command, vp *viper.Viper) {
...
	flags.Bool(option.EnableHubble, false, "Enable hubble server")
 	option.BindEnv(vp, option.EnableHubble)

Now:

var defaultConfig = config{
 	EnableHubble: true,

Helm cilium configmap template

{{- if .Values.hubble.enabled }}
  # Enable Hubble gRPC service.
  enable-hubble: {{ .Values.hubble.enabled | quote }}
Fix a regression that made it impossible to disable Hubble via Helm charts

@devodev devodev requested a review from a team as a code owner February 12, 2025 16:01
@devodev devodev requested a review from rolinh February 12, 2025 16:01
@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 Feb 12, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Feb 12, 2025
@devodev
daemon: fix hubble can't be disabled
1b5c729 
As part of cilium#35206, we
refactored Hubble as a Hive cell, and we inadvertently changed
the default EnableHubble to true.

Since the helm chart defaults to not setting EnableHubble
at all if the option is false in the values file, it is currently
not possible to disable Hubble.

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>
@devodev devodev force-pushed the pr/devodev/fix-hubble-cant-be-disabled branch from fc6f04e to 1b5c729 Compare February 12, 2025 18:51
@rolinh rolinh added sig/hubble area/hubble Impacts hubble server or relay release-note/bug This PR fixes an issue in a previous release of Cilium. labels Feb 13, 2025
@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 Feb 13, 2025
@rolinh rolinh added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Feb 13, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 13, 2025
rolinh
rolinh approved these changes Feb 13, 2025
View reviewed changes
Copy link
Member
@rolinh rolinh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good catch! As of note, Hubble remains enabled by default via Helm charts (which is what we want).

PS: please, make sure to use the "release-note" section for such cases with a clear one-liner intended for end-users when the read the release notes.

@rolinh
Copy link
Member
rolinh commented Feb 13, 2025

/test

@rolinh rolinh enabled auto-merge February 13, 2025 07:50
@rolinh rolinh added this pull request to the merge queue Feb 13, 2025
@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 Feb 13, 2025
Merged via the queue into cilium:main with commit 97aeb9e Feb 13, 2025
64 checks passed
@nbusseneau nbusseneau mentioned this pull request Feb 14, 2025
Merged
22 tasks
@nbusseneau nbusseneau added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Feb 14, 2025
This was referenced Feb 18, 2025
Closed
Merged
@github-actions github-actions bot added backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. and removed backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. labels Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rolinh rolinh rolinh approved these changes

Assignees

@devodev devodev

Labels
area/hubble Impacts hubble server or relay backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. kind/community-contribution This was a contribution made by a community member. 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.
3 participants
@devodev @rolinh @nbusseneau
0