NETOBSERV-1965: Initial intg with UDN interface mapping api by msherif1234 · Pull Request #487 · netobserv/netobserv-ebpf-agent

msherif1234 · 2024-12-14T14:48:18Z

Description

Dependencies

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.

msherif1234 · 2024-12-19T17:11:28Z

/ok-to-test

github-actions · 2024-12-19T17:16:46Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:6d1822e

It will expire after two weeks.

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

USER=netobserv VERSION=6d1822e make set-agent-image

msherif1234 · 2024-12-19T17:52:13Z

/ok-to-test

github-actions · 2024-12-19T17:56:51Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:4319c79

It will expire after two weeks.

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

USER=netobserv VERSION=4319c79 make set-agent-image

codecov · 2024-12-19T18:07:21Z

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 28.40%. Comparing base (2c96c42) to head (64b226b).

Files with missing lines	Patch %	Lines
pkg/agent/agent.go	0.00%	1 Missing ⚠️
pkg/exporter/ipfix.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #487      +/-   ##
==========================================
+ Coverage   28.34%   28.40%   +0.06%     
==========================================
  Files          46       46              
  Lines        4741     4745       +4     
==========================================
+ Hits         1344     1348       +4     
  Misses       3295     3295              
  Partials      102      102

Flag	Coverage Δ
unittests	`28.40% <81.81%> (+0.06%)`	⬆️

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

Files with missing lines	Coverage Δ
pkg/agent/config.go	`8.33% <ø> (ø)`
pkg/decode/decode_protobuf.go	`30.52% <100.00%> (+0.40%)`	⬆️
pkg/model/record.go	`85.50% <100.00%> (+0.43%)`	⬆️
pkg/agent/agent.go	`31.09% <0.00%> (ø)`
pkg/exporter/ipfix.go	`0.00% <0.00%> (ø)`

msherif1234 · 2024-12-19T19:40:21Z

/ok-to-test

github-actions · 2024-12-19T19:45:13Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:4599f62

It will expire after two weeks.

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

USER=netobserv VERSION=4599f62 make set-agent-image

msherif1234 · 2024-12-19T19:58:03Z

/ok-to-test

github-actions · 2024-12-19T20:02:49Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:80a056e

It will expire after two weeks.

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

USER=netobserv VERSION=80a056e make set-agent-image

openshift-ci-robot · 2025-01-06T16:38:26Z

github-actions · 2025-01-20T17:20:45Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:b7a449f

It will expire after two weeks.

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

USER=netobserv VERSION=b7a449f make set-agent-image

msherif1234 · 2025-01-20T17:27:29Z

/ok-to-test

github-actions · 2025-01-20T17:32:43Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:096385d

It will expire after two weeks.

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

USER=netobserv VERSION=096385d make set-agent-image

openshift-ci · 2025-01-21T01:05:53Z

@msherif1234: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/qe-e2e-tests	`8c5d63c`	link	false	`/test qe-e2e-tests`

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

jotak · 2025-01-21T08:56:38Z

pkg/model/record.go

+	ovnmodel "github.com/ovn-org/ovn-kubernetes/go-controller/observability-lib/model"
+	ovnobserv "github.com/ovn-org/ovn-kubernetes/go-controller/observability-lib/sampledecoder"


I don't want to block this PR further, but I think we should get rid of these dependencies in our model package
So, no blocker - I suggest to add that to a follow-up task - possibly for 1.9.

An option could be to pass an interface, or callbacks, instead of the actual sampledecoder.
(Another option which we briefly discussed is to add a new stage instead of doing this decoration directly in Record, but I'm not sure it's any better)

yeah we can see how to clean this further in 1.9 I briefly looked at new stage but I quickly realized it adds some overhead and wasn't sure how to correctly wire things

jotak · 2025-01-21T09:00:12Z

pkg/model/record.go

+		if err != nil {
+			return IntfDirUdn{Interface: intf, Direction: dir, Udn: ""}
+		}
+		udnsCache = m


this cache is never updated? I can add that also as a follow-up if you want

well the caches is reset every eviction then read from ovnk after that its just ready only no update is needed during the eviction processing

oh yes I didn't noticed in NewRecord

jotak · 2025-01-21T09:03:53Z

pkg/pbflow/proto.go

-			}
-		}
+
+	pbflowRecord.NetworkEventsMetadata = make([]*NetworkEvent, 0)


nit / follow-up: avoid creating NetworkEventsMetadata if fr.NetworkMonitorEventsMD is empty?

jotak · 2025-01-21T09:10:06Z

