NETOBSERV-1927 scopes config #634

jpinsonneau · 2024-11-07T16:35:04Z

Description

Allow topology scope and groups configurations

Dependencies

netobserv/network-observability-operator#834

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.

openshift-ci · 2024-11-07T16:35:08Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2024-11-07T16:35:10Z

[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 jpinsonneau. 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

github-actions · 2024-11-08T08:37:40Z

New image:
quay.io/netobserv/network-observability-console-plugin:c527049

It will expire after two weeks.

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

USER=netobserv VERSION=c527049 make set-plugin-image

pkg/config/config.go

Co-authored-by: Joel Takvorian <joel.takvorian@homeblocks.net>

jotak · 2024-11-08T08:55:26Z

pkg/config/config.go

+		"dnsRCode":     {"DnsFlagsResponseCode"},
+	}
+	for i := range c.Scopes {
+		keyLabels[c.Scopes[i].ID] = sc.Labels


oops .. compile error, my bad!

Ahah yeah we need an IDE integration in github !

If you want, you can use GitHub Pull Requests extension in vscode
https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github

It allows you to pull a PR, do the changes you want and create suggestions directly from your IDE

jotak · 2024-11-08T09:05:40Z

web/src/model/flow-query.ts

@@ -8,7 +8,7 @@ export type PacketLoss = 'dropped' | 'hasDrops' | 'sent' | 'all';
 export type MetricFunction = 'count' | 'sum' | 'avg' | 'min' | 'max' | 'p90' | 'p99' | 'rate';
 export type StatFunction = MetricFunction | 'last';
 export type MetricType = 'Flows' | 'DnsFlows' | Field;
-export type FlowScope = 'app' | 'cluster' | 'zone' | 'host' | 'namespace' | 'owner' | 'resource';
+export type FlowScope = 'app' | 'cluster' | 'zone' | 'host' | 'namespace' | 'owner' | 'resource' | string;


since it's now config based maybe just FlowScope = string is enough? (or is there still any good benefit from typing some of known scopes?)
same remark for groups below.

there are some specific cases indeed especially for owner & resource but I will remove the ones not used

web/src/model/scope.ts

jotak · 2024-11-08T09:46:53Z

code lgtm (beside one or two comments)
there are test failures
also, we'll need the operator PR with the new config to test

github-actions · 2024-11-08T10:59:29Z

New image:
quay.io/netobserv/network-observability-console-plugin:14cd831

It will expire after two weeks.

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

USER=netobserv VERSION=14cd831 make set-plugin-image

codecov · 2024-11-08T11:41:13Z

Codecov Report

Attention: Patch coverage is 64.55026% with 67 lines in your changes missing coverage. Please review.

Project coverage is 55.73%. Comparing base (4f95f98) to head (0c65c7f).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
web/src/model/topology.ts	22.22%	21 Missing ⚠️
web/src/model/scope.ts	72.97%	10 Missing ⚠️
web/src/components/dropdowns/scope-dropdown.tsx	14.28%	6 Missing ⚠️
web/src/components/netflow-traffic.tsx	44.44%	5 Missing ⚠️
...b/src/components/drawer/element/element-fields.tsx	33.33%	4 Missing ⚠️
...abs/netflow-topology/2d/styles/styleDecorators.tsx	20.00%	4 Missing ⚠️
...s/netflow-topology/3d/three-d-topology-content.tsx	0.00%	4 Missing ⚠️
web/src/components/slider/scope-slider.tsx	25.00%	3 Missing ⚠️
pkg/handler/topology.go	66.66%	2 Missing ⚠️
...ents/tabs/netflow-topology/2d/topology-content.tsx	50.00%	2 Missing ⚠️
... and 5 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #634      +/-   ##
==========================================
+ Coverage   55.38%   55.73%   +0.35%     
==========================================
  Files         193      195       +2     
  Lines       10088    10058      -30     
  Branches     1215     1199      -16     
==========================================
+ Hits         5587     5606      +19     
+ Misses       4124     4083      -41     
+ Partials      377      369       -8

Flag	Coverage Δ
uitests	`58.08% <56.46%> (+0.41%)`	⬆️
unittests	`50.58% <92.85%> (+0.28%)`	⬆️

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

Files with missing lines	Coverage Δ
pkg/config/config.go	`43.79% <100.00%> (+7.02%)`	⬆️
pkg/loki/topology_query.go	`73.24% <100.00%> (+0.17%)`	⬆️
pkg/prometheus/query.go	`85.84% <100.00%> (+0.12%)`	⬆️
web/src/api/loki.ts	`85.71% <ø> (ø)`
web/src/components/__tests-data__/scopes.ts	`100.00% <100.00%> (ø)`
...b/src/components/drawer/netflow-traffic-drawer.tsx	`45.54% <ø> (ø)`
web/src/components/dropdowns/group-dropdown.tsx	`100.00% <100.00%> (+29.41%)`	⬆️
...components/dropdowns/overview-display-dropdown.tsx	`63.63% <ø> (ø)`
.../components/dropdowns/overview-display-options.tsx	`80.00% <ø> (ø)`
...components/dropdowns/topology-display-dropdown.tsx	`66.66% <ø> (ø)`
... and 24 more

... and 2 files with indirect coverage changes

jotak · 2024-11-08T12:39:56Z

web/src/model/topology.ts

-    return next;
+  const index = scopes.findIndex(sc => sc.id === current);
+  if (index >= 0 && index < scopes.length) {
+    next = scopes[index].id;


step into doesn't work and I believe there's something missing here? (should it be next = scopes[index + 1].id; ?)
But that said, in this PR I made the "scope graph" not linear on purpose (because the next logical step after node is pod, not namespace) .. so how to represent that with the new model? Should we add a new next field in the scopes definition?

yeah I wanted to avoid that but I guess we don't have the choice if it's not linear there

done in 0c65c7f

github-actions · 2024-11-12T11:42:28Z

New image:
quay.io/netobserv/network-observability-console-plugin:63a469a

It will expire after two weeks.

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

USER=netobserv VERSION=63a469a make set-plugin-image

jotak · 2024-11-12T11:58:51Z

web/src/model/topology.ts

-  );
-};
-
-export const getStepIntoNext = (current: FlowScope, allowedScopes: FlowScope[]): FlowScope | undefined => {


I think you need to reintroduce this function (with a few changes), because it was a recursive function that would continue looking for "next" step in case the current found one is not allowed.
This allows to "jump" over disabled scopes. Two examples:

When you enable multi-clusters but not zones: stepping into cluster should bring you directly to nodes, by-passing zones

When disabling Loki, stepping into a Owner should not bring you to Resource (it should be disallowed)

The current implementation breaks these use cases

Restored recursive behavior + added tests
00e7f3b

thanks for the tests, indeed that was missing

github-actions · 2024-11-14T09:12:59Z

New image:
quay.io/netobserv/network-observability-console-plugin:d02b8f5

It will expire after two weeks.

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

USER=netobserv VERSION=d02b8f5 make set-plugin-image

jotak

LGTM

scopes config

a27407e

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

jpinsonneau requested a review from jotak November 7, 2024 16:35

jpinsonneau marked this pull request as ready for review November 7, 2024 16:35

openshift-ci bot removed the do-not-merge/work-in-progress label Nov 7, 2024

jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 8, 2024

jotak reviewed Nov 8, 2024

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

Update pkg/config/config.go

987690b

Co-authored-by: Joel Takvorian <joel.takvorian@homeblocks.net>

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

jotak reviewed Nov 8, 2024

View reviewed changes

web/src/model/scope.ts Outdated Show resolved Hide resolved

fix compile

f70ee2e

jpinsonneau added 3 commits November 8, 2024 10:57

scope & groups types

d5b3958

mandatory fields

4077304

missing fields in go type

6720e95

jpinsonneau mentioned this pull request Nov 8, 2024

NETOBSERV-1927 scopes config netobserv/network-observability-operator#834

Merged

10 tasks

fixes

aeac5b8

jpinsonneau force-pushed the 1927 branch from 9f8eed2 to aeac5b8 Compare November 8, 2024 10:48

jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 8, 2024

jotak reviewed Nov 8, 2024

View reviewed changes

step into

0c65c7f

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

jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 12, 2024

jotak reviewed Nov 12, 2024

View reviewed changes

fix step into + add tests

00e7f3b

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

jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 14, 2024

jotak approved these changes Nov 14, 2024

View reviewed changes

openshift-ci bot assigned jotak Nov 14, 2024

openshift-ci bot added the lgtm label Nov 14, 2024

jotak merged commit c12294d into netobserv:main Nov 18, 2024
7 of 9 checks passed

jpinsonneau mentioned this pull request Nov 26, 2024

NETOBSERV-1927 fix topology issues #657

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NETOBSERV-1927 scopes config #634

NETOBSERV-1927 scopes config #634

jpinsonneau commented Nov 7, 2024 •

edited

Loading

openshift-ci bot commented Nov 7, 2024

openshift-ci bot commented Nov 7, 2024

github-actions bot commented Nov 8, 2024

jotak Nov 8, 2024

jpinsonneau Nov 8, 2024

jotak Nov 8, 2024

jpinsonneau Nov 8, 2024

jpinsonneau Nov 8, 2024

jotak commented Nov 8, 2024 •

edited

Loading

github-actions bot commented Nov 8, 2024

codecov bot commented Nov 8, 2024 •

edited

Loading

jotak Nov 8, 2024

jpinsonneau Nov 8, 2024

jpinsonneau Nov 12, 2024

github-actions bot commented Nov 12, 2024

jotak Nov 12, 2024

jpinsonneau Nov 13, 2024

jotak Nov 14, 2024

github-actions bot commented Nov 14, 2024

jotak left a comment

NETOBSERV-1927 scopes config #634

NETOBSERV-1927 scopes config #634

Conversation

jpinsonneau commented Nov 7, 2024 • edited Loading

Description

Dependencies

Checklist

openshift-ci bot commented Nov 7, 2024

openshift-ci bot commented Nov 7, 2024

github-actions bot commented Nov 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jotak commented Nov 8, 2024 • edited Loading

github-actions bot commented Nov 8, 2024

codecov bot commented Nov 8, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 14, 2024

jotak left a comment

Choose a reason for hiding this comment

jpinsonneau commented Nov 7, 2024 •

edited

Loading

jotak commented Nov 8, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading