Skip to content

Conversation

jpinsonneau
Copy link
Contributor

@jpinsonneau jpinsonneau commented Jul 29, 2025

Description

  • append "..." to truncated text
  • auto-generate schema and examples
  • 2nd pass on field descriptions, fields to hide, etc.
  • code style (camelCase for constants)
  • fix linter warnings introduced
  • remove < br > tags in descriptions
  • add a delete button on edit forms

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.

  • 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).

Copy link

openshift-ci bot commented Jul 29, 2025

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

Copy link

codecov bot commented Jul 29, 2025

Codecov Report

❌ Patch coverage is 47.56098% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.37%. Comparing base (ff0630c) to head (48e50ba).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
web/moduleMapper/dummy.tsx 30.76% 18 Missing ⚠️
...nts/tabs/netflow-topology/2d/layouts/baseLayout.ts 8.33% 11 Missing ⚠️
...nents/tabs/netflow-topology/2d/components/node.tsx 25.00% 3 Missing ⚠️
web/src/components/drawer/record/record-field.tsx 66.66% 2 Missing ⚠️
web/src/model/metrics.ts 33.33% 2 Missing ⚠️
web/src/utils/dscp.ts 71.42% 2 Missing ⚠️
web/src/utils/filter-options.ts 33.33% 2 Missing ⚠️
web/src/utils/metrics.ts 50.00% 2 Missing ⚠️
web/src/components/forms/dynamic-form/utils.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #956      +/-   ##
==========================================
- Coverage   54.42%   54.37%   -0.06%     
==========================================
  Files         202      203       +1     
  Lines       10759    10780      +21     
  Branches     1257     1261       +4     
==========================================
+ Hits         5856     5862       +6     
- Misses       4393     4408      +15     
  Partials      510      510              
Flag Coverage Δ
uitests 56.65% <47.56%> (-0.08%) ⬇️
unittests 49.27% <ø> (ø)

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

Files with missing lines Coverage Δ
web/moduleMapper/schemas.ts 100.00% <ø> (ø)
web/moduleMapper/templates.ts 57.14% <100.00%> (ø)
...nents/tabs/netflow-topology/2d/components/edge.tsx 23.07% <ø> (ø)
...nts/tabs/netflow-topology/2d/layouts/colaLayout.ts 7.69% <100.00%> (ø)
...ents/tabs/netflow-topology/2d/styles/styleNode.tsx 28.57% <ø> (ø)
web/src/model/topology.ts 25.11% <100.00%> (ø)
web/src/utils/context.ts 69.69% <100.00%> (ø)
web/src/utils/icmp.ts 57.40% <100.00%> (ø)
web/src/utils/overview-panels.ts 68.13% <100.00%> (ø)
web/src/utils/pkt-drop.ts 75.00% <100.00%> (ø)
... and 10 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jpinsonneau jpinsonneau force-pushed the 2350 branch 2 times, most recently from 6855190 to 50542a5 Compare July 29, 2025 16:20
@jpinsonneau jpinsonneau marked this pull request as ready for review August 5, 2025 10:29
@jpinsonneau jpinsonneau added the needs-review Tells that the PR needs a review label Aug 7, 2025
}

