Fix tests

Signed-off-by: Ashish Singh <[email protected]>
ashking94 · Aug 31, 2024 · 798cf4b · 798cf4b
1 parent 596b6de
commit 798cf4b
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 29 deletions.
diff --git a/...ClusterTest/java/org/opensearch/action/admin/cluster/repositories/RepositoryBlocksIT.java b/...ClusterTest/java/org/opensearch/action/admin/cluster/repositories/RepositoryBlocksIT.java
@@ -63,7 +63,8 @@ public void testPutRepositoryWithBlocks() {
                     "fs",
                     false,
                     settings,
-                    null
+                    null,
+                    false
                 ),
                 Metadata.CLUSTER_READ_ONLY_BLOCK
             );

diff --git a/...ernalClusterTest/java/org/opensearch/remotestore/RemoteStoreRepositoryRegistrationIT.java b/...ernalClusterTest/java/org/opensearch/remotestore/RemoteStoreRepositoryRegistrationIT.java
@@ -137,7 +137,8 @@ public void testMultiNodeClusterRandomNodeRecoverNetworkIsolationPostNonRestrict
             repositoryMetadata.type(),
             true,
             updatedSettings,
-            null
+            null,
+            false
         ).get();
 
         ensureStableCluster(3, nodesInOneSide.stream().findAny().get());

diff --git a/...src/internalClusterTest/java/org/opensearch/snapshots/CorruptedBlobStoreRepositoryIT.java b/...src/internalClusterTest/java/org/opensearch/snapshots/CorruptedBlobStoreRepositoryIT.java
@@ -40,9 +40,13 @@
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.cluster.metadata.RepositoriesMetadata;
+import org.opensearch.common.blobstore.BlobPath;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.io.IOUtils;
 import org.opensearch.core.common.unit.ByteSizeUnit;
+import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm;
+import org.opensearch.index.remote.RemoteStoreEnums.PathType;
+import org.opensearch.index.remote.RemoteStorePathStrategy;
 import org.opensearch.repositories.IndexId;
 import org.opensearch.repositories.RepositoriesService;
 import org.opensearch.repositories.Repository;
@@ -58,6 +62,7 @@
 import java.util.Map;
 import java.util.stream.Stream;
 
+import static org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm.FNV_1A_COMPOSITE_1;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertRequestBuilderThrows;
 import static org.hamcrest.Matchers.containsString;
