From 8019d2745b60f5cb4555c5edc2c94e96e8904a4b Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 13 Feb 2017 14:53:20 +0100 Subject: [PATCH 1/1] Clarify config pusher message Reword the message logged so it is more obvious what is going on. Also lower it from warning to info and perform a general code cleanup. Change-Id: I37035e8df15b10f40568c3e894ffe2875b935d95 Signed-off-by: Robert Varga --- .../internal/FeatureConfigPusher.java | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/opendaylight/config/config-persister-feature4-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigPusher.java b/opendaylight/config/config-persister-feature4-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigPusher.java index 7f254d5393..ed1b620381 100644 --- a/opendaylight/config/config-persister-feature4-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigPusher.java +++ b/opendaylight/config/config-persister-feature4-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigPusher.java @@ -11,13 +11,14 @@ import com.google.common.base.Optional; import com.google.common.collect.LinkedHashMultimap; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; import org.apache.karaf.features.Feature; import org.apache.karaf.features.FeaturesService; import org.opendaylight.controller.config.persist.api.ConfigPusher; -import org.opendaylight.controller.config.persist.api.ConfigSnapshotHolder; import org.opendaylight.controller.config.persist.storage.file.xml.XmlFileStorageAdapter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -27,22 +28,25 @@ import org.slf4j.LoggerFactory; */ public class FeatureConfigPusher { private static final Logger LOG = LoggerFactory.getLogger(FeatureConfigPusher.class); - private static final int MAX_RETRIES=100; - private static final int RETRY_PAUSE_MILLIS=1; + private static final int MAX_RETRIES = 100; + private static final int RETRY_PAUSE_MILLIS = 1; + private FeaturesService featuresService = null; private ConfigPusher pusher = null; + /* * A LinkedHashSet (to preserve order and insure uniqueness) of the pushedConfigs * This is used to prevent pushing duplicate configs if a Feature is in multiple dependency * chains. Also, preserves the *original* Feature chain for which we pushed the config. * (which is handy for logging). */ - Set pushedConfigs = new LinkedHashSet<>(); + private final Set pushedConfigs = new LinkedHashSet<>(); + /* * LinkedHashMultimap to track which configs we pushed for each Feature installation * For future use */ - LinkedHashMultimap feature2configs = LinkedHashMultimap.create(); + private final LinkedHashMultimap feature2configs = LinkedHashMultimap.create(); /* * @param p - ConfigPusher to push ConfigSnapshotHolders @@ -60,11 +64,10 @@ public class FeatureConfigPusher { * If a Feature is not in the returned LinkedHashMultimap then we couldn't push its configs * (Ususally because it was not yet installed) */ - public LinkedHashMultimap pushConfigs(final List features) throws Exception { + public LinkedHashMultimap pushConfigs(final List features) + throws Exception { LinkedHashMultimap pushedFeatures = LinkedHashMultimap.create(); for (Feature feature : features) { - - Set configSnapShots = pushConfig(feature); if (!configSnapShots.isEmpty()) { pushedFeatures.putAll(feature, configSnapShots); @@ -74,50 +77,55 @@ public class FeatureConfigPusher { } private Set pushConfig(final Feature feature) throws Exception { - Set configs = new LinkedHashSet<>(); // Ignore feature conditions — these encode conditions on other features and shouldn't be processed here if (feature.getName().contains("-condition-")) { LOG.debug("Ignoring conditional feature {}", feature); - } else if (isInstalled(feature)) { - // FIXME Workaround for BUG-2836, features service returns null for feature: standard-condition-webconsole_0_0_0, 3.0.1 - if (featuresService.getFeature(feature.getName(), feature.getVersion()) == null) { - LOG.debug("Feature: {}, {} is missing from features service. Skipping", feature.getName(), feature.getVersion()); - } else { - ChildAwareFeatureWrapper wrappedFeature = new ChildAwareFeatureWrapper(feature, featuresService); - configs = wrappedFeature.getFeatureConfigSnapshotHolders(); - if (!configs.isEmpty()) { - configs = pushConfig(configs, feature); - feature2configs.putAll(feature, configs); - } - } + return Collections.emptySet(); + } + + if (!isInstalled(feature)) { + return Collections.emptySet(); + } + // FIXME Workaround for BUG-2836, features service returns null for feature: + // standard-condition-webconsole_0_0_0, 3.0.1 + if (featuresService.getFeature(feature.getName(), feature.getVersion()) == null) { + LOG.debug("Feature: {}, {} is missing from features service. Skipping", feature.getName(), + feature.getVersion()); + return Collections.emptySet(); + } + + ChildAwareFeatureWrapper wrappedFeature = new ChildAwareFeatureWrapper(feature, featuresService); + Set configs = wrappedFeature.getFeatureConfigSnapshotHolders(); + if (!configs.isEmpty()) { + configs = pushConfig(configs, feature); + feature2configs.putAll(feature, configs); } return configs; } - private boolean isInstalled(final Feature feature) { + private boolean isInstalled(final Feature feature) throws InterruptedException { for (int retries = 0; retries < MAX_RETRIES; retries++) { try { List installedFeatures = Arrays.asList(featuresService.listInstalledFeatures()); - if(installedFeatures.contains(feature)) { + if (installedFeatures.contains(feature)) { return true; - } else { - LOG.warn("Karaf featuresService.listInstalledFeatures() has not yet finished installing feature (retry {}) {} {}",retries,feature.getName(),feature.getVersion()); } + + LOG.info("Karaf Feature Service has not yet finished installing feature {}/{} (retry {})", + feature.getName(), feature.getVersion(), retries); } catch (Exception e) { LOG.warn("Karaf featuresService.listInstalledFeatures() has thrown an exception, retry {}", retries, e); } - try { - Thread.sleep(RETRY_PAUSE_MILLIS); - } catch (InterruptedException e1) { - throw new IllegalStateException(e1); - } + + TimeUnit.MILLISECONDS.sleep(RETRY_PAUSE_MILLIS); } - LOG.error("Giving up (after {} retries) on Karaf featuresService.listInstalledFeatures() which has not yet finished installing feature {} {}",MAX_RETRIES,feature.getName(),feature.getVersion()); + LOG.error("Giving up (after {} retries) on Karaf featuresService.listInstalledFeatures() which has not yet finished installing feature {} {}", + MAX_RETRIES, feature.getName(), feature.getVersion()); return false; } - private Set pushConfig(final Set configs, final Feature feature) - throws InterruptedException { + private Set pushConfig(final Set configs, + final Feature feature) throws InterruptedException { Set configsToPush = new LinkedHashSet<>(configs); configsToPush.removeAll(pushedConfigs); if (!configsToPush.isEmpty()) { @@ -129,7 +137,7 @@ public class FeatureConfigPusher { LOG.warn("Ignoring default configuration {} for feature {}, the configuration is present in {}", configsToPush, feature.getId(), currentCfgPusher.get()); } else { - pusher.pushConfigs(new ArrayList(configsToPush)); + pusher.pushConfigs(new ArrayList<>(configsToPush)); } pushedConfigs.addAll(configsToPush); -- 2.36.6