Skip to content

NETOBSERV-2519: implement form for FlowCollectorSlice #1166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
jotak wants to merge 1 commit into
base: main
Choose a base branch
from
Open

NETOBSERV-2519: implement form for FlowCollectorSlice #1166

jotak wants to merge 1 commit into from

Conversation

@jotak
Copy link
Member

@jotak jotak commented Dec 11, 2025 •
edited
Loading

Description

  • Implement missing form for FlowCollectorSlice
  • Add a script to generate schemas for tests
  • Fix URL declaration for FlowMetrics form (fixes NETOBSERV-2520) This url wasn't declared as it should: /k8s/cluster/flows.netobserv.iov1alpha1FlowMetric/~new
  • Move the delete modal into its own component; adjust some texts

Dependencies

Operator: netobserv/network-observability-operator#2171

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

@jotak
NETOBSERV-2519: implement form for FlowCollectorSlice
d2175b4 
- Implement missing form for FlowCollectorSlice
- Add a script to generate schemas for tests
- Fix URL declaration for FlowMetrics form (fixes NETOBSERV-2520)
    This url wasn't declared as it should: /k8s/cluster/flows.netobserv.io~v1alpha1~FlowMetric/~new
- Move the delete modal into its own component; adjust some texts
@jotak jotak requested a review from jpinsonneau December 11, 2025 14:18
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Dec 11, 2025 •
edited by openshift-ci bot
Loading

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

Details

In response to this:

Description

  • Implement missing form for FlowCollectorSlice
  • Add a script to generate schemas for tests
  • Fix URL declaration for FlowMetrics form (fixes NETOBSERV-2520) This url wasn't declared as it should: /k8s/cluster/flows.netobserv.iov1alpha1FlowMetric/~new
  • Move the delete modal into its own component; adjust some texts

Dependencies

Operator:

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 Dec 11, 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 oliviercazade for approval. For more information see the Code Review Process.

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

Details 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 Dec 11, 2025 •
edited by openshift-ci bot
Loading

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

Details

In response to this:

Description

  • Implement missing form for FlowCollectorSlice
  • Add a script to generate schemas for tests
  • Fix URL declaration for FlowMetrics form (fixes NETOBSERV-2520) This url wasn't declared as it should: /k8s/cluster/flows.netobserv.iov1alpha1FlowMetric/~new
  • Move the delete modal into its own component; adjust some texts

Dependencies

Operator:

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 mentioned this pull request Dec 11, 2025
Merged
11 tasks
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Dec 11, 2025 •
edited by openshift-ci bot
Loading

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

Details

In response to this:

Description

  • Implement missing form for FlowCollectorSlice
  • Add a script to generate schemas for tests
  • Fix URL declaration for FlowMetrics form (fixes NETOBSERV-2520) This url wasn't declared as it should: /k8s/cluster/flows.netobserv.iov1alpha1FlowMetric/~new
  • Move the delete modal into its own component; adjust some texts

Dependencies

Operator: netobserv/network-observability-operator#2171

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 requested a review from Amoghrd December 11, 2025 14:20
@Amoghrd
Copy link
Member

Amoghrd commented Dec 11, 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 Dec 11, 2025
@github-actions
Copy link

github-actions bot commented Dec 11, 2025

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

It will expire after two weeks.

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

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

@openshift-ci
Copy link

openshift-ci bot commented Dec 11, 2025

@jotak: 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-console-tests d2175b4 link false /test qe-e2e-console-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.

@Amoghrd
Copy link
Member

Amoghrd commented Dec 11, 2025

/label qe-approved

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

openshift-ci-robot commented Dec 11, 2025 •
edited by openshift-ci bot
Loading

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

Details

In response to this:

Description

  • Implement missing form for FlowCollectorSlice
  • Add a script to generate schemas for tests
  • Fix URL declaration for FlowMetrics form (fixes NETOBSERV-2520) This url wasn't declared as it should: /k8s/cluster/flows.netobserv.iov1alpha1FlowMetric/~new
  • Move the delete modal into its own component; adjust some texts

Dependencies

Operator: netobserv/network-observability-operator#2171

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.

@Amoghrd
Copy link
Member

Amoghrd commented Dec 12, 2025

/label needs-review

@openshift-ci
Copy link

openshift-ci bot commented Dec 12, 2025

@Amoghrd: The label(s) /label needs-review cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, ux-approved, no-qe, downstream-change-needed, rebase/manual, cluster-config-api-changed, run-integration-tests, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/valid-bug, ok-to-test, stability-fix-approved, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

Details

In response to this:

/label needs-review

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.

@jotak jotak added the needs-review Tells that the PR needs a review label Dec 12, 2025
@jotak jotak requested a review from OlivierCazade December 15, 2025 08:10
jpinsonneau
jpinsonneau reviewed Dec 17, 2025
View reviewed changes
scripts/generate-schemas.sh
Comment on lines +3 to +5
fcSchema=`kubectl get crd flowcollectors.flows.netobserv.io -ojsonpath='{.spec.versions[0].schema.openAPIV3Schema}' | jq`
fcsSchema=`kubectl get crd flowcollectorslices.flows.netobserv.io -ojsonpath='{.spec.versions[0].schema.openAPIV3Schema}' | jq`
fmSchema=`kubectl get crd flowmetrics.flows.netobserv.io -ojsonpath='{.spec.versions[0].schema.openAPIV3Schema}' | jq`
Copy link
Contributor

@jpinsonneau jpinsonneau Dec 17, 2025

Choose a reason for hiding this comment

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

Why not pulling operator main branch and parsing those from YAMLs ?
https://github.com/search?q=repo%3Anetobserv%2Fnetwork-observability-operator+openAPIV3Schema+language%3AYAML&type=code&l=YAML

Copy link
Member Author

@jotak jotak Dec 19, 2025

Choose a reason for hiding this comment

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

I found it more convenient to do it from a running cluster, rather than having to wait for the operator PR to be merged. Of course we could pass a custom repo/branch to the script, as another alternative. But idk, doing it from the cluster seemed just good enough to me

web/src/app.tsx
Comment on lines +208 to +209
case 'flowCollectorSlice':
return <FlowCollectorSliceForm name="flowCollectorSlice-sample" />;
Copy link
Contributor

@jpinsonneau jpinsonneau Dec 17, 2025

Choose a reason for hiding this comment

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

I think we should create a wizard for FlowCollectorSlice too

That would be especially useful when it will grow with additionnal options.

Copy link
Member Author

@jotak jotak Dec 17, 2025

Choose a reason for hiding this comment

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

I didn't because it's just 2 fields, I don't see the need for simplified setup at the moment, but yes if that grows up then it can become useful

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

Reviewers

@jpinsonneau jpinsonneau jpinsonneau left review comments

@Amoghrd Amoghrd Awaiting requested review from Amoghrd

@OlivierCazade OlivierCazade Awaiting requested review from OlivierCazade

Assignees

No one assigned

Labels

jira/valid-reference 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jotak @openshift-ci-robot @Amoghrd @jpinsonneau