From 50dff91a3aa5176dadadf802b6e82b0962701e1a Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Thu, 20 Apr 2017 15:19:25 +0100 Subject: [PATCH] Watcher: Fix resetting of ack status on unmet condition (elastic/x-pack-elasticsearch#1141) When a condition is unmet, the ack status of the actions needs to be resetted again, so that new alerts can be triggered. Due to a bugfix this functionality was removed from ES 5.0.0-alpha5 onwards. relates elastic/x-pack-elasticsearch#1123 Original commit: elastic/x-pack-elasticsearch@83db2cecf98029d57190aba184d6422f0ed2774d --- .../xpack/watcher/watch/WatchStatus.java | 4 + .../xpack/watcher/watch/WatchStatusTests.java | 34 ++++++ .../30_reset_ack_after_unmet_condition.yaml | 105 ++++++++++++++++++ 3 files changed, 143 insertions(+) create mode 100644 plugin/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchStatusTests.java create mode 100644 plugin/src/test/resources/rest-api-spec/test/watcher/ack_watch/30_reset_ack_after_unmet_condition.yaml diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java index c084078bab3..6b17d41319a 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java @@ -117,6 +117,10 @@ public class WatchStatus implements ToXContentObject, Streamable { lastChecked = timestamp; if (metCondition) { lastMetCondition = timestamp; + } else { + for (ActionStatus status : actions.values()) { + status.resetAckStatus(timestamp); + } } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchStatusTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchStatusTests.java new file mode 100644 index 00000000000..85ad27ddb2a --- /dev/null +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchStatusTests.java @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.watcher.watch; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.watcher.actions.ActionStatus; +import org.elasticsearch.xpack.watcher.actions.ActionStatus.AckStatus.State; +import org.elasticsearch.xpack.watcher.actions.logging.LoggingAction; + +import java.util.HashMap; + +import static org.hamcrest.Matchers.is; +import static org.joda.time.DateTime.now; + +public class WatchStatusTests extends ESTestCase { + + public void testAckStatusIsResetOnUnmetCondition() { + HashMap myMap = new HashMap<>(); + ActionStatus actionStatus = new ActionStatus(now()); + myMap.put("foo", actionStatus); + + actionStatus.update(now(), new LoggingAction.Result.Success("foo")); + actionStatus.onAck(now()); + assertThat(actionStatus.ackStatus().state(), is(State.ACKED)); + + WatchStatus status = new WatchStatus(now(), myMap); + status.onCheck(false, now()); + + assertThat(status.actionStatus("foo").ackStatus().state(), is(State.AWAITS_SUCCESSFUL_EXECUTION)); + } +} \ No newline at end of file diff --git a/plugin/src/test/resources/rest-api-spec/test/watcher/ack_watch/30_reset_ack_after_unmet_condition.yaml b/plugin/src/test/resources/rest-api-spec/test/watcher/ack_watch/30_reset_ack_after_unmet_condition.yaml new file mode 100644 index 00000000000..5479de679a9 --- /dev/null +++ b/plugin/src/test/resources/rest-api-spec/test/watcher/ack_watch/30_reset_ack_after_unmet_condition.yaml @@ -0,0 +1,105 @@ +--- +setup: + - do: + cluster.health: + wait_for_status: yellow + +--- +teardown: + - do: + xpack.watcher.delete_watch: + id: "my_watch" + ignore: 404 + +# See https://github.com/elastic/x-pack-elasticsearch/issues/1123 +--- +"Ensure that ack status is reset after unsuccesful execution": + + - do: + xpack.watcher.put_watch: + id: "my_watch" + body: > + { + "trigger": { + "schedule": { + "interval": "1m" + } + }, + "input": { + "simple" : { "match" : "true" } + }, + "condition": { + "compare": { + "ctx.payload.match": { + "eq": "true" + } + } + }, + "actions": { + "indexme" : { + "index" : { + "index" : "my-index", + "doc_type" : "my-type", + "doc_id": "my-id" + } + } + } + } + + - do: + xpack.watcher.execute_watch: + id: "my_watch" + body: > + { + "record_execution" : true + } + - match: { watch_record._status.actions.indexme.ack.state: "ackable" } + + - do: + xpack.watcher.ack_watch: + watch_id: "my_watch" + - match: { "_status.actions.indexme.ack.state" : "acked" } + + - do: + xpack.watcher.get_watch: + id: "my_watch" + - match: { "_status.actions.indexme.ack.state" : "acked" } + + # having a false result will reset the ack state + - do: + xpack.watcher.execute_watch: + id: "my_watch" + body: > + { + "record_execution" : true, + "alternative_input" : { + "match" : "false" + }, + "action_modes" : { + "indexme" : "force_execute" + } + } + - match: { watch_record._status.actions.indexme.ack.state: "awaits_successful_execution" } + + - do: + xpack.watcher.get_watch: + id: "my_watch" + - match: { "_status.actions.indexme.ack.state" : "awaits_successful_execution" } + + - do: + xpack.watcher.execute_watch: + id: "my_watch" + body: > + { + "record_execution" : true, + "action_modes" : { + "indexme" : "force_execute" + } + } + - match: { watch_record._status.actions.indexme.ack.state: "ackable" } + + - do: + xpack.watcher.get_watch: + id: "my_watch" + - match: { "_status.actions.indexme.ack.state" : "ackable" } +