NETOBSERV-341 configure netflow v5 separately (breaking change if using v5)

[jotak]

Fixes #200

@eranra @KalmanMeth
Is it ok with you to have a breaking changes here? (people who used legacy v5 now have to configure it explicitly)
If that's a problem, we could default to the previous behaviour (legacy port = main port +1) and have another setting to disable v5, but I think it's more simple to keep default as 0 = legacy disabled.

[codecov-commenter]

Codecov Report

Merging #207 (2051cf5) into main (8c5e6ef) will decrease coverage by 0.37%.
The diff coverage is 42.30%.

@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
- Coverage   58.16%   57.78%   -0.38%     
==========================================
  Files          58       58              
  Lines        3361     3364       +3     
==========================================
- Hits         1955     1944      -11     
- Misses       1276     1289      +13     
- Partials      130      131       +1     

Flag	Coverage Δ
unittests	57.78% <42.30%> (-0.38%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/pipeline/ingest/ingest_collector.go	53.22% <42.30%> (-10.42%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c5e6ef...2051cf5. Read the comment docs.

[eranra]

@jotak I reviews the PR and it looks clean to me and can be merged. I think that this is better to be restrictive here, so totally ok with that.

BUT:: Merging this PR will break both FLP and NOO testing/deployment over kind::

Need to add the legacy port here:

https://github.com/netobserv/flowlogs-pipeline/blob/main/contrib/kubernetes/flowlogs-pipeline.conf.yaml#L7

So that this will work with the Netflow simulator

https://github.com/netobserv/flowlogs-pipeline/blob/main/contrib/kubernetes/deployment-netflow-simulator.yaml#L22

A similar thing will happen also to NOO:

In https://github.com/netobserv/network-observability-operator/blob/main/.mk/local.mk#L34 We will not expose port 2056 anymore so need to add that to the Operator code ???

[jotak]

@eranra thanks, good catches. I've updated the confgen, see the second commit.
I'm not able to test on my machine (not enough RAM for kind), however, I wonder why the e2e tests passed in CI if it was relying on netflow v5 traffic.

About the netobserv operator, we can talk about that, but it's not supposed to support netflow v5, we only support ipfix, so I'd rather remove the v5 stuff that is used in make targets, rather than updating the code to handle non-supported v5

[eranra]

@eranra thanks, good catches. I've updated the confgen, see the second commit. I'm not able to test on my machine (not enough RAM for kind), however, I wonder why the e2e tests passed in CI if it was relying on netflow v5 traffic.

About the netobserv operator, we can talk about that, but it's not supposed to support netflow v5, we only support ipfix, so I'd rather remove the v5 stuff that is used in make targets, rather than updating the code to handle non-supported v5

Looks good, LGTM ... :-)

As for the e2e tests ... I looked and we are not really using the simulator in the tests ... just in https://github.com/netobserv/flowogs-pipeline/blob/main/Makefile#L253

so actually this does call for better e2e tests ... we might decide to do that just in the level of NOO and not in FLP ... need to decide.

[jotak]

Thanks Eran