[CI] Fix testClientsLifeCycleForSingleProject (elastic#128718)

More robust test for closed clients holder. Also changes
IllegalStateException to AlreadyClosedException for both closed manager
and holder.

Resolves: elastic#128707

Author: ywangd

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientsManager.java

@@ -9,6 +9,7 @@
 
 package org.elasticsearch.repositories.s3;
 
+import org.apache.lucene.store.AlreadyClosedException;
 import org.elasticsearch.cluster.ClusterChangedEvent;
 import org.elasticsearch.cluster.ClusterStateApplier;
 import org.elasticsearch.cluster.metadata.ProjectId;
@@ -293,7 +294,7 @@ S3ClientSettings singleClientSettings(RepositoryMetadata repositoryMetadata) {
          * @param repositoryMetadata The metadata of the repository for which the Amazon S3 client is required.
          * @return An {@link AmazonS3Reference} instance corresponding to the repository metadata.
          * @throws IllegalArgumentException If no client settings exist for the given repository metadata.
-         * @throws IllegalStateException If the client manager is closed and a new client cannot be created.
+         * @throws AlreadyClosedException If either the clients manager or the holder is closed
          */
         final AmazonS3Reference client(RepositoryMetadata repositoryMetadata) {
             final var clientKey = clientKey(repositoryMetadata);
@@ -313,12 +314,12 @@ final AmazonS3Reference client(RepositoryMetadata repositoryMetadata) {
                 }
                 if (closed.get()) {
                     // Not adding a new client once the clients holder is closed since there won't be anything to close it
-                    throw new IllegalStateException("Project [" + projectId() + "] clients holder is closed");
+                    throw new AlreadyClosedException("Project [" + projectId() + "] clients holder is closed");
                 }
                 if (managerClosed.get()) {
                     // This clients holder must be added after the manager is closed. It must have no cached clients.
                     assert clientsCache.isEmpty() : "expect empty cache, but got " + clientsCache;
-                    throw new IllegalStateException("s3 clients manager is closed");
+                    throw new AlreadyClosedException("s3 clients manager is closed");
                 }
                 // The close() method maybe called after we checked it, it is ok since we are already inside the synchronized block.
                 // The close method calls clearCache() which will clear the newly added client.

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientsManagerTests.java

@@ -14,6 +14,7 @@
 import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity;
 import software.amazon.awssdk.regions.Region;
 
+import org.apache.lucene.store.AlreadyClosedException;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.metadata.ProjectId;
@@ -41,6 +42,7 @@
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -50,6 +52,7 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.sameInstance;
 import static org.mockito.ArgumentMatchers.any;
@@ -203,11 +206,18 @@ public void testClientsLifeCycleForSingleProject() throws Exception {
         }
         assertClientNotFound(projectId, clientName);
 
-        assertBusy(() -> assertTrue(clientsHolder.isClosed()));
-        final var e = expectThrows(
-            IllegalStateException.class,
-            () -> clientsHolder.client(createRepositoryMetadata(randomFrom(clientName, anotherClientName)))
-        );
+        final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
+        assertBusy(() -> {
+            assertTrue(clientsHolder.isClosed());
+            try (var client = clientsHolder.client(createRepositoryMetadata(randomFrom(clientName, anotherClientName)))) {
+                fail("client should be closed"); // the cache is still being cleared out
+            } catch (Exception e) {
+                exceptionRef.compareAndSet(null, e); // the first exception must be expected and is checked below
+            }
+        });
+
+        final var e = exceptionRef.get();
+        assertThat(e, instanceOf(AlreadyClosedException.class));
         assertThat(e.getMessage(), containsString("Project [" + projectId + "] clients holder is closed"));
     }
 
@@ -306,8 +316,8 @@ public void testClientsHolderAfterManagerClosed() {
             assertNotNull(clientsHolder);
             assertFalse(clientsHolder.isClosed());
 
-            final IllegalStateException e = expectThrows(
-                IllegalStateException.class,
+            final var e = expectThrows(
+                AlreadyClosedException.class,
                 () -> s3ClientsManager.client(projectId, createRepositoryMetadata(clientName))
             );
             assertThat(e.getMessage(), containsString("s3 clients manager is closed"));

muted-tests.yml

@@ -504,9 +504,6 @@ tests:
 - class: org.elasticsearch.xpack.esql.expression.function.scalar.string.RLikeTests
   method: testEvaluateInManyThreads {TestCase=100 random code points matches self case insensitive with keyword}
   issue: https://github.com/elastic/elasticsearch/issues/128706
-- class: org.elasticsearch.repositories.s3.S3ClientsManagerTests
-  method: testClientsLifeCycleForSingleProject
-  issue: https://github.com/elastic/elasticsearch/issues/128707
 - class: org.elasticsearch.xpack.esql.expression.function.scalar.string.RLikeTests
   method: testCrankyEvaluateBlockWithNulls {TestCase=100 random code points matches self case insensitive with text}
   issue: https://github.com/elastic/elasticsearch/issues/128710