Use the correct Environment object in NativeControllerHolder (elastic/x-pack-elasticsearch#2847)

We should not be constructing a temporary Environment object in production
code.  This currently isn't causing any problems, but it might in the future
if elastic/elasticsearch#27144 or something similar is ever merged.  Instead
the master Environment of the node should always be used.

Original commit: elastic/x-pack-elasticsearch@6276a54a45

plugin/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java

@@ -313,7 +313,7 @@ public class MachineLearning implements ActionPlugin {
         NormalizerProcessFactory normalizerProcessFactory;
         if (AUTODETECT_PROCESS.get(settings) && MachineLearningFeatureSet.isRunningOnMlPlatform(true)) {
             try {
-                NativeController nativeController = NativeControllerHolder.getNativeController(settings);
+                NativeController nativeController = NativeControllerHolder.getNativeController(env);
                 if (nativeController == null) {
                     // This will only only happen when path.home is not set, which is disallowed in production
                     throw new ElasticsearchException("Failed to create native process controller for Machine Learning");
@@ -341,7 +341,7 @@ public class MachineLearning implements ActionPlugin {
         DatafeedJobBuilder datafeedJobBuilder = new DatafeedJobBuilder(internalClient, jobProvider, auditor, System::currentTimeMillis);
         DatafeedManager datafeedManager = new DatafeedManager(threadPool, internalClient, clusterService, datafeedJobBuilder,
                 System::currentTimeMillis, auditor, persistentTasksService);
-        MlLifeCycleService mlLifeCycleService = new MlLifeCycleService(settings, clusterService, datafeedManager, autodetectProcessManager);
+        MlLifeCycleService mlLifeCycleService = new MlLifeCycleService(env, clusterService, datafeedManager, autodetectProcessManager);
         InvalidLicenseEnforcer invalidLicenseEnforcer =
                 new InvalidLicenseEnforcer(settings, licenseState, threadPool, datafeedManager, autodetectProcessManager);
         PersistentTasksExecutorRegistry persistentTasksExecutorRegistry = new PersistentTasksExecutorRegistry(Settings.EMPTY, Arrays.asList(

plugin/src/main/java/org/elasticsearch/xpack/ml/MachineLearningFeatureSet.java

@@ -20,6 +20,7 @@ import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.env.Environment;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.plugins.Platforms;
 import org.elasticsearch.xpack.XPackFeatureSet;
@@ -64,9 +65,9 @@ public class MachineLearningFeatureSet implements XPackFeatureSet {
     private final Map<String, Object> nativeCodeInfo;
 
     @Inject
-    public MachineLearningFeatureSet(Settings settings, ClusterService clusterService, Client client,
+    public MachineLearningFeatureSet(Environment environment, ClusterService clusterService, Client client,
                                      @Nullable XPackLicenseState licenseState) {
-        this.enabled = XPackSettings.MACHINE_LEARNING_ENABLED.get(settings);
+        this.enabled = XPackSettings.MACHINE_LEARNING_ENABLED.get(environment.settings());
         this.clusterService = Objects.requireNonNull(clusterService);
         this.client = Objects.requireNonNull(client);
         this.licenseState = licenseState;
@@ -74,10 +75,11 @@ public class MachineLearningFeatureSet implements XPackFeatureSet {
         // Don't try to get the native code version if ML is disabled - it causes too much controversy
         // if ML has been disabled because of some OS incompatibility.  Also don't try to get the native
         // code version in the transport or tribe client - the controller process won't be running.
-        if (enabled && XPackPlugin.transportClientMode(settings) == false && XPackPlugin.isTribeClientNode(settings) == false) {
+        if (enabled && XPackPlugin.transportClientMode(environment.settings()) == false
+                && XPackPlugin.isTribeClientNode(environment.settings()) == false) {
             try {
                 if (isRunningOnMlPlatform(true)) {
-                    NativeController nativeController = NativeControllerHolder.getNativeController(settings);
+                    NativeController nativeController = NativeControllerHolder.getNativeController(environment);
                     if (nativeController != null) {
                         nativeCodeInfo = nativeController.getNativeCodeInfo();
                     }

plugin/src/main/java/org/elasticsearch/xpack/ml/MlLifeCycleService.java

@@ -8,7 +8,7 @@ package org.elasticsearch.xpack.ml;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.component.LifecycleListener;
-import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.env.Environment;
 import org.elasticsearch.xpack.ml.datafeed.DatafeedManager;
 import org.elasticsearch.xpack.ml.job.process.NativeController;
 import org.elasticsearch.xpack.ml.job.process.NativeControllerHolder;
@@ -18,16 +18,18 @@ import java.io.IOException;
 
 public class MlLifeCycleService extends AbstractComponent {
 
+    private final Environment environment;
     private final DatafeedManager datafeedManager;
     private final AutodetectProcessManager autodetectProcessManager;
 
-    public MlLifeCycleService(Settings settings, ClusterService clusterService) {
+    public MlLifeCycleService(Environment environment, ClusterService clusterService) {
-        this(settings, clusterService, null, null);
+        this(environment, clusterService, null, null);
     }
 
-    public MlLifeCycleService(Settings settings, ClusterService clusterService, DatafeedManager datafeedManager,
+    public MlLifeCycleService(Environment environment, ClusterService clusterService, DatafeedManager datafeedManager,
                               AutodetectProcessManager autodetectProcessManager) {
-        super(settings);
+        super(environment.settings());
+        this.environment = environment;
         this.datafeedManager = datafeedManager;
         this.autodetectProcessManager = autodetectProcessManager;
         clusterService.addLifecycleListener(new LifecycleListener() {
@@ -47,7 +49,7 @@ public class MlLifeCycleService extends AbstractComponent {
                 if (datafeedManager != null) {
                     datafeedManager.isolateAllDatafeedsOnThisNode();
                 }
-                NativeController nativeController = NativeControllerHolder.getNativeController(settings);
+                NativeController nativeController = NativeControllerHolder.getNativeController(environment);
                 if (nativeController != null) {
                     // This kills autodetect processes WITHOUT closing the jobs, so they get reallocated.
                     if (autodetectProcessManager != null) {

plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/NativeControllerHolder.java

@@ -5,7 +5,6 @@
  */
 package org.elasticsearch.xpack.ml.job.process;
 
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.xpack.ml.MachineLearning;
 import org.elasticsearch.xpack.ml.utils.NamedPipeHelper;
@@ -29,16 +28,16 @@ public class NativeControllerHolder {
      *
      * The NativeController is created lazily to allow time for the C++ process to be started before connection is attempted.
      *
-     * null is returned to tests that haven't bothered to set up path.home and all runs where xpack.ml.autodetect_process=false.
+     * <code>null</code> is returned to tests where xpack.ml.autodetect_process=false.
      *
      * Calls may throw an exception if initial connection to the C++ process fails.
      */
-    public static NativeController getNativeController(Settings settings) throws IOException {
+    public static NativeController getNativeController(Environment environment) throws IOException {
 
-        if (Environment.PATH_HOME_SETTING.exists(settings) && MachineLearning.AUTODETECT_PROCESS.get(settings)) {
+        if (MachineLearning.AUTODETECT_PROCESS.get(environment.settings())) {
             synchronized (lock) {
                 if (nativeController == null) {
-                    nativeController = new NativeController(new Environment(settings), new NamedPipeHelper());
+                    nativeController = new NativeController(environment, new NamedPipeHelper());
                     nativeController.tailLogsInThread();
                 }
             }
@@ -51,7 +50,7 @@ public class NativeControllerHolder {
      * Get a reference to the singleton native process controller.
      *
      * Assumes that if it is possible for a native controller to exist that it will already have been created.
-     * Designed for use by objects that don't have access to settings but know a native controller must exist
+     * Designed for use by objects that don't have access to the environment but know a native controller must exist
      * for the object calling this method to exist.
      */
     public static NativeController getNativeController() {

plugin/src/test/java/org/elasticsearch/xpack/ml/MachineLearningFeatureSetTests.java

@@ -17,6 +17,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.env.Environment;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.XPackFeatureSet;
@@ -53,12 +54,17 @@ import static org.mockito.Mockito.when;
 
 public class MachineLearningFeatureSetTests extends ESTestCase {
 
+    private Settings commonSettings;
     private ClusterService clusterService;
     private Client client;
     private XPackLicenseState licenseState;
 
     @Before
     public void init() throws Exception {
+        commonSettings = Settings.builder()
+                .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath())
+                .put(MachineLearning.AUTODETECT_PROCESS.getKey(), false)
+                .build();
         clusterService = mock(ClusterService.class);
         client = mock(Client.class);
         licenseState = mock(XPackLicenseState.class);
@@ -82,7 +88,8 @@ public class MachineLearningFeatureSetTests extends ESTestCase {
     }
 
     public void testAvailable() throws Exception {
-        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(Settings.EMPTY, clusterService, client, licenseState);
+        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(new Environment(commonSettings), clusterService, client,
+                licenseState);
         boolean available = randomBoolean();
         when(licenseState.isMachineLearningAllowed()).thenReturn(available);
         assertThat(featureSet.available(), is(available));
@@ -100,13 +107,14 @@ public class MachineLearningFeatureSetTests extends ESTestCase {
     public void testEnabled() throws Exception {
         boolean useDefault = randomBoolean();
         boolean enabled = true;
-        Settings.Builder settings = Settings.builder();
+        Settings.Builder settings = Settings.builder().put(commonSettings);
         if (useDefault == false) {
             enabled = randomBoolean();
             settings.put("xpack.ml.enabled", enabled);
         }
         boolean expected = enabled || useDefault;
-        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(settings.build(), clusterService, client, licenseState);
+        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(new Environment(settings.build()), clusterService, client,
+                licenseState);
         assertThat(featureSet.enabled(), is(expected));
         PlainActionFuture<Usage> future = new PlainActionFuture<>();
         featureSet.usage(future);
@@ -121,7 +129,7 @@ public class MachineLearningFeatureSetTests extends ESTestCase {
 
     public void testUsage() throws Exception {
         when(licenseState.isMachineLearningAllowed()).thenReturn(true);
-        Settings.Builder settings = Settings.builder();
+        Settings.Builder settings = Settings.builder().put(commonSettings);
         settings.put("xpack.ml.enabled", true);
 
         Job opened1 = buildJob("opened1", Arrays.asList(buildMinDetector("foo")));
@@ -139,7 +147,8 @@ public class MachineLearningFeatureSetTests extends ESTestCase {
                 buildDatafeedStats(DatafeedState.STOPPED)
         ));
 
-        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(settings.build(), clusterService, client, licenseState);
+        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(new Environment(settings.build()), clusterService, client,
+                licenseState);
         PlainActionFuture<Usage> future = new PlainActionFuture<>();
         featureSet.usage(future);
         XPackFeatureSet.Usage mlUsage = future.get();
@@ -201,11 +210,11 @@ public class MachineLearningFeatureSetTests extends ESTestCase {
 
     public void testUsageGivenMlMetadataNotInstalled() throws Exception {
         when(licenseState.isMachineLearningAllowed()).thenReturn(true);
-        Settings.Builder settings = Settings.builder();
+        Settings.Builder settings = Settings.builder().put(commonSettings);
         settings.put("xpack.ml.enabled", true);
         when(clusterService.state()).thenReturn(ClusterState.EMPTY_STATE);
 
-        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(settings.build(),
+        MachineLearningFeatureSet featureSet = new MachineLearningFeatureSet(new Environment(settings.build()),
                 clusterService, client, licenseState);
         PlainActionFuture<Usage> future = new PlainActionFuture<>();
         featureSet.usage(future);