From 6af3f1abc17f951306bcaa3858fb4d8cd966ae46 Mon Sep 17 00:00:00 2001 From: Matt Jeanes Date: Tue, 27 Jul 2021 11:23:10 +0100 Subject: [PATCH] Add --alert-firing-only parameter to only consider firing alerts --- README.md | 6 +++ cmd/kured/main.go | 9 ++++- cmd/kured/main_test.go | 2 +- kured-ds.yaml | 1 + pkg/alerts/prometheus.go | 4 +- pkg/alerts/prometheus_test.go | 76 +++++++++++++++++++++-------------- 6 files changed, 62 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 72fa67d..66b3c2e 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ The following arguments can be passed to kured via the daemonset pod template: ```console Flags: --alert-filter-regexp regexp.Regexp alert names to ignore when checking for active alerts + --alert-firing-only bool only consider firing alerts when checking for active alerts --blocking-pod-selector stringArray label selector identifying pods whose presence should prevent reboots --drain-grace-period int time in seconds given to each pod to terminate gracefully, if negative, the default value specified in the pod will be used (default: -1) --skip-wait-for-delete-timeout int when seconds is greater than zero, skip waiting for the pods whose deletion timestamp is older than N seconds while draining a node (default: 0) @@ -166,6 +167,11 @@ will block reboots, however you can ignore specific alerts: --alert-filter-regexp=^(RebootRequired|AnotherBenignAlert|...$ ``` +You can also only block reboots for firing alerts: +```console +--alert-firing-only=true +``` + See the section on Prometheus metrics for an important application of this filter. diff --git a/cmd/kured/main.go b/cmd/kured/main.go index ae3e23f..9964434 100644 --- a/cmd/kured/main.go +++ b/cmd/kured/main.go @@ -52,6 +52,7 @@ var ( prometheusURL string preferNoScheduleTaintName string alertFilter *regexp.Regexp + alertFiringOnly bool rebootSentinelFile string rebootSentinelCommand string notifyURL string @@ -121,6 +122,8 @@ func main() { "Prometheus instance to probe for active alerts") rootCmd.PersistentFlags().Var(®expValue{&alertFilter}, "alert-filter-regexp", "alert names to ignore when checking for active alerts") + rootCmd.PersistentFlags().BoolVar(&alertFiringOnly, "alert-firing-only", false, + "only consider firing alerts when checking for active alerts (default: false)") rootCmd.PersistentFlags().StringVar(&rebootSentinelFile, "reboot-sentinel", "/var/run/reboot-required", "path to file whose existence triggers the reboot command") rootCmd.PersistentFlags().StringVar(&preferNoScheduleTaintName, "prefer-no-schedule-taint", "", @@ -234,6 +237,8 @@ type PrometheusBlockingChecker struct { promClient *alerts.PromClient // regexp used to get alerts filter *regexp.Regexp + // bool to indicate if only firing alerts should be considered + firingOnly bool } // KubernetesBlockingChecker contains info for connecting @@ -248,7 +253,7 @@ type KubernetesBlockingChecker struct { func (pb PrometheusBlockingChecker) isBlocked() bool { - alertNames, err := pb.promClient.ActiveAlerts(pb.filter) + alertNames, err := pb.promClient.ActiveAlerts(pb.filter, pb.firingOnly) if err != nil { log.Warnf("Reboot blocked: prometheus query error: %v", err) return true @@ -540,7 +545,7 @@ func rebootAsRequired(nodeID string, rebootCommand []string, sentinelCommand []s var blockCheckers []RebootBlocker if prometheusURL != "" { - blockCheckers = append(blockCheckers, PrometheusBlockingChecker{promClient: promClient, filter: alertFilter}) + blockCheckers = append(blockCheckers, PrometheusBlockingChecker{promClient: promClient, filter: alertFilter, firingOnly: alertFiringOnly}) } if podSelectors != nil { blockCheckers = append(blockCheckers, KubernetesBlockingChecker{client: client, nodename: nodeID, filter: podSelectors}) diff --git a/cmd/kured/main_test.go b/cmd/kured/main_test.go index da29b0b..78dab65 100644 --- a/cmd/kured/main_test.go +++ b/cmd/kured/main_test.go @@ -32,7 +32,7 @@ func Test_rebootBlocked(t *testing.T) { if err != nil { log.Fatal("Can't create prometheusClient: ", err) } - brokenPrometheusClient := PrometheusBlockingChecker{promClient: promClient, filter: nil} + brokenPrometheusClient := PrometheusBlockingChecker{promClient: promClient, filter: nil, firingOnly: false} type args struct { blockers []RebootBlocker diff --git a/kured-ds.yaml b/kured-ds.yaml index cf9973b..8acad86 100644 --- a/kured-ds.yaml +++ b/kured-ds.yaml @@ -55,6 +55,7 @@ spec: # - --lock-ttl=0 # - --prometheus-url=http://prometheus.monitoring.svc.cluster.local # - --alert-filter-regexp=^RebootRequired$ +# - --alert-firing-only=false # - --reboot-sentinel=/var/run/reboot-required # - --prefer-no-schedule-taint="" # - --reboot-sentinel-command="" diff --git a/pkg/alerts/prometheus.go b/pkg/alerts/prometheus.go index d974f00..08777f7 100644 --- a/pkg/alerts/prometheus.go +++ b/pkg/alerts/prometheus.go @@ -36,7 +36,7 @@ func NewPromClient(conf papi.Config) (*PromClient, error) { // filter by regexp means when the regex finds the alert-name; the alert is exluded from the // block-list and will NOT block rebooting. query by includeLabel means, // if the query finds an alert, it will include it to the block-list and it WILL block rebooting. -func (p *PromClient) ActiveAlerts(filter *regexp.Regexp) ([]string, error) { +func (p *PromClient) ActiveAlerts(filter *regexp.Regexp, firingOnly bool) ([]string, error) { // get all alerts from prometheus value, _, err := p.api.Query(context.Background(), "ALERTS", time.Now()) @@ -49,7 +49,7 @@ func (p *PromClient) ActiveAlerts(filter *regexp.Regexp) ([]string, error) { activeAlertSet := make(map[string]bool) for _, sample := range vector { if alertName, isAlert := sample.Metric[model.AlertNameLabel]; isAlert && sample.Value != 0 { - if filter == nil || !filter.MatchString(string(alertName)) { + if (filter == nil || !filter.MatchString(string(alertName))) && (!firingOnly || sample.Metric["alertstate"] == "firing") { activeAlertSet[string(alertName)] = true } } diff --git a/pkg/alerts/prometheus_test.go b/pkg/alerts/prometheus_test.go index b0e126e..7122ab5 100644 --- a/pkg/alerts/prometheus_test.go +++ b/pkg/alerts/prometheus_test.go @@ -45,48 +45,62 @@ func TestActiveAlerts(t *testing.T) { addr := "http://localhost:10001" for _, tc := range []struct { - it string - rFilter string - respBody string - aName string - wantN int + it string + rFilter string + respBody string + aName string + wantN int + firingOnly bool }{ { - it: "should return no active alerts", - respBody: responsebody, - rFilter: "", - wantN: 0, + it: "should return no active alerts", + respBody: responsebody, + rFilter: "", + wantN: 0, + firingOnly: false, }, { - it: "should return a subset of all alerts", - respBody: responsebody, - rFilter: "Pod", - wantN: 3, + it: "should return a subset of all alerts", + respBody: responsebody, + rFilter: "Pod", + wantN: 3, + firingOnly: false, }, { - it: "should return all active alerts by regex", - respBody: responsebody, - rFilter: "*", - wantN: 5, + it: "should return all active alerts by regex", + respBody: responsebody, + rFilter: "*", + wantN: 5, + firingOnly: false, }, { - it: "should return all active alerts by regex filter", - respBody: responsebody, - rFilter: "*", - wantN: 5, + it: "should return all active alerts by regex filter", + respBody: responsebody, + rFilter: "*", + wantN: 5, + firingOnly: false, }, { - it: "should return ScheduledRebootFailing active alerts", - respBody: `{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"ALERTS","alertname":"ScheduledRebootFailing","alertstate":"pending","severity":"warning","team":"platform-infra"},"value":[1622472933.973,"1"]}]}}`, - aName: "ScheduledRebootFailing", - rFilter: "*", - wantN: 1, + it: "should return only firing alerts if firingOnly is true", + respBody: responsebody, + rFilter: "*", + wantN: 4, + firingOnly: true, }, { - it: "should not return an active alert if RebootRequired is firing (regex filter)", - respBody: `{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"ALERTS","alertname":"RebootRequired","alertstate":"pending","severity":"warning","team":"platform-infra"},"value":[1622472933.973,"1"]}]}}`, - rFilter: "RebootRequired", - wantN: 0, + it: "should return ScheduledRebootFailing active alerts", + respBody: `{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"ALERTS","alertname":"ScheduledRebootFailing","alertstate":"pending","severity":"warning","team":"platform-infra"},"value":[1622472933.973,"1"]}]}}`, + aName: "ScheduledRebootFailing", + rFilter: "*", + wantN: 1, + firingOnly: false, + }, + { + it: "should not return an active alert if RebootRequired is firing (regex filter)", + respBody: `{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"ALERTS","alertname":"RebootRequired","alertstate":"pending","severity":"warning","team":"platform-infra"},"value":[1622472933.973,"1"]}]}}`, + rFilter: "RebootRequired", + wantN: 0, + firingOnly: false, }, } { // Start mockServer @@ -111,7 +125,7 @@ func TestActiveAlerts(t *testing.T) { log.Fatal(err) } - result, err := p.ActiveAlerts(regex) + result, err := p.ActiveAlerts(regex, tc.firingOnly) if err != nil { log.Fatal(err) }