NETOBSERV-2534 Have a way to pause Network Observability functions#2362
NETOBSERV-2534 Have a way to pause Network Observability functions#2362jpinsonneau wants to merge 2 commits intonetobserv:mainfrom
Conversation
|
[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 |
jpinsonneau
changed the title
jpinsonneau
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:93e5ad9 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-93e5ad9Or 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-93e5ad9
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2362 +/- ##
==========================================
- Coverage 72.23% 71.93% -0.30%
==========================================
Files 93 93
Lines 10346 10485 +139
==========================================
+ Hits 7473 7542 +69
- Misses 2401 2468 +67
- Partials 472 475 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
bot
removed
the
ok-to-test
| if err := deleteResourcesByType(ctx, cl, listObj, labelSelector); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
(nit) Is there any reason to separate it into two lists?
There was a problem hiding this comment.
That was for clarity but there is no technical reason here.
Would you prefer all in one keeping the comments to separate namespaced resources and cluster ones ?
| }).WithTimeout(timeout).WithPolling(interval).Should(BeTrue()) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Wow, that's a lot of testing code for something that's not likely to break. This is more a comment than something to do.
The most likely issue in the future is that a new resource is used and the list of resources to delete from isn't updated.
There was a problem hiding this comment.
I agree it's a defensive approach and we can simplify it. The goal is to cover the most important resources types and ensure it doesn't delete the ones not managed by the operator.
If we prefer, we can reduce that test to the strict minimal and check the list of types in another test.
There was a problem hiding this comment.
Thinking more about this, we could even parse the RBAC to ensure the list of kinds we delete is in sync with the roles 🤔
But the test file will be even bigger 😆
2 participants
Description
HOLDflag in CSV to hold controller reconciliation and delete all resources appart from user CRs and NamespacesAlso polished the plugin for clarity: netobserv/network-observability-console-plugin#1224
Dependencies
n/a
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.