upload committed voting config to remote in state commit phase

Signed-off-by: Shivansh Arora <[email protected]>

server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteStatePublicationIT.java

@@ -11,9 +11,11 @@
 import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
 import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
 import org.opensearch.client.Client;
+import org.opensearch.cluster.coordination.PersistedStateRegistry;
 import org.opensearch.common.blobstore.BlobPath;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.FeatureFlags;
+import org.opensearch.gateway.GatewayMetaState;
 import org.opensearch.gateway.remote.model.RemoteClusterMetadataManifest;
 import org.opensearch.indices.recovery.RecoverySettings;
 import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase;
@@ -41,9 +43,6 @@
 import static org.opensearch.gateway.remote.model.RemotePersistentSettingsMetadata.SETTING_METADATA;
 import static org.opensearch.gateway.remote.model.RemoteTemplatesMetadata.TEMPLATES_METADATA;
 import static org.opensearch.gateway.remote.model.RemoteTransientSettingsMetadata.TRANSIENT_SETTING_METADATA;
-import org.opensearch.cluster.coordination.PersistedStateRegistry;
-import org.opensearch.gateway.GatewayMetaState;
-
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY;

server/src/main/java/org/opensearch/gateway/GatewayMetaState.java

@@ -786,10 +786,14 @@ public void markLastAcceptedStateAsCommitted() {
                 assert lastAcceptedState != null : "Last accepted state is not present";
                 assert lastAcceptedManifest != null : "Last accepted manifest is not present";
                 ClusterState clusterState = lastAcceptedState;
-                Metadata.Builder metadataBuilder = commitVotingConfiguration(lastAcceptedState);
+                boolean shouldCommitVotingConfig = shouldCommitVotingConfig();
+
                 if (lastAcceptedState.metadata().clusterUUID().equals(Metadata.UNKNOWN_CLUSTER_UUID) == false
                     && lastAcceptedState.metadata().clusterUUIDCommitted() == false) {
-                    if (metadataBuilder == null) {
+                    Metadata.Builder metadataBuilder;
+                    if (shouldCommitVotingConfig) {
+                        metadataBuilder = commitVotingConfiguration(lastAcceptedState);
+                    } else {
                         metadataBuilder = Metadata.builder(lastAcceptedState.metadata());
                     }
                     metadataBuilder.clusterUUIDCommitted(true);
@@ -798,7 +802,8 @@ public void markLastAcceptedStateAsCommitted() {
                 if (clusterState.getNodes().isLocalNodeElectedClusterManager()) {
                     final RemoteClusterStateManifestInfo committedManifestDetails = remoteClusterStateService.markLastStateAsCommitted(
                         clusterState,
-                        lastAcceptedManifest
+                        lastAcceptedManifest,
+                        shouldCommitVotingConfig
                     );
                     assert committedManifestDetails != null;
                     setLastAcceptedManifest(committedManifestDetails.getClusterMetadataManifest());
@@ -817,6 +822,10 @@ public void close() throws IOException {
             remoteClusterStateService.close();
         }
 
+        private boolean shouldCommitVotingConfig() {
+            return !lastAcceptedState.getLastAcceptedConfiguration().equals(lastAcceptedState.getLastCommittedConfiguration());
+        }
+
         private void handleExceptionOnWrite(Exception e) {
             throw ExceptionsHelper.convertToRuntime(e);
         }

server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java

@@ -850,18 +850,41 @@ public RemoteClusterStateCleanupManager getCleanupManager() {
     }
 
     @Nullable
-    public RemoteClusterStateManifestInfo markLastStateAsCommitted(ClusterState clusterState, ClusterMetadataManifest previousManifest)
-        throws IOException {
+    public RemoteClusterStateManifestInfo markLastStateAsCommitted(
+        ClusterState clusterState,
+        ClusterMetadataManifest previousManifest,
+        boolean commitVotingConfig
+    ) throws IOException {
         assert clusterState != null : "Last accepted cluster state is not set";
         if (clusterState.nodes().isLocalNodeElectedClusterManager() == false) {
             logger.error("Local node is not elected cluster manager. Exiting");
             return null;
         }
         assert previousManifest != null : "Last cluster metadata manifest is not set";
+        UploadedMetadataAttribute uploadedCoordinationMetadata = previousManifest.getCoordinationMetadata();
+        if (commitVotingConfig) {
+            // update the coordination metadata if voting config is committed
+            uploadedCoordinationMetadata = writeMetadataInParallel(
+                clusterState,
+                emptyList(),
+                emptyMap(),
+                emptyMap(),
+                true,
+                false,
+                false,
+                false,
+                false,
+                false,
+                emptyMap(),
+                false,
+                emptyList(),
+                emptyMap()
+            ).uploadedCoordinationMetadata;
+        }
         UploadedMetadataResults uploadedMetadataResults = new UploadedMetadataResults(
             previousManifest.getIndices(),
             previousManifest.getCustomMetadataMap(),
-            previousManifest.getCoordinationMetadata(),
+            uploadedCoordinationMetadata,
             previousManifest.getSettingsMetadata(),
             previousManifest.getTemplatesMetadata(),
             previousManifest.getTransientSettingsMetadata(),

server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java

@@ -78,6 +78,7 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.nullValue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -977,16 +978,15 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep
         Mockito.verify(remoteClusterStateService, Mockito.times(1)).writeFullMetadata(clusterState, previousClusterUUID);
         assertThat(persistedStateRegistry.getPersistedState(PersistedStateType.REMOTE).getLastAcceptedState(), equalTo(clusterState));
 
-        when(remoteClusterStateService.markLastStateAsCommitted(any(), any())).thenReturn(
+        when(remoteClusterStateService.markLastStateAsCommitted(any(), any(), eq(false))).thenReturn(
             new RemoteClusterStateManifestInfo(manifest, "path/to/manifest")
         );
         coordinationState.handlePreCommit();
         ClusterState committedClusterState = ClusterState.builder(clusterState)
             .metadata(Metadata.builder(clusterState.metadata()).clusterUUIDCommitted(true).build())
             .build();
-        // Mockito.verify(remoteClusterStateService, Mockito.times(1)).markLastStateAsCommitted(committedClusterState, manifest);
         ArgumentCaptor<ClusterState> clusterStateCaptor = ArgumentCaptor.forClass(ClusterState.class);
-        verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(clusterStateCaptor.capture(), any());
+        verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(clusterStateCaptor.capture(), any(), eq(false));
         assertThat(clusterStateCaptor.getValue().metadata().indices(), equalTo(committedClusterState.metadata().indices()));
         assertThat(clusterStateCaptor.getValue().metadata().clusterUUID(), equalTo(committedClusterState.metadata().clusterUUID()));
         assertThat(clusterStateCaptor.getValue().stateUUID(), equalTo(committedClusterState.stateUUID()));

server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java

@@ -109,6 +109,7 @@
 import static org.hamcrest.Matchers.nullValue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -773,11 +774,11 @@ public void testRemotePersistedState() throws IOException {
         assertThat(remotePersistedState.getLastAcceptedState(), equalTo(secondClusterState));
         assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm));
 
-        when(remoteClusterStateService.markLastStateAsCommitted(Mockito.any(), Mockito.any())).thenReturn(
+        when(remoteClusterStateService.markLastStateAsCommitted(Mockito.any(), Mockito.any(), eq(false))).thenReturn(
             new RemoteClusterStateManifestInfo(manifest, "path/to/manifest")
         );
         remotePersistedState.markLastAcceptedStateAsCommitted();
-        Mockito.verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(Mockito.any(), Mockito.any());
+        Mockito.verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(Mockito.any(), Mockito.any(), eq(false));
 
         assertThat(remotePersistedState.getLastAcceptedState(), equalTo(secondClusterState));
         assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm));
@@ -840,7 +841,7 @@ public void testRemotePersistentState_FollowerNode() throws IOException {
         assertFalse(remotePersistedState.getLastAcceptedManifest().isCommitted());
 
         remotePersistedState.markLastAcceptedStateAsCommitted();
-        Mockito.verify(remoteClusterStateService, never()).markLastStateAsCommitted(Mockito.any(), Mockito.any());
+        Mockito.verify(remoteClusterStateService, never()).markLastStateAsCommitted(Mockito.any(), Mockito.any(), eq(false));
 
         assertEquals(secondClusterState, remotePersistedState.getLastAcceptedState());
         assertEquals(clusterTerm, remotePersistedState.getCurrentTerm());

server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java

@@ -2466,7 +2466,7 @@ public void testMarkLastStateAsCommittedSuccess() throws IOException {
         List<UploadedIndexMetadata> indices = List.of(uploadedIndexMetadata);
         final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(indices).build();
 
-        final ClusterMetadataManifest manifest = remoteClusterStateService.markLastStateAsCommitted(clusterState, previousManifest)
+        final ClusterMetadataManifest manifest = remoteClusterStateService.markLastStateAsCommitted(clusterState, previousManifest, false)
             .getClusterMetadataManifest();
 
         final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder()