Skip to content

Commit bc0e562

Browse files
psychbotSachin Kale
authored and
Sachin Kale
committed
Introducing verifylocally to perform verification without cluster being formed such that a node is able to connect to connect and has necessary permissions
Signed-off-by: Dharmesh 💤 <sdharms@amazon.com>
1 parent 23cc00f commit bc0e562

File tree

7 files changed

+101
-63
lines changed

7 files changed

+101
-63
lines changed

server/src/main/java/org/opensearch/action/admin/cluster/remotestore/RemoteStoreNode.java

+13-11
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@
1414
import org.opensearch.common.settings.Settings;
1515

1616
import java.util.ArrayList;
17-
import java.util.Collections;
17+
import java.util.HashSet;
1818
import java.util.List;
1919
import java.util.Locale;
2020
import java.util.Map;
2121
import java.util.Objects;
22+
import java.util.Set;
2223
import java.util.stream.Collectors;
2324

2425
/**
@@ -85,19 +86,20 @@ private RepositoryMetadata buildRepositoryMetadata(String name) {
8586
}
8687

8788
private RepositoriesMetadata buildRepositoriesMetadata() {
88-
String segmentRepositoryName = validateAttributeNonNull(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY);
89-
String translogRepositoryName = validateAttributeNonNull(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY);
90-
if (segmentRepositoryName.equals(translogRepositoryName)) {
91-
return new RepositoriesMetadata(Collections.singletonList(buildRepositoryMetadata(segmentRepositoryName)));
92-
} else {
93-
List<RepositoryMetadata> repositoryMetadataList = new ArrayList<>();
94-
repositoryMetadataList.add(buildRepositoryMetadata(segmentRepositoryName));
95-
repositoryMetadataList.add(buildRepositoryMetadata(translogRepositoryName));
96-
return new RepositoriesMetadata(repositoryMetadataList);
89+
List<RepositoryMetadata> repositoryMetadataList = new ArrayList<>();
90+
Set<String> repositoryNames = new HashSet<>();
91+
92+
repositoryNames.add(validateAttributeNonNull(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY));
93+
repositoryNames.add(validateAttributeNonNull(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY));
94+
95+
for (String repositoryName : repositoryNames) {
96+
repositoryMetadataList.add(buildRepositoryMetadata(repositoryName));
9797
}
98+
99+
return new RepositoriesMetadata(repositoryMetadataList);
98100
}
99101

100-
RepositoriesMetadata getRepositoriesMetadata() {
102+
public RepositoriesMetadata getRepositoriesMetadata() {
101103
return this.repositoriesMetadata;
102104
}
103105

server/src/main/java/org/opensearch/action/admin/cluster/remotestore/RemoteStoreService.java

+3-45
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,12 @@
1919
import org.opensearch.common.settings.Setting;
2020
import org.opensearch.repositories.RepositoriesService;
2121
import org.opensearch.repositories.Repository;
22-
import org.opensearch.repositories.RepositoryVerificationException;
2322
import org.opensearch.threadpool.ThreadPool;
2423

2524
import java.util.ArrayList;
2625
import java.util.Collections;
2726
import java.util.List;
2827
import java.util.Locale;
29-
import java.util.concurrent.CountDownLatch;
30-
import java.util.concurrent.TimeUnit;
3128
import java.util.function.Supplier;
3229

3330
/**
@@ -87,50 +84,11 @@ public RemoteStoreService(Supplier<RepositoriesService> repositoriesService, Thr
8784
* repository mentioned. This verification will happen on a local node to validate if the node is able to connect
8885
* to the repository.
8986
*/
90-
public void verifyRepository(List<Repository> repositories, DiscoveryNode localNode) {
91-
/*
87+
public void verifyRepositoriesLocally(List<Repository> repositories, DiscoveryNode localNode) {
9288
for (Repository repository : repositories) {
93-
String verificationToken = repository.startVerification();
9489
String repositoryName = repository.getMetadata().name();
95-
try {
96-
repository.verify(verificationToken, localNode);
97-
logger.info(() -> new ParameterizedMessage("successfully verified [{}] repository", repositoryName));
98-
} catch (Exception e) {
99-
logger.warn(() -> new ParameterizedMessage("[{}] failed to verify repository", repository), e);
100-
throw new RepositoryVerificationException(repositoryName, e.getMessage());
101-
}
102-
}
103-
Replace the below code with this once #9088 is merged.
104-
*/
105-
106-
for (Repository repository : repositories) {
107-
String verificationToken = repository.startVerification();
108-
String repositoryName = repository.getMetadata().name();
109-
CountDownLatch repositoryVerificationLatch = new CountDownLatch(1);
110-
threadPool.executor(ThreadPool.Names.GENERIC).execute(() -> {
111-
try {
112-
repository.verify(verificationToken, localNode);
113-
logger.info(() -> new ParameterizedMessage("successfully verified [{}] repository", repositoryName));
114-
repositoryVerificationLatch.countDown();
115-
} catch (Exception e) {
116-
logger.warn(() -> new ParameterizedMessage("[{}] failed to verify repository", repository), e);
117-
throw new RepositoryVerificationException(repositoryName, e.getMessage());
118-
}
119-
});
120-
121-
// TODO: See if using listener here which is async makes sense, made this sync as
122-
// we need the repository registration for remote store backed node to be completed before the
123-
// bootstrap completes.
124-
try {
125-
if (repositoryVerificationLatch.await(1000, TimeUnit.MILLISECONDS) == false) {
126-
throw new RepositoryVerificationException(
127-
repository.getMetadata().name(),
128-
"could not complete " + "repository verification within timeout."
129-
);
130-
}
131-
} catch (InterruptedException e) {
132-
throw new RepositoryVerificationException(repository.getMetadata().name(), e.getMessage());
133-
}
90+
repository.verifyLocally(localNode);
91+
logger.info(() -> new ParameterizedMessage("successfully verified [{}] repository", repositoryName));
13492
}
13593
}
13694

