Skip to content
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

Networkevents feature is DevPreview #811

Merged
merged 3 commits into from
Oct 14, 2024
Merged

Networkevents feature is DevPreview #811

merged 3 commits into from
Oct 14, 2024

Conversation

jotak
Copy link
Member

@jotak jotak commented Oct 10, 2024

No description provided.

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 10, 2024

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

@codecov Codecov
Copy link

codecov bot commented Oct 10, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.50%. Comparing base (994b2b9) to head (e0dc798).
Report is 3 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #811      +/-   ##
==========================================
- Coverage   66.64%   66.50%   -0.14%     
==========================================
  Files          75       75              
  Lines        8739     8739              
==========================================
- Hits         5824     5812      -12     
- Misses       2506     2515       +9     
- Partials      409      412       +3
Flag Coverage Δ
unittests 66.50% <ø> (-0.14%) ⬇️

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

Files with missing lines Coverage Δ
apis/flowcollector/v1beta2/flowcollector_types.go 100.00% <ø> (ø)

... and 6 files with indirect coverage changes

@jotak jotak changed the title Add required version for networkevents Networkevents feature is DevPreview Oct 10, 2024
@jotak jotak marked this pull request as ready for review October 10, 2024 14:51
jotak added a commit to jotak/network-observability-operator that referenced this pull request Oct 10, 2024
@jotak
Networkevents feature is DevPreview [1.7 backport]
2948902 
Backport of netobserv#811
@jotak jotak mentioned this pull request Oct 10, 2024
Merged
@jotak jotak requested a review from skrthomas October 10, 2024 14:53
skrthomas
skrthomas reviewed Oct 10, 2024
View reviewed changes
docs/FlowCollector.md Outdated
- `NetworkEvents`: enable the Network events monitoring feature. This feature requires mounting
the kernel debug filesystem, so the eBPF pod has to run as privileged.<br/>
- `NetworkEvents`: enable the Network events monitoring feature. This feature requires mounting
the kernel debug filesystem, so the eBPF pod has to run as privileged. It is currently a Developer Preview.<br><br/>
Copy link
Contributor

@skrthomas skrthomas Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
the kernel debug filesystem, so the eBPF pod has to run as privileged. It is currently a Developer Preview.<br><br/>
the kernel debug filesystem, so the eBPF pod has to run as privileged.
+
[IMPORTANT]
====
`NetworkEvents` is available as a Developer Preview.
====
<br><br/>

Copy link
Contributor

Choose a reason for hiding this comment

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

Or, minimally, if the admonition doesn't translate between the source and asciidoc, just using all caps IMPORTANT:

stleerh
stleerh reviewed Oct 10, 2024
View reviewed changes
Copy link
Contributor

@stleerh stleerh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It should mention that it requires OVN support and to check that your version supports it.

@jotak
Copy link
Member Author

jotak commented Oct 11, 2024

@skrthomas @stleerh I changed the text to:

It requires using the OVN-Kubernetes network plugin with the Observability feature.
IMPORTANT: this feature is available as a Developer Preview.

stleerh
stleerh approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@stleerh stleerh 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 Oct 11, 2024
@skrthomas
Copy link
Contributor

/lgtm

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

The pull request process is described 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-merge-bot bot pushed a commit that referenced this pull request Oct 14, 2024
@jotak
Networkevents feature is DevPreview [1.7 backport] (#812)
1f8b565 
* Networkevents feature is DevPreview [1.7 backport]

Backport of #811

* mention IMPORTANT and OVN needed
@openshift-merge-bot openshift-merge-bot bot merged commit 63c1b7e into netobserv:main Oct 14, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skrthomas skrthomas skrthomas left review comments

@stleerh stleerh stleerh approved these changes

@msherif1234 msherif1234 Awaiting requested review from msherif1234

Assignees

@skrthomas skrthomas

@stleerh stleerh

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jotak @skrthomas @stleerh @jpinsonneau