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

api, cmd, policy: Show rule labels in policy selectors output #27838

Conversation

christarazi
Copy link
Member

@christarazi christarazi commented Aug 31, 2023

  • policy: Plumb rule labels through selector cache
  • api, cmd, policy: Show rule labels in policy selectors output

Commits for convenience of review:

policy: Plumb rule labels through selector cache

In order to make investigating the output of cilium policy selectors
easier to understand, plumb the rule labels from KNP, CNP, and CCNPs.
This makes it so that the user can very easily see which policy the
selector came from.

Benchmark results before and after given that selectorcache can be a hot
path in the policy engine:

$ go test -v ./pkg/policy -run '^$' -bench 'BenchmarkRegenerateL3EgressPolicyRules' -test.benchtime 100x -test.benchmem -test.count 10
...
$ benchstat old.txt new.txt
name                              old time/op    new time/op    delta
RegenerateL3EgressPolicyRules-16    8.13ms ±13%    8.01ms ±11%     ~     (p=0.912 n=10+10)

name                              old alloc/op   new alloc/op   delta
RegenerateL3EgressPolicyRules-16    1.75MB ± 0%    1.94MB ± 0%  +10.78%  (p=0.000 n=10+9)

name                              old allocs/op  new allocs/op  delta
RegenerateL3EgressPolicyRules-16     24.1k ± 0%     25.1k ± 0%   +4.15%  (p=0.000 n=10+10)

api, cmd, policy: Show rule labels in policy selectors output

Use the functionality implemented in previous commits to output the rule
labels in the selector output. This is useful for understanding which
policy the selector comes from, which makes debugging issues much
easier.

Example output:

root@kind-worker:/home/cilium# cilium policy selectors
SELECTOR                                                                                                                                                            LABELS                          USERS   IDENTITIES
&LabelSelector{MatchLabels:map[string]string{k8s.io.kubernetes.pod.namespace: kube-system,k8s.k8s-app: kube-dns,},MatchExpressions:[]LabelSelectorRequirement{},}   default/tofqdn-dns-visibility   1       16500
&LabelSelector{MatchLabels:map[string]string{reserved.none: ,},MatchExpressions:[]LabelSelectorRequirement{},}                                                      default/tofqdn-dns-visibility   1
MatchName: , MatchPattern: *                                                                                                                                        default/tofqdn-dns-visibility   1       16777217
                                                                                                                                                                                                            16777218
                                                                                                                                                                                                            16777219
Improve the usability of the `cilium policy selectors` command by including the policy name and namespace in order to easily understand which selector comes from what policy

Related: #27804

@christarazi christarazi added kind/enhancement This would improve or streamline existing functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. area/cli Impacts the command line interface of any command in the repository. labels Aug 31, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Aug 31, 2023
@christarazi christarazi force-pushed the pr/christarazi/improve-policy-selector-debugging branch from 3191db7 to 3b8c355 Compare August 31, 2023 01:10
@christarazi christarazi changed the title pr/christarazi/improve policy selector debugging api, cmd, policy: Show rule labels in policy selectors output Aug 31, 2023
@christarazi christarazi force-pushed the pr/christarazi/improve-policy-selector-debugging branch from 3b8c355 to 803706a Compare August 31, 2023 01:15
@christarazi christarazi marked this pull request as ready for review August 31, 2023 01:15
@christarazi christarazi requested review from a team as code owners August 31, 2023 01:15
@christarazi
Copy link
Member Author

/test

@christarazi christarazi changed the title api, cmd, policy: Show rule labels in policy selectors output api, cmd, policy: Show rule labels in policy selectors output Aug 31, 2023
@christarazi christarazi force-pushed the pr/christarazi/improve-policy-selector-debugging branch from 803706a to 15d2a10 Compare August 31, 2023 21:24
@christarazi christarazi requested review from a team as code owners August 31, 2023 21:24
@christarazi christarazi force-pushed the pr/christarazi/improve-policy-selector-debugging branch from 15d2a10 to 8178cfe Compare August 31, 2023 21:24
@christarazi
Copy link
Member Author

christarazi commented Aug 31, 2023

@christarazi
Copy link
Member Author

/test

@christarazi
Copy link
Member Author

christarazi commented Aug 31, 2023

/test

Edit: Hit #27688

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, this is a really useful change 🚀

In order to make investigating the output of `cilium policy selectors`
easier to understand, plumb the rule labels from KNP, CNP, and CCNPs.
This makes it so that the user can very easily see which policy the
selector came from.

Benchmark results before and after given that selectorcache can be a hot
path in the policy engine:

```
$ go test -v ./pkg/policy -run '^$' -bench 'BenchmarkRegenerateL3EgressPolicyRules' -test.benchtime 100x -test.benchmem -test.count 10
...
$ benchstat old.txt new.txt
name                              old time/op    new time/op    delta
RegenerateL3EgressPolicyRules-16    8.13ms ±13%    8.01ms ±11%     ~     (p=0.912 n=10+10)

name                              old alloc/op   new alloc/op   delta
RegenerateL3EgressPolicyRules-16    1.75MB ± 0%    1.94MB ± 0%  +10.78%  (p=0.000 n=10+9)

name                              old allocs/op  new allocs/op  delta
RegenerateL3EgressPolicyRules-16     24.1k ± 0%     25.1k ± 0%   +4.15%  (p=0.000 n=10+10)
```

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Use the functionality implemented in previous commits to output the rule
labels in the selector output. This is useful for understanding which
policy the selector comes from, which makes debugging issues much
easier.

Example output:

```
root@kind-worker:/home/cilium# cilium policy selectors
SELECTOR                                                                                                                                                            LABELS                          USERS   IDENTITIES
&LabelSelector{MatchLabels:map[string]string{k8s.io.kubernetes.pod.namespace: kube-system,k8s.k8s-app: kube-dns,},MatchExpressions:[]LabelSelectorRequirement{},}   default/tofqdn-dns-visibility   1       16500
&LabelSelector{MatchLabels:map[string]string{reserved.none: ,},MatchExpressions:[]LabelSelectorRequirement{},}                                                      default/tofqdn-dns-visibility   1
MatchName: , MatchPattern: *                                                                                                                                        default/tofqdn-dns-visibility   1       16777217
                                                                                                                                                                                                            16777218
                                                                                                                                                                                                            16777219
```

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi force-pushed the pr/christarazi/improve-policy-selector-debugging branch from c593e5d to 83019d7 Compare October 2, 2023 17:11
@christarazi
Copy link
Member Author

/test

@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 Oct 2, 2023
@christarazi christarazi merged commit 1e41011 into cilium:main Oct 3, 2023
59 of 61 checks passed
@christarazi christarazi deleted the pr/christarazi/improve-policy-selector-debugging branch October 3, 2023 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Impacts the command line interface of any command in the repository. kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants