Fix Encrypted Repo Test and RepositoriesService Logging (#68469)

This test creates a strange situation when it restarts the
two master nodes back to back without strong guarantees about
whether or not the cluster has cleanly formed again. Since we really just want to
make sure that the master is after the restart is actually running
with the changed password, it seemed easiest to simply do a full restart rather than the
current loop.

Also, this commit fixes the logging around repository creation and deletion which made this
hard to debug and by logging before a CS update has actually been processed would also
log incorrectly.
Lastly, I removed the now unnecessary hacks of using the generic pool on a node to fetch
repository data. This logic is not needed any longer and the repository will now automatically
use the correct thread under the hood. The hack effectively just adds another possible failure
mode around node restarts since the generic pool quietly rejects tasks after shutdown.

Closes #67834

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

@@ -168,13 +168,15 @@ public void registerRepository(final PutRepositoryRequest request, final ActionL
         clusterService.submitStateUpdateTask("put_repository [" + request.name() + "]",
             new AckedClusterStateUpdateTask(request, acknowledgementStep) {
 
+                private boolean found = false;
+                private boolean changed = false;
+
                 @Override
                 public ClusterState execute(ClusterState currentState) {
                     ensureRepositoryNotInUse(currentState, request.name());
                     Metadata metadata = currentState.metadata();
                     Metadata.Builder mdBuilder = Metadata.builder(currentState.metadata());
                     RepositoriesMetadata repositories = metadata.custom(RepositoriesMetadata.TYPE, RepositoriesMetadata.EMPTY);
-                    boolean found = false;
                     List<RepositoryMetadata> repositoriesMetadata = new ArrayList<>(repositories.repositories().size() + 1);
                     for (RepositoryMetadata repositoryMetadata : repositories.repositories()) {
                         if (repositoryMetadata.name().equals(newRepositoryMetadata.name())) {
@@ -189,13 +191,11 @@ public ClusterState execute(ClusterState currentState) {
                         }
                     }
                     if (found == false) {
-                        logger.info("put repository [{}]", request.name());
                         repositoriesMetadata.add(new RepositoryMetadata(request.name(), request.type(), request.settings()));
-                    } else {
-                        logger.info("update repository [{}]", request.name());
                     }
                     repositories = new RepositoriesMetadata(repositoriesMetadata);
                     mdBuilder.putCustom(RepositoriesMetadata.TYPE, repositories);
+                    changed = true;
                     return ClusterState.builder(currentState).metadata(mdBuilder).build();
                 }
 
@@ -214,6 +214,13 @@ public boolean mustAck(DiscoveryNode discoveryNode) {
 
                 @Override
                 public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
+                    if (changed) {
+                        if (found) {
+                            logger.info("updated repository [{}]", request.name());
+                        } else {
+                            logger.info("put repository [{}]", request.name());
+                        }
+                    }
                     publicationStep.onResponse(null);
                 }
             });
@@ -288,6 +295,8 @@ public void unregisterRepository(final DeleteRepositoryRequest request, final Ac
         clusterService.submitStateUpdateTask("delete_repository [" + request.name() + "]",
             new AckedClusterStateUpdateTask(request, listener) {
 
+                private final List<String> deletedRepositories = new ArrayList<>();
+
                 @Override
                 public ClusterState execute(ClusterState currentState) {
                     Metadata metadata = currentState.metadata();
@@ -299,7 +308,7 @@ public ClusterState execute(ClusterState currentState) {
                         for (RepositoryMetadata repositoryMetadata : repositories.repositories()) {
                             if (Regex.simpleMatch(request.name(), repositoryMetadata.name())) {
                                 ensureRepositoryNotInUse(currentState, repositoryMetadata.name());
-                                logger.info("delete repository [{}]", repositoryMetadata.name());
+                                deletedRepositories.add(repositoryMetadata.name());
                                 changed = true;
                             } else {
                                 repositoriesMetadata.add(repositoryMetadata);
@@ -317,6 +326,13 @@ public ClusterState execute(ClusterState currentState) {
                     throw new RepositoryMissingException(request.name());
                 }
 
+                @Override
+                public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
+                    if (deletedRepositories.isEmpty() == false) {
+                        logger.info("deleted repositories [{}]", deletedRepositories);
+                    }
+                }
+
                 @Override
                 public boolean mustAck(DiscoveryNode discoveryNode) {
                     // repository was created on both master and data nodes

x-pack/plugin/repository-encrypted/src/internalClusterTest/java/org/elasticsearch/repositories/encrypted/EncryptedFSBlobStoreRepositoryIntegTests.java

@@ -6,7 +6,6 @@
  */
 package org.elasticsearch.repositories.encrypted;
 
-import org.elasticsearch.action.ActionRunnable;
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.common.settings.MockSecureSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -157,9 +156,7 @@ public void testTamperedEncryptionMetadata() throws Exception {
         assertThat(
             expectThrows(
                 RepositoryException.class,
-                () -> PlainActionFuture.<RepositoryData, Exception>get(
-                    f -> blobStoreRepository.threadPool().generic().execute(ActionRunnable.wrap(f, blobStoreRepository::getRepositoryData))
-                )
+                () -> PlainActionFuture.<RepositoryData, Exception>get(blobStoreRepository::getRepositoryData)
             ).getMessage(),
             containsString("the encryption metadata in the repository has been corrupted")
         );

x-pack/plugin/repository-encrypted/src/internalClusterTest/java/org/elasticsearch/repositories/encrypted/EncryptedRepositorySecretIntegTests.java

@@ -8,7 +8,6 @@
 package org.elasticsearch.repositories.encrypted;
 
 import org.elasticsearch.ElasticsearchSecurityException;
-import org.elasticsearch.action.ActionRunnable;
 import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryResponse;
 import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
 import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
@@ -40,7 +39,6 @@
 
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
@@ -549,7 +547,6 @@ public void testWrongRepositoryPassword() throws Exception {
         internalCluster().startNodes(2, Settings.builder().setSecureSettings(secureSettingsWithPassword).build());
         ensureStableCluster(2);
         createRepository(repositoryName, repositorySettings, true);
-        // create empty smapshot
         final String snapshotName = randomName();
         logger.info("-->  create empty snapshot {}:{}", repositoryName, snapshotName);
         CreateSnapshotResponse createSnapshotResponse = client().admin()
@@ -575,14 +572,9 @@ public void testWrongRepositoryPassword() throws Exception {
             EncryptedRepositoryPlugin.ENCRYPTION_PASSWORD_SETTING.getConcreteSettingForNamespace(repositoryName).getKey(),
             wrongPassword
         );
-        Set<String> nodesWithWrongPassword = new HashSet<>();
-        do {
-            String masterNodeName = internalCluster().getMasterName();
-            logger.info("-->  restart master node {}", masterNodeName);
-            internalCluster().restartNode(masterNodeName, new InternalTestCluster.RestartCallback());
-            nodesWithWrongPassword.add(masterNodeName);
-            ensureStableCluster(2);
-        } while (false == nodesWithWrongPassword.contains(internalCluster().getMasterName()));
+
+        internalCluster().fullRestart();
+        ensureStableCluster(2);
         // maybe recreate the repository
         if (randomBoolean()) {
             deleteRepository(repositoryName);
@@ -594,9 +586,7 @@ public void testWrongRepositoryPassword() throws Exception {
         ).repository(repositoryName);
         RepositoryException e = expectThrows(
             RepositoryException.class,
-            () -> PlainActionFuture.<RepositoryData, Exception>get(
-                f -> blobStoreRepository.threadPool().generic().execute(ActionRunnable.wrap(f, blobStoreRepository::getRepositoryData))
-            )
+            () -> PlainActionFuture.<RepositoryData, Exception>get(blobStoreRepository::getRepositoryData)
         );
         assertThat(e.getCause().getMessage(), containsString("repository password is incorrect"));
         e = expectThrows(
@@ -626,13 +616,8 @@ public void testWrongRepositoryPassword() throws Exception {
             EncryptedRepositoryPlugin.ENCRYPTION_PASSWORD_SETTING.getConcreteSettingForNamespace(repositoryName).getKey(),
             goodPassword
         );
-        do {
-            String masterNodeName = internalCluster().getMasterName();
-            logger.info("-->  restart master node {}", masterNodeName);
-            internalCluster().restartNode(masterNodeName, new InternalTestCluster.RestartCallback());
-            nodesWithWrongPassword.remove(masterNodeName);
-            ensureStableCluster(2);
-        } while (nodesWithWrongPassword.contains(internalCluster().getMasterName()));
+        internalCluster().fullRestart();
+        ensureStableCluster(2);
         // ensure get snapshot works
         getSnapshotResponse = client().admin().cluster().prepareGetSnapshots(repositoryName).get();
         assertThat(getSnapshotResponse.getFailedResponses().keySet(), empty());