From 92a026b3b9be18803f11e49bd44ef0afbfdb6c13 Mon Sep 17 00:00:00 2001
From: Adrien Grand <jpountz@gmail.com>
Date: Tue, 21 Jan 2014 16:56:33 +0100
Subject: [PATCH] Throw an ElasticsearchIllegalArgumentException when
 allocating on a non-data node.

Today, it would fail with a NullPointerException.

Close #4833
---
 .../allocation/command/AllocateAllocationCommand.java    | 9 +++++++++
 .../routing/allocation/AllocationCommandsTests.java      | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/src/main/java/org/elasticsearch/cluster/routing/allocation/command/AllocateAllocationCommand.java b/src/main/java/org/elasticsearch/cluster/routing/allocation/command/AllocateAllocationCommand.java
index 2999cc6d7de..58301a06c34 100644
--- a/src/main/java/org/elasticsearch/cluster/routing/allocation/command/AllocateAllocationCommand.java
+++ b/src/main/java/org/elasticsearch/cluster/routing/allocation/command/AllocateAllocationCommand.java
@@ -21,6 +21,7 @@ package org.elasticsearch.cluster.routing.allocation.command;
 
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ElasticsearchIllegalArgumentException;
+import org.elasticsearch.ElasticsearchIllegalStateException;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.routing.MutableShardRouting;
@@ -183,6 +184,14 @@ public class AllocateAllocationCommand implements AllocationCommand {
         }
 
         RoutingNode routingNode = allocation.routingNodes().node(discoNode.id());
+        if (routingNode == null) {
+            if (!discoNode.dataNode()) {
+                throw new ElasticsearchIllegalArgumentException("Allocation can only be done on data nodes, not [" + node + "]");
+            } else {
+                throw new ElasticsearchIllegalStateException("Could not find [" + node + "] among the routing nodes");
+            }
+        }
+
         Decision decision = allocation.deciders().canAllocate(shardRouting, routingNode, allocation);
         if (decision.type() == Decision.Type.NO) {
             throw new ElasticsearchIllegalArgumentException("[allocate] allocation of " + shardId + " on node " + discoNode + " is not allowed, reason: " + decision);
diff --git a/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java b/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java
index 497bfb385dc..2e76b20a5b7 100644
--- a/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java
+++ b/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.cluster.routing.allocation;
 
+import com.google.common.collect.ImmutableMap;
 import org.elasticsearch.ElasticsearchIllegalArgumentException;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -117,6 +118,7 @@ public class AllocationCommandsTests extends ElasticsearchAllocationTestCase {
                 .put(newNode("node1"))
                 .put(newNode("node2"))
                 .put(newNode("node3"))
+                .put(newNode("node4", ImmutableMap.of("data", Boolean.FALSE.toString())))
         ).build();
         RoutingAllocation.Result rerouteResult = allocation.reroute(clusterState);
         clusterState = ClusterState.builder(clusterState).routingTable(rerouteResult.routingTable()).build();
@@ -129,6 +131,13 @@ public class AllocationCommandsTests extends ElasticsearchAllocationTestCase {
         } catch (ElasticsearchIllegalArgumentException e) {
         }
 
+        logger.info("--> allocating to non-data node, should fail");
+        try {
+            rerouteResult = allocation.reroute(clusterState, new AllocationCommands(new AllocateAllocationCommand(new ShardId("test", 0), "node4", true)));
+            fail();
+        } catch (ElasticsearchIllegalArgumentException e) {
+        }
+
         logger.info("--> allocating with primary flag set to true");
         rerouteResult = allocation.reroute(clusterState, new AllocationCommands(new AllocateAllocationCommand(new ShardId("test", 0), "node1", true)));
         assertThat(rerouteResult.changed(), equalTo(true));