Introduce dynamic hubble flow logs exporters based on config file #28873

marqc · 2023-10-30T15:26:53Z

All code is covered by unit and/or runtime tests where feasible.
All commits contain a well written commit description including a title,
description and a Fixes: #XXX line if the commit addresses a particular
GitHub issue.
All commits are signed off. See the section Developer’s Certificate of Origin
Provide a title or release-note blurb suitable for the release notes.

Introduce dynamic hubble flowlog exporters configuration guided by config file.
Changes in the config file are reflected without a need to restart cilium-agent, so it allows to dynamically specify flowlog requests. Each request contains the output file path, field mask, include filters (to allow flows), exclude filters (to deny flows) and end time at which flowlog will automatically stop collecting flows. The low-level implementation reuses a current hubble exporter. The config file will be populated from configmap (but allows customization if needed).
The config file is in YAML format, sample config file.

flowlogs:
- name: "test001"
  filePath: "/var/log/network/flow-log/pa/test001.log"
  fieldMask: []
  includeFilters: []
  excludeFilters: []
  end: "2023-10-09T23:59:59-07:00"
- name: "test002"
  filePath: "/var/log/network/flow-log/pa/test002.log"
  fieldMask: ["source.namespace", "source.pod_name", "destination.namespace", "destination.pod_name", "verdict"]
  includeFilters:
  - source_pod: ["default/"]
    event_type:
    - type: 1
  - destination_pod: ["frontend/nginx-975996d4c-7hhgt"]
  excludeFilters: []
  end: "2023-10-09T23:59:59-07:00"

This is a follow up from discussion in #28220 cc: @chancez @AwesomePatrol @nathanperkins

Fixes: #25508

TODO list:

~~replace periodic file reads with inotify~~ As this PR is already quite big I will do it in a separate PR
add metrics (gauge with active flowlogs, cumulative counters with number of config changes applied)
modify helm charts to cover new dynamic flowlog feature (deploy empty config as CM and mount to cilium-agent)
docs

Add dynamic flowlog exporters configured by yaml file (configmap) without a need of agent restart.

pkg/hubble/exporter/api.go

chancez · 2023-10-31T16:38:40Z

I'll take a deeper look a bit later this week but so far the general approach looks good.

One thought: can get some metrics? A few useful metrics:

a hash of the config and expose that as a gauge. It would help identify when nodes all have the most up to date configuration.
a counter for number of configuration reload failures. If the new config is invalid, it would be ideal to have a metric that can be used to detect this case.

derailed

@marqc Nice work! Cool feature.

pkg/hubble/exporter/api.go

pkg/hubble/exporter/config_watcher.go

pkg/hubble/exporter/dynamic_exporter.go

marqc · 2023-11-02T14:46:20Z

One thought: can get some metrics? A few useful metrics:

a hash of the config and expose that as a gauge. It would help identify when nodes all have the most up to date configuration.

a counter for number of configuration reload failures. If the new config is invalid, it would be ideal to have a metric that can be used to detect this case.

Done. As flowlog already has a "name" identifying it I use it instead of calculating hash in metrics.

marqc · 2023-11-27T10:28:52Z

/test

rolinh

It looks like I'm late to the review party, sorry about that.

That's a great contribution Marek! The implementation looks pretty solid overall, I only have a few comments (see below).

pkg/hubble/exporter/api.go

pkg/hubble/exporter/config_watcher.go

pkg/hubble/exporter/dynamic_exporter.go

rolinh

Great work 🚀 ! Let's get this in before the v1.15 cut.

rolinh · 2023-11-29T14:51:37Z

/test

ql-owo-lp

Overall good job. Thanks!

install/kubernetes/cilium/templates/cilium-flowlog-configmap.yaml

pkg/hubble/exporter/api.go

rolinh · 2023-12-01T14:01:06Z

/test

Signed-off-by: Marek Chodor <mchodor@google.com>

marqc · 2023-12-01T14:53:33Z

/test

rolinh · 2023-12-01T15:57:21Z

/ci-ipsec-upgrade

aanm · 2023-12-01T16:38:43Z

For some reason, travis didn't run. Travis is only running arm64 unit tests so it's safe to assume that this PR can be merged. Thank you for the contribution.

