NETOBSERV-2596: Make console plugin controller use health metadata for config, set some default rules as recording#2388
Conversation
| TrendDuration: &v1.Duration{Duration: time.Hour}, | ||
| TrendOffset: &v1.Duration{Duration: time.Minute * 30}, | ||
| TrendDuration: &v1.Duration{Duration: time.Minute * 30}, | ||
| // TrendOffset: &v1.Duration{Duration: 24 * time.Hour}, |
There was a problem hiding this comment.
TODO: rollback those temporary changes
There was a problem hiding this comment.
Is this for testing ?
Maybe we could have a way to put some default values for developments without changing the code all the time. Similar to the useMocks in plugin 😉
| } | ||
|
|
||
| func (r *latencyTrend) RecordingName() string { | ||
| return buildRecordingRuleName(r.ctx, "tcp_latency_p90", "2m") |
There was a problem hiding this comment.
todo: rename including the comparison offset
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2388 +/- ##
==========================================
+ Coverage 71.92% 72.18% +0.25%
==========================================
Files 93 104 +11
Lines 10501 10619 +118
==========================================
+ Hits 7553 7665 +112
- Misses 2469 2479 +10
+ Partials 479 475 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jotak
added
the
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:7f084ed make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-7f084edOr 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-sha-7f084ed
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
leandroberetta
left a comment
leandroberetta
left a comment
There was a problem hiding this comment.
First round of review, looks good. I understand the changes and looks cleaner for sure.
| Sampling int `yaml:"sampling" json:"sampling"` | ||
| Features []string `yaml:"features" json:"features"` | ||
| Fields []FieldConfig `yaml:"fields" json:"fields"` | ||
| RecordingAnnotations map[string]map[string]string `yaml:"recordingAnnotations,omitempty" json:"recordingAnnotations,omitempty"` |
There was a problem hiding this comment.
Question although I think I know the answer, the change here is to just send annotations vs healthrules, right?.
There was a problem hiding this comment.
yes, it's to 100% fill the gap between alerts and recording, since the console plugin was able to get annotations on alerts but not on recordings
jpinsonneau
left a comment
jpinsonneau
left a comment
There was a problem hiding this comment.
Code looks good
Thanks @jotak !
github-actions
bot
removed
the
ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
jotak
added
the
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:823f507 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-823f507Or 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-sha-823f507
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
…me default rules as recording
- Refactor all alerts to implement a HealthRule interface
- HealthRule provides the Annotations, RecordingName and the
PrometheusRule
- RecordingName now provided explicitly
- Split logic between "builder" and "context"
- Console plugin controller just dumps annotations to config
- Change some defaults to Recording
Since these annotations are a user-exposed API, it is preferable to use a more k8s-standard terminology, ie "workload" instead of "owner".
jotak
force-pushed
the
unify-data-model
branch
from
ef407a2 to
0bb7f34
Compare
github-actions
bot
removed
the
ok-to-test
jotak
added
the
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:88f417c make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-88f417cOr 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-sha-88f417c
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
github-actions
bot
removed
the
ok-to-test
jotak
added
the
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:18b8d5f make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-18b8d5fOr 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-sha-18b8d5f
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
jotak
force-pushed
the
unify-data-model
branch
from
f047cfb to
3556c06
Compare
jotak
force-pushed
the
unify-data-model
branch
from
3556c06 to
fd8629c
Compare
github-actions
bot
removed
the
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:f9d7c1e make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-f9d7c1eOr 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-sha-f9d7c1e
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
jotak
added
the
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:73d01f7 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-73d01f7Or 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-sha-73d01f7
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
| NodeLabels []string `json:"nodeLabels,omitempty"` | ||
| NamespaceLabels []string `json:"namespaceLabels,omitempty"` | ||
| WorkloadLabels []string `json:"workloadLabels,omitempty"` | ||
| KindLabels []string `json:"kindLabels,omitempty"` |
There was a problem hiding this comment.
Can you please ensure the doc is updated ?
https://github.com/netobserv/network-observability-operator/blob/b15953ed03f305d7502ab2293d6490a683733de1/docs/HealthRules.md?plain=1#L147-L149
There was a problem hiding this comment.
oh yeah I missed that, good catch - fixed in last commit
github-actions
bot
removed
the
ok-to-test
jotak
force-pushed
the
unify-data-model
branch
from
b5ba902 to
fd8629c
Compare
jotak
changed the title
|
@jotak: This pull request references NETOBSERV-2596 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.22.0" version, but no target version was set. DetailsIn 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. |
jotak
added
the
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:c37987f make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-c37987fOr 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-sha-c37987f
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
leandroberetta
left a comment
leandroberetta
left a comment
There was a problem hiding this comment.
lgtm although there are some todos.
github-actions
bot
removed
the
ok-to-test
|
/label qe-approved |
|
@jotak: This pull request references NETOBSERV-2596 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.22.0" version, but no target version was set. DetailsIn 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. |
|
/cherry-pick release-1.11 |
|
@jotak: once the present PR merges, I will cherry-pick it on top of DetailsIn 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 kubernetes-sigs/prow repository. |
|
@jotak: new pull request created: #2413 DetailsIn 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 kubernetes-sigs/prow repository. |
7 participants
Description
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.