pkg/decode/decode_protobuf.go

 	for _, intf := range fr.Interfaces {
 		directions = append(directions, intf.Direction)
 		interfaces = append(interfaces, intf.Interface)
+		if intf.Udn != "" {


shouldn't we append udns here, regardless of it's presence? It depends how this data is processed in the console plugin. If the udn array is correlated with the interface array, the indexes in udn array must match the one from the interfaces. Not totally sure about that.

in the console side they are two separate list? u mean when user correlate between the two in the console ui they won't be in sync so maybe its better to show it regardless I will remove this check

honestly I don't remember how it's done in the console plugin ... For interfaces and direction I know that we have a column that shows both "Interfaces and Directions" so the 2 arrays are correlated; but did we do something similar for udn, I'm not sure

with my len check we get this

but w/o len check we will get n/a and xlat-test/test-network i.e 2 rows which might be better for readability because now u don't know this tag for which interface so I think better to show "" for ovn-udn1 interface

jotak

/lgtm
thanks @msherif1234
So I created this follow-up for 1.9: https://issues.redhat.com/browse/NETOBSERV-2069 - we can continue discuss here and amend that story accordingly

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>

jotak

/lgtm

msherif1234 · 2025-01-21T10:53:08Z

/approve

openshift-ci · 2025-01-21T10:53:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msherif1234

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [msherif1234]

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

…v#487) * intg netobserv agent with new api to get udn mapping Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com> * update vendered pkg and go mod Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com> * move sample decoding logic out protobuf logic Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com> --------- Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>

openshift-ci bot added the do-not-merge/work-in-progress label Dec 14, 2024

msherif1234 marked this pull request as draft December 14, 2024 14:48

msherif1234 force-pushed the udn-mapping branch 6 times, most recently from 148c2ff to 41dd289 Compare December 19, 2024 17:10

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

msherif1234 force-pushed the udn-mapping branch from 41dd289 to f6cb719 Compare December 19, 2024 17:51

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 19, 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 Dec 19, 2024

msherif1234 force-pushed the udn-mapping branch from f6cb719 to d625a2e Compare December 19, 2024 19:38

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 19, 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 Dec 19, 2024

msherif1234 force-pushed the udn-mapping branch from d625a2e to bf1b744 Compare December 19, 2024 19:57

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 19, 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 Dec 19, 2024

msherif1234 changed the title ~~WIP: Initial intg with UDN interface mapping api~~ NETOBSERV-1965: Initial intg with UDN interface mapping api Jan 6, 2025

openshift-ci-robot added the jira/valid-reference label Jan 6, 2025

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

msherif1234 force-pushed the udn-mapping branch from a2e1562 to a6c3e54 Compare January 20, 2025 17:27

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jan 20, 2025

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

msherif1234 force-pushed the udn-mapping branch from a6c3e54 to 8c5d63c Compare January 20, 2025 20:14

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jan 20, 2025

jotak reviewed Jan 21, 2025

View reviewed changes

jotak approved these changes Jan 21, 2025

View reviewed changes

openshift-ci bot assigned jotak Jan 21, 2025

openshift-ci bot added the lgtm label Jan 21, 2025

move sample decoding logic out protobuf logic

8c0a7a8

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>

msherif1234 force-pushed the udn-mapping branch from 8c5d63c to 8c0a7a8 Compare January 21, 2025 10:30

openshift-ci bot removed the lgtm label Jan 21, 2025

msherif1234 requested a review from jotak January 21, 2025 10:30

jotak approved these changes Jan 21, 2025

View reviewed changes

openshift-ci bot added the lgtm label Jan 21, 2025

openshift-ci bot added the approved label Jan 21, 2025

openshift-merge-bot bot merged commit 5110b52 into netobserv:main Jan 21, 2025
9 of 10 checks passed

msherif1234 deleted the udn-mapping branch January 21, 2025 10:57

		ovnmodel "github.com/ovn-org/ovn-kubernetes/go-controller/observability-lib/model"
		ovnobserv "github.com/ovn-org/ovn-kubernetes/go-controller/observability-lib/sampledecoder"

Conversation

msherif1234 commented Dec 14, 2024 • edited by jotak Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Checklist

Uh oh!

msherif1234 commented Dec 19, 2024

Uh oh!

github-actions bot commented Dec 19, 2024

Uh oh!

msherif1234 commented Dec 19, 2024

Uh oh!

github-actions bot commented Dec 19, 2024

Uh oh!

codecov bot commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

msherif1234 commented Dec 19, 2024

Uh oh!

github-actions bot commented Dec 19, 2024

Uh oh!

msherif1234 commented Dec 19, 2024

Uh oh!

github-actions bot commented Dec 19, 2024

Uh oh!

openshift-ci-robot commented Jan 6, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Checklist

Uh oh!

github-actions bot commented Jan 20, 2025

Uh oh!

msherif1234 commented Jan 20, 2025

Uh oh!

github-actions bot commented Jan 20, 2025

Uh oh!

openshift-ci bot commented Jan 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jotak Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msherif1234 Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jotak left a comment

Choose a reason for hiding this comment

Uh oh!

jotak left a comment

Choose a reason for hiding this comment

Uh oh!

msherif1234 commented Jan 21, 2025

Uh oh!

openshift-ci bot commented Jan 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

msherif1234 commented Dec 14, 2024 •

edited by jotak

Loading

codecov bot commented Dec 19, 2024 •

edited

Loading

openshift-ci-robot commented Jan 6, 2025 •

edited by openshift-ci bot

Loading

jotak Jan 21, 2025 •

edited

Loading

msherif1234 Jan 21, 2025 •

edited

Loading