Deprecate the pidfile setting (elastic#45938)

This commit deprecates the pidfile setting in favor of node.pidfile.
original-brownbear · Aug 24, 2019 · 3ed8b5c · 3ed8b5c
1 parent 67b3414
commit 3ed8b5c
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 9 deletions.
diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy
@@ -370,7 +370,7 @@ class ClusterFormationTasks {
         Map esConfig = [
                 'cluster.name'                 : node.clusterName,
                 'node.name'                    : "node-" + node.nodeNum,
-                'pidfile'                      : node.pidFile,
+                (node.nodeVersion.onOrAfter('8.0.0') ? 'node.pidfile' : 'pidfile') : node.pidFile,
                 'path.repo'                    : "${node.sharedDir}/repo",
                 'path.shared_data'             : "${node.sharedDir}/",
                 // Define a node attribute so we can test that it exists

diff --git a/dev-tools/smoke_test_rc.py b/dev-tools/smoke_test_rc.py
@@ -200,7 +200,7 @@ def smoke_test_release(release, files, hash, plugins):
       headers = {}
     print('  Starting elasticsearch deamon from [%s]' % es_dir)
     try:
-      run('%s; %s -Enode.name=smoke_tester -Ecluster.name=prepare_release -Erepositories.url.allowed_urls=http://snapshot.test* %s -Epidfile=%s -Enode.portsfile=true'
+      run('%s; %s -Enode.name=smoke_tester -Ecluster.name=prepare_release -Erepositories.url.allowed_urls=http://snapshot.test* %s -Enode.pidfile=%s -Enode.portsfile=true'
           % (java_exe(), es_run_path, '-d', os.path.join(es_dir, 'es-smoke.pid')))
       if not wait_for_node_startup(es_dir, header=headers):
         print("elasticsearch logs:")

diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java
@@ -85,7 +85,7 @@ public void testEnvironmentPaths() throws Exception {
                 esHome.resolve("data2").toString());
         settingsBuilder.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), esHome.resolve("custom").toString());
         settingsBuilder.put(Environment.PATH_LOGS_SETTING.getKey(), esHome.resolve("logs").toString());
-        settingsBuilder.put(Environment.PIDFILE_SETTING.getKey(), esHome.resolve("test.pid").toString());
+        settingsBuilder.put(Environment.NODE_PIDFILE_SETTING.getKey(), esHome.resolve("test.pid").toString());
         Settings settings = settingsBuilder.build();
 
         Path fakeTmpDir = createTempDir();

diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
@@ -256,7 +256,7 @@ private static Environment createEnvironment(
             final Path configPath) {
         Settings.Builder builder = Settings.builder();
         if (pidFile != null) {
-            builder.put(Environment.PIDFILE_SETTING.getKey(), pidFile);
+            builder.put(Environment.NODE_PIDFILE_SETTING.getKey(), pidFile);
         }
         builder.put(initialSettings);
         if (secureSettings != null) {

diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
@@ -363,6 +363,7 @@ public void apply(Settings value, Settings current, Settings previous) {
             Environment.PATH_REPO_SETTING,
             Environment.PATH_SHARED_DATA_SETTING,
             Environment.PIDFILE_SETTING,
+            Environment.NODE_PIDFILE_SETTING,
             NodeEnvironment.NODE_ID_SEED_SETTING,
             Node.INITIAL_STATE_TIMEOUT_SETTING,
             DiscoveryModule.DISCOVERY_TYPE_SETTING,

diff --git a/server/src/main/java/org/elasticsearch/env/Environment.java b/server/src/main/java/org/elasticsearch/env/Environment.java
@@ -60,7 +60,8 @@ public class Environment {
     public static final Setting<List<String>> PATH_REPO_SETTING =
         Setting.listSetting("path.repo", Collections.emptyList(), Function.identity(), Property.NodeScope);
     public static final Setting<String> PATH_SHARED_DATA_SETTING = Setting.simpleString("path.shared_data", Property.NodeScope);
-    public static final Setting<String> PIDFILE_SETTING = Setting.simpleString("pidfile", Property.NodeScope);
+    public static final Setting<String> PIDFILE_SETTING = Setting.simpleString("pidfile", Property.Deprecated, Property.NodeScope);
+    public static final Setting<String> NODE_PIDFILE_SETTING = Setting.simpleString("node.pidfile", PIDFILE_SETTING, Property.NodeScope);
 
     private final Settings settings;
 
@@ -154,8 +155,8 @@ public Environment(final Settings settings, final Path configPath) {
             logsFile = homeFile.resolve("logs");
         }
 
-        if (PIDFILE_SETTING.exists(settings)) {
-            pidFile = PathUtils.get(PIDFILE_SETTING.get(settings)).toAbsolutePath().normalize();
+        if (NODE_PIDFILE_SETTING.exists(settings) || PIDFILE_SETTING.exists(settings)) {
+            pidFile = PathUtils.get(NODE_PIDFILE_SETTING.get(settings)).toAbsolutePath().normalize();
         } else {
             pidFile = null;
         }
@@ -179,7 +180,10 @@ public Environment(final Settings settings, final Path configPath) {
             assert sharedDataFile != null;
             finalSettings.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), sharedDataFile.toString());
         }
-        if (PIDFILE_SETTING.exists(settings)) {
+        if (NODE_PIDFILE_SETTING.exists(settings)) {
+            assert pidFile != null;
+            finalSettings.put(Environment.NODE_PIDFILE_SETTING.getKey(), pidFile.toString());
+        } else if (PIDFILE_SETTING.exists(settings)) {
             assert pidFile != null;
             finalSettings.put(Environment.PIDFILE_SETTING.getKey(), pidFile.toString());
         }

diff --git a/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java
@@ -19,6 +19,7 @@
 package org.elasticsearch.env;
 
 import org.elasticsearch.common.io.PathUtils;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESTestCase;
 
@@ -174,13 +175,20 @@ public void testTempPathValidationWhenRegularFile() throws IOException {
 
     // test that environment paths are absolute and normalized
     public void testPathNormalization() throws IOException {
+        final Setting<String> pidFileSetting;
+        if (randomBoolean()) {
+            pidFileSetting = Environment.NODE_PIDFILE_SETTING;
+        } else {
+            pidFileSetting = Environment.PIDFILE_SETTING;
+        }
+
         final Settings settings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), "home")
             .put(Environment.PATH_DATA_SETTING.getKey(), "./home/../home/data")
             .put(Environment.PATH_LOGS_SETTING.getKey(), "./home/../home/logs")
             .put(Environment.PATH_REPO_SETTING.getKey(), "./home/../home/repo")
             .put(Environment.PATH_SHARED_DATA_SETTING.getKey(), "./home/../home/shared_data")
-            .put(Environment.PIDFILE_SETTING.getKey(), "./home/../home/pidfile")
+            .put(pidFileSetting.getKey(), "./home/../home/pidfile")
             .build();
 
         // the above paths will be treated as relative to the working directory
@@ -205,6 +213,13 @@ public void testPathNormalization() throws IOException {
 
         final String sharedDataPath = Environment.PATH_SHARED_DATA_SETTING.get(environment.settings());
         assertPath(sharedDataPath, home.resolve("shared_data"));
+
+        final String pidFile = pidFileSetting.get(environment.settings());
+        assertPath(pidFile, home.resolve("pidfile"));
+
+        if (pidFileSetting.isDeprecated()) {
+            assertSettingDeprecationsAndWarnings(new Setting<?>[] { pidFileSetting });
+        }
     }
 
     private void assertPath(final String actual, final Path expected) {