ROX-33361: Per-namespace persistence in SecuredCluster CR#20102
ROX-33361: Per-namespace persistence in SecuredCluster CR#20102
Conversation
🚀 Build Images ReadyImages are ready for commit 57f78fa. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-721-g57f78fa954 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #20102 +/- ##
=======================================
Coverage 49.68% 49.68%
=======================================
Files 2766 2766
Lines 209299 209359 +60
=======================================
+ Hits 103989 104028 +39
- Misses 97616 97639 +23
+ Partials 7694 7692 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/test ocp-4-21-operator-e2e-tests |
|
|
||
| enableOpenShiftMonitoring: false | ||
|
|
||
| [<- if .FeatureFlags.ROX_PROCESS_INDICATORS_PER_NAMESPACE >] |
There was a problem hiding this comment.
Do we really need the feature flag for this? The act of updating the config would sort of handle that from a user perspective would it not?
There was a problem hiding this comment.
There was a discussion about this with the Install team, unfortunately without any decisive conclusions yet.
dashrews78
left a comment
There was a problem hiding this comment.
LGTM, but please get approval from install team before merging.
|
Screenshots of the Openshift console with these configuration in the PR description might be nice. You can merge without my approval. |
Good point, forgot about this requirement. |
Commit 405c8cf ("ROX-33361: Per-namespace persistence for process indicators (#19957)") has introduced a possibility to configure per-namespace persistence for process indicators, but did not wire it up anywhere. Allow to provide new dynamic config fields via SecuredCluster CR, in the form: spec: processIndicators: persistence: true excludeNamespaceFilter: namespace-without-persistence excludeOpenshiftNs: false It works in exactly the same way as above mentioned dynamic config counterpart, except the reversed "persistence" field. Contrary to a protobuf dynamic config, SecuredCluster CR allows to distinguish not set values, thus we choose more natural to read version. It will be converted into the "noPersistence" during translation.
0c4b745 to
57f78fa
Compare
|
@erthalion: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
Description
NOTE: It's been split from #19455, for the purposes of simplifying the review. The PR contains only the second commit, wiring everything up into the operator. The implementation is exactly the same as in the original PR.
Commit 405c8cf ("ROX-33361: Per-namespace persistence for process indicators (#19957)") has introduced a possibility to configure per-namespace persistence for process indicators, but did not wire it up anywhere.
Allow to provide new dynamic config fields via SecuredCluster CR, in the form:
It works in exactly the same way as above mentioned dynamic config counterpart, except the reversed "persistence" field. Contrary to a protobuf dynamic config, SecuredCluster CR allows to distinguish not set values, thus we choose more natural to read version. It will be converted into the "noPersistence" during translation.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Manual validation (creating an operator-managed cluster and modifying the configuration), as well as E2E tests. Split from #19455 to simplify the review.