Flatten Get Snapshots Response (elastic#74451)

This PR returns the get snapshots API to the 7.x format (and transport client behavior) and enhances it for requests that ask for multiple repositories. The changes for requests that target multiple repositories are: * Add `repository` field to `SnapshotInfo` and REST response * Add `failures` map alongside `snapshots` list instead of returning just an exception response as done for single repo requests * Pagination now works across repositories instead of being per repository for multi-repository requests closes elastic#69108 closes elastic#43462
original-brownbear · Jun 29, 2021 · 307dbc6 · 307dbc6
1 parent 1462704
commit 307dbc6
Show file tree

Hide file tree

Showing 25 changed files with 392 additions and 153 deletions.
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java
@@ -33,22 +33,23 @@
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.repositories.fs.FsRepository;
 import org.elasticsearch.rest.RestStatus;
+import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase;
 import org.elasticsearch.snapshots.RestoreInfo;
 import org.elasticsearch.snapshots.SnapshotInfo;
+import org.mockito.internal.util.collections.Sets;
 
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.snapshots.SnapshotsService.NO_FEATURE_STATES_VALUE;
 import static org.elasticsearch.tasks.TaskResultsService.TASKS_FEATURE_NAME;
-import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 
 public class SnapshotIT extends ESRestHighLevelClientTestCase {
@@ -177,50 +178,54 @@ public void testCreateSnapshot() throws Exception {
     }
 
     public void testGetSnapshots() throws IOException {
-        String repository = "test_repository";
+        String repository1 = "test_repository1";
+        String repository2 = "test_repository2";
         String snapshot1 = "test_snapshot1";
         String snapshot2 = "test_snapshot2";
 
-        AcknowledgedResponse putRepositoryResponse = createTestRepository(repository, FsRepository.TYPE, "{\"location\": \".\"}");
+        AcknowledgedResponse putRepositoryResponse =
+                createTestRepository(repository1, FsRepository.TYPE, "{\"location\": \"loc1\"}");
         assertTrue(putRepositoryResponse.isAcknowledged());
 
-        CreateSnapshotRequest createSnapshotRequest1 = new CreateSnapshotRequest(repository, snapshot1);
+        AcknowledgedResponse putRepositoryResponse2 =
+                createTestRepository(repository2, FsRepository.TYPE, "{\"location\": \"loc2\"}");
+        assertTrue(putRepositoryResponse2.isAcknowledged());
+
+        CreateSnapshotRequest createSnapshotRequest1 = new CreateSnapshotRequest(repository1, snapshot1);
         createSnapshotRequest1.waitForCompletion(true);
         CreateSnapshotResponse putSnapshotResponse1 = createTestSnapshot(createSnapshotRequest1);
-        CreateSnapshotRequest createSnapshotRequest2 = new CreateSnapshotRequest(repository, snapshot2);
+        CreateSnapshotRequest createSnapshotRequest2 = new CreateSnapshotRequest(repository2, snapshot2);
         createSnapshotRequest2.waitForCompletion(true);
-        Map<String, Object> originalMetadata = randomUserMetadata();
+        Map<String, Object> originalMetadata = AbstractSnapshotIntegTestCase.randomUserMetadata();
         createSnapshotRequest2.userMetadata(originalMetadata);
         CreateSnapshotResponse putSnapshotResponse2 = createTestSnapshot(createSnapshotRequest2);
         // check that the request went ok without parsing JSON here. When using the high level client, check acknowledgement instead.
         assertEquals(RestStatus.OK, putSnapshotResponse1.status());
         assertEquals(RestStatus.OK, putSnapshotResponse2.status());
 
-        GetSnapshotsRequest request;
-        if (randomBoolean()) {
-            request = new GetSnapshotsRequest(repository);
-        } else if (randomBoolean()) {
-            request = new GetSnapshotsRequest(repository, new String[] {"_all"});
+        GetSnapshotsRequest request = new GetSnapshotsRequest(
+                randomFrom(new String[]{"_all"}, new String[]{"*"}, new String[]{repository1, repository2}),
+                randomFrom(new String[]{"_all"}, new String[]{"*"}, new String[]{snapshot1, snapshot2})
+        );
+        request.ignoreUnavailable(true);
 
-        } else {
-            request = new GetSnapshotsRequest(repository, new String[] {snapshot1, snapshot2});
-        }
         GetSnapshotsResponse response = execute(request, highLevelClient().snapshot()::get, highLevelClient().snapshot()::getAsync);
 
-        assertEquals(2, response.getSnapshots().size());
-        assertThat(response.getSnapshots().stream().map((s) -> s.snapshotId().getName()).collect(Collectors.toList()),
-            contains("test_snapshot1", "test_snapshot2"));
-        Optional<Map<String, Object>> returnedMetadata = response.getSnapshots().stream()
-            .filter(s -> s.snapshotId().getName().equals("test_snapshot2"))
-            .findFirst()
-            .map(SnapshotInfo::userMetadata);
-        if (returnedMetadata.isPresent()) {
-            assertEquals(originalMetadata, returnedMetadata.get());
-        } else {
-            assertNull("retrieved metadata is null, expected non-null metadata", originalMetadata);
-        }
+        assertThat(response.isFailed(), is(false));
+        assertEquals(
+            Sets.newSet(repository1, repository2),
+            response.getSnapshots().stream().map(SnapshotInfo::repository).collect(Collectors.toSet())
+        );
+
+        assertThat(response.getSnapshots(), hasSize(2));
+        assertThat(response.getSnapshots().get(0).snapshotId().getName(), equalTo(snapshot1));
+        assertThat(response.getSnapshots().get(0).repository(), equalTo(repository1));
+        assertThat(response.getSnapshots().get(1).snapshotId().getName(), equalTo(snapshot2));
+        assertThat(response.getSnapshots().get(1).userMetadata(), equalTo(originalMetadata));
+        assertThat(response.getSnapshots().get(1).repository(), equalTo(repository2));
     }
 
+
     public void testSnapshotsStatus() throws IOException {
         String testRepository = "test";
         String testSnapshot = "snapshot";

diff --git a/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc
@@ -176,6 +176,7 @@ The API returns the following response:
   "snapshot": {
     "snapshot": "snapshot_2",
     "uuid": "vdRctLCxSketdKb54xw67g",
+    "repository": "my_repository",
     "version_id": <version_id>,
     "version": <version>,
     "indices": [],

diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc
@@ -59,7 +59,8 @@ Use the get snapshot API to return information about one or more snapshots, incl
 
 `<repository>`::
 (Required, string)
-Snapshot repository name used to limit the request.
+Comma-separated list of snapshot repository names used to limit the request.
+Wildcard (`*`) expressions are supported.
 +
 To get information about all snapshot repositories registered in the
 cluster, omit this parameter or use `*` or `_all`.
@@ -301,6 +302,7 @@ The API returns the following response:
     {
       "snapshot": "snapshot_2",
       "uuid": "vdRctLCxSketdKb54xw67g",
+      "repository": "my_repository",
       "version_id": <version_id>,
       "version": <version>,
       "indices": [],
@@ -310,7 +312,7 @@ The API returns the following response:
       "state": "SUCCESS",
       "start_time": "2020-07-06T21:55:18.129Z",
       "start_time_in_millis": 1593093628850,
-      "end_time": "2020-07-06T21:55:18.876Z",
+      "end_time": "2020-07-06T21:55:18.129Z",
       "end_time_in_millis": 1593094752018,
       "duration_in_millis": 0,
       "failures": [],
@@ -328,7 +330,7 @@ The API returns the following response:
 // TESTRESPONSE[s/"version": <version>/"version": $body.snapshots.0.version/]
 // TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.snapshots.0.start_time/]
 // TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.snapshots.0.start_time_in_millis/]
-// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.876Z"/"end_time": $body.snapshots.0.end_time/]
+// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.snapshots.0.end_time/]
 // TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.snapshots.0.end_time_in_millis/]
 // TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.snapshots.0.duration_in_millis/]
 

diff --git a/...src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java b/...src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java
@@ -28,7 +28,6 @@
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.notNullValue;
 
 @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST)
 public class URLSnapshotRestoreIT extends ESIntegTestCase {
@@ -105,7 +104,6 @@ public void testUrlRepository() throws Exception {
 
         logger.info("--> list available shapshots");
         GetSnapshotsResponse getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get();
-        assertThat(getSnapshotsResponse.getSnapshots(), notNullValue());
         assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1));
 
         logger.info("--> delete snapshot");
@@ -114,7 +112,6 @@ public void testUrlRepository() throws Exception {
 
         logger.info("--> list available shapshot again, no snapshots should be returned");
         getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get();
-        assertThat(getSnapshotsResponse.getSnapshots(), notNullValue());
         assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(0));
     }
 }
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json
@@ -42,6 +42,10 @@
         "type":"boolean",
         "description":"Whether to include details of each index in the snapshot, if those details are available. Defaults to false."
       },
+      "include_repository":{
+        "type":"boolean",
+        "description":"Whether to include the repository name in the snapshot info. Defaults to true."
+      },
       "verbose":{
         "type":"boolean",
         "description":"Whether to show verbose snapshot info or only show the basic info found in the repository index blob"

diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/snapshot.get/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/snapshot.get/10_basic.yml
@@ -186,6 +186,7 @@ setup:
   - match: { snapshots.0.state: SUCCESS }
   - match: { snapshots.0.metadata.taken_by: test }
   - match: { snapshots.0.metadata.foo.bar: baz }
+  - is_false: snapshots.0.index_details
 
   - do:
       snapshot.delete:
@@ -231,3 +232,40 @@ setup:
       snapshot.delete:
         repository: test_repo_get_1
         snapshot: test_snapshot_with_index_details
+
+---
+"Get snapshot info without repository names":
+  - skip:
+      version: " - 7.13.99"
+      reason: "7.14 changes get snapshots response format"
+
+  - do:
+      indices.create:
+        index: test_index
+        body:
+          settings:
+            number_of_shards:   1
+            number_of_replicas: 0
+
+  - do:
+      snapshot.create:
+        repository: test_repo_get_1
+        snapshot: test_snapshot_no_repo_name
+        wait_for_completion: true
+
+  - do:
+      snapshot.get:
+        repository: test_repo_get_1
+        snapshot: test_snapshot_no_repo_name
+        include_repository: false
+        human: true
+
+  - is_true: snapshots
+  - match: { snapshots.0.snapshot: test_snapshot_no_repo_name }
+  - match: { snapshots.0.state: SUCCESS }
+  - is_false: snapshots.0.repository
+
+  - do:
+      snapshot.delete:
+        repository: test_repo_get_1
+        snapshot: test_snapshot_no_repo_name
diff --git a/.../internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/.../internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java
@@ -1071,7 +1071,6 @@ public void testReadonlyRepository() throws Exception {
 
         logger.info("--> list available shapshots");
         GetSnapshotsResponse getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("readonly-repo").get();
-        assertThat(getSnapshotsResponse.getSnapshots(), notNullValue());
         assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1));
 
         logger.info("--> try deleting snapshot");