Add protoc-gen-gotag plugin to builder image. Add yaml annotations to flow proto gen files. Introduce dynamic hubble flow logs exporters based on config file. Signed-off-by: Marek Chodor <mchodor@google.com> Carry-Patch-Approval: b/309936968 Upstream-PR: cilium#28873 Upstream-PR: cilium#28943 Upstream-PR: cilium#28944 Change-Id: I7871d8b119822c3228a6e959b86dd1b6b23d275a Reviewed-on: https://gke-internal-review.googlesource.com/c/third_party/cilium/+/860928 Unit-Verified: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Reviewed-by: Akhil Velagapudi <avelagap@google.com> Reviewed-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Tested-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com>

Carry-Patch-Approval: b/309936968 Upstream-PR: cilium#28873 Change-Id: Ie0028dfba1ad9f6185ddb996582a934eb11b5452 Reviewed-on: https://gke-internal-review.googlesource.com/c/third_party/cilium/+/866204 Reviewed-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Tested-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Prow-Auto-Submit: Marek Chodor <mchodor@google.com> Reviewed-by: Alan Kutniewski <kutniewski@google.com> Unit-Verified: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com>

The new parser utilizes json tags to keep API consistent. This also reverts cherry-pick of cilium#28944 as this is no longer needed. Carry-Patch-Approval: b/309936968 Upstream-PR: cilium#28873 Change-Id: I8d44e7f937fb56f3c5c6c4ed943586d39a4ea17b Reviewed-on: https://gke-internal-review.googlesource.com/c/third_party/cilium/+/867331 Tested-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Unit-Verified: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Reviewed-by: Akhil Velagapudi <avelagap@google.com> Reviewed-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Prow-Auto-Submit: Marek Chodor <mchodor@google.com>

Carry-Patch-Approval: b/309936968 Upstream-PR: cilium#28873 Change-Id: Ib38ed5f1581900ab06da92cb2f3931b0cadad152 Reviewed-on: https://gke-internal-review.googlesource.com/c/third_party/cilium/+/867864 Prow-Auto-Submit: Marek Chodor <mchodor@google.com> Reviewed-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Unit-Verified: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Reviewed-by: Akhil Velagapudi <avelagap@google.com> Tested-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com>

Carry-Patch-Approval: b/309936968 Upstream-PR: cilium#28873 Change-Id: Id7a619b9fb19ae6ee4f1ecc2549ac7dec5c64019 Reviewed-on: https://gke-internal-review.googlesource.com/c/third_party/cilium/+/873423 Reviewed-by: Dorde Lapcevic <dordel@google.com> Prow-Auto-Submit: Marek Chodor <mchodor@google.com> Tested-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Reviewed-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Unit-Verified: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com>

Upstream-PR: cilium#28873 Signed-off-by: Marek Chodor <mchodor@google.com> Change-Id: Ib2b6937ba276d7fee127b856c02212053d9bb3ee Reviewed-on: https://gke-internal-review.googlesource.com/c/third_party/cilium/+/899008 Unit-Verified: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Reviewed-by: Akhil Velagapudi <avelagap@google.com> Reviewed-by: Aleksander Mistewicz <amistewicz@google.com> Tested-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com> Reviewed-by: Prow_Bot_V2 <425329972751-compute@developer.gserviceaccount.com>

marqc requested a review from a team as a code owner October 30, 2023 15:26

marqc requested a review from a user October 30, 2023 15:26

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 Oct 30, 2023

marqc marked this pull request as draft October 30, 2023 15:26

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Oct 30, 2023

marqc force-pushed the dynamic_hubble_flow_logs branch from 890c558 to 43f3aa0 Compare October 31, 2023 14:14

marqc marked this pull request as ready for review October 31, 2023 14:27

marqc requested review from a team as code owners October 31, 2023 14:27

marqc requested review from marseel and derailed October 31, 2023 14:27

marqc commented Oct 31, 2023

View reviewed changes

pkg/hubble/exporter/api.go Outdated Show resolved Hide resolved

ghost added the sig/hubble label Oct 31, 2023

michi-covalent added the release-note/major This PR introduces major new functionality to Cilium. label Oct 31, 2023

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 Oct 31, 2023

