Sumo monitors installation as a part of the setup job #2250
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -3,7 +3,7 @@ | |||
| set -e | |||
|
|
|||
| echo "Checking the bash scripts with shellcheck..." | |||
| find . ! -path '*deploy/helm/sumologic/conf/setup/setup.sh' ! -path "*/tmp/*" -name '*.sh' -type 'f' -print | | |||
| find . ! -path '*deploy/helm/sumologic/conf/setup/setup.sh' ! -path '*deploy/helm/sumologic/conf/setup/monitors.sh' ! -path "*/tmp/*" -name '*.sh' -type 'f' -print | | |||
There was a problem hiding this comment.
Could you add analogical check like for setup.sh (is below)?
deploy/helm/sumologic/values.yaml
Outdated
| setupEnabled: true | ||
|
|
||
| ## If true, the installed monitors will be disabled by default | ||
| monitoringDisabled: false |
There was a problem hiding this comment.
I would change this flag to default enableByDefault or sth like that. There is no need to repeat monitoring keyword here
| echo "The monitors were already installed in ${MONITORS_FOLDER_NAME}." | ||
| echo "You can (re)install them manually with:" | ||
| echo "https://github.com/SumoLogic/terraform-sumologic-sumo-logic-monitor/tree/main/monitor_packages/kubernetes" | ||
| fi |
There was a problem hiding this comment.
Please add newline at EOF
There was a problem hiding this comment.
For other files as well 😅
| echo "Installation of the Sumo Logic monitors is disabled." | ||
| echo "You can install them manually later with:" | ||
| echo "https://github.com/SumoLogic/terraform-sumologic-sumo-logic-monitor/tree/main/monitor_packages/kubernetes" |
There was a problem hiding this comment.
nobody will see this message, as setup pod is being removed just after it finish it jobs (except failures). please add this info to https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/50b1a462b19790110ecafdc33c75cd848c2fdca8/deploy/helm/sumologic/templates/NOTES.txt
…gic.setup.monitors.disabled`
deploy/helm/sumologic/values.yaml
Outdated
| ## If enabled, a pre-install hook will create k8s monitors in Sumo Logic | ||
| setupEnabled: true | ||
|
|
||
| ## If true, the installed monitors will be disabled by default | ||
| disabled: false |
There was a problem hiding this comment.
This set of configuration variables is a bit misleading in the context of the rest of the Chart. It's customary to have an enabled: key which can be used to control the whole feature. To stay consistent with that, I'd do the following:
enabled: true - controls the whole feature, creates monitors in setup and enables them
monitorStatus: enabled - sets the monitor status, can be used to disable them
There was a problem hiding this comment.
OK, that makes sense. I'll update that.
tests/helm/terraform/static/monitors_with_email_notifications.input.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Dominik Rosiek <58699848+sumo-drosiek@users.noreply.github.com>
Description
In order to simplify k8s onboarding in Sumo Logic, we want to install monitors automatically from the setup job.
Behaviour:
Kuberenetesmonitors folder if it doesn't exist yetsumologic.setup.monitors.enabledsumologic.setup.monitors.monitorStatussumologic.setup.monitors.notificationEmailsChecklist
Testing performed