Skip to content

Commit 58a4bad

Browse files
committed
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.
1 parent 1e9c505 commit 58a4bad

File tree

2 files changed

+24
-14
lines changed

2 files changed

+24
-14
lines changed

modules/systemd/src/main/java/org/elasticsearch/systemd/SystemdPlugin.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121

2222
import org.apache.logging.log4j.LogManager;
2323
import org.apache.logging.log4j.Logger;
24-
import org.apache.lucene.util.Constants;
25-
import org.elasticsearch.Assertions;
2624
import org.elasticsearch.Build;
2725
import org.elasticsearch.plugins.ClusterPlugin;
2826
import org.elasticsearch.plugins.Plugin;
@@ -39,15 +37,22 @@ final boolean isEnabled() {
3937

4038
@SuppressWarnings("unused")
4139
public SystemdPlugin() {
42-
this(true, Constants.LINUX, System.getenv("ES_SD_NOTIFY"));
40+
this(true, Build.CURRENT.type(), System.getenv("ES_SD_NOTIFY"));
4341
}
4442

45-
SystemdPlugin(final boolean assertIsPackageDistribution, final boolean isLinux, final String esSDNotify) {
46-
if (Assertions.ENABLED && assertIsPackageDistribution) {
43+
SystemdPlugin(final boolean assertIsPackageDistribution, final Build.Type buildType, final String esSDNotify) {
44+
final boolean isPackageDistribution = buildType == Build.Type.DEB || buildType == Build.Type.RPM;
45+
if (assertIsPackageDistribution) {
4746
// our build is configured to only include this module in the package distributions
48-
assert Build.CURRENT.type() == Build.Type.DEB || Build.CURRENT.type() == Build.Type.RPM : Build.CURRENT.type();
47+
assert isPackageDistribution : buildType;
4948
}
50-
if (isLinux == false || esSDNotify == null) {
49+
if (isPackageDistribution == false) {
50+
logger.debug("disabling sd_notify as the build type [{}] is not a package distribution", buildType);
51+
enabled = false;
52+
return;
53+
}
54+
logger.trace("ES_SD_NOTIFY is set to [{}]", esSDNotify);
55+
if (esSDNotify == null) {
5156
enabled = false;
5257
return;
5358
}

modules/systemd/src/test/java/org/elasticsearch/systemd/SystemdPluginTests.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.systemd;
2121

22+
import org.elasticsearch.Build;
2223
import org.elasticsearch.common.CheckedConsumer;
2324
import org.elasticsearch.test.ESTestCase;
2425
import org.elasticsearch.test.hamcrest.OptionalMatchers;
@@ -37,23 +38,27 @@
3738

3839
public class SystemdPluginTests extends ESTestCase {
3940

41+
private Build.Type randomPackageBuildType = randomFrom(Build.Type.DEB, Build.Type.RPM);
42+
private Build.Type randomNonPackageBuildType =
43+
randomValueOtherThanMany(t -> t == Build.Type.DEB || t == Build.Type.RPM, () -> randomFrom(Build.Type.values()));
44+
4045
public void testIsEnabled() {
41-
final SystemdPlugin plugin = new SystemdPlugin(false, true, Boolean.TRUE.toString());
46+
final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, Boolean.TRUE.toString());
4247
assertTrue(plugin.isEnabled());
4348
}
4449

45-
public void testIsNotLinux() {
46-
final SystemdPlugin plugin = new SystemdPlugin(false, false, Boolean.TRUE.toString());
50+
public void testIsNotPackageDistribution() {
51+
final SystemdPlugin plugin = new SystemdPlugin(false, randomNonPackageBuildType, Boolean.TRUE.toString());
4752
assertFalse(plugin.isEnabled());
4853
}
4954

5055
public void testIsImplicitlyNotEnabled() {
51-
final SystemdPlugin plugin = new SystemdPlugin(false, true, null);
56+
final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, null);
5257
assertFalse(plugin.isEnabled());
5358
}
5459

5560
public void testIsExplicitlyNotEnabled() {
56-
final SystemdPlugin plugin = new SystemdPlugin(false, true, Boolean.FALSE.toString());
61+
final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, Boolean.FALSE.toString());
5762
assertFalse(plugin.isEnabled());
5863
}
5964

@@ -62,7 +67,7 @@ public void testInvalid() {
6267
s -> Boolean.TRUE.toString().equals(s) || Boolean.FALSE.toString().equals(s),
6368
() -> randomAlphaOfLength(4));
6469
final RuntimeException e = expectThrows(RuntimeException.class,
65-
() -> new SystemdPlugin(false, true, esSDNotify));
70+
() -> new SystemdPlugin(false, randomPackageBuildType, esSDNotify));
6671
assertThat(e, hasToString(containsString("ES_SD_NOTIFY set to unexpected value [" + esSDNotify + "]")));
6772
}
6873

@@ -137,7 +142,7 @@ private void runTest(
137142
final AtomicBoolean invoked = new AtomicBoolean();
138143
final AtomicInteger invokedUnsetEnvironment = new AtomicInteger();
139144
final AtomicReference<String> invokedState = new AtomicReference<>();
140-
final SystemdPlugin plugin = new SystemdPlugin(false, true, esSDNotify) {
145+
final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, esSDNotify) {
141146

142147
@Override
143148
int sd_notify(final int unset_environment, final String state) {

0 commit comments

Comments
 (0)