@@ -358,9 +363,7 @@ public void testSnapshotWithCorruptedShardIndexFile() throws Exception {
         assertThat(indexIds.size(), equalTo(1));
 
         final IndexId corruptedIndex = indexIds.get(indexName);
-        final Path shardIndexFile = repo.resolve("indices")
-            .resolve(corruptedIndex.getId())
-            .resolve("0")
+        final Path shardIndexFile = repo.resolve(resolvePath(corruptedIndex, "0"))
             .resolve("index-" + repositoryData.shardGenerations().getShardGen(corruptedIndex, 0));
 
         logger.info("-->  truncating shard index file [{}]", shardIndexFile);
@@ -435,7 +438,7 @@ public void testDeleteSnapshotWithMissingIndexAndShardMetadata() throws Exceptio
 
         logger.info("--> delete index metadata and shard metadata");
         for (String index : indices) {
-            Path shardZero = indicesPath.resolve(indexIds.get(index).getId()).resolve("0");
+            Path shardZero = repo.resolve(resolvePath(indexIds.get(index), "0"));
             if (randomBoolean()) {
                 Files.delete(
                     shardZero.resolve("index-" + getRepositoryData("test-repo").shardGenerations().getShardGen(indexIds.get(index), 0))
@@ -628,10 +631,9 @@ public void testSnapshotWithMissingShardLevelIndexFile() throws Exception {
         clusterAdmin().prepareCreateSnapshot("test-repo", "test-snap-1").setWaitForCompletion(true).setIndices("test-idx-*").get();
 
         logger.info("--> deleting shard level index file");
-        final Path indicesPath = repo.resolve("indices");
         for (IndexId indexId : getRepositoryData("test-repo").getIndices().values()) {
             final Path shardGen;
-            try (Stream<Path> shardFiles = Files.list(indicesPath.resolve(indexId.getId()).resolve("0"))) {
+            try (Stream<Path> shardFiles = Files.list(repo.resolve(resolvePath(indexId, "0")))) {
                 shardGen = shardFiles.filter(file -> file.getFileName().toString().startsWith(BlobStoreRepository.INDEX_FILE_PREFIX))
                     .findFirst()
                     .orElseThrow(() -> new AssertionError("Failed to find shard index blob"));
@@ -681,4 +683,16 @@ private void assertRepositoryBlocked(Client client, String repo, String existing
             containsString("Could not read repository data because the contents of the repository do not match its expected state.")
         );
     }
+
+    private static String resolvePath(IndexId indexId, String shardId) {
+        PathType pathType = PathType.fromCode(indexId.getShardPathType());
+        RemoteStorePathStrategy.SnapshotShardPathInput shardPathInput = new RemoteStorePathStrategy.SnapshotShardPathInput.Builder()
+            .basePath(BlobPath.cleanPath())
+            .indexUUID(indexId.getId())
+            .shardId(shardId)
+            .build();
+        PathHashAlgorithm pathHashAlgorithm = pathType != PathType.FIXED ? FNV_1A_COMPOSITE_1 : null;
+        BlobPath blobPath = pathType.path(shardPathInput, pathHashAlgorithm);
+        return blobPath.buildAsString();
+    }
 }
diff --git a/.../internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/.../internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java
@@ -766,7 +766,8 @@ public void testRegistrationFailure() {
                 "mock",
                 false,
                 Settings.builder().put("location", randomRepoPath()),
-                null
+                null,
+                false
             ).get();
         }
         logger.info("--> make sure that properly setup repository can be registered on all nodes");
@@ -776,7 +777,8 @@ public void testRegistrationFailure() {
             "fs",
             true,
             Settings.builder().put("location", randomRepoPath()),
-            null
+            null,
+            false
         ).get();
     }
 

diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/MultiClusterRepoAccessIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/MultiClusterRepoAccessIT.java
@@ -121,7 +121,8 @@ public void testConcurrentDeleteFromOtherCluster() throws InterruptedException {
             "fs",
             true,
             Settings.builder().put("location", repoPath),
-            null
+            null,
+            false
         ).get();
 
         createIndexWithRandomDocs("test-idx-1", randomIntBetween(1, 100));

diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java
@@ -269,7 +269,8 @@ public void testRepositoryVerification() throws Exception {
                 "mock",
                 true,
                 Settings.builder().put(settings),
-                null
+                null,
+                false
             ),
             RepositoryVerificationException.class
         );
@@ -282,7 +283,8 @@ public void testRepositoryVerification() throws Exception {
                 "mock",
                 true,
                 Settings.builder().put(readonlySettings),
-                null
+                null,
+                false
             ),
             RepositoryVerificationException.class
         );

diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java
@@ -279,7 +279,7 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
 
     public static final Setting<PathType> SHARD_PATH_TYPE = new Setting<>(
         "shard_path_type",
-        PathType.HASHED_PREFIX.toString(),
+        PathType.FIXED.toString(),
         PathType::parseString
     );
 
@@ -1896,7 +1896,7 @@ private List<String> findMatchingShardPaths(String indexId, Map<String, BlobMeta
      */
     private Optional<String> findHighestGenerationShardPaths(List<String> matchingShardPaths) {
         return matchingShardPaths.stream()
-            .map(s -> s.split(SnapshotShardPaths.DELIMITER))
+            .map(s -> s.split("\\" + SnapshotShardPaths.DELIMITER))
             .sorted((a, b) -> Integer.parseInt(b[2]) - Integer.parseInt(a[2]))
             .map(parts -> String.join(SnapshotShardPaths.DELIMITER, parts))
             .findFirst();
@@ -2117,11 +2117,11 @@ public void finalizeSnapshot(
      */
     private void cleanupRedundantSnapshotShardPaths(Set<String> updatedShardPathsIndexIds) {
         Set<String> updatedIndexIds = updatedShardPathsIndexIds.stream()
-            .map(s -> s.split(SnapshotShardPaths.DELIMITER)[0])
+            .map(s -> s.split("\\" + SnapshotShardPaths.DELIMITER)[0])
             .collect(Collectors.toSet());
         Set<String> indexIdShardPaths = getSnapshotShardPaths().keySet();
         List<String> staleShardPaths = indexIdShardPaths.stream().filter(s -> updatedShardPathsIndexIds.contains(s) == false).filter(s -> {
-            String indexId = s.split(SnapshotShardPaths.DELIMITER)[0];
+            String indexId = s.split("\\" + SnapshotShardPaths.DELIMITER)[0];
             return updatedIndexIds.contains(indexId);
         }).collect(Collectors.toList());
         try {

diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotShardPaths.java b/server/src/main/java/org/opensearch/snapshots/SnapshotShardPaths.java
@@ -27,7 +27,7 @@ public class SnapshotShardPaths implements ToXContent {
 
     public static final String DIR = "snapshot_shard_paths";
 
-    public static final String DELIMITER = "#";
+    public static final String DELIMITER = ".";
 
     public static final String FILE_NAME_FORMAT = "%s";
 
@@ -96,7 +96,7 @@ public static SnapshotShardPaths fromXContent(XContentParser ignored) {
      * @throws IllegalArgumentException if the shard path format is invalid or cannot be parsed.
      */
     public static ShardInfo parseShardPath(String shardPath) {
-        String[] parts = shardPath.split(SnapshotShardPaths.DELIMITER);
+        String[] parts = shardPath.split("\\" + SnapshotShardPaths.DELIMITER);
         if (parts.length != 5) {
             throw new IllegalArgumentException("Invalid shard path format: " + shardPath);
         }

diff --git a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java
@@ -435,7 +435,7 @@ public void testWriteAndReadShardPaths() throws Exception {
             assertNotNull("IndexId should not be null", indexId);
             assertEquals("Index ID should match", shardInfo.getIndexId().getId(), indexId.getId());
             assertEquals("Shard path type should match", shardInfo.getIndexId().getShardPathType(), indexId.getShardPathType());
-            String[] parts = shardPathBlobName.split(SnapshotShardPaths.DELIMITER);
+            String[] parts = shardPathBlobName.split("\\" + SnapshotShardPaths.DELIMITER);
             assertEquals(
                 "Path hash algorithm should be FNV_1A_COMPOSITE_1",
                 RemoteStoreEnums.PathHashAlgorithm.FNV_1A_COMPOSITE_1,

diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java
@@ -2581,7 +2581,7 @@ protected void updateRepository(String repoName, String type, Settings.Builder s
     }
 
     public static void putRepository(ClusterAdminClient adminClient, String repoName, String type, Settings.Builder settings) {
-        assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null));
+        assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null, false));
     }
 
     public static void putRepository(
@@ -2591,7 +2591,7 @@ public static void putRepository(
         String timeout,
         Settings.Builder settings
     ) {
-        assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, timeout));
+        assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, timeout, false));
     }
 
     public static void putRepository(
@@ -2601,7 +2601,17 @@ public static void putRepository(
         boolean verify,
         Settings.Builder settings
     ) {
-        assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, verify, settings, null));
+        assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, verify, settings, null, false));
+    }
+
+    public static void putRepositoryWithNoSettingOverrides(
+        ClusterAdminClient adminClient,
+        String repoName,
+        String type,
+        boolean verify,
+        Settings.Builder settings
+    ) {
+        assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, verify, settings, null, true));
     }
 
     public static void putRepository(
@@ -2611,7 +2621,7 @@ public static void putRepository(
         Settings.Builder settings,
         ActionListener<AcknowledgedResponse> listener
     ) {
-        putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null).execute(listener);
+        putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null, false).execute(listener);
     }
 
     public static PutRepositoryRequestBuilder putRepositoryRequestBuilder(
@@ -2620,15 +2630,17 @@ public static PutRepositoryRequestBuilder putRepositoryRequestBuilder(
         String type,
         boolean verify,
         Settings.Builder settings,
-        String timeout
+        String timeout,
+        boolean finalSettings
     ) {
-        PutRepositoryRequestBuilder builder = adminClient.preparePutRepository(repoName)
-            .setType(type)
-            .setVerify(verify)
-            .setSettings(settings);
+        PutRepositoryRequestBuilder builder = adminClient.preparePutRepository(repoName).setType(type).setVerify(verify);
         if (timeout != null) {
             builder.setTimeout(timeout);
         }
+        if (finalSettings == false) {
+            settings.put(BlobStoreRepository.SHARD_PATH_TYPE.getKey(), randomFrom(PathType.values()));
+        }
+        builder.setSettings(settings);
         return builder;
     }