derailed requested changes Nov 1, 2023

View reviewed changes

marqc force-pushed the dynamic_hubble_flow_logs branch from 43f3aa0 to 1c1e15f Compare November 2, 2023 10:33

This was referenced Nov 2, 2023

Add protoc-gen-gotag plugin to builder image #28943

Closed

Gen yaml annotations for flow proto #28944

Closed

marqc force-pushed the dynamic_hubble_flow_logs branch from 1c1e15f to 970218b Compare November 2, 2023 12:23

marqc requested a review from a team as a code owner November 2, 2023 12:23

marqc requested a review from rolinh November 2, 2023 12:23

marqc force-pushed the dynamic_hubble_flow_logs branch from 970218b to d0d532e Compare November 2, 2023 12:31

bimmlerd mentioned this pull request Nov 2, 2023

restore full go vet behaviour #28945

Merged

marqc force-pushed the dynamic_hubble_flow_logs branch 2 times, most recently from 58cbaac to 40ceaec Compare November 2, 2023 14:28

marqc force-pushed the dynamic_hubble_flow_logs branch from 40ceaec to d4f57ff Compare November 2, 2023 14:48

marqc requested a review from derailed November 2, 2023 14:53

rolinh requested changes Nov 27, 2023

View reviewed changes

marqc force-pushed the dynamic_hubble_flow_logs branch from 6f2b41c to e510e02 Compare November 28, 2023 10:19

marqc requested a review from rolinh November 28, 2023 10:21

rolinh approved these changes Nov 28, 2023

View reviewed changes

marqc force-pushed the dynamic_hubble_flow_logs branch from e510e02 to d33a197 Compare November 28, 2023 10:53

ql-owo-lp approved these changes Dec 1, 2023

View reviewed changes

install/kubernetes/cilium/templates/cilium-flowlog-configmap.yaml Outdated Show resolved Hide resolved

pkg/hubble/exporter/api.go Show resolved Hide resolved

marqc force-pushed the dynamic_hubble_flow_logs branch from d33a197 to 2ff9189 Compare December 1, 2023 13:53

marqc force-pushed the dynamic_hubble_flow_logs branch from 2ff9189 to e20dea7 Compare December 1, 2023 14:37

Introduce dynamic hubble flow logs exporters based on config file.

6aa8733

Signed-off-by: Marek Chodor <mchodor@google.com>

marqc force-pushed the dynamic_hubble_flow_logs branch from e20dea7 to 6aa8733 Compare December 1, 2023 14:46

nathanjsweet approved these changes Dec 1, 2023

View reviewed changes

rolinh enabled auto-merge December 1, 2023 16:38

aanm merged commit f51bc04 into cilium:main Dec 1, 2023

chancez mentioned this pull request Dec 14, 2023

Add rate limiting on BPF events map #29711

Merged

8 tasks

chancez mentioned this pull request Feb 15, 2024

Hubble: add no-metrics label suport for namespace or pod #30789

Closed

glibsm mentioned this pull request May 16, 2024

CI: TestDynamicExporterLifecycle #32587

Closed

joestringer mentioned this pull request Nov 8, 2024

hubble: Lock exporters while gathering metrics #35860

Merged

ozerovandrei mentioned this pull request Dec 10, 2024

Add Hubble exporter compression to Helm chart #36489

Closed

Introduce dynamic hubble flow logs exporters based on config file #28873

Introduce dynamic hubble flow logs exporters based on config file #28873

Uh oh!

Conversation

marqc commented Oct 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

chancez commented Oct 31, 2023

Uh oh!

derailed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marqc commented Nov 2, 2023

Uh oh!

marqc commented Nov 27, 2023

Uh oh!

rolinh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rolinh left a comment

Choose a reason for hiding this comment

Uh oh!

rolinh commented Nov 29, 2023

Uh oh!

ql-owo-lp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rolinh commented Dec 1, 2023

Uh oh!

marqc commented Dec 1, 2023

Uh oh!

rolinh commented Dec 1, 2023

Uh oh!

aanm commented Dec 1, 2023

Uh oh!

Uh oh!

marqc commented Oct 30, 2023 •

edited

Loading