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.

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

move add_if and add_regex_if to transform_filter from transform_network #453

Conversation

KalmanMeth
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Patch coverage: 77.27% and no project coverage change.

Comparison is base (7513aa9) 66.15% compared to head (7ae760f) 66.15%.
Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #453   +/-   ##
=======================================
  Coverage   66.15%   66.15%           
=======================================
  Files          94       94           
  Lines        6867     6867           
=======================================
  Hits         4543     4543           
  Misses       2065     2065           
  Partials      259      259           
Flag Coverage Δ
unittests 66.15% <77.27%> (ø)

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

Files Changed Coverage Δ
pkg/api/transform_filter.go 100.00% <ø> (ø)
pkg/api/transform_network.go 83.33% <ø> (ø)
pkg/pipeline/transform/transform_network.go 72.56% <ø> (-0.56%) ⬇️
pkg/pipeline/transform/transform_filter.go 89.23% <77.27%> (-6.12%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jotak jotak self-requested a review July 25, 2023 09:29
@jotak jotak added the breaking-change This pull request has breaking changes. They should be described in PR description. label Jul 25, 2023
@jotak jotak requested a review from ronensc July 25, 2023 09:30
@jotak
Copy link
Member

jotak commented Jul 25, 2023

cc @ronensc @eranra wdyt?
It would be a breaking change of course, but it sounds like some of the filters were wrongly defined as "network filters" whereas they were completely unrelated to network.
We'll need to document this change (and how to "migrate") in the release note

@jotak
Copy link
Member

jotak commented Jul 26, 2023

@KalmanMeth could you rebase please? Some CI checks aren't triggered because you it is several weeks behind main

@KalmanMeth KalmanMeth force-pushed the move-stuff-to-transform-filter branch from 11caaab to 7ae760f Compare July 26, 2023 13:36
@KalmanMeth
Copy link
Collaborator Author

@KalmanMeth could you rebase please? Some CI checks aren't triggered because you it is several weeks behind main

done

@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 26, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/flowlogs-pipeline:4f2115e

It will expire after two weeks.

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

USER=netobserv VERSION=4f2115e make set-flp-image

@jotak jotak added no-qe This PR doesn't necessitate QE approval no-doc This PR doesn't require documentation change on the NetObserv operator labels Jul 26, 2023
@jotak
Copy link
Member

jotak commented Jul 26, 2023

I just verified that there is no regression with netobserv (these moved rules aren't used, so that's fine)
Code looks good to me

/lgtm

@KalmanMeth
Copy link
Collaborator Author

@jotak @ronensc I think this PR just needs a formal approval from a reviewer before we can merge.

@jotak
Copy link
Member

jotak commented Sep 6, 2023

/approve

@openshift-ci
Copy link

openshift-ci bot commented Sep 6, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak

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-ci openshift-ci bot added the approved label Sep 6, 2023
@openshift-merge-robot openshift-merge-robot merged commit e958fe7 into netobserv:main Sep 6, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved breaking-change This pull request has breaking changes. They should be described in PR description. lgtm no-doc This PR doesn't require documentation change on the NetObserv operator no-qe This PR doesn't necessitate QE approval ok-to-test To set manually when a PR is safe to test. Triggers image build on PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants