NETOBSERV-1875: Make enrichment indexes configurable #711

jotak · 2024-09-13T14:31:25Z

Description

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those.

This PR makes it all configurable in that way (example):

secondaryNetworks:
- name: my-network
  index:
    mac: true
    ip: true

More than one network can be configured that way.

Additionally, the api.K8sRule object needs to be configured with the Interface field as well. Breaking change: also the IP and MAC field names have been renamed for consistency. It's now, for instance:

    kubernetes:
      ipField:  "SrcAddr",
      macField: "SrcMAC",
      interfacesField: "Interfaces",

Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql:
sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)

This metric can also report errors and unexpected multiple matches

Breaking change tl;dr

In k8s enrichment rule, rename <rule>.kubernetes.input into <rule>.kubernetes.ipField.

Dependencies

Operator: netobserv/network-observability-operator#772

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those. This PR makes it all configurable in that way (example): ```bash secondaryNetworks: - name: my-network index: mac: true ip: true ``` More than one network can be configured that way. Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql: `sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)` This metric can also report errors and unexpected multiple matches

openshift-ci-robot · 2024-09-13T14:31:29Z

@jotak: This pull request references NETOBSERV-1875 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those.

This PR makes it all configurable in that way (example):
secondaryNetworks:
- name: my-network
 index:
   mac: true
   ip: true
More than one network can be configured that way.

Additionally, the api.K8sRule object needs to be configured with the Interface field as well. Breaking change: also the IP and MAC field names have been renamed for consistency. It's now, for instance:
   kubernetes:
     ipField:  "SrcAddr",
     macField: "SrcMAC",
     interfacesField: "Interfaces",
Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql:
sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)

This metric can also report errors and unexpected multiple matches

Breaking change tl;dr

In k8s enrichment rule, rename <rule>.kubernetes.input into <rule>.kubernetes.ipField.

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

Will this change affect NetObserv / Network Observability operator? If not, you can ignore the rest of this checklist.

Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).

Does this PR require product documentation?

If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.

Does this PR require a product release notes entry?

If so, fill in "Release Note Text" in the JIRA.

Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.

If so, make sure it is described in the JIRA ticket.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-09-13T14:31:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jotak. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2024-09-13T14:31:50Z

@jotak: This pull request references NETOBSERV-1875 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those.

This PR makes it all configurable in that way (example):
secondaryNetworks:
- name: my-network
 index:
   mac: true
   ip: true
More than one network can be configured that way.

Additionally, the api.K8sRule object needs to be configured with the Interface field as well. Breaking change: also the IP and MAC field names have been renamed for consistency. It's now, for instance:
   kubernetes:
     ipField:  "SrcAddr",
     macField: "SrcMAC",
     interfacesField: "Interfaces",
Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql:
sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)

This metric can also report errors and unexpected multiple matches

Breaking change tl;dr

In k8s enrichment rule, rename <rule>.kubernetes.input into <rule>.kubernetes.ipField.

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

Will this change affect NetObserv / Network Observability operator? If not, you can ignore the rest of this checklist.

Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).

Does this PR require product documentation?

If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.

Does this PR require a product release notes entry?

If so, fill in "Release Note Text" in the JIRA.

Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.

If so, make sure it is described in the JIRA ticket.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-09-13T14:32:00Z

@jotak: This pull request references NETOBSERV-1875 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those.

This PR makes it all configurable in that way (example):
secondaryNetworks:
- name: my-network
 index:
   mac: true
   ip: true
More than one network can be configured that way.

Additionally, the api.K8sRule object needs to be configured with the Interface field as well. Breaking change: also the IP and MAC field names have been renamed for consistency. It's now, for instance:
   kubernetes:
     ipField:  "SrcAddr",
     macField: "SrcMAC",
     interfacesField: "Interfaces",
Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql:
sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)

This metric can also report errors and unexpected multiple matches

Breaking change tl;dr

In k8s enrichment rule, rename <rule>.kubernetes.input into <rule>.kubernetes.ipField.

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

Will this change affect NetObserv / Network Observability operator? If not, you can ignore the rest of this checklist.

Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).

Does this PR require product documentation?

If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.

Does this PR require a product release notes entry?

If so, fill in "Release Note Text" in the JIRA.

Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.

If so, make sure it is described in the JIRA ticket.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-09-13T14:32:07Z

@jotak: This pull request references NETOBSERV-1875 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those.

