Skip to content

Commit

Permalink
Fix and re-enable two monitoring migrate alerts test on 7.x branch (#…
Browse files Browse the repository at this point in the history
…69326)

Re-enabled TransportMonitoringMigrateAlertsActionTests#testLocalAlertsRemoval and TransportMonitoringMigrateAlertsActionTests#testRepeatedLocalAlertsRemoval on 7.x branch.

Includes changes from #69139 and #68752
Relates to #66586

Included commits:
* Add more trace logging when installing monitor watches and (#68752)

unmute TransportMonitoringMigrateAlertsActionTests#testLocalAlertsRemoval and
TransportMonitoringMigrateAlertsActionTests#testRepeatedLocalAlertsRemoval tests

Somehow during these tests the monitor watches are not installed. Both
tests use the local exporter and this exporter only installs the watches
under specific conditions via the elected master node. I suspect the
conditions are never met. The http exporter is more relaxed when attempting
to install monitor watches and the tests using the http exporter seem
not to be prone by the fact that tests fail because monitor watches have
not been installed.

Relates to #66586

* Manually trigger local exporter to open a bulk in some monitor tests. (#69139)

Change tests to use monitor bulk api on elected master node before verifying watcher index exists.
Sometimes the monitor service on the elected master doesn't yet export monitor documents resulting in tests using the `ensureInitialLocalResources(...)` method to fail.
Cluster alerts watcher are only installed when local exporter tries to resolve local bulk.

Relates to #66586
  • Loading branch information
martijnvg committed Feb 22, 2021
1 parent 3e4b0aa commit c0a4076
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,20 @@ private void setupClusterAlertsTasks(ClusterState clusterState, boolean clusterS
if (watches != null && watches.allPrimaryShardsActive() == false) {
logger.trace("cannot manage cluster alerts because [.watches] index is not allocated");
} else if ((watches == null || indexExists) && watcherSetup.compareAndSet(false, true)) {
logger.trace("installing monitoring watches");
getClusterAlertsInstallationAsyncActions(indexExists, asyncActions, pendingResponses);
} else {
logger.trace("skipping installing monitoring watches, watches=[{}], indexExists=[{}], watcherSetup=[{}]",
watches, indexExists, watcherSetup.get());
}
} else {
logger.trace("watches shouldn't be setup, because state=[{}] and clusterStateChange=[{}]", state.get(), clusterStateChange);
}
} else {
logger.trace("watches can't be used, because xpack.watcher.enabled=[{}] and " +
"xpack.monitoring.exporters._local.cluster_alerts.management.enabled=[{}]",
XPackSettings.WATCHER_ENABLED.get(config.settings()),
CLUSTER_ALERTS_MANAGEMENT_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()));
}
}

Expand Down Expand Up @@ -581,6 +592,7 @@ private void getClusterAlertsInstallationAsyncActions(final boolean indexExists,
new ResponseActionListener<>("watch", uniqueWatchId, pendingResponses)));
}
} else if (addWatch) {
logger.trace("adding monitoring watch [{}]", uniqueWatchId);
asyncActions.add(() -> putWatch(watcher, watchId, uniqueWatchId, pendingResponses));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.http.MockRequest;
import org.elasticsearch.test.http.MockResponse;
import org.elasticsearch.test.http.MockWebServer;
import org.elasticsearch.test.junit.annotations.TestLogging;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkAction;
import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkRequest;
import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkResponse;
import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsAction;
import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsRequest;
import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsResponse;
Expand All @@ -49,6 +54,7 @@
import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil;
import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter;
import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter;
import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporterIntegTests;
import org.elasticsearch.xpack.monitoring.test.MonitoringIntegTestCase;
import org.junit.After;
import org.junit.Before;
Expand Down Expand Up @@ -124,7 +130,9 @@ private void stopMonitoring() {
));
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/66586")
@TestLogging(
value = "org.elasticsearch.xpack.monitoring.exporter.local:trace",
reason = "to ensure we log local exporter on trace level")
public void testLocalAlertsRemoval() throws Exception {
try {
// start monitoring service
Expand Down Expand Up @@ -159,6 +167,9 @@ public void testLocalAlertsRemoval() throws Exception {
}
}

@TestLogging(
value = "org.elasticsearch.xpack.monitoring.exporter.local:trace",
reason = "to ensure we log local exporter on trace level")
public void testRepeatedLocalAlertsRemoval() throws Exception {
try {
// start monitoring service
Expand Down Expand Up @@ -474,6 +485,18 @@ public void testRemoteAlertsRemoteDisallowsWatcher() throws Exception {
}

private void ensureInitialLocalResources() throws Exception {
// Should trigger setting up alert watches via LocalExporter#openBulk(...) and
// then eventually to LocalExporter#setupIfElectedMaster(...)
// Sometimes this last method doesn't install watches, because elected master node doesn't export monitor documents.
// and then these assertions here fail.
{
MonitoringBulkRequest request = new MonitoringBulkRequest();
request.add(LocalExporterIntegTests.createMonitoringBulkDoc());
String masterNode = internalCluster().getMasterName();
MonitoringBulkResponse response = client(masterNode).execute(MonitoringBulkAction.INSTANCE, request).actionGet();
assertThat(response.status(), equalTo(RestStatus.OK));
}

waitForWatcherIndices();
assertBusy(() -> {
assertThat(indexExists(".monitoring-*"), is(true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ private void checkMonitoringDocs() {
}
}

private static MonitoringBulkDoc createMonitoringBulkDoc() throws IOException {
public static MonitoringBulkDoc createMonitoringBulkDoc() throws IOException {
final MonitoredSystem system = randomFrom(BEATS, KIBANA, LOGSTASH);
final XContentType xContentType = randomFrom(XContentType.values());
final BytesReference source;
Expand Down

0 comments on commit c0a4076

Please sign in to comment.