From: Tomas Olvecky Date: Mon, 2 Dec 2013 15:39:12 +0000 (+0100) Subject: Add warning when user destroys default module. X-Git-Tag: jenkins-controller-bulk-release-prepare-only-2-1~280^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=6117dc1c03128e9f7eac249b4a997449e254cdd6 Add warning when user destroys default module. Change-Id: I0a2b27fa8bac7e00b138847935cf803bb1ee8156 Signed-off-by: Tomas Olvecky --- diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java index 0fd6c52794..1b695a9bda 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java @@ -129,10 +129,18 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe */ @Override public synchronized ObjectName beginConfig() { - return beginConfigInternal().getControllerObjectName(); + return beginConfig(false); } - private synchronized ConfigTransactionControllerInternal beginConfigInternal() { + /** + * @param blankTransaction true if this transaction is created automatically by + * org.opendaylight.controller.config.manager.impl.osgi.BlankTransactionServiceTracker + */ + public synchronized ObjectName beginConfig(boolean blankTransaction) { + return beginConfigInternal(blankTransaction).getControllerObjectName(); + } + + private synchronized ConfigTransactionControllerInternal beginConfigInternal(boolean blankTransaction) { versionCounter++; String transactionName = "ConfigTransaction-" + version + "-" + versionCounter; TransactionJMXRegistrator transactionRegistrator = baseJMXRegistrator @@ -140,7 +148,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe Map> allCurrentFactories = Collections.unmodifiableMap(resolver.getAllFactories()); ConfigTransactionControllerInternal transactionController = new ConfigTransactionControllerImpl( transactionName, transactionRegistrator, version, - versionCounter, allCurrentFactories, transactionsMBeanServer, configMBeanServer, bundleContext); + versionCounter, allCurrentFactories, transactionsMBeanServer, configMBeanServer, blankTransaction); try { transactionRegistrator.registerMBean(transactionController, transactionController.getControllerObjectName()); } catch (InstanceAlreadyExistsException e) { @@ -223,7 +231,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator; if (entry.hasOldModule() == false) { runtimeBeanRegistrator = baseJMXRegistrator - .createRuntimeBeanRegistrator(entry.getName()); + .createRuntimeBeanRegistrator(entry.getIdentifier()); } else { // reuse old JMX registrator runtimeBeanRegistrator = entry.getOldInternalInfo() @@ -236,7 +244,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe .setRuntimeBeanRegistrator(runtimeBeanRegistrator); } // save it to info so it is accessible afterwards - runtimeRegistrators.put(entry.getName(), runtimeBeanRegistrator); + runtimeRegistrators.put(entry.getIdentifier(), runtimeBeanRegistrator); } // can register runtime beans @@ -264,8 +272,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // determine if current instance was recreated or reused or is new // rules for closing resources: - // osgi registration - will be (re)created every time, so it needs - // to be closed here + // osgi registration - will be reused if possible. // module jmx registration - will be (re)created every time, needs // to be closed here // runtime jmx registration - should be taken care of by module @@ -279,7 +286,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe ModuleInternalInfo oldInternalInfo = entry.getOldInternalInfo(); DynamicReadableWrapper oldReadableConfigBean = oldInternalInfo .getReadableModule(); - currentConfig.remove(entry.getName()); + currentConfig.remove(entry.getIdentifier()); // test if old instance == new instance if (oldReadableConfigBean.getInstance().equals(module.getInstance())) { @@ -324,7 +331,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe BundleContext bc = configTransactionController. getModuleFactoryBundleContext(moduleFactory.getImplementationName()); osgiRegistration = beanToOsgiServiceManager.registerToOsgi(module.getClass(), - newReadableConfigBean.getInstance(), entry.getName(), bc); + newReadableConfigBean.getInstance(), entry.getIdentifier(), bc); } else { throw new NullPointerException(entry.getIdentifier().getFactoryName() + " ModuleFactory not found."); } @@ -332,11 +339,11 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = runtimeRegistrators - .get(entry.getName()); + .get(entry.getIdentifier()); ModuleInternalInfo newInfo = new ModuleInternalInfo( - entry.getName(), newReadableConfigBean, osgiRegistration, + entry.getIdentifier(), newReadableConfigBean, osgiRegistration, runtimeBeanRegistrator, newModuleJMXRegistrator, - orderingIdx); + orderingIdx, entry.isDefaultBean()); newConfigEntries.put(module, newInfo); orderingIdx++; @@ -367,7 +374,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe /** * Abort open transactions and unregister read only modules. Since this * class is not responsible for registering itself under - * {@link ConfigRegistryMXBean#OBJECT_NAME}, it will not unregister itself + * {@link org.opendaylight.controller.config.api.ConfigRegistry#OBJECT_NAME}, it will not unregister itself * here. */ @Override diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java index a9ab664fd6..3e53a7a217 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java @@ -78,13 +78,14 @@ class ConfigTransactionControllerImpl implements configBeanModificationDisabled); private final MBeanServer configMBeanServer; - private final BundleContext bundleContext; + private final boolean blankTransaction; public ConfigTransactionControllerImpl(String transactionName, TransactionJMXRegistrator transactionRegistrator, long parentVersion, long currentVersion, Map> currentlyRegisteredFactories, - MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer, BundleContext bundleContext) { + MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer, + boolean blankTransaction) { this.transactionIdentifier = new TransactionIdentifier(transactionName); this.controllerON = ObjectNameUtil @@ -100,7 +101,7 @@ class ConfigTransactionControllerImpl implements this.dependencyResolverManager = new DependencyResolverManager(transactionName, transactionStatus); this.transactionsMBeanServer = transactionsMBeanServer; this.configMBeanServer = configMBeanServer; - this.bundleContext = bundleContext; + this.blankTransaction = blankTransaction; } @Override @@ -143,7 +144,8 @@ class ConfigTransactionControllerImpl implements // ensure default module to be registered to jmx even if its module factory does not use dependencyResolverFactory DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(module.getIdentifier()); try { - putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null, dependencyResolver); + boolean defaultBean = true; + putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null, dependencyResolver, defaultBean); } catch (InstanceAlreadyExistsException e) { throw new IllegalStateException(e); } @@ -184,7 +186,8 @@ class ConfigTransactionControllerImpl implements "Error while copying old configuration from %s to %s", oldConfigBeanInfo, moduleFactory), e); } - putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo, dependencyResolver); + putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo, dependencyResolver, + oldConfigBeanInfo.isDefaultBean()); } @Override @@ -201,14 +204,15 @@ class ConfigTransactionControllerImpl implements DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier); Module module = moduleFactory.createModule(instanceName, dependencyResolver, getModuleFactoryBundleContext(moduleFactory.getImplementationName())); + boolean defaultBean = false; return putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, - moduleFactory, null, dependencyResolver); + moduleFactory, null, dependencyResolver, defaultBean); } private synchronized ObjectName putConfigBeanToJMXAndInternalMaps( ModuleIdentifier moduleIdentifier, Module module, ModuleFactory moduleFactory, - @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver) + @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver, boolean isDefaultBean) throws InstanceAlreadyExistsException { logger.debug("Adding module {} to transaction {}", moduleIdentifier, this); @@ -232,14 +236,14 @@ class ConfigTransactionControllerImpl implements .registerMBean(writableDynamicWrapper, writableON); ModuleInternalTransactionalInfo moduleInternalTransactionalInfo = new ModuleInternalTransactionalInfo( moduleIdentifier, module, moduleFactory, - maybeOldConfigBeanInfo, transactionModuleJMXRegistration); + maybeOldConfigBeanInfo, transactionModuleJMXRegistration, isDefaultBean); dependencyResolverManager.put(moduleInternalTransactionalInfo); return writableON; } @Override - public void destroyModule(ObjectName objectName) + public synchronized void destroyModule(ObjectName objectName) throws InstanceNotFoundException { String foundTransactionName = ObjectNameUtil .getTransactionName(objectName); @@ -253,9 +257,17 @@ class ConfigTransactionControllerImpl implements destroyModule(moduleIdentifier); } - private void destroyModule(ModuleIdentifier moduleIdentifier) { + private synchronized void destroyModule(ModuleIdentifier moduleIdentifier) { logger.debug("Destroying module {} in transaction {}", moduleIdentifier, this); transactionStatus.checkNotAborted(); + + if (blankTransaction == false) { + ModuleInternalTransactionalInfo found = + dependencyResolverManager.findModuleInternalTransactionalInfo(moduleIdentifier); + if (found.isDefaultBean()) { + logger.warn("Warning: removing default bean. This will be forbidden in next version of config-subsystem"); + } + } ModuleInternalTransactionalInfo removedTInfo = dependencyResolverManager.destroyModule(moduleIdentifier); // remove from jmx removedTInfo.getTransactionModuleJMXRegistration().close(); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java index c675db5bff..14a706dd9b 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java @@ -35,12 +35,14 @@ public class ModuleInternalInfo implements Comparable, private final OsgiRegistration osgiRegistration; private final ModuleJMXRegistrator moduleJMXRegistrator; private final int orderingIdx; + private final boolean isDefaultBean; public ModuleInternalInfo(ModuleIdentifier name, @Nullable DynamicReadableWrapper readableModule, OsgiRegistration osgiRegistration, RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator, - ModuleJMXRegistrator moduleJMXRegistrator, int orderingIdx) { + ModuleJMXRegistrator moduleJMXRegistrator, int orderingIdx, + boolean isDefaultBean) { if (osgiRegistration == null) { throw new IllegalArgumentException( @@ -56,6 +58,7 @@ public class ModuleInternalInfo implements Comparable, this.name = name; this.moduleJMXRegistrator = moduleJMXRegistrator; this.orderingIdx = orderingIdx; + this.isDefaultBean = isDefaultBean; } public DynamicReadableWrapper getReadableModule() { @@ -117,4 +120,8 @@ public class ModuleInternalInfo implements Comparable, public ModuleIdentifier getIdentifier() { return name; } + + public boolean isDefaultBean() { + return isDefaultBean; + } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java index c4f40fbeeb..0a4ceacb43 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java @@ -24,27 +24,22 @@ public class ModuleInternalTransactionalInfo implements Identifiable result = new ArrayList<>(); for( ModuleInternalTransactionalInfo info : modulesHolder.getAllInfos()) { if (factory.equals(info.getModuleFactory())) { - result.add(info.getName()); + result.add(info.getIdentifier()); } } return result; diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModulesHolder.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModulesHolder.java index 6bbd7875a9..7c7d9f95a4 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModulesHolder.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModulesHolder.java @@ -85,7 +85,7 @@ class ModulesHolder implements TransactionHolder { public Map getAllModules() { Map result = new HashMap<>(); for (ModuleInternalTransactionalInfo entry : commitMap.values()) { - ModuleIdentifier name = entry.getName(); + ModuleIdentifier name = entry.getIdentifier(); result.put(name, entry.getModule()); } return result; @@ -94,17 +94,17 @@ class ModulesHolder implements TransactionHolder { @Override public void put( ModuleInternalTransactionalInfo moduleInternalTransactionalInfo) { - commitMap.put(moduleInternalTransactionalInfo.getName(), + commitMap.put(moduleInternalTransactionalInfo.getIdentifier(), moduleInternalTransactionalInfo); } @Override public ModuleInternalTransactionalInfo destroyModule( ModuleIdentifier moduleIdentifier) { - ModuleInternalTransactionalInfo found = commitMap - .remove(moduleIdentifier); - if (found == null) + ModuleInternalTransactionalInfo found = commitMap.remove(moduleIdentifier); + if (found == null) { throw new IllegalStateException("Not found:" + moduleIdentifier); + } if (found.hasOldModule()) { unorderedDestroyedFromPreviousTransactions.add(found); } @@ -123,4 +123,13 @@ class ModulesHolder implements TransactionHolder { public Collection getAllInfos(){ return commitMap.values(); } + + @Override + public ModuleInternalTransactionalInfo findModuleInternalTransactionalInfo(ModuleIdentifier moduleIdentifier) { + ModuleInternalTransactionalInfo found = commitMap.get(moduleIdentifier); + if (found == null) { + throw new IllegalStateException("Not found:" + moduleIdentifier); + } + return found; + } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java index 8f05ed75ae..bccd453af0 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java @@ -36,4 +36,6 @@ interface TransactionHolder { void assertNotExists(ModuleIdentifier moduleIdentifier) throws InstanceAlreadyExistsException; + ModuleInternalTransactionalInfo findModuleInternalTransactionalInfo(ModuleIdentifier moduleIdentifier); + } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java index 3d0decb93d..ec1a290cd2 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java @@ -44,7 +44,8 @@ public class BlankTransactionServiceTracker implements ServiceTrackerCustomizer< for (int i = 0; i < 10; i++) { try { // create transaction - ObjectName tx = configRegistry.beginConfig(); + boolean blankTransaction = true; + ObjectName tx = configRegistry.beginConfig(blankTransaction); CommitStatus commitStatus = configRegistry.commitConfig(tx); logger.debug("Committed blank transaction with status {}", commitStatus); return; diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java index 3cb0f447b4..22a959060c 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java @@ -67,7 +67,7 @@ public class ConfigTransactionControllerImplTest extends testedTxController = new ConfigTransactionControllerImpl( transactionName123, jmxRegistrator123, 1, 1, currentlyRegisteredFactories, transactionsMBeanServer, - ManagementFactory.getPlatformMBeanServer(), null); + ManagementFactory.getPlatformMBeanServer(), false); TransactionModuleJMXRegistrator transactionModuleJMXRegistrator123 = testedTxController .getTxModuleJMXRegistrator(); transactionModuleJMXRegistrator123.registerMBean( diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java index a79cb83a38..65b010532b 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java @@ -86,7 +86,7 @@ public class DependencyResolverManagerTest { private static void mockGetInstance(DependencyResolverManager tested, ModuleIdentifier moduleIdentifier) { ModuleInternalTransactionalInfo mock = mock(ModuleInternalTransactionalInfo.class); - doReturn(moduleIdentifier).when(mock).getName(); + doReturn(moduleIdentifier).when(mock).getIdentifier(); doReturn(mockedModule()).when(mock).getModule(); tested.put(mock); }