Skip to content

Commit

Permalink
Fix FollowIndexSecurityIT.testAutoFollowPatterns (#87853) (#87915)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tlrx authored Jun 22, 2022
1 parent daf612f commit 0d8412a
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 17 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugin/ccr/qa/security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/ccr/qa/security/follower-roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/ccr/qa/security/leader-roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(() -> {
Expand Down Expand Up @@ -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()) {
Expand All @@ -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);
Expand Down Expand Up @@ -306,6 +316,30 @@ public void testUnPromoteAndFollowDataStream() throws Exception {
}
}

private static void withMonitoring(Logger logger, CheckedRunnable<Exception> 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<String, Object> clusterState = toMap(adminClient().performRequest(new Request("GET", "/_cluster/state")));
List<?> tasks = (List<?>) XContentMapValues.extractValue("metadata.persistent_tasks.tasks", clusterState);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, ?> 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);
Expand All @@ -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<String, Object> toMap(Response response) throws IOException {
Expand Down

0 comments on commit 0d8412a

Please sign in to comment.