Skip to content

Conversation

pasteley
Copy link
Contributor

@pasteley pasteley commented Jun 24, 2025

This PR allows you to configure the policy map pressure via Helm values. Previously, it was difficult to determine the baseline map pressure before applying a network policy, which could potentially cause a sudden increase (e.g., from 9% to 100%). With this change, you have better control and visibility over policy map pressure settings

Allow setting threshold for bpf policy map pressure metrics via Helm

@pasteley pasteley requested review from a team as code owners June 24, 2025 12:46
@pasteley pasteley requested review from marseel and derailed June 24, 2025 12:46
@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 Jun 24, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jun 24, 2025
@pasteley pasteley force-pushed the IK8SNET-681 branch 2 times, most recently from 97284fd to 0cbcfaa Compare June 24, 2025 14:34
@joestringer joestringer added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs labels Jun 24, 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 Jun 24, 2025
@squeed
Copy link
Contributor

squeed commented Jun 25, 2025

Oh, this makes total sense. Once v1.18 has cut, we can land this in main.

I had a wild idea: we could theoretically expose a histogram of policy map pressure; given that histograms are intended to expose sampled data. The only problem is that histograms, as implemented in prometheus-go are intended for "streaming", e.g. individual request duration. There's no obvious easy way to fill all the buckets at once.

Anyways, no need to take this seriously, just some brain cells this PR ticked.

@squeed
Copy link
Contributor

squeed commented Jun 25, 2025

On a more relevant note: the documentation CI job is failing. Please run make -C Documentation update-cmdref

squeed
squeed previously requested changes Jun 25, 2025
Copy link
Contributor

@squeed squeed left a comment

Choose a reason for hiding this comment

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

one change, otherwise looks good.

@pasteley
Copy link
Contributor Author

Oh, this makes total sense. Once v1.18 has cut, we can land this in main.

Don't you mind if we backport it to 1.17 as well?

@pasteley pasteley requested a review from squeed June 25, 2025 15:33
@marseel
Copy link
Contributor

marseel commented Jun 26, 2025

@pasteley seems like you need to update couple of tests: https://github.com/cilium/cilium/actions/runs/15878956787/job/44773797407?pr=40188
and also Documentation: https://github.com/cilium/cilium/actions/runs/15878956808/job/44773797208?pr=40188 by running make -C Documentation update-helm-values

@pasteley pasteley requested a review from a team as a code owner June 26, 2025 13:26
@pasteley pasteley requested a review from a team as a code owner July 14, 2025 17:13
@pasteley pasteley requested a review from hemanthmalla July 14, 2025 17:13
Copy link
Contributor

@tommyp1ckles tommyp1ckles left a comment

Choose a reason for hiding this comment

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

Looks good, just some lint issues.

@pasteley pasteley force-pushed the IK8SNET-681 branch 3 times, most recently from 58eb5c6 to 011e15e Compare July 15, 2025 12:14
@pasteley
Copy link
Contributor Author

@squeed We need your review here again

@jrajahalme
Copy link
Member

/test

@joestringer
Copy link
Member

joestringer commented Aug 6, 2025

@cilium/fqdn looks like this still needs approval from your codeowners, @squeed is on vacation currently.

@pasteley this PR has conflicts against main that need to be resolved, would you mind doing one more rebase?

@bimmlerd bimmlerd removed the request for review from squeed August 7, 2025 08:19
@bimmlerd
Copy link
Member

bimmlerd commented Aug 7, 2025

Ah - unfortunately I lack the power to dismiss @squeed's review - hence when I removed his review request so that this PR can progress, GitHub translated that back to a "Changes Requested". Could a committer dismiss his review (I feel it's adequately addressed) so this PR can progress, once rebased?

@joestringer joestringer requested a review from squeed August 7, 2025 14:46
@joestringer
Copy link
Member

Normally there's some box near the bottom of the page that allows committers to dismiss the review and post a text comment why, but I'm not seeing that UI element on the page right now.

Based on the UI state it looks like this is non-blocking however, I can click the 'Merge when ready' button. I will do so and kick off the tests.

@joestringer joestringer enabled auto-merge August 7, 2025 14:47
@joestringer
Copy link
Member

/test

Signed-off-by: pasteley <ceasebeing@gmail.com>
auto-merge was automatically disabled August 7, 2025 19:50

Head branch was pushed to by a user without write access

@joestringer
Copy link
Member

/test

@parlakisik
Copy link
Contributor

@pasteley Can you rebase your code. The failing check was fixed in the main stream.

@joestringer joestringer dismissed squeed’s stale review August 21, 2025 03:01

Feedback was addressed

@joestringer joestringer enabled auto-merge August 21, 2025 03:01
@joestringer joestringer added this pull request to the merge queue Aug 21, 2025
Merged via the queue into cilium:main with commit 51901b1 Aug 21, 2025
68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. 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.