Browse Source

Delay S3 repo warning if default region absent (#133848) (#133917)

Today we emit a log message at startup if we fail to look up the default
region for use by the S3 repository. However, this is only a potential
issue if the default region is needed. If the user doesn't use
`repository-s3` at all, or specifies the `region` for each S3
repository, then the absence of the default region is not an issue.

With this commit we delay logging anything until we encounter a
situation where we actually need the default region for something.
David Turner 1 tháng trước cách đây
mục cha
commit
bef8554449

+ 5 - 0
docs/changelog/133848.yaml

@@ -0,0 +1,5 @@
+pr: 133848
+summary: Delay S3 repo warning if default region absent
+area: Snapshot/Restore
+type: bug
+issues: []

+ 2 - 1
modules/repository-s3/qa/insecure-credentials/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java

@@ -12,6 +12,7 @@ package org.elasticsearch.repositories.s3;
 import software.amazon.awssdk.auth.credentials.AwsCredentials;
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
 import software.amazon.awssdk.http.SdkHttpClient;
+import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.s3.S3Client;
 
 import org.apache.logging.log4j.LogManager;
@@ -306,7 +307,7 @@ public class RepositoryCredentialsTests extends ESSingleNodeTestCase {
                 ProjectResolver projectResolver,
                 ResourceWatcherService resourceWatcherService
             ) {
-                super(environment, clusterService, projectResolver, resourceWatcherService, () -> null);
+                super(environment, clusterService, projectResolver, resourceWatcherService, () -> Region.of(randomIdentifier()));
             }
 
             @Override

+ 61 - 0
modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolder.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.repositories.s3;
+
+import software.amazon.awssdk.regions.Region;
+
+import org.elasticsearch.common.util.concurrent.RunOnce;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
+
+import java.util.function.Supplier;
+
+/**
+ * Holds onto the {@link Region} provided by the given supplier (think: the AWS SDK's default provider chain) in case it's needed for a S3
+ * repository. If the supplier fails with an exception, the first call to {@link #getDefaultRegion} will log a warning message recording
+ * the exception.
+ */
+class S3DefaultRegionHolder {
+
+    private static final Logger logger = LogManager.getLogger(S3DefaultRegionHolder.class);
+
+    // no synchronization required, assignments happen in start() which happens-before all reads
+    private Region defaultRegion;
+    private Runnable defaultRegionFailureLogger = () -> {};
+
+    private final Runnable initializer;
+
+    /**
+     * @param delegateRegionSupplier Supplies a non-null {@link Region} or throws a {@link RuntimeException}.
+     *                               <p>
+     *                               Retained until its first-and-only invocation when {@link #start()} is called, and then released.
+     */
+    S3DefaultRegionHolder(Supplier<Region> delegateRegionSupplier) {
+        initializer = new RunOnce(() -> {
+            try {
+                defaultRegion = delegateRegionSupplier.get();
+                assert defaultRegion != null;
+            } catch (Exception e) {
+                defaultRegion = null;
+                defaultRegionFailureLogger = new RunOnce(() -> logger.warn("failed to obtain region from default provider chain", e));
+            }
+        });
+    }
+
+    void start() {
+        initializer.run();
+    }
+
+    Region getDefaultRegion() {
+        assert defaultRegion != null || defaultRegionFailureLogger instanceof RunOnce : "not initialized";
+        defaultRegionFailureLogger.run();
+        return defaultRegion;
+    }
+}

+ 1 - 6
modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java

@@ -98,12 +98,7 @@ public class S3RepositoryPlugin extends Plugin implements RepositoryPlugin, Relo
     }
 
     private static Region getDefaultRegion() {
-        try {
-            return DefaultAwsRegionProviderChain.builder().build().getRegion();
-        } catch (Exception e) {
-            logger.info("failed to obtain region from default provider chain", e);
-            return null;
-        }
+        return DefaultAwsRegionProviderChain.builder().build().getRegion();
     }
 
     @Override

+ 4 - 6
modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java

@@ -48,7 +48,6 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.util.concurrent.RunOnce;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Releasable;
 import org.elasticsearch.core.Releasables;
@@ -94,8 +93,7 @@ class S3Service extends AbstractLifecycleComponent {
         Setting.Property.NodeScope
     );
 
