ROX-31149: Improve monitoring deployment customization#83
ROX-31149: Improve monitoring deployment customization#83JoukoVirtanen wants to merge 6 commits intomainfrom
Conversation
tommartensen
left a comment
tommartensen
left a comment
There was a problem hiding this comment.
The approach works, but I have a concern about the general structure of the scripts in the start-secured-cluster directory.
| # Replace the prometheus ConfigMap with one that doesn't scrape as much info from berserker containers | ||
| kubectl -n stackrox delete configmap prometheus | ||
| kubectl create -f "${SCRIPT_DIR}"/prometheus.yaml | ||
| # Compare version to determine which script to use |
There was a problem hiding this comment.
Why do we have two different versions, what is the difference between them? I would prefer to have just start-secured-cluster.sh and if necessary control logic and functions for each version there.
That should reduce code duplication and make the intents clearer.
There was a problem hiding this comment.
I have combined the scripts.
| version_major=$(echo "${version_major_minor}" | cut -d. -f1) | ||
| version_minor=$(echo "${version_major_minor}" | cut -d. -f2) | ||
|
|
||
| # Determine if version is 4.11 or later (compare as integers, not floats) |
There was a problem hiding this comment.
Please add a Jira under https://issues.redhat.com/browse/ROX-33013 for the cleanup, when 4.11 is the last lowest supported ACS version.
There was a problem hiding this comment.
Here is the ticket https://issues.redhat.com/browse/ROX-33578
| --set resources.requests.memory="8Gi" | ||
| --set resources.limits.memory="8Gi" |
There was a problem hiding this comment.
Why can't these two values be set through the same mechanism as for < 4.11 ?
In the end, they're both overriding the resources.requests.memory and resources.limits.memory value in the monitoring chart.
| # Replace the prometheus ConfigMap with one that doesn't scrape as much info from berserker containers | ||
| kubectl -n stackrox delete configmap prometheus | ||
| kubectl create -f "${SCRIPT_DIR}"/prometheus.yaml | ||
| # Pre-4.11 only: Replace prometheus ConfigMap |
There was a problem hiding this comment.
Could we use kubectl apply -f?
2 participants
Currently monitoring for the long running clusters the config for the Prometheus is overwritten. This was done in a PR for increasing the berserker load. That change required changes to the Prometheus config. A comment in that PR suggested another method for adjusting the Prometheus config for the long running cluster. This PR implements that suggestion.
See the companion stackrox/stackrox PR stackrox/stackrox#18928
Testing is explained there.