-
Notifications
You must be signed in to change notification settings - Fork 25
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
NETOBSERV-1500: Update after refactoring of FLP transform network API #562
Conversation
/hold because the FLP dependency needs to be updated once the PR is merged |
5587596
to
cc121a7
Compare
/retest |
cc121a7
to
05be1ec
Compare
@OlivierCazade: This pull request references NETOBSERV-1500 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.16.0" version, but no target version was set. In response to this:
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. |
@OlivierCazade: This pull request references NETOBSERV-1500 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.16.0" version, but no target version was set. In response to this:
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. |
@@ -47,6 +47,8 @@ func newPipelineBuilder( | |||
} | |||
} | |||
|
|||
const openshiftNamespacesPrefixes = "openshift" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be configurable to allow non openshift deployments to identify their infra too ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps as a follow-up / or waiting to see if there's demand?
People can also play with quick filters if they want to add conditions to the app/infra filters in the gui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could introduce new tagging possibilities, but I think this could be confusing to let a user mix non openshift deployment under the infra tag.
{ | ||
Name: "openshift", | ||
Namespace: "default", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's this exactly, do you know? I've never seen it used / never appears in my flows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two services are deployed in the default namespace, kubernetes
and openshift
they can be used to reach respectively kubernetes and openshift API.
Now, I don't exactly know what is exactly exposed in the openshift API.
go.mod
Outdated
@@ -84,3 +84,5 @@ require ( | |||
) | |||
|
|||
replace github.com/prometheus/common v0.46.0 => github.com/netobserv/prometheus-common v0.46.0-netobserv | |||
|
|||
replace github.com/netobserv/flowlogs-pipeline v0.1.11-0.20240125165124-4289f5a17934 => github.com/OlivierCazade/flowlogs-pipeline v0.0.2-0.20240202153633-a34a92595834 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't forget to remove before merge :)
/lgtm |
05be1ec
to
eff0d2d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #562 +/- ##
==========================================
+ Coverage 67.15% 67.19% +0.03%
==========================================
Files 71 71
Lines 8407 8417 +10
==========================================
+ Hits 5646 5656 +10
Misses 2409 2409
Partials 352 352
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
/ok-to-test |
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:aeb2063 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-aeb2063 Or as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-aeb2063
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
/lgtm |
eff0d2d
to
7865f27
Compare
7865f27
to
024fe69
Compare
github.com/onsi/ginkgo/v2 v2.15.0 | ||
github.com/onsi/gomega v1.31.1 | ||
github.com/openshift/api v0.0.0-20220112145620-704957ce4980 | ||
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.71.2 | ||
github.com/prometheus/common v0.46.0 | ||
github.com/prometheus/common v0.48.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still have the replace directive for 0.46.0 below
024fe69
to
1235503
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: OlivierCazade 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 |
@OlivierCazade: The following test failed, say
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/test-infra repository. I understand the commands that are listed here. |
Description
Update of FLP dependency
Dependencies
netobserv/flowlogs-pipeline#580
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.