Skip to content

Commit

Permalink
Show only committed cluster UUID in GET / (#114275)
Browse files Browse the repository at this point in the history
Today we show `Metadata#clusterUUID` in the response to `GET /`
regardless of whether this value is committed or not, which means that
in theory users may see this value change even if nothing is going
wrong. To avoid any doubt about the stability of this cluster UUID, this
commit suppresses the cluster UUID in this API response until it is
committed.
  • Loading branch information
DaveCTurner authored Oct 8, 2024
1 parent 4ecc5bd commit bb9d612
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
Expand Down Expand Up @@ -48,7 +49,7 @@ protected void doExecute(Task task, MainRequest request, ActionListener<MainResp
nodeName,
IndexVersion.current().luceneVersion().toString(),
clusterState.getClusterName(),
clusterState.metadata().clusterUUID(),
clusterState.metadata().clusterUUIDCommitted() ? clusterState.metadata().clusterUUID() : Metadata.UNKNOWN_CLUSTER_UUID,
Build.current()
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@

package org.elasticsearch.rest.root;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.ActionTestUtils;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.block.ClusterBlock;
import org.elasticsearch.cluster.block.ClusterBlockLevel;
import org.elasticsearch.cluster.block.ClusterBlocks;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.RestStatus;
Expand All @@ -26,7 +27,7 @@
import org.elasticsearch.transport.TransportService;

import java.util.Collections;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.atomic.AtomicBoolean;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
Expand All @@ -39,7 +40,7 @@ public void testMainActionClusterAvailable() {
final ClusterService clusterService = mock(ClusterService.class);
final ClusterName clusterName = new ClusterName("elasticsearch");
final Settings settings = Settings.builder().put("node.name", "my-node").build();
ClusterBlocks blocks;
final ClusterBlocks blocks;
if (randomBoolean()) {
if (randomBoolean()) {
blocks = ClusterBlocks.EMPTY_CLUSTER_BLOCK;
Expand Down Expand Up @@ -73,7 +74,12 @@ public void testMainActionClusterAvailable() {
)
.build();
}
ClusterState state = ClusterState.builder(clusterName).blocks(blocks).build();
final Metadata.Builder metadata = new Metadata.Builder();
if (randomBoolean()) {
metadata.clusterUUID(randomUUID());
metadata.clusterUUIDCommitted(randomBoolean());
}
final ClusterState state = ClusterState.builder(clusterName).metadata(metadata).blocks(blocks).build();
when(clusterService.state()).thenReturn(state);

TransportService transportService = new TransportService(
Expand All @@ -85,21 +91,21 @@ public void testMainActionClusterAvailable() {
null,
Collections.emptySet()
);
TransportMainAction action = new TransportMainAction(settings, transportService, mock(ActionFilters.class), clusterService);
AtomicReference<MainResponse> responseRef = new AtomicReference<>();
action.doExecute(mock(Task.class), new MainRequest(), new ActionListener<>() {
@Override
public void onResponse(MainResponse mainResponse) {
responseRef.set(mainResponse);
}

@Override
public void onFailure(Exception e) {
logger.error("unexpected error", e);
}
});
final AtomicBoolean listenerCalled = new AtomicBoolean();
new TransportMainAction(settings, transportService, mock(ActionFilters.class), clusterService).doExecute(
mock(Task.class),
new MainRequest(),
ActionTestUtils.assertNoFailureListener(mainResponse -> {
assertNotNull(mainResponse);
assertEquals(
state.metadata().clusterUUIDCommitted() ? state.metadata().clusterUUID() : Metadata.UNKNOWN_CLUSTER_UUID,
mainResponse.getClusterUuid()
);
assertFalse(listenerCalled.getAndSet(true));
})
);

assertNotNull(responseRef.get());
assertTrue(listenerCalled.get());
verify(clusterService, times(1)).state();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,11 @@ public long version() {
return this.version;
}

/**
* @return A UUID which identifies this cluster. Nodes record the UUID of the cluster they first join on disk, and will then refuse to
* join clusters with different UUIDs. Note that when the cluster is forming for the first time this value may not yet be committed,
* and therefore it may change. Check {@link #clusterUUIDCommitted()} to verify that the value is committed if needed.
*/
public String clusterUUID() {
return this.clusterUUID;
}
Expand Down

0 comments on commit bb9d612

Please sign in to comment.