TemplateUpgradeService should only run on the master (#27294)

The `TemplateUpgradeService` allows plugins to register a call back that mutates index templates upon recovery. This is handy for upgrade logic that needs to make sure that an existing index template is updated once the cluster is upgraded to a new version of the plugin (and ES).

Currently, the service has complicated logic to decide which node should perform the upgrade. It will prefer the master node, if it is of the highest version of the cluster and otherwise it will fall back to one of the non-coordinating nodes which are on the latest version. While this attempts to make sure that new nodes can assume their template version is in place (but old node still need to be able to operate under both old and new template), it has an inherent problem in that the master (on an old version) may not be able to process the put template request with the new template - it may miss certain features.

This PR changes the logic to be simpler and always rely on the current master nodes. This comes at the price that new nodes need to operate both with old templates and new. That price is small as they need to operate with old indices regardless of the template. On the flip side we reduce a lot of complexity in what can happen in the cluster.

Author: bleskes

core/src/main/java/org/elasticsearch/cluster/metadata/TemplateUpgradeService.java

@@ -116,7 +116,7 @@ public void clusterChanged(ClusterChangedEvent event) {
             return;
         }
 
-        if (shouldLocalNodeUpdateTemplates(state.nodes()) == false) {
+        if (state.nodes().isLocalNodeElectedMaster() == false) {
             return;
         }
 
@@ -133,43 +133,6 @@ public void clusterChanged(ClusterChangedEvent event) {
         }
     }
 
-    /**
-     * Checks if the current node should update the templates
-     *
-     * If the master has the newest verison in the cluster - it will be dedicated template updater.
-     * Otherwise the node with the highest id among nodes with the highest version should update the templates
-     */
-    boolean shouldLocalNodeUpdateTemplates(DiscoveryNodes nodes) {
-        DiscoveryNode localNode = nodes.getLocalNode();
-        // Only data and master nodes should update the template
-        if (localNode.isDataNode() || localNode.isMasterNode()) {
-            DiscoveryNode masterNode = nodes.getMasterNode();
-            if (masterNode == null) {
-                return false;
-            }
-            Version maxVersion = nodes.getLargestNonClientNodeVersion();
-            if (maxVersion.equals(masterNode.getVersion())) {
-                // If the master has the latest version - we will allow it to handle the update
-                return nodes.isLocalNodeElectedMaster();
-            } else {
-                if (maxVersion.equals(localNode.getVersion()) == false) {
-                    // The localhost node doesn't have the latest version - not going to update
-                    return false;
-                }
-                for (ObjectCursor<DiscoveryNode> node : nodes.getMasterAndDataNodes().values()) {
-                    if (node.value.getVersion().equals(maxVersion) && node.value.getId().compareTo(localNode.getId()) > 0) {
-                        // We have a node with higher id then mine - it should update
-                        return false;
-                    }
-                }
-                // We have the highest version and highest id - we should perform the update
-                return true;
-            }
-        } else {
-            return false;
-        }
-    }
-
     void updateTemplates(Map<String, BytesReference> changes, Set<String> deletions) {
         for (Map.Entry<String, BytesReference> change : changes.entrySet()) {
             PutIndexTemplateRequest request =

core/src/test/java/org/elasticsearch/cluster/metadata/TemplateUpgradeServiceTests.java

@@ -341,51 +341,6 @@ public void testClusterStateUpdate() {
 
     private static final int NODE_TEST_ITERS = 100;
 
-    public void testOnlyOneNodeRunsTemplateUpdates() {
-        TemplateUpgradeService service = new TemplateUpgradeService(Settings.EMPTY, null, clusterService, null, Collections.emptyList());
-        for (int i = 0; i < NODE_TEST_ITERS; i++) {
-            int nodesCount = randomIntBetween(1, 10);
-            int clientNodesCount = randomIntBetween(0, 4);
-            DiscoveryNodes nodes = randomNodes(nodesCount, clientNodesCount);
-            int updaterNode = -1;
-            for (int j = 0; j < nodesCount; j++) {
-                DiscoveryNodes localNodes = DiscoveryNodes.builder(nodes).localNodeId(nodes.resolveNode("node_" + j).getId()).build();
-                if (service.shouldLocalNodeUpdateTemplates(localNodes)) {
-                    assertThat("Expected only one node to update template, found " + updaterNode + " and " + j, updaterNode, lessThan(0));
-                    updaterNode = j;
-                }
-            }
-            assertThat("Expected one node to update template", updaterNode, greaterThanOrEqualTo(0));
-        }
-    }
-
-    public void testIfMasterHasTheHighestVersionItShouldRunsTemplateUpdates() {
-        for (int i = 0; i < NODE_TEST_ITERS; i++) {
-            int nodesCount = randomIntBetween(1, 10);
-            int clientNodesCount = randomIntBetween(0, 4);
-            DiscoveryNodes nodes = randomNodes(nodesCount, clientNodesCount);
-            DiscoveryNodes.Builder builder = DiscoveryNodes.builder(nodes).localNodeId(nodes.resolveNode("_master").getId());
-            nodes = builder.build();
-            TemplateUpgradeService service = new TemplateUpgradeService(Settings.EMPTY, null, clusterService, null,
-                Collections.emptyList());
-            assertThat(service.shouldLocalNodeUpdateTemplates(nodes),
-                equalTo(nodes.getLargestNonClientNodeVersion().equals(nodes.getMasterNode().getVersion())));
-        }
-    }
-
-    public void testClientNodeDontRunTemplateUpdates() {
-        for (int i = 0; i < NODE_TEST_ITERS; i++) {
-            int nodesCount = randomIntBetween(1, 10);
-            int clientNodesCount = randomIntBetween(1, 4);
-            DiscoveryNodes nodes = randomNodes(nodesCount, clientNodesCount);
-            int testClient = randomIntBetween(0, clientNodesCount - 1);
-            DiscoveryNodes.Builder builder = DiscoveryNodes.builder(nodes).localNodeId(nodes.resolveNode("client_" + testClient).getId());
-            TemplateUpgradeService service = new TemplateUpgradeService(Settings.EMPTY, null, clusterService, null,
-                Collections.emptyList());
-            assertThat(service.shouldLocalNodeUpdateTemplates(builder.build()), equalTo(false));
-        }
-    }
-
     private DiscoveryNodes randomNodes(int dataAndMasterNodes, int clientNodes) {
         DiscoveryNodes.Builder builder = DiscoveryNodes.builder();
         String masterNodeId = null;