Merge pull request #16210 from javanna/enhancement/node_ingest_setting

Use Setting class to register node.ingest setting

core/src/main/java/org/elasticsearch/action/ingest/IngestProxyActionFilter.java

@@ -35,17 +35,10 @@ import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.common.Randomness;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.node.NodeModule;
 import org.elasticsearch.tasks.Task;
-import org.elasticsearch.threadpool.ThreadPool;
-import org.elasticsearch.transport.TransportException;
 import org.elasticsearch.transport.TransportResponse;
-import org.elasticsearch.transport.TransportResponseHandler;
 import org.elasticsearch.transport.TransportService;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
 public final class IngestProxyActionFilter implements ActionFilter {
@@ -110,7 +103,7 @@ public final class IngestProxyActionFilter implements ActionFilter {
     }
 
     private DiscoveryNode randomIngestNode() {
-        assert NodeModule.isNodeIngestEnabled(clusterService.localNode().attributes()) == false;
+        assert clusterService.localNode().isIngestNode() == false;
         DiscoveryNodes nodes = clusterService.state().getNodes();
         DiscoveryNode[] ingestNodes = nodes.getIngestNodes().values().toArray(DiscoveryNode.class);
         if (ingestNodes.length == 0) {

core/src/main/java/org/elasticsearch/client/transport/TransportClient.java

@@ -19,10 +19,6 @@
 
 package org.elasticsearch.client.transport;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.TimeUnit;
-
 import org.elasticsearch.Version;
 import org.elasticsearch.action.Action;
 import org.elasticsearch.action.ActionListener;
@@ -49,7 +45,6 @@ import org.elasticsearch.common.settings.SettingsModule;
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.indices.breaker.CircuitBreakerModule;
 import org.elasticsearch.monitor.MonitorService;
-import org.elasticsearch.node.NodeModule;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.PluginsModule;
@@ -60,6 +55,10 @@ import org.elasticsearch.threadpool.ThreadPoolModule;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.transport.netty.NettyTransport;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
 import static org.elasticsearch.common.settings.Settings.settingsBuilder;
 
 /**
@@ -117,7 +116,7 @@ public class TransportClient extends AbstractClient {
                 .put("node.client", true)
                 .put(CLIENT_TYPE_SETTING, CLIENT_TYPE);
             return new PluginsService(settingsBuilder.build(), null, null, pluginClasses);
-        };
+        }
 
         /**
          * Builds a new instance of the transport client.
@@ -151,8 +150,7 @@ public class TransportClient extends AbstractClient {
                         // noop
                     }
                 });
-                boolean ingestEnabled = NodeModule.isNodeIngestEnabled(settings);
+                modules.add(new ActionModule(false, true));
-                modules.add(new ActionModule(ingestEnabled, true));
                 modules.add(new CircuitBreakerModule(settings));
 
                 pluginsService.processModules(modules);

core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java

@@ -32,6 +32,7 @@ import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.common.transport.TransportAddressSerializers;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.node.Node;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -87,6 +88,10 @@ public class DiscoveryNode implements Streamable, ToXContent {
         return Booleans.isExplicitTrue(data);
     }
 
+    public static boolean ingestNode(Settings settings) {
+        return Node.NODE_INGEST_SETTING.get(settings);
+    }
+
     public static final List<DiscoveryNode> EMPTY_LIST = Collections.emptyList();
 
     private String nodeName = "";
@@ -316,6 +321,14 @@ public class DiscoveryNode implements Streamable, ToXContent {
         return masterNode();
     }
 
+    /**
+     * Returns a boolean that tells whether this an ingest node or not
+     */
+    public boolean isIngestNode() {
+        String ingest = attributes.get("ingest");
+        return ingest == null ? true : Booleans.parseBooleanExact(ingest);
+    }
+
     public Version version() {
         return this.version;
     }

core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java

@@ -31,7 +31,6 @@ import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.transport.TransportAddress;
-import org.elasticsearch.node.NodeModule;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -678,7 +677,7 @@ public class DiscoveryNodes extends AbstractDiffable<DiscoveryNodes> implements
                     masterNodesBuilder.put(nodeEntry.key, nodeEntry.value);
                     minNonClientNodeVersion = Version.smallest(minNonClientNodeVersion, nodeEntry.value.version());
                 }
-                if (NodeModule.isNodeIngestEnabled(nodeEntry.value.getAttributes())) {
+                if (nodeEntry.value.isIngestNode()) {
                     ingestNodesBuilder.put(nodeEntry.key, nodeEntry.value);
                 }
                 minNodeVersion = Version.smallest(minNodeVersion, nodeEntry.value.version());

core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -44,6 +44,7 @@ import org.elasticsearch.index.store.IndexStoreConfig;
 import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService;
 import org.elasticsearch.indices.recovery.RecoverySettings;
 import org.elasticsearch.indices.ttl.IndicesTTLService;
+import org.elasticsearch.node.Node;
 import org.elasticsearch.search.SearchService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.Transport;
@@ -158,5 +159,6 @@ public final class ClusterSettings extends AbstractScopedSettings {
         Transport.TRANSPORT_TCP_COMPRESS,
         IndexSettings.QUERY_STRING_ANALYZE_WILDCARD,
         IndexSettings.QUERY_STRING_ALLOW_LEADING_WILDCARD,
-        PrimaryShardAllocator.NODE_INITIAL_SHARDS_SETTING)));
+        PrimaryShardAllocator.NODE_INITIAL_SHARDS_SETTING,
+        Node.NODE_INGEST_SETTING)));
 }

