Changes as per review comments - Round 2

killbill · Nov 27, 2023 · fe38be6 · fe38be6
1 parent 88861bc
commit fe38be6
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 15 deletions.
diff --git a/lifecycle/spotbugs-exclude.xml b/lifecycle/spotbugs-exclude.xml
@@ -33,4 +33,10 @@
  <Method name="initialize" />
  <Bug pattern="EI_EXPOSE_REP2" />
  </Match>
+
+ <Match>
+ <Class name="org.killbill.billing.lifecycle.DefaultLifecycle" />
+ <Method name="doFireStage" />
+ <Bug pattern="DM_EXIT" />
+ </Match>
 </FindBugsFilter>
diff --git a/lifecycle/src/main/java/org/killbill/billing/lifecycle/DefaultLifecycle.java b/lifecycle/src/main/java/org/killbill/billing/lifecycle/DefaultLifecycle.java
@@ -42,7 +42,6 @@
 import org.slf4j.LoggerFactory;
 
 import com.google.inject.ConfigurationException;
-
 import com.google.inject.Injector;
 import com.google.inject.ProvisionException;
 
@@ -53,10 +52,14 @@ public class DefaultLifecycle implements Lifecycle {
  // See https://github.com/killbill/killbill-commons/issues/143
  private final Map<LifecycleLevel, SortedSet<LifecycleHandler<? extends KillbillService>>> handlersByLevel;
 
+ private static final String EXIT_ON_LIFECYCLE_ERROR_PROPERTY = "org.killbill.server.exit.on.lifecycle.error";
+ private boolean exitOnError;
+
  @Inject
  public DefaultLifecycle(final Injector injector) {
  this();
  final ServiceFinder<KillbillService> serviceFinder = new ServiceFinder<>(DefaultLifecycle.class.getClassLoader(), KillbillService.class.getName());
+ exitOnError = System.getProperty(EXIT_ON_LIFECYCLE_ERROR_PROPERTY) != null && Boolean.parseBoolean(System.getProperty(EXIT_ON_LIFECYCLE_ERROR_PROPERTY));
  init(serviceFinder, injector);
  }
 
@@ -155,6 +158,10 @@ private void doFireStage(final LifecycleHandlerType.LifecycleLevel level) {
  method.invoke(target);
  } catch (final Exception e) {
  logWarn("Killbill lifecycle failed to invoke lifecycle handler", e);
+ if (exitOnError) {
+ log.info("{} is set, so exiting..", EXIT_ON_LIFECYCLE_ERROR_PROPERTY);
+ System.exit(1);
+ }
  }
  }
 

diff --git a/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java b/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java
@@ -100,7 +100,7 @@ private void stopAndUninstallBundle(final Bundle bundle, final String pluginName
  registry.remove(pluginName);
  }
 
- public void startBundles(final Iterable<String> mandatoryPlugins) {
+ public void startBundles(final Iterable<String> mandatoryPlugins) throws Exception {
  log.info("List of mandatory plugins: {}", mandatoryPlugins);
  final List<String> pluginsStarted = new LinkedList<>();
  for (final BundleWithConfig bundleWithConfig : bundleWithConfigs) {
@@ -109,8 +109,7 @@ public void startBundles(final Iterable<String> mandatoryPlugins) {
  final String pluginName = getPluginName(bundleWithConfig);
  if (isBundleStarted) {
  pluginsStarted.add(pluginName);
- }
- else {
+ } else {
  registry.remove(pluginName);
  }
 
@@ -125,18 +124,14 @@ public void startBundles(final Iterable<String> mandatoryPlugins) {
 
  }
 
- private void checkIfMandatoryPluginsAreStarted(final List<String> pluginsStarted, final Iterable<String> mandatoryPlugins){
- boolean allMandatoryPluginsStarted = true;
- for (String pluginName: mandatoryPlugins) {
- if(!pluginsStarted.contains(pluginName)) {
- log.error("Mandatory plugin {} not started", pluginName); //TODO_1911 - Exit here?
- allMandatoryPluginsStarted = false;
+ private void checkIfMandatoryPluginsAreStarted(final List<String> pluginsStarted, final Iterable<String> mandatoryPlugins) throws Exception {
+ for (final String pluginName : mandatoryPlugins) {
+ if (!pluginsStarted.contains(pluginName)) {
+ log.warn("Mandatory plugin {} not started", pluginName);
+ throw new Exception("Mandatory plugin " + pluginName + " not started");
  }
  }
- if (allMandatoryPluginsStarted) {
- log.info("All mandatory plugins are started");
- }
-
+ log.info("All mandatory plugins are started");
  }
 
 

diff --git a/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java b/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java
@@ -99,7 +99,7 @@ public void initialize() {
  }
 
  @LifecycleHandlerType(LifecycleHandlerType.LifecycleLevel.START_PLUGIN)
- public void start() {
+ public void start() throws Exception {
  final String mandatoryPlugins = osgiConfig.getMandatoryPluginsList();
  final Set<String> mandatoryPluginsList = mandatoryPlugins != null && !mandatoryPlugins.isEmpty() ? Set.of(mandatoryPlugins.split("\\s*,\\s*")) : Collections.emptySet();
  // This will call the start() method for the bundles