This PR makes it all configurable in that way (example):
secondaryNetworks:
- name: my-network
 index:
   mac: true
   ip: true
More than one network can be configured that way.

Additionally, the api.K8sRule object needs to be configured with the Interface field as well. Breaking change: also the IP and MAC field names have been renamed for consistency. It's now, for instance:
   kubernetes:
     ipField:  "SrcAddr",
     macField: "SrcMAC",
     interfacesField: "Interfaces",
Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql:
sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)

This metric can also report errors and unexpected multiple matches

Breaking change tl;dr

In k8s enrichment rule, rename <rule>.kubernetes.input into <rule>.kubernetes.ipField.

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

Will this change affect NetObserv / Network Observability operator? If not, you can ignore the rest of this checklist.

Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).

Does this PR require product documentation?

If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.

Does this PR require a product release notes entry?

If so, fill in "Release Note Text" in the JIRA.

Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.

If so, make sure it is described in the JIRA ticket.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-09-13T14:41:33Z

@jotak: This pull request references NETOBSERV-1875 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those.

This PR makes it all configurable in that way (example):
secondaryNetworks:
- name: my-network
 index:
   mac: true
   ip: true
More than one network can be configured that way.

Additionally, the api.K8sRule object needs to be configured with the Interface field as well. Breaking change: also the IP and MAC field names have been renamed for consistency. It's now, for instance:
   kubernetes:
     ipField:  "SrcAddr",
     macField: "SrcMAC",
     interfacesField: "Interfaces",
Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql:
sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)

This metric can also report errors and unexpected multiple matches

Breaking change tl;dr

In k8s enrichment rule, rename <rule>.kubernetes.input into <rule>.kubernetes.ipField.

Dependencies

Operator: netobserv/network-observability-operator#772

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

Will this change affect NetObserv / Network Observability operator? If not, you can ignore the rest of this checklist.

Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).

Does this PR require product documentation?

If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.

Does this PR require a product release notes entry?

If so, fill in "Release Note Text" in the JIRA.

Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.

If so, make sure it is described in the JIRA ticket.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

codecov · 2024-09-13T14:43:45Z

Codecov Report

Attention: Patch coverage is 61.58192% with 68 lines in your changes missing coverage. Please review.

Project coverage is 65.47%. Comparing base (df4bbeb) to head (f2e1166).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
...peline/transform/kubernetes/informers/informers.go	36.36%	45 Missing and 4 partials ⚠️
pkg/pipeline/transform/kubernetes/cni/multus.go	70.96%	14 Missing and 4 partials ⚠️
...e/transform/kubernetes/informers/informers-mock.go	95.83%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #711      +/-   ##
==========================================
+ Coverage   65.38%   65.47%   +0.08%     
==========================================
  Files         108      108              
  Lines        6960     7026      +66     
==========================================
+ Hits         4551     4600      +49     
- Misses       2099     2108       +9     
- Partials      310      318       +8

Flag	Coverage Δ
unittests	`65.47% <61.58%> (+0.08%)`	⬆️

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

Files with missing lines	Coverage Δ
pkg/api/transform_network.go	`100.00% <ø> (ø)`
pkg/operational/metrics.go	`78.08% <100.00%> (+0.30%)`	⬆️
pkg/pipeline/pipeline_builder.go	`64.19% <100.00%> (ø)`
...ipeline/transform/kubernetes/cni/ovn_kubernetes.go	`68.57% <ø> (+3.70%)`	⬆️
pkg/pipeline/transform/kubernetes/enrich.go	`79.12% <100.00%> (-0.45%)`	⬇️
pkg/pipeline/transform/transform_network.go	`68.02% <100.00%> (ø)`
...e/transform/kubernetes/informers/informers-mock.go	`97.19% <95.83%> (+0.13%)`	⬆️
pkg/pipeline/transform/kubernetes/cni/multus.go	`70.96% <70.96%> (+7.33%)`	⬆️
...peline/transform/kubernetes/informers/informers.go	`22.09% <36.36%> (+3.79%)`	⬆️

…t network

memodi · 2024-09-19T22:01:18Z

/ok-to-test

github-actions · 2024-09-20T01:06:14Z

New image:
quay.io/netobserv/flowlogs-pipeline:331b818

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=331b818 make set-flp-image

memodi

/label qe-approved

openshift-ci-robot · 2024-09-20T02:58:41Z

