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

helm: Add hubble section #10358

Merged
merged 2 commits into from Mar 5, 2020
Merged

helm: Add hubble section #10358

merged 2 commits into from Mar 5, 2020

Conversation

michi-covalent
Copy link
Contributor

@michi-covalent michi-covalent commented Feb 27, 2020

2 commits:

  • add a hidden cilium observe subcommand to get flows from hubble.
  • update helm.

This change is Reviewable

@michi-covalent michi-covalent requested a review from a team February 27, 2020 01:40
@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 45.43% when pulling 1ee747a on pr/michi/helm into 7a24487 on master.

@coveralls
Copy link

coveralls commented Feb 27, 2020

Coverage Status

Coverage increased (+0.002%) to 45.618% when pulling fd9f75c on pr/michi/helm into 9c34630 on master.

@aanm aanm added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Feb 27, 2020
@aanm aanm added the dont-merge/waiting-for-upstream Only merge once upstream library or kernel features have landed label Feb 27, 2020
Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

I don't see where HUBBLE_DEFAULT_SOCKET_PATH nor HUBBLE_GROUP_NAME are used in the cilium's code base so I assume this is waiting for #10238 to be merged first?

@aanm aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 27, 2020
@michi-covalent
Copy link
Contributor Author

I don't see where HUBBLE_DEFAULT_SOCKET_PATH nor HUBBLE_GROUP_NAME are used in the cilium's code base so I assume this is waiting for #10238 to be merged first?

right. these are env variables in hubble that will be available once #10238 is merged, although this pr is not really blocked on it since it doesn't add any hubble-related settings by default unless you specify them in helm command.

@michi-covalent michi-covalent requested review from a team as code owners March 5, 2020 00:47
@michi-covalent michi-covalent added wip and removed dont-merge/waiting-for-upstream Only merge once upstream library or kernel features have landed dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. labels Mar 5, 2020
@michi-covalent michi-covalent changed the title helm: Add hubble section WIP: helm: Add hubble section Mar 5, 2020
The observe subcommand executes Hubble's observe command. We are still
trying to figure out how to distribute Hubble's CLI. This is a short
term workaround so that people can use it to interact with Hubble in
the meantime.

Ref #9925

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Add hubble section to values.yaml, and add new Hubble-related fields
to the configmap. The configmap has these new hubble-related fields:

- hubble-listen-addresses
    List of addresses for Hubble to listen to. Disabled if empty.
- hubble-flow-buffer-size
    Number of recent flows for Hubble to cache. Defaults to 4096.
- hubble-event-queue-size
    Buffer size of the events channel. Defaults to 128.
- hubble-metrics-server
    Address for the metric server to listen to. Disabled if empty.
- hubble-metrics
    List of metrics to collect.

This PR also adds 2 environment variables if Hubble is enabled:

- HUBBLE_GROUP_NAME
     Group for Hubble's unix domain sockets. Hardcoded to be `cilium`.
- HUBBLE_DEFAULT_SOCKET_PATH
     Default Hubble gRPC endpoint for observe/status commands. This
     is set to the first address in hubble-listen-addresses.

Here is a sample helm command to configure Hubble-related settings:

    helm template cilium \
      --set global.hubble.listenAddresses="{unix:///var/run/cilium/hubble.sock,unix:///var/run/cilium/hubble.sock2}" \
      --set global.hubble.eventQueueSize=1234 \
      --set global.hubble.flowBufferSize=5678 \
      --set global.hubble.metricsServer=":7071" \
      --set global.hubble.metrics="{dns:query;ignoreAAAA,drop,tcp,flow,port-distribution,icmp,http}"

Ref #9925

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@michi-covalent michi-covalent changed the title WIP: helm: Add hubble section helm: Add hubble section Mar 5, 2020
@michi-covalent
Copy link
Contributor Author

test-me-please

@aanm aanm requested a review from gandro March 5, 2020 08:49
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.

See comment regarding event queue size. I'm fine if we fix this in a follow-up PR though.

install/kubernetes/cilium/values.yaml Show resolved Hide resolved
@tgraf tgraf merged commit b3b29b3 into master Mar 5, 2020
1.8.0 automation moved this from In progress to Merged Mar 5, 2020
@tgraf tgraf deleted the pr/michi/helm branch March 5, 2020 09:30
@rolinh rolinh added the sig/hubble Impacts hubble server or relay label Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/hubble Impacts hubble server or relay
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants