From e4c6ae08543f245ed9fe0e389fe0527f0595e43a Mon Sep 17 00:00:00 2001 From: reshmabidikar Date: Mon, 20 Nov 2023 16:10:42 +0530 Subject: [PATCH 1/3] Initial changes for 1911 --- .idea/misc.xml | 1 + .../killbill/billing/osgi/BundleRegistry.java | 37 +++++++++++++++++-- .../billing/osgi/DefaultOSGIService.java | 5 ++- .../billing/osgi/config/OSGIConfig.java | 5 +++ .../osgi/pluginconf/TestPluginFinder.java | 5 +++ platform-api/pom.xml | 2 +- platform-test/pom.xml | 4 +- 7 files changed, 52 insertions(+), 7 deletions(-) diff --git a/.idea/misc.xml b/.idea/misc.xml index 35c3ad8d..f4a499a6 100644 --- a/.idea/misc.xml +++ b/.idea/misc.xml @@ -1,5 +1,6 @@ + diff --git a/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java b/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java index d512e5f7..4e3a5a27 100644 --- a/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java +++ b/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -99,16 +100,46 @@ private void stopAndUninstallBundle(final Bundle bundle, final String pluginName registry.remove(pluginName); } - public void startBundles() { + public void startBundles(final List mandatoryPlugins) { + log.info("List of mandatory plugins: {}",mandatoryPlugins); + final List pluginsStarted = new LinkedList<>(); for (final BundleWithConfig bundleWithConfig : bundleWithConfigs) { final boolean isBundleStarted = fileInstall.startBundle(bundleWithConfig.getBundle()); - if (!isBundleStarted) { - registry.remove(getPluginName(bundleWithConfig)); + final String pluginName = getPluginName(bundleWithConfig); + if (isBundleStarted) { + pluginsStarted.add(pluginName); } + else { + registry.remove(pluginName); + } + + } + + if (!mandatoryPlugins.isEmpty()) { + checkIfMandatoryPluginsAreStarted(pluginsStarted, mandatoryPlugins); } + else { + log.info("Mandatory plugins not specified, skipping mandatory plugins check"); + } + } + private void checkIfMandatoryPluginsAreStarted(List pluginsStarted, List 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; + } + } + if(allMandatoryPluginsStarted) { + log.info("All mandatory plugins are started"); + } + + } + + public void stopBundles() { for (final BundleWithConfig bundleWithConfig : bundleWithConfigs) { try { diff --git a/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java b/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java index 72f15110..4570e8f9 100644 --- a/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java +++ b/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java @@ -20,6 +20,7 @@ package org.killbill.billing.osgi; import java.io.File; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -98,8 +99,10 @@ public void initialize() { @LifecycleHandlerType(LifecycleHandlerType.LifecycleLevel.START_PLUGIN) public void start() { + final String mandatoryPlugins = osgiConfig.getMandatoryPluginsList(); + final List mandatoryPluginsList = mandatoryPlugins != null && !mandatoryPlugins.isEmpty() ? List.of(mandatoryPlugins.split(",")) : Collections.emptyList(); // This will call the start() method for the bundles - bundleRegistry.startBundles(); + bundleRegistry.startBundles(mandatoryPluginsList); // Tell the plugins all bundles have started killbillActivator.sendEvent("org/killbill/billing/osgi/lifecycle/STARTED", new HashMap()); } diff --git a/osgi/src/main/java/org/killbill/billing/osgi/config/OSGIConfig.java b/osgi/src/main/java/org/killbill/billing/osgi/config/OSGIConfig.java index 881a8b35..560a2848 100644 --- a/osgi/src/main/java/org/killbill/billing/osgi/config/OSGIConfig.java +++ b/osgi/src/main/java/org/killbill/billing/osgi/config/OSGIConfig.java @@ -267,4 +267,9 @@ public interface OSGIConfig extends KillbillPlatformConfig { @Default("") public String getSystemBundleExportPackagesExtra(); + @Config("org.killbill.billing.plugin.mandatory.plugins") + @Description("List of mandatory plugins") + @Default("") + public String getMandatoryPluginsList(); + } diff --git a/osgi/src/test/java/org/killbill/billing/osgi/pluginconf/TestPluginFinder.java b/osgi/src/test/java/org/killbill/billing/osgi/pluginconf/TestPluginFinder.java index 480e3c46..6d60c744 100644 --- a/osgi/src/test/java/org/killbill/billing/osgi/pluginconf/TestPluginFinder.java +++ b/osgi/src/test/java/org/killbill/billing/osgi/pluginconf/TestPluginFinder.java @@ -210,6 +210,11 @@ public String getSystemBundleExportPackagesJava() { public String getSystemBundleExportPackagesExtra() { return null; } + @Override + public String getMandatoryPluginsList() { + return null; + } + }; } diff --git a/platform-api/pom.xml b/platform-api/pom.xml index 321e1b77..b2d09733 100644 --- a/platform-api/pom.xml +++ b/platform-api/pom.xml @@ -28,5 +28,5 @@ killbill-platform-api jar killbill-platform-api - + diff --git a/platform-test/pom.xml b/platform-test/pom.xml index 6e06fc4a..b549934c 100644 --- a/platform-test/pom.xml +++ b/platform-test/pom.xml @@ -278,8 +278,8 @@ initialize - - + + From 88861bc2c371b750d76cdd6412f43748c2163cbd Mon Sep 17 00:00:00 2001 From: reshmabidikar Date: Wed, 22 Nov 2023 18:12:54 +0530 Subject: [PATCH 2/3] Changes as per review comments - Round 1 --- .../org/killbill/billing/osgi/BundleRegistry.java | 12 ++++++------ .../killbill/billing/osgi/DefaultOSGIService.java | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java b/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java index 4e3a5a27..172ddef4 100644 --- a/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java +++ b/osgi/src/main/java/org/killbill/billing/osgi/BundleRegistry.java @@ -100,8 +100,8 @@ private void stopAndUninstallBundle(final Bundle bundle, final String pluginName registry.remove(pluginName); } - public void startBundles(final List mandatoryPlugins) { - log.info("List of mandatory plugins: {}",mandatoryPlugins); + public void startBundles(final Iterable mandatoryPlugins) { + log.info("List of mandatory plugins: {}", mandatoryPlugins); final List pluginsStarted = new LinkedList<>(); for (final BundleWithConfig bundleWithConfig : bundleWithConfigs) { final boolean isBundleStarted = fileInstall.startBundle(bundleWithConfig.getBundle()); @@ -116,7 +116,7 @@ public void startBundles(final List mandatoryPlugins) { } - if (!mandatoryPlugins.isEmpty()) { + if (mandatoryPlugins.iterator().hasNext()) { checkIfMandatoryPluginsAreStarted(pluginsStarted, mandatoryPlugins); } else { @@ -125,15 +125,15 @@ public void startBundles(final List mandatoryPlugins) { } - private void checkIfMandatoryPluginsAreStarted(List pluginsStarted, List mandatoryPlugins){ + private void checkIfMandatoryPluginsAreStarted(final List pluginsStarted, final Iterable mandatoryPlugins){ boolean allMandatoryPluginsStarted = true; for (String pluginName: mandatoryPlugins) { if(!pluginsStarted.contains(pluginName)) { - log.error("Mandatory plugin {} not started",pluginName); //TODO_1911 - Exit here? + log.error("Mandatory plugin {} not started", pluginName); //TODO_1911 - Exit here? allMandatoryPluginsStarted = false; } } - if(allMandatoryPluginsStarted) { + if (allMandatoryPluginsStarted) { 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 index 4570e8f9..1f8dd02c 100644 --- a/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java +++ b/osgi/src/main/java/org/killbill/billing/osgi/DefaultOSGIService.java @@ -25,6 +25,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import javax.inject.Inject; import javax.inject.Named; @@ -100,7 +101,7 @@ public void initialize() { @LifecycleHandlerType(LifecycleHandlerType.LifecycleLevel.START_PLUGIN) public void start() { final String mandatoryPlugins = osgiConfig.getMandatoryPluginsList(); - final List mandatoryPluginsList = mandatoryPlugins != null && !mandatoryPlugins.isEmpty() ? List.of(mandatoryPlugins.split(",")) : Collections.emptyList(); + final Set mandatoryPluginsList = mandatoryPlugins != null && !mandatoryPlugins.isEmpty() ? Set.of(mandatoryPlugins.split("\\s*,\\s*")) : Collections.emptySet(); // This will call the start() method for the bundles bundleRegistry.startBundles(mandatoryPluginsList); // Tell the plugins all bundles have started From fe38be67882f2a93c67c7858148f5beb2ea257c6 Mon Sep 17 00:00:00 2001 From: reshmabidikar Date: Mon, 27 Nov 2023 12:13:58 +0530 Subject: [PATCH 3/3] Changes as per review comments - Round 2 --- lifecycle/spotbugs-exclude.xml | 6 ++++++ .../billing/lifecycle/DefaultLifecycle.java | 9 +++++++- .../killbill/billing/osgi/BundleRegistry.java | 21 +++++++------------ .../billing/osgi/DefaultOSGIService.java | 2 +- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/lifecycle/spotbugs-exclude.xml b/lifecycle/spotbugs-exclude.xml index 3f9fa31f..1cc1caf1 100644 --- a/lifecycle/spotbugs-exclude.xml +++ b/lifecycle/spotbugs-exclude.xml @@ -33,4 +33,10 @@ + + + + + + \ No newline at end of file diff --git a/lifecycle/src/main/java/org/killbill/billing/lifecycle/DefaultLifecycle.java b/lifecycle/src/main/java/org/killbill/billing/lifecycle/DefaultLifecycle.java index 4f95043d..5b2a8496 100644 --- 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>> 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 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 index 172ddef4..dd326e62 100644 --- 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 mandatoryPlugins) { + public void startBundles(final Iterable mandatoryPlugins) throws Exception { log.info("List of mandatory plugins: {}", mandatoryPlugins); final List pluginsStarted = new LinkedList<>(); for (final BundleWithConfig bundleWithConfig : bundleWithConfigs) { @@ -109,8 +109,7 @@ public void startBundles(final Iterable 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 mandatoryPlugins) { } - private void checkIfMandatoryPluginsAreStarted(final List pluginsStarted, final Iterable 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 pluginsStarted, final Iterable 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 index 1f8dd02c..244eee64 100644 --- 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 mandatoryPluginsList = mandatoryPlugins != null && !mandatoryPlugins.isEmpty() ? Set.of(mandatoryPlugins.split("\\s*,\\s*")) : Collections.emptySet(); // This will call the start() method for the bundles