core/src/main/java/org/elasticsearch/node/Node.java

@@ -30,6 +30,7 @@ import org.elasticsearch.cluster.ClusterModule;
 import org.elasticsearch.cluster.ClusterNameModule;
 import org.elasticsearch.cluster.ClusterService;
 import org.elasticsearch.cluster.action.index.MappingUpdatedAction;
+import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.routing.RoutingService;
 import org.elasticsearch.common.StopWatch;
 import org.elasticsearch.common.component.Lifecycle;
@@ -46,6 +47,7 @@ import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
 import org.elasticsearch.common.settings.SettingsModule;
@@ -119,6 +121,7 @@ import static org.elasticsearch.common.settings.Settings.settingsBuilder;
  */
 public class Node implements Releasable {
 
+    public static final Setting<Boolean> NODE_INGEST_SETTING = Setting.boolSetting("node.ingest", true, false, Setting.Scope.CLUSTER);
     private static final String CLIENT_TYPE = "node";
     public static final String HTTP_ENABLED = "http.enabled";
     private final Lifecycle lifecycle = new Lifecycle();
@@ -190,8 +193,7 @@ public class Node implements Releasable {
             modules.add(new ClusterModule(this.settings));
             modules.add(new IndicesModule());
             modules.add(new SearchModule(settings, namedWriteableRegistry));
-            boolean ingestEnabled = NodeModule.isNodeIngestEnabled(settings);
+            modules.add(new ActionModule(DiscoveryNode.ingestNode(settings), false));
-            modules.add(new ActionModule(ingestEnabled, false));
             modules.add(new GatewayModule(settings));
             modules.add(new NodeClientModule());
             modules.add(new PercolatorModule());

core/src/main/java/org/elasticsearch/node/NodeModule.java

@@ -20,10 +20,7 @@
 package org.elasticsearch.node;
 
 import org.elasticsearch.cache.recycler.PageCacheRecycler;
-import org.elasticsearch.common.Booleans;
-import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.ingest.ProcessorsRegistry;
 import org.elasticsearch.ingest.core.Processor;
@@ -113,14 +110,4 @@ public class NodeModule extends AbstractModule {
     public void registerProcessor(String type, Function<TemplateService, Processor.Factory<?>> processorFactoryProvider) {
         processorsRegistry.registerProcessor(type, processorFactoryProvider);
     }
-
-    public static boolean isNodeIngestEnabled(Settings settings) {
-        return settings.getAsBoolean("node.ingest", true);
-    }
-
-    public static boolean isNodeIngestEnabled(ImmutableOpenMap<String, String> nodeAttributes) {
-        String ingestEnabled = nodeAttributes.get("ingest");
-        //reproduces same logic used in settings.getAsBoolean used above
-        return Booleans.parseBoolean(ingestEnabled, true);
-    }
 }

core/src/test/java/org/elasticsearch/action/ingest/IngestProxyActionFilterTests.java

@@ -34,7 +34,6 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.common.transport.DummyTransportAddress;
-import org.elasticsearch.node.NodeModule;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.VersionUtils;
@@ -246,7 +245,7 @@ public class IngestProxyActionFilterTests extends ESTestCase {
 
         @Override
         protected boolean matchesSafely(DiscoveryNode node) {
-            return NodeModule.isNodeIngestEnabled(node.getAttributes());
+            return node.isIngestNode();
         }
     }
 }

core/src/test/java/org/elasticsearch/node/NodeModuleTests.java

@@ -1,80 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.node;
-
-import org.elasticsearch.common.collect.ImmutableOpenMap;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.test.ESTestCase;
-
-import static org.hamcrest.Matchers.equalTo;
-
-public class NodeModuleTests extends ESTestCase {
-
-    public void testIsNodeIngestEnabledSettings() {
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.EMPTY), equalTo(true));
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.builder().put("node.ingest", true).build()), equalTo(true));
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.builder().put("node.ingest", "true").build()), equalTo(true));
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.builder().put("node.ingest", false).build()), equalTo(false));
-
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.builder().put("node.ingest", "false").build()), equalTo(false));
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.builder().put("node.ingest", "off").build()), equalTo(false));
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.builder().put("node.ingest", "no").build()), equalTo(false));
-        assertThat(NodeModule.isNodeIngestEnabled(Settings.builder().put("node.ingest", "0").build()), equalTo(false));
-    }
-
-    public void testIsIngestEnabledAttributes() {
-        assertThat(NodeModule.isNodeIngestEnabled(ImmutableOpenMap.<String, String>builder().build()), equalTo(true));
-
-        ImmutableOpenMap.Builder<String, String> builder = ImmutableOpenMap.<String, String>builder();
-        builder.put("ingest", "true");
-        assertThat(NodeModule.isNodeIngestEnabled(builder.build()), equalTo(true));
-
-        builder = ImmutableOpenMap.<String, String>builder();
-        builder.put("ingest", "false");
-        assertThat(NodeModule.isNodeIngestEnabled(builder.build()), equalTo(false));
-
-        builder = ImmutableOpenMap.<String, String>builder();
-        builder.put("ingest", "off");
-        assertThat(NodeModule.isNodeIngestEnabled(builder.build()), equalTo(false));
-
-        builder = ImmutableOpenMap.<String, String>builder();
-        builder.put("ingest", "no");
-        assertThat(NodeModule.isNodeIngestEnabled(builder.build()), equalTo(false));
-
-        builder = ImmutableOpenMap.<String, String>builder();
-        builder.put("ingest", "0");
-        assertThat(NodeModule.isNodeIngestEnabled(builder.build()), equalTo(false));
-    }
-
-    public void testIsIngestEnabledMethodsReturnTheSameValue() {
-        String randomString;
-        if (randomBoolean()) {
-            randomString = randomFrom("true", "false", "on", "off", "yes", "no", "0", "1");
-        } else {
-            randomString = randomAsciiOfLengthBetween(1, 5);
-        }
-        Settings settings = Settings.builder().put("node.ingest", randomString).build();
-        ImmutableOpenMap.Builder<String, String> builder = ImmutableOpenMap.<String, String>builder();
-        builder.put("ingest", randomString);
-        ImmutableOpenMap<String, String> attributes = builder.build();
-
-        assertThat(NodeModule.isNodeIngestEnabled(settings), equalTo(NodeModule.isNodeIngestEnabled(attributes)));
-    }
-}