Replace global checkpoint sync test

This commit replaces the REST test that the global checkpoint sync
action runs successfully as a privileged user. The test needs to be
replaced because it has a small race condition. Namely, the check that
the post-operation global checkpoint sync was successful could run
before the sync finishes running. To address this, we replace the REST
test with a test where we have a little more control and can assert busy
to avoid this race from failing the test.

Relates elastic/x-pack-elasticsearch#2749

Original commit: elastic/x-pack-elasticsearch@ea585b843c

qa/multi-node/build.gradle

@@ -1,17 +1,11 @@
-import org.elasticsearch.gradle.test.RestIntegTestTask
+apply plugin: 'elasticsearch.standalone-rest-test'
-
+apply plugin: 'elasticsearch.rest-test'
-apply plugin: 'elasticsearch.standalone-test'
 
 dependencies {
   testCompile project(path: ':x-pack-elasticsearch:plugin', configuration: 'runtime')
-  testCompile project(path: ':x-pack-elasticsearch:plugin', configuration: 'testArtifacts')
 }
 
-task multiNodeTest(type: RestIntegTestTask) {
+integTestCluster {
-  mustRunAfter(precommit)
-}
-
-multiNodeTestCluster {
   distribution = 'zip'
   numNodes = 2
   clusterName = 'multi-node'
@@ -33,6 +27,3 @@ multiNodeTestCluster {
     return tmpFile.exists()
   }
 }
-
-test.enabled = false // no unit tests for multi-node, only the rest integration test
-check.dependsOn(multiNodeTest)

qa/multi-node/src/test/java/org/elasticsearch/multi_node/GlobalCheckpointSyncActionIT.java

@@ -0,0 +1,97 @@
+/*
+ * 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.multi_node;
+
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.test.rest.ESRestTestCase;
+import org.elasticsearch.test.rest.yaml.ObjectPath;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
+import static org.hamcrest.Matchers.equalTo;
+
+public class GlobalCheckpointSyncActionIT extends ESRestTestCase {
+
+    @Override
+    protected Settings restClientSettings() {
+        return getClientSettings("test-user", "x-pack-test-password");
+    }
+
+    @Override
+    protected Settings restAdminSettings() {
+        return getClientSettings("super-user", "x-pack-super-password");
+    }
+
+    private Settings getClientSettings(final String username, final String password) {
+        final String token = basicAuthHeaderValue(username, new SecureString(password.toCharArray()));
+        return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build();
+    }
+
+    /*
+     * The post-operation global checkpoint sync runs privileged as the system user otherwise the sync would be denied to restricted users.
+     * This test ensures that these post-operation syncs are successful otherwise the global checkpoint would not have advanced on the
+     * replica.
+     */
+    public void testGlobalCheckpointSyncActionRunsAsPrivilegedUser() throws Exception {
+        // create the test-index index
+        try (XContentBuilder builder = jsonBuilder()) {
+            builder.startObject();
+            {
+                builder.startObject("settings");
+                {
+                    builder.field("index.number_of_shards", 1);
+                    builder.field("index.number_of_replicas", 1);
+                }
+                builder.endObject();
+            }
+            builder.endObject();
+            final StringEntity entity = new StringEntity(builder.string(), ContentType.APPLICATION_JSON);
+            client().performRequest("PUT", "test-index", Collections.emptyMap(), entity);
+        }
+
+        // wait for the replica to recover
+        client().performRequest("GET", "/_cluster/health", Collections.singletonMap("wait_for_status", "green"));
+
+        // index some documents
+        final int numberOfDocuments = randomIntBetween(0, 128);
+        for (int i = 0; i < numberOfDocuments; i++) {
+            try (XContentBuilder builder = jsonBuilder()) {
+                builder.startObject();
+                {
+                    builder.field("foo", i);
+                }
+                builder.endObject();
+                final StringEntity entity = new StringEntity(builder.string(), ContentType.APPLICATION_JSON);
+                client().performRequest("PUT", "/test-index/test-type/" + i, Collections.emptyMap(), entity);
+            }
+        }
+
+        // we have to wait for the post-operation global checkpoint sync to propagate to the replica
+        assertBusy(() -> {
+            final Map<String, String> params = new HashMap<>(2);
+            params.put("level", "shards");
+            params.put("filter_path", "**.seq_no");
+            final Response response = client().performRequest("GET", "/test-index/_stats", params);
+            final ObjectPath path = ObjectPath.createFromResponse(response);
+            // int looks funny here since global checkpoints are longs but the response parser does not know enough to treat them as long
+            final int shard0GlobalCheckpoint = path.evaluate("indices.test-index.shards.0.0.seq_no.global_checkpoint");
+            assertThat(shard0GlobalCheckpoint, equalTo(numberOfDocuments - 1));
+            final int shard1GlobalCheckpoint = path.evaluate("indices.test-index.shards.0.1.seq_no.global_checkpoint");
+            assertThat(shard1GlobalCheckpoint, equalTo(numberOfDocuments - 1));
+        });
+    }
+
+}

qa/multi-node/src/test/java/org/elasticsearch/multi_node/MultiNodeIT.java

@@ -1,45 +0,0 @@
-/*
- * 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.multi_node;
-
-import com.carrotsearch.randomizedtesting.annotations.Name;
-import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
-import org.elasticsearch.common.settings.SecureString;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.util.concurrent.ThreadContext;
-import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
-import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase;
-import org.elasticsearch.xpack.security.SecurityClusterClientYamlTestCase;
-
-import static org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
-
-public class MultiNodeIT extends SecurityClusterClientYamlTestCase {
-
-    public MultiNodeIT(@Name("yaml") final ClientYamlTestCandidate testCandidate) {
-        super(testCandidate);
-    }
-
-    @ParametersFactory
-    public static Iterable<Object[]> parameters() throws Exception {
-        return ESClientYamlSuiteTestCase.createParameters();
-    }
-
-    @Override
-    protected Settings restClientSettings() {
-        return getClientSettings("test-user", "x-pack-test-password");
-    }
-
-    @Override
-    protected Settings restAdminSettings() {
-        return getClientSettings("super-user", "x-pack-super-password");
-    }
-
-    private Settings getClientSettings(final String username, final String password) {
-        final String token = basicAuthHeaderValue(username, new SecureString(password.toCharArray()));
-        return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build();
-    }
-
-}

qa/multi-node/src/test/resources/rest-api-spec/test/global_checkpoint_sync/10_basic.yml

@@ -1,31 +0,0 @@
----
-"Global checkpoint sync action runs as a privileged user":
-
- - do:
-     indices.create:
-       index: test-index
-       body:
-         settings:
-           index:
-             number_of_shards: 1
-             number_of_replicas: 1
-
- - do:
-     cluster.health:
-       wait_for_status: green
-
- - do:
-     index:
-       index: test-index
-       type: test-type
-       id: 1
-       body: { foo: bar }
-
- - do:
-     indices.stats:
-       index: test-index
-       level: shards
-       filter_path: "**.seq_no"
-
- - match: { indices.test-index.shards.0.0.seq_no.global_checkpoint: 0 }
- - match: { indices.test-index.shards.0.1.seq_no.global_checkpoint: 0 }