Skip to content

Commit c5fbc80

Browse files
ywelschywelsch
committed
Do not access snapshot repo on dedicated voting-only master node (#61016)
Today a snapshot repository verification ensures that all master-eligible and data nodes have write access to the snapshot repository (and can see each other's data) since taking a snapshot requires data nodes and the currently elected master to write to the repository. However, a dedicated voting-only master-eligible node is not a data node and will never be the elected master so we should not require it to have write access to the repository. Closes #59649
1 parent c85157f commit c5fbc80

File tree

3 files changed

+121
-3
lines changed

3 files changed

+121
-3
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.elasticsearch.cluster.metadata.RepositoriesMetadata;
4040
import org.elasticsearch.cluster.metadata.RepositoryMetadata;
4141
import org.elasticsearch.cluster.node.DiscoveryNode;
42+
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
4243
import org.elasticsearch.cluster.service.ClusterService;
4344
import org.elasticsearch.common.Strings;
4445
import org.elasticsearch.common.component.AbstractLifecycleComponent;
@@ -56,6 +57,7 @@
5657
import java.util.HashMap;
5758
import java.util.List;
5859
import java.util.Map;
60+
import java.util.Set;
5961

6062
/**
6163
* Service responsible for maintaining and providing access to snapshot repositories on nodes.
@@ -86,7 +88,9 @@ public RepositoriesService(Settings settings, ClusterService clusterService, Tra
8688
// Doesn't make sense to maintain repositories on non-master and non-data nodes
8789
// Nothing happens there anyway
8890
if (DiscoveryNode.isDataNode(settings) || DiscoveryNode.isMasterNode(settings)) {
89-
clusterService.addHighPriorityApplier(this);
91+
if (isDedicatedVotingOnlyNode(DiscoveryNode.getRolesFromSettings(settings)) == false) {
92+
clusterService.addHighPriorityApplier(this);
93+
}
9094
}
9195
this.verifyAction = new VerifyNodeRepositoryAction(transportService, clusterService, this);
9296
}
@@ -279,6 +283,10 @@ protected void doRun() {
279283
});
280284
}
281285

286+
static boolean isDedicatedVotingOnlyNode(Set<DiscoveryNodeRole> roles) {
287+
return roles.contains(DiscoveryNodeRole.MASTER_ROLE) && roles.contains(DiscoveryNodeRole.DATA_ROLE) == false &&
288+
roles.stream().anyMatch(role -> role.roleName().equals("voting_only"));
289+
}
282290

283291
/**
284292
* Checks if new repositories appeared in or disappeared from cluster metadata and updates current list of

‎server/src/main/java/org/elasticsearch/repositories/VerifyNodeRepositoryAction.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ public void verify(String repository, String verificationToken, final ActionList
7575
final List<DiscoveryNode> nodes = new ArrayList<>();
7676
for (ObjectCursor<DiscoveryNode> cursor : masterAndDataNodes) {
7777
DiscoveryNode node = cursor.value;
78-
nodes.add(node);
78+
if (RepositoriesService.isDedicatedVotingOnlyNode(node.getRoles()) == false) {
79+
nodes.add(node);
80+
}
7981
}
8082
final CopyOnWriteArrayList<VerificationFailure> errors = new CopyOnWriteArrayList<>();
8183
final AtomicInteger counter = new AtomicInteger(nodes.size());

‎x-pack/plugin/voting-only-node/src/test/java/org/elasticsearch/cluster/coordination/VotingOnlyNodePluginTests.java‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,47 @@
55
*/
66
package org.elasticsearch.cluster.coordination;
77

8+
import org.elasticsearch.Version;
9+
import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryResponse;
10+
import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
11+
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
12+
import org.elasticsearch.client.Client;
13+
import org.elasticsearch.cluster.metadata.RepositoryMetadata;
14+
import org.elasticsearch.cluster.node.DiscoveryNode;
15+
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
16+
import org.elasticsearch.cluster.service.ClusterService;
817
import org.elasticsearch.common.Priority;
18+
import org.elasticsearch.common.blobstore.BlobStore;
919
import org.elasticsearch.common.settings.Settings;
20+
import org.elasticsearch.common.util.set.Sets;
21+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
1022
import org.elasticsearch.discovery.DiscoverySettings;
1123
import org.elasticsearch.discovery.MasterNotDiscoveredException;
24+
import org.elasticsearch.env.Environment;
25+
import org.elasticsearch.indices.recovery.RecoverySettings;
1226
import org.elasticsearch.plugins.Plugin;
27+
import org.elasticsearch.plugins.RepositoryPlugin;
28+
import org.elasticsearch.repositories.Repository;
29+
import org.elasticsearch.repositories.fs.FsRepository;
30+
import org.elasticsearch.snapshots.SnapshotInfo;
31+
import org.elasticsearch.snapshots.SnapshotState;
1332
import org.elasticsearch.test.ESIntegTestCase;
1433
import org.elasticsearch.test.ESIntegTestCase.Scope;
34+
import org.hamcrest.Matchers;
1535

36+
import java.util.Arrays;
1637
import java.util.Collection;
1738
import java.util.Collections;
39+
import java.util.List;
40+
import java.util.Map;
1841

1942
import static org.elasticsearch.test.NodeRoles.addRoles;
2043
import static org.elasticsearch.test.NodeRoles.onlyRole;
44+
import static org.elasticsearch.test.NodeRoles.onlyRoles;
45+
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
2146
import static org.hamcrest.CoreMatchers.containsString;
2247
import static org.hamcrest.CoreMatchers.equalTo;
48+
import static org.hamcrest.Matchers.greaterThan;
2349
import static org.hamcrest.Matchers.hasSize;
2450
import static org.hamcrest.Matchers.nullValue;
2551

@@ -28,7 +54,7 @@ public class VotingOnlyNodePluginTests extends ESIntegTestCase {
2854

2955
@Override
3056
protected Collection<Class<? extends Plugin>> nodePlugins() {
31-
return Collections.singleton(LocalStateVotingOnlyNodePlugin.class);
57+
return Arrays.asList(LocalStateVotingOnlyNodePlugin.class, RepositoryVerifyAccessPlugin.class);
3258
}
3359

3460
public void testRequireVotingOnlyNodeToBeMasterEligible() {
@@ -114,4 +140,86 @@ public void testVotingOnlyNodesCannotBeMasterWithoutFullMasterNodes() throws Exc
114140
final String newMasterId = client().admin().cluster().prepareState().get().getState().nodes().getMasterNodeId();
115141
assertNotEquals(oldMasterId, newMasterId);
116142
}
143+
144+
public void testBasicSnapshotRestoreWorkFlow() {
145+
internalCluster().setBootstrapMasterNodeIndex(0);
146+
internalCluster().startNodes(2);
147+
// dedicated voting-only master node
148+
final String dedicatedVotingOnlyNode = internalCluster().startNode(
149+
onlyRoles(Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE, VotingOnlyNodePlugin.VOTING_ONLY_NODE_ROLE)));
150+
// voting-only master node that also has data
151+
final String nonDedicatedVotingOnlyNode = internalCluster().startNode(
152+
addRoles(Sets.newHashSet(VotingOnlyNodePlugin.VOTING_ONLY_NODE_ROLE)));
153+
154+
assertAcked(client().admin().cluster().preparePutRepository("test-repo")
155+
.setType("verifyaccess-fs").setSettings(Settings.builder().put("location", randomRepoPath())
156+
.put("compress", randomBoolean())));
157+
createIndex("test-idx-1");
158+
createIndex("test-idx-2");
159+
createIndex("test-idx-3");
160+
ensureGreen();
161+
162+
VerifyRepositoryResponse verifyResponse = client().admin().cluster().prepareVerifyRepository("test-repo").get();
163+
// only the da
164+
assertEquals(3, verifyResponse.getNodes().size());
165+
assertTrue(verifyResponse.getNodes().stream().noneMatch(nw -> nw.getName().equals(dedicatedVotingOnlyNode)));
166+
assertTrue(verifyResponse.getNodes().stream().anyMatch(nw -> nw.getName().equals(nonDedicatedVotingOnlyNode)));
167+
168+
final String[] indicesToSnapshot = {"test-idx-*", "-test-idx-3"};
169+
170+
logger.info("--> snapshot");
171+
Client client = client();
172+
CreateSnapshotResponse createSnapshotResponse = client.admin().cluster().prepareCreateSnapshot("test-repo", "test-snap")
173+
.setWaitForCompletion(true).setIndices(indicesToSnapshot).get();
174+
assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(0));
175+
assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(),
176+
Matchers.equalTo(createSnapshotResponse.getSnapshotInfo().totalShards()));
177+
178+
List<SnapshotInfo> snapshotInfos = client.admin().cluster().prepareGetSnapshots("test-repo")
179+
.setSnapshots(randomFrom("test-snap", "_all", "*", "*-snap", "test*")).get().getSnapshots();
180+
assertThat(snapshotInfos.size(), Matchers.equalTo(1));
181+
SnapshotInfo snapshotInfo = snapshotInfos.get(0);
182+
assertThat(snapshotInfo.state(), Matchers.equalTo(SnapshotState.SUCCESS));
183+
assertThat(snapshotInfo.version(), Matchers.equalTo(Version.CURRENT));
184+
185+
logger.info("--> close indices");
186+
client.admin().indices().prepareClose("test-idx-1", "test-idx-2").get();
187+
188+
logger.info("--> restore all indices from the snapshot");
189+
RestoreSnapshotResponse restoreSnapshotResponse = client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap")
190+
.setWaitForCompletion(true).execute().actionGet();
191+
assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), greaterThan(0));
192+
193+
ensureGreen();
194+
}
195+
196+
public static class RepositoryVerifyAccessPlugin extends org.elasticsearch.plugins.Plugin implements RepositoryPlugin {
197+
198+
@Override
199+
public Map<String, Repository.Factory> getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry,
200+
ClusterService clusterService, RecoverySettings recoverySettings) {
201+
return Collections.singletonMap("verifyaccess-fs", (metadata) ->
202+
new AccessVerifyingRepo(metadata, env, namedXContentRegistry, clusterService, recoverySettings));
203+
}
204+
205+
private static class AccessVerifyingRepo extends FsRepository {
206+
207+
private final ClusterService clusterService;
208+
209+
private AccessVerifyingRepo(RepositoryMetadata metadata, Environment environment, NamedXContentRegistry namedXContentRegistry,
210+
ClusterService clusterService, RecoverySettings recoverySettings) {
211+
super(metadata, environment, namedXContentRegistry, clusterService, recoverySettings);
212+
this.clusterService = clusterService;
213+
}
214+
215+
@Override
216+
protected BlobStore createBlobStore() throws Exception {
217+
final DiscoveryNode localNode = clusterService.state().nodes().getLocalNode();
218+
if (localNode.getRoles().contains(VotingOnlyNodePlugin.VOTING_ONLY_NODE_ROLE)) {
219+
assertTrue(localNode.isDataNode());
220+
}
221+
return super.createBlobStore();
222+
}
223+
}
224+
}
117225
}

0 commit comments

Comments
 (0)