feat: Control verbosity of kube-network-policies sidecar container - #14255
feat: Control verbosity of kube-network-policies sidecar container#14255olivergregorius wants to merge 2 commits into
Conversation
Add `kubeNetworkPoliciesVerbose` to the KubeFlannelCNIConfig document, controlling the `--v` flag passed to the kube-network-policies sidecar container Flannel deploys when `kubeNetworkPoliciesEnabled` is set. When true (the default, preserving prior behavior), the container runs with `--v=2`; when explicitly set to false, it runs with `--v=0`. The field is a pointer so "unset" can be distinguished from "explicitly false" and still resolve to the previous always-verbose behavior. Signed-off-by: Oliver Gregorius <oliver@gregorius.dev>
80a6035 to
55bc15a
Compare
| // Enables verbose logging for kube-network-policies. | ||
| // | ||
| // This enables verbose logging for the kube-network-policies container. | ||
| FlannelKubeNetworkPoliciesVerbose *bool `yaml:"kubeNetworkPoliciesVerbose,omitempty"` |
There was a problem hiding this comment.
please don't update legacy configs - they are deprecated/not documented anymore
There was a problem hiding this comment.
Should I remove this change or is it just a hint from you?
There was a problem hiding this comment.
yes, you should l remove changes to legacy configs - they will not be updated.
Also see #13391 - looks like this approach is not good.
The --v=2 is not a good default, and --v=0 might not good either, I wonder if we should just drop all of them, and add extraArgs as a field.
There was a problem hiding this comment.
I wonder if we should just drop all of them, and add extraArgs as a field.
I already had this in mind as well, but as a consequence the kubeNetworkPoliciesEnabled flag should be moved to a surrounding object:
flannel:
kubeNetworkPolicies:
enabled: true
extraArgs:
- --v=2That's a bigger and also a breaking change. But it would be much more clean than having two flags, of course.
There was a problem hiding this comment.
we still can do changes like this one for 1.15+ and we can keep it backwards compatible as well.
There was a problem hiding this comment.
yes, you should l remove changes to legacy configs
Done with 0d411b4
|
Nevermind my approval, opened the wrong PR to approve. |
KubeNetworkPoliciesVerbose was added to both the deprecated FlannelCNIConfig (v1alpha1) and its replacement, KubeFlannelCNIConfigV1Alpha1, in the same change. Since FlannelCNIConfig is being phased out in favor of KubeFlannelCNIConfigV1Alpha1, the new field should only exist on the replacement document. Remove the field from FlannelCNIConfig, keeping the KubeNetworkPoliciesVerbose() method (required by the shared config.K8sFlannelCNIConfig interface) hardcoded to true, matching how other unsupported knobs are handled on the deprecated type. Signed-off-by: Oliver Gregorius <oliver@gregorius.dev>
2cb23da to
0d411b4
Compare
|
Putting this on hold until #13391 comes up with a final design. We need a proper way to expose this in the machine config, which will not require changes every now and then. |
Pull Request
What? (description)
Added a new flag kubeNetworkPoliciesVerbose to KubeFlannelCNIConfigV1Alpha1 to control verbosity of kube-network-policies sidecar container.
Why? (reasoning)
By default when network policies are enabled for flannel, it logs each and every packet flooding the logs overwhelming log collectors like Promtail.
Acceptance
Please use the following checklist:
make conformance)make fmt)make lint)make docs)make unit-tests)