daemon: updates the CEP in-place #32574
Conversation
Adds the base logic for handling the dynamic label filter for limiting the set of identity relevant labels used for creating the CIDs. Many labels are not useful for policy enforcement or visibility, the objective is to create CIDs only for labels used in network policies to greatly reduce the number of CIDs. Signed-off-by: Ovidiu Tirla <otirla@google.com>
Add a daemon feature flag for the dynamic label filter which is disabled by default for now. The goal is to have it enabled by default and completely replace the need to manually set the identity relevant labels. Signed-off-by: Ovidiu Tirla <otirla@google.com>
Signed-off-by: Ovidiu Tirla <otirla@google.com>
Modifies the CEP to reflect the dynamic filter labels changes Signed-off-by: Ovidiu Tirla <otirla@google.com>
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
github-actions
bot
added
the
kind/community-contribution
There was a problem hiding this comment.
Hi, thanks a bunch for the contribution! Thanks for taking the time and using hive/cell.
I've asked some code level questions, but the biggest question on my mind is whether this can turn out to be a performance bottleneck. ModifyIdentityLabels I'd assume is a pretty expensive operation - if things change, I think this regenerates all affected endpoints, correct? Do you have some insight in how this performs in a cluster?
| // If the SOURCE is any the transformed value will be empty | ||
| // k8s.Foo converts to k8s:Foo, any.Foo converts to :Foo | ||
| func normalizeKeyLabel(labelWithSourcePrefix string) string { | ||
| before, after, found := strings.Cut(labelWithSourcePrefix, ".") |
There was a problem hiding this comment.
I was under the impression that label keys can contain dots. If there's no intentional source but the label key contains a dot, does this still do the right thing?
| return exitingLabels | ||
| } | ||
|
|
||
| // getRelevantKeyLabels extracts the label key from the MatchLabels and MatchExpressions. |
There was a problem hiding this comment.
nit: I'm a bit confused by this naming - what are key labels? To me, this seems to extract a set of label keys from both the matchLabels and the matchExpressionLabels. So should this be named extractLabelKeys?
| exitingLabels[l.Key] = l | ||
| } | ||
| // Merge with pod labels | ||
| exitingLabels.MergeLabels(labels.Map2Labels(podLabels, labels.LabelSourceK8s)) |
There was a problem hiding this comment.
I have slight performance worries here - we're doing a lot of string/map allocs and deallocs shuffling these different label representations around. How often do you expect this code to run?
A more efficient representation could be to use sorted lists of immutable labels, to avoid the constant allocation and deallocation.
| identityLabelsMap := identityLabels.StringMap() | ||
| labelsToRemove := labels.Labels{} | ||
|
|
||
| for _, label := range endpointLabels { |
There was a problem hiding this comment.
Will this break in interesting ways if endpoint labels have duplicates?
| ) | ||
|
|
||
| type controller struct { | ||
|
|
|
|
||
| // EnableDynamicLabelFilter enables to filter dynamically the label prefixes used to determine identities based | ||
| // on network policy labels. | ||
| EnableDynamicLabelFilter bool |
There was a problem hiding this comment.
Let's try to avoid adding new fields to the DaemonConfig. You're creating a new cell, you can use cell.Config to scope the configuration to your cell.
| case <-ctx.Done(): | ||
| l.Info("Cilium Dynamic Label Filter Controller shut down") | ||
| return | ||
| case <-c.Signal.Signal: |
There was a problem hiding this comment.
Should this be rate-limited in some way? There's the signal coalescing if triggers arrive faster than the reconciliation can occur, but in effect (assuming fast trigger rate) the bottleneck is the rate at which reconciliation can happen - it's not clear to me that that's the right choice, given (I believe) this can kick of identity/ep regenerations.
based on #32501
Adds the logic required to update the CEP in-place when dynamic label filter detect any changes to network policy.
The reconcile loop will fetch all the CEP and compute the labels that needs to be added and removed considering the pod labels, namespace labels and the existing CEP labels.
Fixes: #32576