Skip to content

Conversation

jibi
Copy link
Member

@jibi jibi commented Sep 17, 2024

No description provided.

@jibi jibi added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/agent Cilium agent related. feature/conntrack labels Sep 17, 2024
@jibi jibi requested a review from borkmann September 17, 2024 08:51
@jibi jibi requested review from a team as code owners September 17, 2024 08:51
@jibi jibi force-pushed the pr/jibi/conntrack-accounting-flag branch 2 times, most recently from 8bca9e6 to 7d58f26 Compare September 17, 2024 09:19
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.

Helm LGTM. But is there a reason there is no usage or help flag like so?:

flags.Bool(option.BPFEventsTraceEnabled, defaults.BPFEventsTraceEnabled, "Expose 'trace' events for Cilium monitor and/or Hubble")
option.BindEnv(vp, option.BPFEventsTraceEnabled)

In particular I'm not sure the configmap bit is picked up without BindEnv. Am I missing something?

Signed-off-by: Gilberto Bertin <jibi@cilium.io>
@jibi jibi force-pushed the pr/jibi/conntrack-accounting-flag branch from 7d58f26 to 61cc03e Compare September 17, 2024 12:21
@jibi
Copy link
Member Author

jibi commented Sep 17, 2024

Helm LGTM. But is there a reason there is no usage or help flag like so?:

nite catch, I just overlooked that

flags.Bool(option.BPFEventsTraceEnabled, defaults.BPFEventsTraceEnabled, "Expose 'trace' events for Cilium monitor and/or Hubble")
option.BindEnv(vp, option.BPFEventsTraceEnabled)

In particular I'm not sure the configmap bit is picked up without BindEnv. Am I missing something?

(didn't notice I was missing it as the agent is still picking up the flag from the configmap)

@jibi
Copy link
Member Author

jibi commented Sep 17, 2024

/test

@gandro
Copy link
Member

gandro commented Sep 17, 2024

(didn't notice I was missing it as the agent is still picking up the flag from the configmap)

Ah right, because the configmap is mounted and read manually, and not via ENV variables

@jibi jibi enabled auto-merge September 18, 2024 07:31
@jibi jibi added this pull request to the merge queue Sep 18, 2024
Merged via the queue into main with commit c308885 Sep 18, 2024
262 checks passed
@jibi jibi deleted the pr/jibi/conntrack-accounting-flag branch September 18, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/agent Cilium agent related. area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. feature/conntrack release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants