From dbac33bf63429b16ff9c5eef1ce35823abe6a2c2 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 00b698f38db4a..ff4a4f857fe32 100644 --- a/x-pack/plugin/ccr/qa/security/build.gradle +++ b/x-pack/plugin/ccr/qa/security/build.gradle @@ -25,7 +25,7 @@ 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 f50ecfab0d457..24eb234716c4e 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.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(() -> { @@ -141,20 +147,22 @@ 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 request = new Request("PUT", "/_ccr/auto_follow/" + pattern); request.setJsonEntity(""" - {"leader_index_patterns": ["logs-*"], "remote_cluster": "leader_cluster"}"""); + {"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 request = new Request("PUT", "/_ccr/auto_follow/" + pattern); request.setJsonEntity(""" - {"leader_index_patterns": ["logs-eu*"], "remote_cluster": "leader_cluster"}"""); + {"leader_index_patterns": ["testautofollowpatterns-eu*"], "remote_cluster": "leader_cluster"}"""); assertOK(client().performRequest(request)); try (RestClient leaderClient = buildLeaderClient()) { @@ -176,12 +184,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 b88f751941a7d..f7df63db15f97 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 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 {