Skip to content

Conversation

ceelias
Copy link
Collaborator

@ceelias ceelias commented Sep 11, 2025

No description provided.

@ceelias ceelias requested a review from gsmith-sas September 11, 2025 14:33
Copy link
Member

@gsmith-sas gsmith-sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add an entry for this to the CHANGELOG.md. I guess it be classified as a CHANGE and indicate that we've decided it was better to deliver alerts as samples rather than deploying them automatically...to allow sites to customize them as needed. Not sure what else we need/want to say about it. Probably should mention that we "fixed" some of the alert definitions that were broken.

Hmmm, for sites that are doing an update-in-place, I guess any previously deployed alerts will stay deployed. I guess that's fine; if they're happy with them, there's no reason for us to uninstall them. Mentioning that we fixed some of the alerts in the CHANGELOG should prompt them to consider redeploying the updated versions.

Copy link
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck -e SC1004' found no issues.

shfmt errors

'shfmt -s -i 4 -bn -sr -ln bash' returned error 1 finding the following formatting issues:

----------
diff monitoring/bin/deploy_monitoring_cluster.sh.orig monitoring/bin/deploy_monitoring_cluster.sh
--- monitoring/bin/deploy_monitoring_cluster.sh.orig
+++ monitoring/bin/deploy_monitoring_cluster.sh
@@ -380,7 +380,7 @@
 log_verbose "Creating Grafana alert rules ConfigMap"
 CUSTOM_ALERT_CONFIG_DIR="$USER_DIR/monitoring/alerting/"
 
-if [ -d "$CUSTOM_ALERT_CONFIG_DIR" ] && [ "$(ls -A "$CUSTOM_ALERT_CONFIG_DIR" 2>/dev/null)" ]; then
+if [ -d "$CUSTOM_ALERT_CONFIG_DIR" ] && [ "$(ls -A "$CUSTOM_ALERT_CONFIG_DIR" 2> /dev/null)" ]; then
     log_debug "Creating configmap for alert rules/notifiers/contact points defined in '$CUSTOM_ALERT_CONFIG_DIR'"
     CM_ARGS=(--from-file="$CUSTOM_ALERT_CONFIG_DIR")
 
----------

You can reformat the above files to meet shfmt's requirements by typing:

  shfmt -s -i 4 -bn -sr -ln bash -w filename


Copy link
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck -e SC1004' found no issues.

shfmt errors

'shfmt -s -i 4 -bn -sr -ln bash' returned error 1 finding the following formatting issues:

----------
diff monitoring/bin/deploy_monitoring_cluster.sh.orig monitoring/bin/deploy_monitoring_cluster.sh
--- monitoring/bin/deploy_monitoring_cluster.sh.orig
+++ monitoring/bin/deploy_monitoring_cluster.sh
@@ -380,7 +380,7 @@
 log_verbose "Creating Grafana alert rules ConfigMap"
 CUSTOM_ALERT_CONFIG_DIR="$USER_DIR/monitoring/alerting/"
 
-if [ -d "$CUSTOM_ALERT_CONFIG_DIR" ] && [ "$(ls -A "$CUSTOM_ALERT_CONFIG_DIR" 2>/dev/null)" ]; then
+if [ -d "$CUSTOM_ALERT_CONFIG_DIR" ] && [ "$(ls -A "$CUSTOM_ALERT_CONFIG_DIR" 2> /dev/null)" ]; then
     log_debug "Creating configmap for alert rules/notifiers/contact points defined in '$CUSTOM_ALERT_CONFIG_DIR'"
     CM_ARGS=(--from-file="$CUSTOM_ALERT_CONFIG_DIR")
 
----------

You can reformat the above files to meet shfmt's requirements by typing:

  shfmt -s -i 4 -bn -sr -ln bash -w filename


Copy link
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck -e SC1004' found no issues.

shfmt errors

'shfmt -s -i 4 -bn -sr -ln bash' returned error 1 finding the following formatting issues:

----------
diff monitoring/bin/deploy_monitoring_cluster.sh.orig monitoring/bin/deploy_monitoring_cluster.sh
--- monitoring/bin/deploy_monitoring_cluster.sh.orig
+++ monitoring/bin/deploy_monitoring_cluster.sh
@@ -380,7 +380,7 @@
 log_verbose "Creating Grafana alert rules ConfigMap"
 CUSTOM_ALERT_CONFIG_DIR="$USER_DIR/monitoring/alerting/"
 
-if [ -d "$CUSTOM_ALERT_CONFIG_DIR" ] && [ "$(ls -A "$CUSTOM_ALERT_CONFIG_DIR" 2>/dev/null)" ]; then
+if [ -d "$CUSTOM_ALERT_CONFIG_DIR" ] && [ "$(ls -A "$CUSTOM_ALERT_CONFIG_DIR" 2> /dev/null)" ]; then
     log_debug "Creating configmap for alert rules/notifiers/contact points defined in '$CUSTOM_ALERT_CONFIG_DIR'"
     CM_ARGS=(--from-file="$CUSTOM_ALERT_CONFIG_DIR")
 
----------

You can reformat the above files to meet shfmt's requirements by typing:

  shfmt -s -i 4 -bn -sr -ln bash -w filename


@ceelias ceelias requested a review from gsmith-sas September 16, 2025 13:30
Copy link
Member

@gsmith-sas gsmith-sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I only have a few suggestions for tweaks to the CHANGELOG.md entry.

@ceelias ceelias requested a review from gsmith-sas September 16, 2025 14:56
Copy link
Member

@gsmith-sas gsmith-sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ceelias ceelias merged commit 7200b4a into main Sep 16, 2025
1 of 2 checks passed
@gsmith-sas gsmith-sas deleted the alerting-samples branch September 17, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants