diff --git a/docs/changelog/85551.yaml b/docs/changelog/85551.yaml new file mode 100644 index 0000000000000..82ec1ed648325 --- /dev/null +++ b/docs/changelog/85551.yaml @@ -0,0 +1,5 @@ +pr: 85551 +summary: "Distinguish missing and invalid repositories" +area: Snapshot/Restore +type: bug +issues: [85550] diff --git a/server/src/internalClusterTest/java/org/elasticsearch/repositories/InvalidRepositoryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/repositories/InvalidRepositoryIT.java new file mode 100644 index 0000000000000..217784cd127e8 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/repositories/InvalidRepositoryIT.java @@ -0,0 +1,147 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.repositories; + +import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesResponse; +import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryResponse; +import org.elasticsearch.cluster.metadata.RepositoryMetadata; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.env.Environment; +import org.elasticsearch.indices.recovery.RecoverySettings; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.RepositoryPlugin; +import org.elasticsearch.snapshots.mockstore.MockRepository; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.xcontent.NamedXContentRegistry; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.isA; + +public class InvalidRepositoryIT extends ESIntegTestCase { + @Override + protected Collection> nodePlugins() { + return Collections.singletonList(UnstableRepository.Plugin.class); + } + + public static class UnstableRepository extends MockRepository { + public static final String TYPE = "unstable"; + public static final Setting> UNSTABLE_NODES = Setting.stringListSetting( + "repository.unstable_nodes", + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + + public UnstableRepository( + RepositoryMetadata metadata, + Environment environment, + NamedXContentRegistry namedXContentRegistry, + ClusterService clusterService, + BigArrays bigArrays, + RecoverySettings recoverySettings + ) { + super(metadata, environment, namedXContentRegistry, clusterService, bigArrays, recoverySettings); + List unstableNodes = UNSTABLE_NODES.get(metadata.settings()); + if (unstableNodes.contains(clusterService.getNodeName())) { + throw new RepositoryException(metadata.name(), "Failed to create repository: current node is not stable"); + } + } + + public static class Plugin extends org.elasticsearch.plugins.Plugin implements RepositoryPlugin { + @Override + public Map getRepositories( + Environment env, + NamedXContentRegistry namedXContentRegistry, + ClusterService clusterService, + BigArrays bigArrays, + RecoverySettings recoverySettings + ) { + return Collections.singletonMap( + TYPE, + (metadata) -> new UnstableRepository(metadata, env, namedXContentRegistry, clusterService, bigArrays, recoverySettings) + ); + } + + @Override + public List> getSettings() { + return List.of(UNSTABLE_NODES); + } + } + } + + public void testCreateInvalidRepository() throws Exception { + internalCluster().ensureAtLeastNumDataNodes(2); + final String repositoryName = "test-duplicate-create-repo"; + + // put repository for the first time: only let master node create repository successfully + createRepository( + repositoryName, + UnstableRepository.TYPE, + Settings.builder() + .put("location", randomRepoPath()) + .putList( + UnstableRepository.UNSTABLE_NODES.getKey(), + Arrays.stream(internalCluster().getNodeNames()) + .filter(name -> name.equals(internalCluster().getMasterName()) == false) + .toList() + ) + ); + // verification should fail with some node has InvalidRepository + final var expectedException = expectThrows( + RepositoryVerificationException.class, + () -> client().admin().cluster().prepareVerifyRepository(repositoryName).get() + ); + for (Throwable suppressed : expectedException.getSuppressed()) { + Throwable outerCause = suppressed.getCause(); + assertThat(outerCause, isA(RepositoryException.class)); + assertThat( + outerCause.getMessage(), + equalTo("[" + repositoryName + "] repository type [" + UnstableRepository.TYPE + "] failed to create on current node") + ); + Throwable innerCause = suppressed.getCause().getCause().getCause(); + assertThat(innerCause, isA(RepositoryException.class)); + assertThat( + innerCause.getMessage(), + equalTo("[" + repositoryName + "] Failed to create repository: current node is not stable") + ); + } + + // restart master + internalCluster().restartNode(internalCluster().getMasterName()); + ensureGreen(); + + // put repository again: let all node can create repository successfully + createRepository(repositoryName, UnstableRepository.TYPE, Settings.builder().put("location", randomRepoPath())); + // verification should succeed with all node create repository successfully + VerifyRepositoryResponse verifyRepositoryResponse = client().admin().cluster().prepareVerifyRepository(repositoryName).get(); + assertEquals(verifyRepositoryResponse.getNodes().size(), internalCluster().numDataAndMasterNodes()); + + } + + private void createRepository(String name, String type, Settings.Builder settings) { + // create + assertAcked(client().admin().cluster().preparePutRepository(name).setType(type).setVerify(false).setSettings(settings).get()); + // get + final GetRepositoriesResponse updatedGetRepositoriesResponse = client().admin().cluster().prepareGetRepositories(name).get(); + // assert + assertThat(updatedGetRepositoriesResponse.repositories(), hasSize(1)); + final RepositoryMetadata updatedRepositoryMetadata = updatedGetRepositoriesResponse.repositories().get(0); + assertThat(updatedRepositoryMetadata.type(), equalTo(type)); + } +} diff --git a/server/src/main/java/org/elasticsearch/repositories/InvalidRepository.java b/server/src/main/java/org/elasticsearch/repositories/InvalidRepository.java new file mode 100644 index 0000000000000..1f11def3c8010 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/repositories/InvalidRepository.java @@ -0,0 +1,189 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.repositories; + +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.RepositoryMetadata; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.common.component.AbstractLifecycleComponent; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus; +import org.elasticsearch.index.store.Store; +import org.elasticsearch.indices.recovery.RecoveryState; +import org.elasticsearch.snapshots.SnapshotId; + +import java.io.IOException; +import java.util.Collection; +import java.util.function.Consumer; +import java.util.function.Function; + +/** + * Represents a repository that exists in the cluster state but could not be instantiated on a node, typically due to invalid configuration. + */ +public class InvalidRepository extends AbstractLifecycleComponent implements Repository { + + private final RepositoryMetadata repositoryMetadata; + private final RepositoryException creationException; + + public InvalidRepository(RepositoryMetadata repositoryMetadata, RepositoryException creationException) { + this.repositoryMetadata = repositoryMetadata; + this.creationException = creationException; + } + + private RepositoryException createCreationException() { + return new RepositoryException( + repositoryMetadata.name(), + "repository type [" + repositoryMetadata.type() + "] failed to create on current node", + creationException + ); + } + + @Override + public RepositoryMetadata getMetadata() { + return repositoryMetadata; + } + + @Override + public void getSnapshotInfo(GetSnapshotInfoContext context) { + throw createCreationException(); + } + + @Override + public Metadata getSnapshotGlobalMetadata(SnapshotId snapshotId) { + throw createCreationException(); + } + + @Override + public IndexMetadata getSnapshotIndexMetaData(RepositoryData repositoryData, SnapshotId snapshotId, IndexId index) throws IOException { + throw createCreationException(); + } + + @Override + public void getRepositoryData(ActionListener listener) { + listener.onFailure(createCreationException()); + } + + @Override + public void finalizeSnapshot(FinalizeSnapshotContext finalizeSnapshotContext) { + finalizeSnapshotContext.onFailure(createCreationException()); + } + + @Override + public void deleteSnapshots( + Collection snapshotIds, + long repositoryStateId, + Version repositoryMetaVersion, + ActionListener listener + ) { + listener.onFailure(createCreationException()); + } + + @Override + public long getSnapshotThrottleTimeInNanos() { + throw createCreationException(); + } + + @Override + public long getRestoreThrottleTimeInNanos() { + throw createCreationException(); + } + + @Override + public String startVerification() { + throw createCreationException(); + } + + @Override + public void endVerification(String verificationToken) { + throw createCreationException(); + } + + @Override + public void verify(String verificationToken, DiscoveryNode localNode) { + throw createCreationException(); + } + + @Override + public boolean isReadOnly() { + // this repository is assumed writable to bypass read-only check and fail with exception produced by this class + return false; + } + + @Override + public void snapshotShard(SnapshotShardContext snapshotShardContext) { + snapshotShardContext.onFailure(createCreationException()); + } + + @Override + public void restoreShard( + Store store, + SnapshotId snapshotId, + IndexId indexId, + ShardId snapshotShardId, + RecoveryState recoveryState, + ActionListener listener + ) { + listener.onFailure(createCreationException()); + } + + @Override + public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, IndexId indexId, ShardId shardId) { + throw createCreationException(); + } + + @Override + public void updateState(ClusterState state) { + + } + + @Override + public void executeConsistentStateUpdate( + Function createUpdateTask, + String source, + Consumer onFailure + ) { + onFailure.accept(createCreationException()); + } + + @Override + public void cloneShardSnapshot( + SnapshotId source, + SnapshotId target, + RepositoryShardId shardId, + ShardGeneration shardGeneration, + ActionListener listener + ) { + listener.onFailure(createCreationException()); + } + + @Override + public void awaitIdle() { + + } + + @Override + protected void doStart() { + + } + + @Override + protected void doStop() { + + } + + @Override + protected void doClose() throws IOException { + + } +} diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 84b628d1ecc2f..0ecd1a6d32c6e 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -515,6 +515,7 @@ public void applyClusterState(ClusterChangedEvent event) { // TODO: this catch is bogus, it means the old repo is already closed, // but we have nothing to replace it logger.warn(() -> new ParameterizedMessage("failed to change repository [{}]", repositoryMetadata.name()), ex); + repository = new InvalidRepository(repositoryMetadata, ex); } } } else { @@ -522,12 +523,12 @@ public void applyClusterState(ClusterChangedEvent event) { repository = createRepository(repositoryMetadata, typesRegistry, RepositoriesService::createUnknownTypeRepository); } catch (RepositoryException ex) { logger.warn(() -> new ParameterizedMessage("failed to create repository [{}]", repositoryMetadata.name()), ex); + repository = new InvalidRepository(repositoryMetadata, ex); } } - if (repository != null) { - logger.debug("registering repository [{}]", repositoryMetadata.name()); - builder.put(repositoryMetadata.name(), repository); - } + assert repository != null : "repository should not be null here"; + logger.debug("registering repository [{}]", repositoryMetadata.name()); + builder.put(repositoryMetadata.name(), repository); } for (Repository repo : builder.values()) { repo.updateState(state); diff --git a/server/src/main/java/org/elasticsearch/repositories/UnknownTypeRepository.java b/server/src/main/java/org/elasticsearch/repositories/UnknownTypeRepository.java index b634f09502f1e..70f764dd9b5e0 100644 --- a/server/src/main/java/org/elasticsearch/repositories/UnknownTypeRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/UnknownTypeRepository.java @@ -30,7 +30,7 @@ /** * This class represents a repository that could not be initialized due to unknown type. - * This could happen whe a user creates a snapshot repository using a type from a plugin and then removes the plugin. + * This could happen when a user creates a snapshot repository using a type from a plugin and then removes the plugin. */ public class UnknownTypeRepository extends AbstractLifecycleComponent implements Repository { diff --git a/server/src/test/java/org/elasticsearch/repositories/InvalidRepositoryTests.java b/server/src/test/java/org/elasticsearch/repositories/InvalidRepositoryTests.java new file mode 100644 index 0000000000000..d6f3d3a8d0383 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/repositories/InvalidRepositoryTests.java @@ -0,0 +1,35 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.repositories; + +import org.elasticsearch.cluster.metadata.RepositoryMetadata; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.isA; + +public class InvalidRepositoryTests extends ESTestCase { + + private InvalidRepository repository = new InvalidRepository( + new RepositoryMetadata("name", "type", Settings.EMPTY), + new RepositoryException("name", "failed to create repository") + ); + + public void testShouldThrowWhenGettingMetadata() { + final var expectedException = expectThrows( + RepositoryException.class, + () -> repository.getSnapshotGlobalMetadata(new SnapshotId("name", "uuid")) + ); + assertThat(expectedException.getMessage(), equalTo("[name] repository type [type] failed to create on current node")); + assertThat(expectedException.getCause(), isA(RepositoryException.class)); + assertThat(expectedException.getCause().getMessage(), equalTo("[name] failed to create repository")); + } +} diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java index d4ec767e5817c..102baacb64bf3 100644 --- a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java @@ -85,6 +85,8 @@ public void setUp() throws Exception { Map typesRegistry = Map.of( TestRepository.TYPE, TestRepository::new, + UnstableRepository.TYPE, + UnstableRepository::new, MeteredRepositoryTypeA.TYPE, metadata -> new MeteredRepositoryTypeA(metadata, clusterService), MeteredRepositoryTypeB.TYPE, @@ -212,6 +214,76 @@ public void onFailure(Exception e) { }); } + // test InvalidRepository is returned if repository failed to create + public void testHandlesCreationFailureWhenApplyingClusterState() { + var repoName = randomAlphaOfLengthBetween(10, 25); + + var clusterState = createClusterStateWithRepo(repoName, UnstableRepository.TYPE); + repositoriesService.applyClusterState(new ClusterChangedEvent("put unstable repository", clusterState, emptyState())); + + var repo = repositoriesService.repository(repoName); + assertThat(repo, isA(InvalidRepository.class)); + } + + // test InvalidRepository can be replaced if current repo is created successfully + public void testReplaceInvalidRepositoryWhenCreationSuccess() { + var repoName = randomAlphaOfLengthBetween(10, 25); + + var clusterState = createClusterStateWithRepo(repoName, UnstableRepository.TYPE); + repositoriesService.applyClusterState(new ClusterChangedEvent("put unstable repository", clusterState, emptyState())); + + var repo = repositoriesService.repository(repoName); + assertThat(repo, isA(InvalidRepository.class)); + + clusterState = createClusterStateWithRepo(repoName, TestRepository.TYPE); + repositoriesService.applyClusterState(new ClusterChangedEvent("put test repository", clusterState, emptyState())); + repo = repositoriesService.repository(repoName); + assertThat(repo, isA(TestRepository.class)); + } + + // test remove InvalidRepository when current repo is removed in cluster state + public void testRemoveInvalidRepositoryTypeWhenApplyingClusterState() { + var repoName = randomAlphaOfLengthBetween(10, 25); + + var clusterState = createClusterStateWithRepo(repoName, UnstableRepository.TYPE); + repositoriesService.applyClusterState(new ClusterChangedEvent("put unstable repository", clusterState, emptyState())); + repositoriesService.applyClusterState(new ClusterChangedEvent("removing repo", emptyState(), clusterState)); + assertThat( + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)).getMessage(), + equalTo("[" + repoName + "] missing") + ); + } + + // InvalidRepository is created when current node is non-master node and failed to create repository by applying cluster state from + // master. When current node become master node later and same repository is put again, current node can create repository successfully + // and replace previous InvalidRepository + public void testRegisterRepositorySuccessAfterCreationFailed() { + // 1. repository creation failed when current node is non-master node and apply cluster state from master + var repoName = randomAlphaOfLengthBetween(10, 25); + + var clusterState = createClusterStateWithRepo(repoName, UnstableRepository.TYPE); + repositoriesService.applyClusterState(new ClusterChangedEvent("put unstable repository", clusterState, emptyState())); + + var repo = repositoriesService.repository(repoName); + assertThat(repo, isA(InvalidRepository.class)); + + // 2. repository creation successfully when current node become master node and repository is put again + var request = new PutRepositoryRequest().name(repoName).type(TestRepository.TYPE); + + repositoriesService.registerRepository(request, new ActionListener<>() { + @Override + public void onResponse(AcknowledgedResponse acknowledgedResponse) { + assertTrue(acknowledgedResponse.isAcknowledged()); + assertThat(repositoriesService.repository(repoName), isA(TestRepository.class)); + } + + @Override + public void onFailure(Exception e) { + assert false : e; + } + }); + } + private ClusterState createClusterStateWithRepo(String repoName, String repoType) { ClusterState.Builder state = ClusterState.builder(new ClusterName("test")); Metadata.Builder mdBuilder = Metadata.builder(); @@ -391,6 +463,15 @@ public void close() { } } + private static class UnstableRepository extends TestRepository { + private static final String TYPE = "unstable"; + + private UnstableRepository(RepositoryMetadata metadata) { + super(metadata); + throw new RepositoryException(TYPE, "failed to create unstable repository"); + } + } + private static class MeteredRepositoryTypeA extends MeteredBlobStoreRepository { private static final String TYPE = "type-a"; private static final RepositoryStats STATS = new RepositoryStats(Map.of("GET", 10L)); diff --git a/x-pack/plugin/repository-encrypted/src/internalClusterTest/java/org/elasticsearch/repositories/encrypted/EncryptedRepositorySecretIntegTests.java b/x-pack/plugin/repository-encrypted/src/internalClusterTest/java/org/elasticsearch/repositories/encrypted/EncryptedRepositorySecretIntegTests.java index 98e780d2ea4df..579af5f4905cb 100644 --- a/x-pack/plugin/repository-encrypted/src/internalClusterTest/java/org/elasticsearch/repositories/encrypted/EncryptedRepositorySecretIntegTests.java +++ b/x-pack/plugin/repository-encrypted/src/internalClusterTest/java/org/elasticsearch/repositories/encrypted/EncryptedRepositorySecretIntegTests.java @@ -420,7 +420,10 @@ public void testSnapshotIsPartialForMissingPassword() throws Exception { incompleteSnapshotResponse.getSnapshotInfo() .shardFailures() .stream() - .allMatch(shardFailure -> shardFailure.reason().contains("[" + repositoryName + "] missing")) + .allMatch( + shardFailure -> shardFailure.reason() + .contains("Secure setting [repository.encrypted." + repositoryName + ".password] must be set") + ) ); assertThat( incompleteSnapshotResponse.getSnapshotInfo().userMetadata(),