design-proposals: multi-target log forwarding from kubernetes-tenants#14
design-proposals: multi-target log forwarding from kubernetes-tenants#14IvanHunters wants to merge 1 commit into
Conversation
Proposes a new optional `addons.monitoringAgents.additionalLogTargets` field on the Kubernetes app CR that lets operators fan out fluent-bit container logs to multiple ancestor VictoriaLogs instances, in addition to the implicit immediate-parent destination. Default behaviour stays bit-for-bit identical. Cilium egress allowance follows from the vlinsert addition tracked in cozystack/cozystack#2910. The proposal is complementary to (not replacing) the split-routing PR cozystack/cozystack#2195. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a design proposal for a multi-target log forwarding feature in Kubernetes tenant clusters. The document specifies how logs can be fan-out from a tenant's immediate parent VictoriaLogs instance to additional ancestor instances via a new optional CR field, Helm templating approach, and network policy integration. ChangesMulti-target log forwarding feature design
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal for multi-target log forwarding from Kubernetes tenant clusters to ancestor VictoriaLogs instances by extending the monitoring-agents chart. The review feedback highlights two key issues in the proposed Helm template and configuration: first, the need to split resolved shorthand targets (like 'ancestors') to prevent rendering invalid host configurations, and second, updating the fluent-bit Match pattern from 'kube.' to '' to ensure all log inputs (including audit and system events) are correctly forwarded.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| {{- range .Values.addons.monitoringAgents.additionalLogTargets | default list }} | ||
| {{- $resolved := include "kubernetes.resolveLogTarget" (dict "shorthand" . "ctx" $) }} | ||
| {{- $targets = append $targets $resolved }} | ||
| {{- end }} |
There was a problem hiding this comment.
In Helm templates, the include function always returns a string. If a shorthand like ancestors resolves to multiple target namespaces (e.g., space-separated), include "kubernetes.resolveLogTarget" will return them as a single string. Appending this directly to $targets will result in a single list element containing multiple space-separated namespaces, which will render an invalid Host configuration (e.g., Host vlinsert-generic.tenant-root tenant-parent.svc.cozy.local).
To handle multi-target shorthands correctly, you should split the resolved string before appending it to the $targets list.
| {{- range .Values.addons.monitoringAgents.additionalLogTargets | default list }} | |
| {{- $resolved := include "kubernetes.resolveLogTarget" (dict "shorthand" . "ctx" $) }} | |
| {{- $targets = append $targets $resolved }} | |
| {{- end }} | |
| {{- range .Values.addons.monitoringAgents.additionalLogTargets | default list }} | |
| {{- $resolved := include "kubernetes.resolveLogTarget" (dict "shorthand" . "ctx" $) }} | |
| {{- range (splitList " " $resolved) }} | |
| {{- if . }} | |
| {{- $targets = append $targets . }} | |
| {{- end }} | |
| {{- end }} | |
| {{- end }} |
| {{- range $targets }} | ||
| [OUTPUT] | ||
| Name http | ||
| Match kube.* |
There was a problem hiding this comment.
The proposal states that multiple inputs (kube container logs, audit, kubelet events) should all respect the same destination list. However, the proposed [OUTPUT] block uses Match kube.*. If audit logs or other system events use different tags (e.g., audit.*), they will not be matched by this output block and thus won't be forwarded to the additional targets. Consider using Match * or matching the exact tags used by all configured inputs.
| {{- range $targets }} | |
| [OUTPUT] | |
| Name http | |
| Match kube.* | |
| {{- range $targets }} | |
| [OUTPUT] | |
| Name http | |
| Match * |
1 participant
Summary
Adds a draft design proposal for fanning out fluent-bit container logs from a
kubernetes(k8s-tenant) cluster to multiple ancestor VictoriaLogs instances, on top of the implicit immediate-parent destination. The default behaviour stays bit-for-bit identical to today; the feature is opt-in via a newaddons.monitoringAgents.additionalLogTargetslist on theKubernetesCR.The proposal is complementary to (not replacing) cozystack/cozystack#2195 — that PR adds split routing for namespace-tenant monitoring-agents, this proposal adds fan-out for kubernetes-tenant monitoring-agents. Both share the underlying mechanism (additional
[OUTPUT]blocks in fluent-bit config).Depends on:
vlinsertto the tenant egress allowlist.tenant.cozystack.io/*labels on grandchild namespaces, which the egress policy relies on.Why now
Reported by the TTK platform operations team: there is no way to view logs from a child
kubernetestenant in the root tenant's Grafana, which breaks the platform operator's incident-response workflow (forced to hop between per-tenant Grafanas). The current production workaround is a manualkubernetes-ht-monitoring-agentsHelmRelease patch plussuspend: trueon the parent — sustainable for a hotfix, not as a permanent state.Reviewers
Pinging the team referenced in similar
community/PRs for visibility — open to discussion on the open questions in the body (shorthand DSL, default ofancestors, whether to centralise the multi-output rendering in themonitoring-agentschart vs. callers).Summary by CodeRabbit
New Features
Documentation