@jotak: This pull request references NETOBSERV-1875 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description

For secondary interfaces, it's better to allow more configurable indexing for pods enrichment. For some network types, pods might be identified by their MAC address advertized in the network-status annotations, while in others it can be by IP, by interface name, or any combination of those.

This PR makes it all configurable in that way (example):
secondaryNetworks:
- name: my-network
 index:
   mac: true
   ip: true
More than one network can be configured that way.

Additionally, the api.K8sRule object needs to be configured with the Interface field as well. Breaking change: also the IP and MAC field names have been renamed for consistency. It's now, for instance:
   kubernetes:
     ipField:  "SrcAddr",
     macField: "SrcMAC",
     interfacesField: "Interfaces",
Additionally, a new metric is added to track which indexer is used for every enrichment hit: promql:
sum(rate(netobserv_secondary_network_indexer_hit[1m])) by (kind, network, error)

This metric can also report errors and unexpected multiple matches

Breaking change tl;dr

In k8s enrichment rule, rename <rule>.kubernetes.input into <rule>.kubernetes.ipField.

Dependencies

Operator: netobserv/network-observability-operator#772

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

Will this change affect NetObserv / Network Observability operator? If not, you can ignore the rest of this checklist.

Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).

Does this PR require product documentation?

If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.

Does this PR require a product release notes entry?

If so, fill in "Release Note Text" in the JIRA.

Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.

If so, make sure it is described in the JIRA ticket.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

jotak added the breaking-change This pull request has breaking changes. They should be described in PR description. label Sep 13, 2024

jotak requested review from OlivierCazade and jpinsonneau September 13, 2024 14:31

openshift-ci-robot added the jira/valid-reference label Sep 13, 2024

jotak mentioned this pull request Sep 13, 2024

NETOBSERV-1875: Make enrichment indexes configurable netobserv/network-observability-operator#772

Open

10 tasks

Add namespace label to metric, add network name enrichment

90329b8

jpinsonneau approved these changes Sep 16, 2024

View reviewed changes

openshift-ci bot assigned jpinsonneau Sep 16, 2024

openshift-ci bot added the lgtm label Sep 16, 2024

do not return from building keys when a field is missing: move to nex…

f2e1166

…t network

openshift-ci bot removed the lgtm label Sep 16, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Sep 19, 2024

memodi approved these changes Sep 20, 2024

View reviewed changes

openshift-ci bot assigned memodi Sep 20, 2024

openshift-ci bot added qe-approved QE has approved this pull request lgtm labels Sep 20, 2024

NETOBSERV-1875: Make enrichment indexes configurable #711

Are you sure you want to change the base?

NETOBSERV-1875: Make enrichment indexes configurable #711

Conversation

jotak commented Sep 13, 2024 • edited Loading

Description

Breaking change tl;dr

Dependencies

Checklist

openshift-ci-robot commented Sep 13, 2024 • edited by openshift-ci bot Loading

Description

Breaking change tl;dr

Dependencies

Checklist

openshift-ci bot commented Sep 13, 2024

openshift-ci-robot commented Sep 13, 2024 • edited by openshift-ci bot Loading

Description

Breaking change tl;dr

Dependencies

Checklist

openshift-ci-robot commented Sep 13, 2024 • edited by openshift-ci bot Loading

Description

Breaking change tl;dr

Dependencies

Checklist

openshift-ci-robot commented Sep 13, 2024 • edited by openshift-ci bot Loading

Description

Breaking change tl;dr

Dependencies

Checklist

openshift-ci-robot commented Sep 13, 2024 • edited by openshift-ci bot Loading

Description

Breaking change tl;dr

Dependencies

Checklist

codecov bot commented Sep 13, 2024 • edited Loading

Codecov Report

memodi commented Sep 19, 2024

github-actions bot commented Sep 20, 2024

memodi left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 20, 2024 • edited by openshift-ci bot Loading

Description

Breaking change tl;dr

Dependencies

Checklist

jotak commented Sep 13, 2024 •

edited

Loading

openshift-ci-robot commented Sep 13, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Sep 13, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Sep 13, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Sep 13, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Sep 13, 2024 •

edited by openshift-ci bot

Loading

codecov bot commented Sep 13, 2024 •

edited

Loading

openshift-ci-robot commented Sep 20, 2024 •

edited by openshift-ci bot

Loading