Align assertion and enable check in systemd plugin

This commit more closely aligns the assertion that we are running in a package distribution with disabling the systemd integration if somehow we running on not a package distribution. This is, previously we had an assertion that we are in a package distribution (RPM or Debian package) but would disable the systemd integration if we are not on Linux. Instead, we should disable the systemd integration if we are not running in a package distribution. Because of our assertion, we expect this to never hold, but we need a fallback for when this assertion is violated and assertions are not enabled.
RagdollNoone · Jul 24, 2019 · ded5ad4 · ded5ad4
1 parent ac5cc15
commit ded5ad4
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 14 deletions.
diff --git a/modules/systemd/src/main/java/org/elasticsearch/systemd/SystemdPlugin.java b/modules/systemd/src/main/java/org/elasticsearch/systemd/SystemdPlugin.java
@@ -21,8 +21,6 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.apache.lucene.util.Constants;
-import org.elasticsearch.Assertions;
 import org.elasticsearch.Build;
 import org.elasticsearch.plugins.ClusterPlugin;
 import org.elasticsearch.plugins.Plugin;
@@ -39,15 +37,22 @@ final boolean isEnabled() {
 
     @SuppressWarnings("unused")
     public SystemdPlugin() {
-        this(true, Constants.LINUX, System.getenv("ES_SD_NOTIFY"));
+        this(true, Build.CURRENT.type(), System.getenv("ES_SD_NOTIFY"));
     }
 
-    SystemdPlugin(final boolean assertIsPackageDistribution, final boolean isLinux, final String esSDNotify) {
-        if (Assertions.ENABLED && assertIsPackageDistribution) {
+    SystemdPlugin(final boolean assertIsPackageDistribution, final Build.Type buildType, final String esSDNotify) {
+        final boolean isPackageDistribution = buildType == Build.Type.DEB || buildType == Build.Type.RPM;
+        if (assertIsPackageDistribution) {
             // our build is configured to only include this module in the package distributions
-            assert Build.CURRENT.type() == Build.Type.DEB || Build.CURRENT.type() == Build.Type.RPM : Build.CURRENT.type();
+            assert isPackageDistribution : buildType;
         }
-        if (isLinux == false || esSDNotify == null) {
+        if (isPackageDistribution == false) {
+            logger.debug("disabling sd_notify as the build type [{}] is not a package distribution", buildType);
+            enabled = false;
+            return;
+        }
+        logger.trace("ES_SD_NOTIFY is set to [{}]", esSDNotify);
+        if (esSDNotify == null) {
             enabled = false;
             return;
         }

diff --git a/modules/systemd/src/test/java/org/elasticsearch/systemd/SystemdPluginTests.java b/modules/systemd/src/test/java/org/elasticsearch/systemd/SystemdPluginTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.systemd;
 
+import org.elasticsearch.Build;
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.hamcrest.OptionalMatchers;
@@ -37,23 +38,27 @@
 
 public class SystemdPluginTests extends ESTestCase {
 
+    private Build.Type randomPackageBuildType = randomFrom(Build.Type.DEB, Build.Type.RPM);
+    private Build.Type randomNonPackageBuildType =
+        randomValueOtherThanMany(t -> t == Build.Type.DEB || t == Build.Type.RPM, () -> randomFrom(Build.Type.values()));
+
     public void testIsEnabled() {
-        final SystemdPlugin plugin = new SystemdPlugin(false, true, Boolean.TRUE.toString());
+        final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, Boolean.TRUE.toString());
         assertTrue(plugin.isEnabled());
     }
 
-    public void testIsNotLinux() {
-        final SystemdPlugin plugin = new SystemdPlugin(false, false, Boolean.TRUE.toString());
+    public void testIsNotPackageDistribution() {
+        final SystemdPlugin plugin = new SystemdPlugin(false, randomNonPackageBuildType, Boolean.TRUE.toString());
         assertFalse(plugin.isEnabled());
     }
 
     public void testIsImplicitlyNotEnabled() {
-        final SystemdPlugin plugin = new SystemdPlugin(false, true, null);
+        final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, null);
         assertFalse(plugin.isEnabled());
     }
 
     public void testIsExplicitlyNotEnabled() {
-        final SystemdPlugin plugin = new SystemdPlugin(false, true, Boolean.FALSE.toString());
+        final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, Boolean.FALSE.toString());
         assertFalse(plugin.isEnabled());
     }
 
@@ -62,7 +67,7 @@ public void testInvalid() {
             s -> Boolean.TRUE.toString().equals(s) || Boolean.FALSE.toString().equals(s),
             () -> randomAlphaOfLength(4));
         final RuntimeException e = expectThrows(RuntimeException.class,
-            () -> new SystemdPlugin(false, true, esSDNotify));
+            () -> new SystemdPlugin(false, randomPackageBuildType, esSDNotify));
         assertThat(e, hasToString(containsString("ES_SD_NOTIFY set to unexpected value [" + esSDNotify + "]")));
     }
 
@@ -137,7 +142,7 @@ private void runTest(
         final AtomicBoolean invoked = new AtomicBoolean();
         final AtomicInteger invokedUnsetEnvironment = new AtomicInteger();
         final AtomicReference<String> invokedState = new AtomicReference<>();
-        final SystemdPlugin plugin = new SystemdPlugin(false, true, esSDNotify) {
+        final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, esSDNotify) {
 
             @Override
             int sd_notify(final int unset_environment, final String state) {