Fixing TestCluster repository clean up logic

opensearch-project · Sep 12, 2023 · 68359ce · 68359ce
1 parent 14f8f3a
commit 68359ce
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 8 deletions.
diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java
@@ -135,6 +135,13 @@ public RepositoriesMetadata updateRepositoriesMetadata(DiscoveryNode joiningNode
                 for (RepositoryMetadata existingRepositoryMetadata : existingRepositories.repositories()) {
                     if (newRepositoryMetadata.name().equals(existingRepositoryMetadata.name())) {
                         try {
+                            // This will help in handling two scenarios -
+                            // 1. When a fresh cluster is formed and a node tries to join the cluster, the repository
+                            // metadata constructed from the node attributes of the joining node will be validated
+                            // against the repository information provided by existing nodes in cluster state.
+                            // 2. It's possible to update repository settings except the restricted ones post the
+                            // creation of a system repository and if a node drops we will need to allow it to join
+                            // even if the non-restricted system repository settings are now different.
                             repositoriesService.get().ensureValidSystemRepositoryUpdate(newRepositoryMetadata, existingRepositoryMetadata);
                             newRepositoryMetadata = existingRepositoryMetadata;
                             repositoryAlreadyPresent = true;

diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java
@@ -259,10 +259,8 @@ public ClusterState execute(ClusterState currentState) {
                                 found = true;
                                 if (isSystemRepositorySettingPresent(repositoryMetadata.settings())) {
                                     ensureValidSystemRepositoryUpdate(updatedRepositoryMetadata, repositoryMetadata);
-                                    repositoriesMetadata.add(updatedRepositoryMetadata);
-                                } else {
-                                    repositoriesMetadata.add(newRepositoryMetadata);
                                 }
+                                repositoriesMetadata.add(updatedRepositoryMetadata);
                             } else {
                                 repositoriesMetadata.add(updatedRepositoryMetadata);
                             }

diff --git a/test/framework/src/main/java/org/opensearch/test/TestCluster.java b/test/framework/src/main/java/org/opensearch/test/TestCluster.java
@@ -42,6 +42,7 @@
 import org.opensearch.client.Client;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.IndexTemplateMetadata;
+import org.opensearch.cluster.metadata.RepositoryMetadata;
 import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
 import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.indices.IndexTemplateMissingException;
@@ -254,12 +255,17 @@ public void wipeRepositories(String... repositories) {
             }
             for (String repository : repositories) {
                 try {
-                    if (BlobStoreRepository.SYSTEM_REPOSITORY_SETTING.get(
-                        client().admin().cluster().prepareGetRepositories(repository).execute().actionGet().repositories().get(0).settings()
-                    ) == false) {
-                        client().admin().cluster().prepareDeleteRepository(repository).execute().actionGet();
-                    } else {
+                    List<RepositoryMetadata> repositoryMetadata = client().admin()
+                        .cluster()
+                        .prepareGetRepositories(repository)
+                        .execute()
+                        .actionGet()
+                        .repositories();
+                    if (repositoryMetadata.isEmpty() == false
+                        && BlobStoreRepository.SYSTEM_REPOSITORY_SETTING.get(repositoryMetadata.get(0).settings()) == true) {
                         client().admin().cluster().prepareCleanupRepository(repository).execute().actionGet();
+                    } else {
+                        client().admin().cluster().prepareDeleteRepository(repository).execute().actionGet();
                     }
                 } catch (RepositoryMissingException ex) {
                     // ignore