From 983a23c73f021355cfa402ac7eef5b3d02401e69 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 21 Jun 2022 17:18:48 +0200 Subject: [PATCH] Fix FollowIndexSecurityIT.testAutoFollowPatterns (#87853) The test FollowIndexSecurityIT.testAutoFollowPatterns sometimes fails when verifying the monitoring documents about auto-follow stats. I wasn't able to reproduce locally but I suspect that monitoring collects auto-follow stats before they are updated. Instead it should collect auto follow stats monitoring documents once indices are effectively followed. There are also some index / auto-follow pattern conflicts with other tests in the same class, so this PR also changes that. In case this fix is not enough, the full monitoring documents should appear in test log to help further debugging. Closes #84888 --- x-pack/plugin/ccr/qa/security/build.gradle | 2 +- .../plugin/ccr/qa/security/follower-roles.yml | 2 +- .../plugin/ccr/qa/security/leader-roles.yml | 2 +- .../xpack/ccr/FollowIndexSecurityIT.java | 56 +++++++++++++++---- .../xpack/ccr/ESCCRRestTestCase.java | 12 +++- 5 files changed, 57 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/ccr/qa/security/build.gradle b/x-pack/plugin/ccr/qa/security/build.gradle index e7e6d4abfdf34..9cdde5e3a9abd 100644 --- a/x-pack/plugin/ccr/qa/security/build.gradle +++ b/x-pack/plugin/ccr/qa/security/build.gradle @@ -34,7 +34,7 @@ def followCluster = testClusters.register('follow-cluster') { } setting 'xpack.license.self_generated.type', 'trial' setting 'xpack.security.enabled', 'true' - setting 'xpack.monitoring.collection.enabled', 'true' + setting 'xpack.monitoring.collection.enabled', 'false' // will be enabled by tests extraConfigFile 'roles.yml', file('follower-roles.yml') user username: "test_admin", role: "superuser" user username: "test_ccr", role: "ccruser" diff --git a/x-pack/plugin/ccr/qa/security/follower-roles.yml b/x-pack/plugin/ccr/qa/security/follower-roles.yml index f547751fd07e7..0f5a84afdaace 100644 --- a/x-pack/plugin/ccr/qa/security/follower-roles.yml +++ b/x-pack/plugin/ccr/qa/security/follower-roles.yml @@ -2,7 +2,7 @@ ccruser: cluster: - manage_ccr indices: - - names: [ 'allowed-index', 'forget-follower', 'logs-eu*' ] + - names: [ 'allowed-index', 'forget-follower', 'logs-eu*', 'testautofollowpatterns-eu*' ] privileges: - monitor - read diff --git a/x-pack/plugin/ccr/qa/security/leader-roles.yml b/x-pack/plugin/ccr/qa/security/leader-roles.yml index 858f76fa0c0e0..ce31f6077f08c 100644 --- a/x-pack/plugin/ccr/qa/security/leader-roles.yml +++ b/x-pack/plugin/ccr/qa/security/leader-roles.yml @@ -2,7 +2,7 @@ ccruser: cluster: - read_ccr indices: - - names: [ 'allowed-index', 'clean-leader', 'forget-leader', 'logs-eu*' ] + - names: [ 'allowed-index', 'clean-leader', 'forget-leader', 'logs-eu*', 'testautofollowpatterns-eu*' ] privileges: - manage - read diff --git a/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java b/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java index d9df9c7734596..a0f2a97a1a973 100644 --- a/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java +++ b/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java @@ -6,14 +6,18 @@ */ package org.elasticsearch.xpack.ccr; +import org.apache.logging.log4j.Logger; import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; +import org.elasticsearch.client.WarningsHandler; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.core.CheckedRunnable; import org.elasticsearch.index.seqno.ReplicationTracker; import org.elasticsearch.test.rest.yaml.ObjectPath; @@ -69,7 +73,9 @@ public void testFollowIndex() throws Exception { followIndex("leader_cluster", allowedIndex, allowedIndex); assertBusy(() -> verifyDocuments(allowedIndex, numDocs, "*:*")); assertThat(getCcrNodeTasks(), contains(new CcrNodeTask("leader_cluster", allowedIndex, allowedIndex, 0))); - assertBusy(() -> verifyCcrMonitoring(allowedIndex, allowedIndex), 120L, TimeUnit.SECONDS); + + withMonitoring(logger, () -> { assertBusy(() -> verifyCcrMonitoring(allowedIndex, allowedIndex), 120L, TimeUnit.SECONDS); }); + pauseFollow(allowedIndex); // Make sure that there are no other ccr relates operations running: assertBusy(() -> { @@ -143,18 +149,20 @@ public void testFollowIndex() throws Exception { public void testAutoFollowPatterns() throws Exception { assumeTrue("Test should only run with target_cluster=follow", "follow".equals(targetCluster)); - String allowedIndex = "logs-eu_20190101"; - String disallowedIndex = "logs-us_20190101"; + final String prefix = getTestName().toLowerCase(Locale.ROOT); + String allowedIndex = prefix + "-eu_20190101"; + String disallowedIndex = prefix + "-us_20190101"; + final String pattern = "pattern_" + prefix; { - Request request = new Request("PUT", "/_ccr/auto_follow/test_pattern"); - request.setJsonEntity("{\"leader_index_patterns\": [\"logs-*\"], \"remote_cluster\": \"leader_cluster\"}"); + Request request = new Request("PUT", "/_ccr/auto_follow/" + pattern); + request.setJsonEntity("{\"leader_index_patterns\": [\"testautofollowpatterns-*\"], \"remote_cluster\": \"leader_cluster\"}"); Exception e = expectThrows(ResponseException.class, () -> assertOK(client().performRequest(request))); - assertThat(e.getMessage(), containsString("insufficient privileges to follow index [logs-*]")); + assertThat(e.getMessage(), containsString("insufficient privileges to follow index [testautofollowpatterns-*]")); } - Request request = new Request("PUT", "/_ccr/auto_follow/test_pattern"); - request.setJsonEntity("{\"leader_index_patterns\": [\"logs-eu*\"], \"remote_cluster\": \"leader_cluster\"}"); + Request request = new Request("PUT", "/_ccr/auto_follow/" + pattern); + request.setJsonEntity("{\"leader_index_patterns\": [\"testautofollowpatterns-eu*\"], \"remote_cluster\": \"leader_cluster\"}"); assertOK(client().performRequest(request)); try (RestClient leaderClient = buildLeaderClient()) { @@ -175,12 +183,14 @@ public void testAutoFollowPatterns() throws Exception { assertBusy(() -> ensureYellow(allowedIndex), 30, TimeUnit.SECONDS); assertBusy(() -> verifyDocuments(allowedIndex, 5, "*:*"), 30, TimeUnit.SECONDS); assertThat(indexExists(disallowedIndex), is(false)); - assertBusy(() -> verifyCcrMonitoring(allowedIndex, allowedIndex), 120L, TimeUnit.SECONDS); - assertBusy(ESCCRRestTestCase::verifyAutoFollowMonitoring, 120L, TimeUnit.SECONDS); + withMonitoring(logger, () -> { + assertBusy(() -> verifyCcrMonitoring(allowedIndex, allowedIndex), 120L, TimeUnit.SECONDS); + assertBusy(ESCCRRestTestCase::verifyAutoFollowMonitoring, 120L, TimeUnit.SECONDS); + }); } finally { // Cleanup by deleting auto follow pattern and pause following: try { - deleteAutoFollowPattern("test_pattern"); + deleteAutoFollowPattern(pattern); pauseFollow(allowedIndex); } catch (Throwable e) { logger.warn("Failed to cleanup after the test", e); @@ -306,6 +316,30 @@ public void testUnPromoteAndFollowDataStream() throws Exception { } } + private static void withMonitoring(Logger logger, CheckedRunnable runnable) throws Exception { + Request enableMonitoring = new Request("PUT", "/_cluster/settings"); + enableMonitoring.setOptions(RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE).build()); + enableMonitoring.setJsonEntity( + "{\"persistent\":{" + "\"xpack.monitoring.collection.enabled\":true," + "\"xpack.monitoring.collection.interval\":\"1s\"" + "}}" + ); + assertOK(adminClient().performRequest(enableMonitoring)); + logger.info("monitoring collection enabled"); + try { + runnable.run(); + } finally { + Request disableMonitoring = new Request("PUT", "/_cluster/settings"); + disableMonitoring.setOptions(RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE).build()); + disableMonitoring.setJsonEntity( + "{\"persistent\":{" + + "\"xpack.monitoring.collection.enabled\":null," + + "\"xpack.monitoring.collection.interval\":null" + + "}}" + ); + assertOK(adminClient().performRequest(disableMonitoring)); + logger.info("monitoring collection disabled"); + } + } + private static void assertNoPersistentTasks() throws IOException { Map clusterState = toMap(adminClient().performRequest(new Request("GET", "/_cluster/state"))); List tasks = (List) XContentMapValues.extractValue("metadata.persistent_tasks.tasks", clusterState); diff --git a/x-pack/plugin/ccr/qa/src/main/java/org/elasticsearch/xpack/ccr/ESCCRRestTestCase.java b/x-pack/plugin/ccr/qa/src/main/java/org/elasticsearch/xpack/ccr/ESCCRRestTestCase.java index 3ee3f193c8a70..cbfe16adc26ac 100644 --- a/x-pack/plugin/ccr/qa/src/main/java/org/elasticsearch/xpack/ccr/ESCCRRestTestCase.java +++ b/x-pack/plugin/ccr/qa/src/main/java/org/elasticsearch/xpack/ccr/ESCCRRestTestCase.java @@ -220,13 +220,15 @@ protected static void verifyCcrMonitoring(final String expectedLeaderIndex, fina protected static void verifyAutoFollowMonitoring() throws IOException { Request request = new Request("GET", "/.monitoring-*/_search"); request.setJsonEntity("{\"query\": {\"term\": {\"type\": \"ccr_auto_follow_stats\"}}}"); + String responseEntity; Map response; try { - response = toMap(adminClient().performRequest(request)); + responseEntity = EntityUtils.toString(adminClient().performRequest(request).getEntity()); + response = toMap(responseEntity); } catch (ResponseException e) { throw new AssertionError("error while searching", e); } - + assertNotNull(responseEntity); int numberOfSuccessfulFollowIndices = 0; List hits = (List) XContentMapValues.extractValue("hits.hits", response); @@ -242,7 +244,11 @@ protected static void verifyAutoFollowMonitoring() throws IOException { numberOfSuccessfulFollowIndices = Math.max(numberOfSuccessfulFollowIndices, foundNumberOfOperationsReceived); } - assertThat(numberOfSuccessfulFollowIndices, greaterThanOrEqualTo(1)); + assertThat( + "Unexpected number of followed indices [" + responseEntity + ']', + numberOfSuccessfulFollowIndices, + greaterThanOrEqualTo(1) + ); } protected static Map toMap(Response response) throws IOException {