server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ public static DiscoveryNode createLocal(
305305
);
306306
RemoteStoreNode remoteStoreNode = new RemoteStoreNode(discoveryNode);
307307
List<Repository> repositories = remoteStoreService.createRepositories(remoteStoreNode);
308-
remoteStoreService.verifyRepository(repositories, discoveryNode);
308+
remoteStoreService.verifyRepositoriesLocally(repositories, discoveryNode);
309309
return discoveryNode;
310310
}
311311

server/src/main/java/org/opensearch/repositories/FilterRepository.java

+5
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ public void verify(String verificationToken, DiscoveryNode localNode) {
162162
in.verify(verificationToken, localNode);
163163
}
164164

165+
@Override
166+
public void verifyLocally(DiscoveryNode localNode) {
167+
in.verifyLocally(localNode);
168+
}
169+
165170
@Override
166171
public boolean isReadOnly() {
167172
return in.isReadOnly();

server/src/main/java/org/opensearch/repositories/Repository.java

+7
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,13 @@ default RepositoryStats stats() {
241241
*/
242242
void verify(String verificationToken, DiscoveryNode localNode);
243243

244+
/**
245+
* Verifies repository settings on local node by reading and writing files onto blobstore without the
246+
* cluster-manager.
247+
* @param localNode the local node information
248+
*/
249+
void verifyLocally(DiscoveryNode localNode);
250+
244251
/**
245252
* Returns true if the repository supports only read operations
246253
* @return true if the repository is read/only

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

+69-6
Original file line numberDiff line numberDiff line change
@@ -3123,12 +3123,6 @@ public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, In
31233123

31243124
@Override
31253125
public void verify(String seed, DiscoveryNode localNode) {
3126-
/*
3127-
if(!isSystemRepository) {
3128-
assertSnapshotOrGenericThread();
3129-
}
3130-
Update the assertion method invocation with this once #9088 is merged.
3131-
*/
31323126
assertSnapshotOrGenericThread();
31333127
if (isReadOnly()) {
31343128
try {
@@ -3182,6 +3176,75 @@ public void verify(String seed, DiscoveryNode localNode) {
31823176
}
31833177
}
31843178

3179+
@Override
3180+
public void verifyLocally(DiscoveryNode localNode) {
3181+
String seed = UUIDs.randomBase64UUID();
3182+
if (isReadOnly()) {
3183+
try {
3184+
latestIndexBlobId();
3185+
} catch (Exception e) {
3186+
throw new RepositoryVerificationException(
3187+
metadata.name(),
3188+
"path " + basePath() + " is not accessible on node " + localNode,
3189+
e
3190+
);
3191+
}
3192+
} else {
3193+
BlobContainer testBlobContainer = blobStore().blobContainer(basePath().add(testBlobPrefix(seed)));
3194+
String blobName = "data-" + localNode.getId() + ".dat";
3195+
3196+
// Writing test data to the repository
3197+
try {
3198+
BytesArray bytes = new BytesArray(seed);
3199+
try (InputStream stream = bytes.streamInput()) {
3200+
testBlobContainer.writeBlob(blobName, stream, bytes.length(), true);
3201+
}
3202+
} catch (Exception exp) {
3203+
throw new RepositoryVerificationException(
3204+
metadata.name(),
3205+
"store location [" + blobStore() + "] is not accessible on the node [" + localNode + "]",
3206+
exp
3207+
);
3208+
}
3209+
3210+
// Reading test data from the repository
3211+
try (InputStream localNodeDat = testBlobContainer.readBlob(blobName)) {
3212+
final String seedRead = Streams.readFully(localNodeDat).utf8ToString();
3213+
if (seedRead.equals(seed) == false) {
3214+
throw new RepositoryVerificationException(
3215+
metadata.name(),
3216+
"Seed read was [" + seedRead + "] but expected seed [" + seed + "]"
3217+
);
3218+
}
3219+
} catch (NoSuchFileException e) {
3220+
throw new RepositoryVerificationException(
3221+
metadata.name(),
3222+
"a file written to the store ["
3223+
+ blobStore()
3224+
+ "] cannot be accessed on the node ["
3225+
+ localNode
3226+
+ "]. "
3227+
+ "This might indicate that the store ["
3228+
+ blobStore()
3229+
+ "] permissions don't allow reading files",
3230+
e
3231+
);
3232+
} catch (Exception e) {
3233+
throw new RepositoryVerificationException(metadata.name(), "Failed to verify repository", e);
3234+
}
3235+
3236+
// Trying to delete the repository once the write and read verification completes. We wont fail the
3237+
// verification if the detete fails.
3238+
// TODO: See if there is a better way to handle this deletion failure.
3239+
try {
3240+
final String testPrefix = testBlobPrefix(seed);
3241+
blobStore().blobContainer(basePath().add(testPrefix)).delete();
3242+
} catch (Exception exp) {
3243+
logger.warn(() -> new ParameterizedMessage("[{}] cannot delete test data at {} {}", metadata.name(), basePath(), exp));
3244+
}
3245+
}
3246+
}
3247+
31853248
@Override
31863249
public String toString() {
31873250
return "BlobStoreRepository[" + "[" + metadata.name() + "], [" + blobStore.get() + ']' + ']';

test/framework/src/main/java/org/opensearch/index/shard/RestoreOnlyRepository.java

+3
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, In
195195
@Override
196196
public void verify(String verificationToken, DiscoveryNode localNode) {}
197197

198+
@Override
199+
public void verifyLocally(DiscoveryNode localNode) {}
200+
198201
@Override
199202
public void updateState(final ClusterState state) {}
200203

0 commit comments

Comments
 (0)