export function k8sDelete(k8s: any): Promise<any> {
console.log("k8sDelete", k8s);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that temporary debug log or something we want to keep?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dummy file is not imported on production builds since it's part of console api. It's useful to see what's been sent in standalone builds.

https://github.com/netobserv/network-observability-console-plugin/blob/main/web/webpack.standalone.js#L81

Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm, will do some tests

@openshift-ci openshift-ci bot added the lgtm label Aug 14, 2025
@jotak jotak changed the title NETOBSERV-2350 Static plugin: feedback follow-up NETOBSERV-2350: Static plugin: feedback follow-up Aug 14, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Aug 14, 2025

@jpinsonneau: This pull request references NETOBSERV-2350 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.20.0" version, but no target version was set.

In response to this:

Description

  • append "..." to truncated text
  • auto-generate schema and examples
  • 2nd pass on field descriptions, fields to hide, etc.
  • code style (camelCase for constants)
  • fix linter warnings introduced
  • remove < br > tags in descriptions
  • add a delete button on edit forms

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.

  • 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 jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 14, 2025
Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=8080bac make set-plugin-image

@jotak
Copy link
Member

jotak commented Aug 14, 2025

Looks like the <br> tags are still visible in some places:
image

@jotak
Copy link
Member

jotak commented Aug 14, 2025

There seems to be some problems in the wizard:

  • the eBPF features/privilege mode have disappeared? Is it on purpose?
  • Chosing Kafka deployment model, or LokiStack for loki, doesn't trigger an update of the form with the relevant features
  • We seem to have lost the "s/NetObserv/network observability/" replacement. Or maybe this is some text overrides that we have lost? E.g. we had before:
image and now: image The previous text was better
  • In this doc I suggested some changes of what to add/remove from the wizard, which would make it shorter and more focused on a smaller set of much used features. What's the status, are we ok to do that?

@openshift-ci openshift-ci bot removed the lgtm label Aug 19, 2025
@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 19, 2025
Copy link

openshift-ci bot commented Aug 19, 2025

[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 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:

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

@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 19, 2025
Copy link

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

It will expire after two weeks.

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

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

@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 25, 2025
@jpinsonneau
Copy link
Contributor Author

@Amoghrd that should make it: 7504c22

I'm forced to recreate the whole graph as updating it results in the crash in some cases 😢

Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=56ccbc3 make set-plugin-image

@Amoghrd
Copy link
Member

Amoghrd commented Aug 25, 2025

On which cases does it crash?

@jpinsonneau
Copy link
Contributor Author

quay.io/netobserv/network-observability-console-plugin:56ccbc3

On refresh, when the FlowCollector status changes.
The topology viewer tries to recreate the edges and fail telling that a parent is missing. I remember I had similar issues with the metrics viewer too !

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 25, 2025
@jpinsonneau
Copy link
Contributor Author

@Amoghrd that one will fix the missing name / namespace fields in FlowMetric: a289d61

I also improved the way errors are displayed.

FYI, by default, the FlowMetric example has an issue. A legend field is missing.

@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 25, 2025
Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=0de26d8 make set-plugin-image

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 26, 2025
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 26, 2025
Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=99f4e6a make set-plugin-image

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 26, 2025
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 26, 2025
Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=35d9ca2 make set-plugin-image

Copy link

openshift-ci bot commented Aug 26, 2025

@jpinsonneau: The following tests 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-console-tests a289d61 link false /test qe-e2e-console-tests
ci/prow/plugin-cypress 5be6e24 link true /test plugin-cypress

Full PR test history. Your PR dashboard.

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.

Copy link
Collaborator

@OlivierCazade OlivierCazade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci openshift-ci bot added the lgtm label Aug 26, 2025
@Amoghrd
Copy link
Member

Amoghrd commented Aug 26, 2025

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Aug 26, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Aug 26, 2025

@jpinsonneau: This pull request references NETOBSERV-2350 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.20.0" version, but no target version was set.

In response to this:

Description

  • append "..." to truncated text
  • auto-generate schema and examples
  • 2nd pass on field descriptions, fields to hide, etc.
  • code style (camelCase for constants)
  • fix linter warnings introduced
  • remove < br > tags in descriptions
  • add a delete button on edit forms

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.

  • 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.

@jpinsonneau jpinsonneau merged commit f585478 into netobserv:main Aug 26, 2025
10 of 14 checks passed
@jotak
Copy link
Member

jotak commented Sep 12, 2025

There seems to be some problems in the wizard:

  • the eBPF features/privilege mode have disappeared? Is it on purpose?

  • Chosing Kafka deployment model, or LokiStack for loki, doesn't trigger an update of the form with the relevant features

  • We seem to have lost the "s/NetObserv/network observability/" replacement. Or maybe this is some text overrides that we have lost? E.g. we had before:

  • In this doc I suggested some changes of what to add/remove from the wizard, which would make it shorter and more focused on a smaller set of much used features. What's the status, are we ok to do that?

I have double checked this and it's still working. I guess you didn't deployed the operator from OLM. You need a valid CSV deployed to retreive the model:

I've retried today and still get the same problem. I didn't deploy from OLM, I deployed from the source repo, however since that's been merged (and I'm up to date) I think it should work, right?
I'm wondering if it's an issue with the openshift version? I'm testing on 4.18 right now...

Beside that, I also mentioned other issues, like a regression on the texts, that I think we should address ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference lgtm needs-review Tells that the PR needs a review ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. qe-approved QE has approved this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants