mbp-1128: Add Network Observability with NooBaa-backed Loki - #165
mbp-1128: Add Network Observability with NooBaa-backed Loki#165pmatouse wants to merge 9 commits into
Conversation
mlorenzofr
left a comment
There was a problem hiding this comment.
overall it's very good, but some small changes need to be made
abcf24c to
091415e
Compare
p-rog
left a comment
There was a problem hiding this comment.
Overall LGTM
I left one suggestion which I found when I was testing it at OCP 4.22.
Additionally, it would be great to add network policies to the Network Observability feature, to follow ZT methodology. You can test my network policy architect skill :)
I've added the the network policies |
89a748b to
04a245d
Compare
mlorenzofr
left a comment
There was a problem hiding this comment.
Overall okay, it's almost ready for LGTM in my opinion, just a couple of minor things.
Additionally:
- Change its status from draft to ready for review.
- Do a rebase to get the latest changes from the
mainbranch and avoid conflicts.
|
@pmatouse the network policies looks very nice. As I can see you didn't restrict specific ports in the ingress rules from Additionally, the Loki Operator channel is pinned to stable-6.6. To avoid OCP compatibility problems, maybe let's remove it like it was done with ODF, Quay, and RHTAS so subscriptions follow the default catalog channel on newer OCP releases. WDYT? |
Install Loki and Network Observability operators via a storage-dependent feature fragment, and deploy LokiStack plus FlowCollector using NooBaa storage. Signed-off-by: Petr Matousek <pmatouse@redhat.com>
Use Helm lookup for the default StorageClass when possible; require an explicit value under Argo CD. LokiStack CRD rejects an omitted field.
Enable FlowCollector operator NPs for netobserv/privileged, and add least-privilege policies for netobserv-loki.
04a245d to
b46e35c
Compare
|
Addressed in Also unpinned Loki channel in |
mlorenzofr
left a comment
There was a problem hiding this comment.
with the current code it is fully functional
If we want to avoid OutOfSync in the Argo CD application, we would have to ignore some attributes that mutate when the FlowCollector is initialized.
I may be wrong, but I think the SYNC-WAVE-INVENTORY.md file is missing the NetworkPolicies entries.
| argocd.argoproj.io/sync-wave: "42" | ||
| overrides: | ||
| - name: loki.storageClassName | ||
| value: "<YOUR-STORAGE-CLASS>" |
There was a problem hiding this comment.
netobserv operator mutates some fields after creation, so every ArgoCD sync after the first will report OutOfSync. I think this would solve it.
| value: "<YOUR-STORAGE-CLASS>" | |
| value: "<YOUR-STORAGE-CLASS>" | |
| ignoreDifferences: | |
| - group: flows.netobserv.io | |
| kind: FlowCollector | |
| jsonPointers: | |
| - /spec/agent/ebpf/cacheActiveTimeout | |
| - /spec/agent/ebpf/cacheMaxFlows | |
| - /spec/agent/ebpf/excludeInterfaces | |
| - /spec/agent/ebpf/imagePullPolicy | |
| - /spec/agent/ebpf/kafkaBatchSize | |
| - /spec/agent/ebpf/logLevel | |
| - /spec/agent/ebpf/metrics | |
| - /spec/agent/ebpf/resources | |
| - /spec/agent/ipfix | |
| - /spec/consolePlugin/autoscaler | |
| - /spec/consolePlugin/imagePullPolicy | |
| - /spec/consolePlugin/logLevel | |
| - /spec/consolePlugin/portNaming | |
| - /spec/consolePlugin/quickFilters | |
| - /spec/consolePlugin/replicas | |
| - /spec/consolePlugin/resources | |
| - /spec/execution | |
| - /spec/kafka | |
| - /spec/loki/manual | |
| - /spec/loki/microservices | |
| - /spec/loki/monolithic | |
| - /spec/loki/readTimeout | |
| - /spec/loki/writeBatchSize | |
| - /spec/loki/writeBatchWait | |
| - /spec/loki/writeTimeout | |
| - /spec/processor | |
| - /spec/prometheus |
| # argocd.argoproj.io/sync-wave: "42" # Deploy after NooBaa storage backend | ||
| # overrides: | ||
| # - name: loki.storageClassName | ||
| # value: "" # Add your own storageClassName here |
There was a problem hiding this comment.
| # value: "" # Add your own storageClassName here | |
| # value: "" # Add your own storageClassName here | |
| # ignoreDifferences: | |
| # - group: flows.netobserv.io | |
| # kind: FlowCollector | |
| # jsonPointers: | |
| # - /spec/agent/ebpf/cacheActiveTimeout | |
| # - /spec/agent/ebpf/cacheMaxFlows | |
| # - /spec/agent/ebpf/excludeInterfaces | |
| # - /spec/agent/ebpf/imagePullPolicy | |
| # - /spec/agent/ebpf/kafkaBatchSize | |
| # - /spec/agent/ebpf/logLevel | |
| # - /spec/agent/ebpf/metrics | |
| # - /spec/agent/ebpf/resources | |
| # - /spec/agent/ipfix | |
| # - /spec/consolePlugin/autoscaler | |
| # - /spec/consolePlugin/imagePullPolicy | |
| # - /spec/consolePlugin/logLevel | |
| # - /spec/consolePlugin/portNaming | |
| # - /spec/consolePlugin/quickFilters | |
| # - /spec/consolePlugin/replicas | |
| # - /spec/consolePlugin/resources | |
| # - /spec/execution | |
| # - /spec/kafka | |
| # - /spec/loki/manual | |
| # - /spec/loki/microservices | |
| # - /spec/loki/monolithic | |
| # - /spec/loki/readTimeout | |
| # - /spec/loki/writeBatchSize | |
| # - /spec/loki/writeBatchWait | |
| # - /spec/loki/writeTimeout | |
| # - /spec/processor | |
| # - /spec/prometheus |
sabre1041
left a comment
There was a problem hiding this comment.
A few small changes. However, functionality wise, the new integration works great. Once these items are addressed, we can integrate
| {{- $sc -}} | ||
| {{- else -}} | ||
| {{- $default := "" -}} | ||
| {{- range (lookup "storage.k8s.io/v1" "StorageClass" "" "").items | default list -}} |
There was a problem hiding this comment.
Since the lookup functionality is not supported in Argo CD, lets remove this from the chart and just fail if this value is not provided. Appropriate guidance via documentation should be provided to instruct the end user the steps that they need to take
| type: {{ .Values.flowcollector.agent.type }} | ||
| ebpf: | ||
| sampling: {{ .Values.flowcollector.agent.sampling }} | ||
| privileged: {{ .Values.flowcollector.agent.privileged }} |
There was a problem hiding this comment.
if privileged is false, the property is dropped from the resulting CR as results in out of sync warnings. Update this property to only provide the value only if true
feat: add Network Observability with NooBaa-backed Loki
Install Loki and Network Observability operators via a storage-dependent
feature fragment, and deploy LokiStack plus FlowCollector using NooBaa
storage.
Signed-off-by: Petr Matousek pmatouse@redhat.com