-    private final Runnable defaultRegionSetter;
-    private volatile Region defaultRegion;
+    private final S3DefaultRegionHolder defaultRegionHolder;
 
     /**
      * Use a signer that does not require to pre-read (and checksum) the body of PutObject and UploadPart requests since we can rely on
@@ -129,7 +127,7 @@ class S3Service extends AbstractLifecycleComponent {
         compareAndExchangeTimeToLive = REPOSITORY_S3_CAS_TTL_SETTING.get(nodeSettings);
         compareAndExchangeAntiContentionDelay = REPOSITORY_S3_CAS_ANTI_CONTENTION_DELAY_SETTING.get(nodeSettings);
         isStateless = DiscoveryNode.isStateless(nodeSettings);
-        defaultRegionSetter = new RunOnce(() -> defaultRegion = defaultRegionSupplier.get());
+        defaultRegionHolder = new S3DefaultRegionHolder(defaultRegionSupplier);
         s3ClientsManager = new S3ClientsManager(
             nodeSettings,
             this::buildClientReference,
@@ -266,7 +264,7 @@ class S3Service extends AbstractLifecycleComponent {
         } else {
             endpointDescription = "no configured endpoint";
         }
-        final var defaultRegion = this.defaultRegion;
+        final var defaultRegion = defaultRegionHolder.getDefaultRegion();
         if (defaultRegion != null) {
             LOGGER.debug("""
                 found S3 client with no configured region and {}, using region [{}] from SDK""", endpointDescription, defaultRegion);
@@ -415,7 +413,7 @@ class S3Service extends AbstractLifecycleComponent {
 
     @Override
     protected void doStart() {
-        defaultRegionSetter.run();
+        defaultRegionHolder.start();
     }
 
     @Override

+ 3 - 2
modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java

@@ -101,6 +101,7 @@ import static org.elasticsearch.repositories.s3.S3ClientSettings.ENDPOINT_SETTIN
 import static org.elasticsearch.repositories.s3.S3ClientSettings.MAX_CONNECTIONS_SETTING;
 import static org.elasticsearch.repositories.s3.S3ClientSettings.MAX_RETRIES_SETTING;
 import static org.elasticsearch.repositories.s3.S3ClientSettings.READ_TIMEOUT_SETTING;
+import static org.elasticsearch.repositories.s3.S3ClientSettingsTests.DEFAULT_REGION_UNAVAILABLE;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.contains;
@@ -134,7 +135,7 @@ public class S3BlobContainerRetriesTests extends AbstractBlobContainerRetriesTes
             ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()),
             TestProjectResolvers.DEFAULT_PROJECT_ONLY,
             Mockito.mock(ResourceWatcherService.class),
-            () -> null
+            DEFAULT_REGION_UNAVAILABLE
         ) {
             private InetAddress[] resolveHost(String host) throws UnknownHostException {
                 assertEquals("127.0.0.1", host);
@@ -1323,7 +1324,7 @@ public class S3BlobContainerRetriesTests extends AbstractBlobContainerRetriesTes
             ),
             TestProjectResolvers.DEFAULT_PROJECT_ONLY,
             Mockito.mock(ResourceWatcherService.class),
-            () -> null
+            DEFAULT_REGION_UNAVAILABLE
         );
         service.start();
         recordingMeterRegistry = new RecordingMeterRegistry();

+ 9 - 1
modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java

@@ -13,6 +13,7 @@ import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
 import software.amazon.awssdk.auth.credentials.AwsSessionCredentials;
 import software.amazon.awssdk.regions.Region;
 
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.cluster.project.TestProjectResolvers;
 import org.elasticsearch.common.settings.MockSecureSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -24,6 +25,7 @@ import org.elasticsearch.watcher.ResourceWatcherService;
 import org.mockito.Mockito;
 
 import java.util.Map;
+import java.util.function.Supplier;
 
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.emptyString;
@@ -190,9 +192,11 @@ public class S3ClientSettingsTests extends ESTestCase {
                 ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()),
                 TestProjectResolvers.DEFAULT_PROJECT_ONLY,
                 Mockito.mock(ResourceWatcherService.class),
-                () -> null
+                DEFAULT_REGION_UNAVAILABLE
             )
         ) {
+            s3Service.start();
+
             var otherSettings = settings.get("other");
             Region otherRegion = s3Service.getClientRegion(otherSettings);
             assertEquals(randomRegion, otherRegion.toString());
@@ -213,4 +217,8 @@ public class S3ClientSettingsTests extends ESTestCase {
         // the default appears in the docs so let's make sure it doesn't change:
         assertEquals(50, S3ClientSettings.Defaults.MAX_CONNECTIONS);
     }
+
+    public static final Supplier<Region> DEFAULT_REGION_UNAVAILABLE = () -> {
+        throw new ElasticsearchException("default region unavailable in this test");
+    };
 }

+ 85 - 0
modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolderTests.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.repositories.s3;
+
+import software.amazon.awssdk.regions.Region;
+
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.LogEvent;
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.MockLog;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+public class S3DefaultRegionHolderTests extends ESTestCase {
+    public void testSuccess() {
+        try (var mockLog = MockLog.capture(S3DefaultRegionHolder.class)) {
+            mockLog.addExpectation(new NoLogEventsExpectation());
+
+            final var region = Region.of(randomIdentifier());
+            final var regionSupplied = new AtomicBoolean();
+            final var regionHolder = new S3DefaultRegionHolder(() -> {
+                assertTrue(regionSupplied.compareAndSet(false, true)); // only called once
+                return region;
+            });
+
+            regionHolder.start();
+            assertTrue(regionSupplied.get());
+            assertSame(region, regionHolder.getDefaultRegion());
+            assertSame(region, regionHolder.getDefaultRegion());
+
+            mockLog.assertAllExpectationsMatched();
+        }
+    }
+
+    public void testFailure() {
+        try (var mockLog = MockLog.capture(S3DefaultRegionHolder.class)) {
+            final var warningSeenExpectation = new MockLog.EventuallySeenEventExpectation(
+                "warning",
+                S3DefaultRegionHolder.class.getCanonicalName(),
+                Level.WARN,
+                "failed to obtain region from default provider chain"
+            );
+            mockLog.addExpectation(warningSeenExpectation);
+
+            final var regionSupplied = new AtomicBoolean();
+            final var regionHolder = new S3DefaultRegionHolder(() -> {
+                assertTrue(regionSupplied.compareAndSet(false, true)); // only called once
+                throw new ElasticsearchException("simulated");
+            });
+
+            regionHolder.start();
+            assertTrue(regionSupplied.get());
+
+            warningSeenExpectation.setExpectSeen(); // not seen yet, but will be seen now
+            regionHolder.getDefaultRegion();
+
+            mockLog.addExpectation(new NoLogEventsExpectation()); // log message not duplicated
+            regionHolder.getDefaultRegion();
+
+            mockLog.assertAllExpectationsMatched();
+        }
+    }
+
+    private static class NoLogEventsExpectation implements MockLog.LoggingExpectation {
+        private boolean seenLogEvent;
+
+        @Override
+        public void match(LogEvent event) {
+            seenLogEvent = true;
+        }
+
+        @Override
+        public void assertMatched() {
+            assertFalse(seenLogEvent);
+        }
+    }
+}

+ 2 - 1
modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java

@@ -10,6 +10,7 @@
 package org.elasticsearch.repositories.s3;
 
 import software.amazon.awssdk.http.SdkHttpClient;
+import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.s3.S3Client;
 
 import org.elasticsearch.cluster.metadata.ProjectId;
@@ -68,7 +69,7 @@ public class S3RepositoryTests extends ESTestCase {
             ProjectResolver projectResolver,
             ResourceWatcherService resourceWatcherService
         ) {
-            super(environment, clusterService, projectResolver, resourceWatcherService, () -> null);
+            super(environment, clusterService, projectResolver, resourceWatcherService, () -> Region.of(randomIdentifier()));
         }
 
         @Override

+ 118 - 53
modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ServiceTests.java

@@ -19,6 +19,8 @@ import software.amazon.awssdk.services.s3.endpoints.internal.DefaultS3EndpointPr
 import software.amazon.awssdk.services.s3.model.S3Exception;
 
 import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.LogEvent;
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.cluster.metadata.ProjectId;
 import org.elasticsearch.cluster.metadata.RepositoryMetadata;
 import org.elasticsearch.cluster.project.TestProjectResolvers;
@@ -33,7 +35,6 @@ import org.elasticsearch.test.MockLog;
 import org.elasticsearch.test.junit.annotations.TestLogging;
 import org.elasticsearch.watcher.ResourceWatcherService;
 
-import java.io.IOException;
 import java.net.URI;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -42,7 +43,7 @@ import static org.mockito.Mockito.mock;
 
 public class S3ServiceTests extends ESTestCase {
 
-    public void testCachedClientsAreReleased() throws IOException {
+    public void testCachedClientsAreReleased() {
         final S3Service s3Service = new S3Service(
             mock(Environment.class),
             ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()),
@@ -107,30 +108,45 @@ public class S3ServiceTests extends ESTestCase {
                 mock(ResourceWatcherService.class),
                 () -> {
                     assertTrue(regionRequested.compareAndSet(false, true));
-                    return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier()), null);
+                    if (randomBoolean()) {
+                        throw new ElasticsearchException("simulated");
+                    } else {
+                        return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier()));
+                    }
                 }
-            )
+            );
+            var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class)
         ) {
+            mockLog.addExpectation(
+                new MockLog.UnseenEventExpectation(
+                    "no default region warning",
+                    S3DefaultRegionHolder.class.getCanonicalName(),
+                    Level.WARN,
+                    "*"
+                )
+            );
+
             s3Service.start();
             assertTrue(regionRequested.get());
 
             final var clientName = randomBoolean() ? "default" : randomIdentifier();
 
             final var region = randomBoolean() ? randomFrom(Region.regions()) : Region.of(randomIdentifier());
-            MockLog.assertThatLogger(
-                () -> assertSame(
-                    region,
-                    s3Service.getClientRegion(
-                        S3ClientSettings.getClientSettings(
-                            Settings.builder().put("s3.client." + clientName + ".region", region.id()).build(),
-                            clientName
-                        )
+
+            mockLog.addExpectation(new MockLog.UnseenEventExpectation("no warning", S3Service.class.getCanonicalName(), Level.WARN, "*"));
+            mockLog.addExpectation(new MockLog.UnseenEventExpectation("no debug", S3Service.class.getCanonicalName(), Level.DEBUG, "*"));
+
+            assertSame(
+                region,
+                s3Service.getClientRegion(
+                    S3ClientSettings.getClientSettings(
+                        Settings.builder().put("s3.client." + clientName + ".region", region.id()).build(),
+                        clientName
                     )
-                ),
-                S3Service.class,
-                new MockLog.UnseenEventExpectation("no warning", S3Service.class.getCanonicalName(), Level.WARN, "*"),
-                new MockLog.UnseenEventExpectation("no debug", S3Service.class.getCanonicalName(), Level.DEBUG, "*")
+                )
             );
+
+            mockLog.assertAllExpectationsMatched();
         }
     }
 
@@ -145,10 +161,24 @@ public class S3ServiceTests extends ESTestCase {
                 mock(ResourceWatcherService.class),
                 () -> {
                     assertTrue(regionRequested.compareAndSet(false, true));
-                    return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier()), null);
+                    if (randomBoolean()) {
+                        throw new ElasticsearchException("simulated");
+                    } else {
+                        return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier()));
+                    }
                 }
-            )
+            );
+            var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class)
         ) {
+            mockLog.addExpectation(
+                new MockLog.UnseenEventExpectation(
+                    "no default region warning",
+                    S3DefaultRegionHolder.class.getCanonicalName(),
+                    Level.WARN,
+                    "*"
+                )
+            );
+
             s3Service.start();
             assertTrue(regionRequested.get());
 
@@ -163,18 +193,7 @@ public class S3ServiceTests extends ESTestCase {
             ).url();
             final var endpoint = randomFrom(endpointUrl.toString(), endpointUrl.getHost());
 
-            MockLog.assertThatLogger(
-                () -> assertEquals(
-                    endpoint,
-                    guessedRegion,
-                    s3Service.getClientRegion(
-                        S3ClientSettings.getClientSettings(
-                            Settings.builder().put("s3.client." + clientName + ".endpoint", endpoint).build(),
-                            clientName
-                        )
-                    )
-                ),
-                S3Service.class,
+            mockLog.addExpectation(
                 new MockLog.SeenEventExpectation(
                     endpoint + " -> " + guessedRegion,
                     S3Service.class.getCanonicalName(),
@@ -188,6 +207,18 @@ public class S3ServiceTests extends ESTestCase {
                     )
                 )
             );
+            assertEquals(
+                endpoint,
+                guessedRegion,
+                s3Service.getClientRegion(
+                    S3ClientSettings.getClientSettings(
+                        Settings.builder().put("s3.client." + clientName + ".endpoint", endpoint).build(),
+                        clientName
+                    )
+                )
+            );
+
+            mockLog.assertAllExpectationsMatched();
         }
     }
 
@@ -205,16 +236,24 @@ public class S3ServiceTests extends ESTestCase {
                     assertTrue(regionRequested.compareAndSet(false, true));
                     return defaultRegion;
                 }
-            )
+            );
+            var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class)
         ) {
+            mockLog.addExpectation(
+                new MockLog.UnseenEventExpectation(
+                    "no default region warning",
+                    S3DefaultRegionHolder.class.getCanonicalName(),
+                    Level.WARN,
+                    "*"
+                )
+            );
+
             s3Service.start();
             assertTrue(regionRequested.get());
 
             final var clientName = randomBoolean() ? "default" : randomIdentifier();
 
-            MockLog.assertThatLogger(
-                () -> assertSame(defaultRegion, s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))),
-                S3Service.class,
+            mockLog.addExpectation(
                 new MockLog.SeenEventExpectation(
                     "warning",
                     S3Service.class.getCanonicalName(),
@@ -224,12 +263,17 @@ public class S3ServiceTests extends ESTestCase {
                         + "] from SDK"
                 )
             );
+
+            assertSame(defaultRegion, s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName)));
+
+            mockLog.assertAllExpectationsMatched();
         }
     }
 
     @TestLogging(reason = "testing WARN log output", value = "org.elasticsearch.repositories.s3.S3Service:WARN")
     public void testGetClientRegionFallbackToUsEast1() {
         final var regionRequested = new AtomicBoolean();
+        final var exceptionMessage = randomIdentifier();
         try (
             var s3Service = new S3Service(
                 mock(Environment.class),
@@ -238,23 +282,39 @@ public class S3ServiceTests extends ESTestCase {
                 mock(ResourceWatcherService.class),
                 () -> {
                     assertTrue(regionRequested.compareAndSet(false, true));
-                    return null;
+                    throw new ElasticsearchException(exceptionMessage);
                 }
-            )
+            );
+            var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class)
         ) {
             s3Service.start();
             assertTrue(regionRequested.get());
 
             final var clientName = randomBoolean() ? "default" : randomIdentifier();
 
-            MockLog.assertThatLogger(
-                () -> assertNull(s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))),
-                S3Service.class,
-                new MockLog.SeenEventExpectation("warning", S3Service.class.getCanonicalName(), Level.WARN, """
-                    found S3 client with no configured region and no configured endpoint, \
-                    falling back to [us-east-1] and enabling cross-region access; \
-                    to suppress this warning, configure the [s3.client.CLIENT_NAME.region] setting on this node""")
+            mockLog.addExpectation(
+                new MockLog.SeenEventExpectation(
+                    "default provider chain failure",
+                    S3DefaultRegionHolder.class.getCanonicalName(),
+                    Level.WARN,
+                    "failed to obtain region from default provider chain"
+                ) {
+                    @Override
+                    public void match(LogEvent event) {
+                        if (event.getThrown() instanceof ElasticsearchException e && exceptionMessage.equals(e.getMessage())) {
+                            super.match(event);
+                        }
+                    }
+                }
             );
+            mockLog.addExpectation(new MockLog.SeenEventExpectation("warning", S3Service.class.getCanonicalName(), Level.WARN, """
+                found S3 client with no configured region and no configured endpoint, \
+                falling back to [us-east-1] and enabling cross-region access; \
+                to suppress this warning, configure the [s3.client.CLIENT_NAME.region] setting on this node"""));
+
+            assertNull(s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName)));
+
+            mockLog.assertAllExpectationsMatched();
         }
     }
 
@@ -302,15 +362,20 @@ public class S3ServiceTests extends ESTestCase {
     }
 
     private URI getEndpointUri(Settings.Builder settings, String clientName) {
-        return new S3Service(
-            mock(Environment.class),
-            ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()),
-            TestProjectResolvers.DEFAULT_PROJECT_ONLY,
-            mock(ResourceWatcherService.class),
-            () -> Region.of(randomIdentifier())
-        ).buildClient(S3ClientSettings.getClientSettings(settings.build(), clientName), mock(SdkHttpClient.class))
-            .serviceClientConfiguration()
-            .endpointOverride()
-            .get();
+        try (
+            var s3Service = new S3Service(
+                mock(Environment.class),
+                ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()),
+                TestProjectResolvers.DEFAULT_PROJECT_ONLY,
+                mock(ResourceWatcherService.class),
+                () -> Region.of(randomIdentifier())
+            )
+        ) {
+            s3Service.start();
+            return s3Service.buildClient(S3ClientSettings.getClientSettings(settings.build(), clientName), mock(SdkHttpClient.class))
+                .serviceClientConfiguration()
+                .endpointOverride()
+                .get();
+        }
     }
 }