Skip to content

Conversation

@jotak
Copy link
Member

@jotak jotak commented May 13, 2025

Description

  • Auto-detect available filters per feature (so we don't need to implement specific code to detect when NetworkName filters (for instance) are available)
  • Add tests for auto-detection
  • Do not display "None" as a UDN label

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.

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

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented May 13, 2025

@jotak: This pull request references NETOBSERV-2227 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 bug to target the "4.20.0" version, but no target version was set.

In response to this:

Description

  • Auto-detect available filters per feature (so we don't need to implement specific code to detect when NetworkName filters (for instance) are available)
  • Add tests for auto-detection
  • Do not display "None" as a UDN label

Dependencies

  • Operator: (todo)

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.

@openshift-ci
Copy link

openshift-ci bot commented May 13, 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 assign jotak for approval. 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

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented May 13, 2025

@jotak: This pull request references NETOBSERV-2227 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 bug to target the "4.20.0" version, but no target version was set.

In response to this:

Description

  • Auto-detect available filters per feature (so we don't need to implement specific code to detect when NetworkName filters (for instance) are available)
  • Add tests for auto-detection
  • Do not display "None" as a UDN label

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.

  • 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 May 14, 2025
@github-actions
Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=3dddc24 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 May 14, 2025
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label May 14, 2025
@github-actions
Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=309506a make set-plugin-image

Comment on lines +438 to +443
value
.map(iName => String(iName))
.filter(iName => iName !== '')
.map(iName => simpleTextWithTooltip(iName)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not showing n/a greyed out for empty ones ?

Currently it's empty. That feels strange compared to other fields:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

done in 6d8cec6

Comment on lines +198 to +205
return getFilterDefinitions(model.config.filters, model.config.columns, t).filter(fd => {
if (fd.id === 'id') {
return isConnectionTracking();
}
return checkFilterAvailable(fd, model.config, model.dataSource);
});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [model.config.columns, model.config.filters, model.config.promLabels, isPromOnly]);
}, [model.config, model.dataSource]);
Copy link
Contributor

@jpinsonneau jpinsonneau May 14, 2025

Choose a reason for hiding this comment

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

I think there was some exceptions here:

  • You already managed the UDN one
  • I see drop state and cause filters when packet drop feature is not enabled

image
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Others filters seems to work properly 👌

Copy link
Member Author

Choose a reason for hiding this comment

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

did you take the operator PR too? The issue with drops is supposed to be fixed from the operator config

Copy link
Contributor

Choose a reason for hiding this comment

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

I was using it yes

Copy link
Member Author

Choose a reason for hiding this comment

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

weird, I don't have them
image

Copy link
Member Author

Choose a reason for hiding this comment

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

the operator generated image was prior to this commit, I guess you used that old one; just triggered a new one

@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 May 14, 2025
@Amoghrd
Copy link
Member

Amoghrd commented May 14, 2025

/ok-to-test

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

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

It will expire after two weeks.

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

USER=netobserv VERSION=392537c 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 May 14, 2025
@jotak jotak added the needs-review Tells that the PR needs a review label May 14, 2025
@Amoghrd
Copy link
Member

Amoghrd commented May 14, 2025

/ok-to-test

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

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

It will expire after two weeks.

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

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

jotak added 2 commits May 16, 2025 10:27
- Auto-detect available filters per feature (so we don't need to
  implement specific code to detect when NetworkName filters (for
instance) are available)
- Add tests for auto-detection
- Do not display "None" as a UDN label
@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 May 16, 2025
@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label May 16, 2025
@github-actions
Copy link

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

It will expire after two weeks.

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

USER=netobserv VERSION=1032074 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 May 16, 2025
@jotak jotak requested a review from jpinsonneau May 16, 2025 09:07
@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label May 16, 2025
@github-actions
Copy link

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

It will expire after two weeks.

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

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

@openshift-ci
Copy link

openshift-ci bot commented May 16, 2025

@jotak: 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/plugin-cypress 00c0312 link true /test plugin-cypress
ci/prow/qe-e2e-console-tests 00c0312 link false /test qe-e2e-console-tests

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.

@Amoghrd
Copy link
Member

Amoghrd commented May 16, 2025

/label qe-approved

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

openshift-ci-robot commented May 16, 2025

@jotak: This pull request references NETOBSERV-2227 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 bug to target the "4.20.0" version, but no target version was set.

In response to this:

Description

  • Auto-detect available filters per feature (so we don't need to implement specific code to detect when NetworkName filters (for instance) are available)
  • Add tests for auto-detection
  • Do not display "None" as a UDN label

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.

  • 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 openshift-ci bot added the lgtm label May 20, 2025
@jotak jotak merged commit ca69ede into netobserv:main May 20, 2025
10 of 13 checks passed
jotak added a commit that referenced this pull request May 22, 2025
* NETOBSERV-2227: UDN adjustments, and auto-detect filters

- Auto-detect available filters per feature (so we don't need to
  implement specific code to detect when NetworkName filters (for
instance) are available)
- Add tests for auto-detection
- Do not display "None" as a UDN label

* Remove unnecessary Filter on Field

* write n/a for no udn

* Fix css alignment for n/a text

* Fix empty udn name
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