Remove old secret as it does not work well with config-persister.
Add parameter maxWaitTime to shutdown RPC that specifies amount of time before
server is forcibly shut down.
Work around buggy yuma message sending xml element without inner text
node if blank value is specified.
Change-Id: Ief3fea74107349c5ff425ffdc184346981254bc4
Signed-off-by: Tomas Olvecky <tolvecky@cisco.com>
@Override
public String toString() {
- return "ExceptionMessageWithStackTrace [message=" + message
- + ", stackTrace=" + stackTrace + "]";
+ return message;
}
}
return null;
}
- private synchronized void blankTransaction() {
+ synchronized void blankTransaction() {
// race condition check: config-persister might push new configuration while server is starting up.
ConflictingVersionException lastException = null;
for (int i = 0; i < 10; i++) {
configRegistryJMXRegistrator.registerToJMX(configRegistry);
// track bundles containing factories
- extenderBundleTracker = new ExtenderBundleTracker(context);
+ BlankTransactionServiceTracker blankTransactionServiceTracker = new BlankTransactionServiceTracker(configRegistry);
+ extenderBundleTracker = new ExtenderBundleTracker(context, blankTransactionServiceTracker);
extenderBundleTracker.open();
- BlankTransactionServiceTracker customizer = new BlankTransactionServiceTracker(configRegistry);
- ServiceTracker<?, ?> serviceTracker = new ServiceTracker(context, ModuleFactory.class, customizer);
+ ServiceTracker<?, ?> serviceTracker = new ServiceTracker(context, ModuleFactory.class, blankTransactionServiceTracker);
serviceTracker.open();
}
* Code based on http://www.toedter.com/blog/?p=236
*/
public class ExtenderBundleTracker extends BundleTracker<Object> {
-
+ private final BlankTransactionServiceTracker blankTransactionServiceTracker;
private static final Logger logger = LoggerFactory.getLogger(ExtenderBundleTracker.class);
- public ExtenderBundleTracker(BundleContext context) {
+ public ExtenderBundleTracker(BundleContext context, BlankTransactionServiceTracker blankTransactionServiceTracker) {
super(context, Bundle.ACTIVE, null);
+ this.blankTransactionServiceTracker = blankTransactionServiceTracker;
logger.trace("Registered as extender with context {}", context);
}
@Override
public void removedBundle(Bundle bundle, BundleEvent event, Object object) {
super.removedBundle(bundle,event,object);
+ // workaround for service tracker not getting removed service event
+ blankTransactionServiceTracker.blankTransaction();
}
// TODO:test
* @param inputSecret must match configured secret of the implementation
* @param reason Optional string to be logged while shutting down
*/
- void shutdown(String inputSecret, Optional<String> reason);
+ void shutdown(String inputSecret, Long maxWaitTime, Optional<String> reason);
}
public final class ShutdownModule extends AbstractShutdownModule {
private final Bundle systemBundle;
- private final ShutdownModule nullableOldModule;
public ShutdownModule(ModuleIdentifier identifier, Bundle systemBundle) {
super(identifier, null);
singletonCheck(identifier);
this.systemBundle = systemBundle;
- this.nullableOldModule = null;
}
public ShutdownModule(ModuleIdentifier identifier, ShutdownModule oldModule, java.lang.AutoCloseable oldInstance,
super(identifier, null, oldModule, oldInstance);
singletonCheck(identifier);
this.systemBundle = systemBundle;
- this.nullableOldModule = oldModule;
}
private static void singletonCheck(ModuleIdentifier identifier) {
throw new UnsupportedOperationException();
}
- @Override
- public String getSecret() {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public String getOldSecret() {
- throw new UnsupportedOperationException();
- }
-
- String getActualSecret() {
- return super.getSecret();
- }
-
- String getActualOldSecret() {
- return super.getOldSecret();
- }
-
@Override
protected void customValidation() {
- JmxAttributeValidationException.checkNotNull(super.getOldSecret(), oldSecretJmxAttribute);
JmxAttributeValidationException.checkNotNull(super.getSecret(), secretJmxAttribute);
- if (nullableOldModule != null) {
- // if nothing changed, remain valid
- boolean sameAsOldModule = isSame(nullableOldModule);
- if (sameAsOldModule == false) {
- boolean valid = getActualOldSecret().equals(nullableOldModule.getActualSecret());
- JmxAttributeValidationException.checkCondition(valid, "Invalid old secret", oldSecretJmxAttribute);
- }
- }
}
@Override
public java.lang.AutoCloseable createInstance() {
- return new ShutdownServiceImpl(getActualSecret(), systemBundle, getRootRuntimeBeanRegistratorWrapper());
+ return new ShutdownServiceImpl(getSecret(), systemBundle, getRootRuntimeBeanRegistratorWrapper());
}
}
package org.opendaylight.controller.config.yang.shutdown.impl;
import org.opendaylight.controller.config.api.DependencyResolver;
+import org.opendaylight.controller.config.api.DependencyResolverFactory;
import org.opendaylight.controller.config.api.ModuleIdentifier;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
-public class ShutdownModuleFactory extends AbstractShutdownModuleFactory {
-
- @Override
- public org.opendaylight.controller.config.spi.Module createModule(String instanceName, org.opendaylight.controller.config.api.DependencyResolver dependencyResolver, org.opendaylight.controller.config.api.DynamicMBeanWithInstance old, org.osgi.framework.BundleContext bundleContext) throws Exception {
- org.opendaylight.controller.config.yang.shutdown.impl.ShutdownModule oldModule = null;
- try {
- oldModule = (org.opendaylight.controller.config.yang.shutdown.impl.ShutdownModule) old.getModule();
- } catch(Exception e) {
- return handleChangedClass(old);
- }
- org.opendaylight.controller.config.yang.shutdown.impl.ShutdownModule module = instantiateModule(instanceName, dependencyResolver, oldModule, old.getInstance(), bundleContext);
-
- module.setOldSecret(oldModule.getActualOldSecret());
- module.setSecret(oldModule.getActualSecret());
-
- return module;
- }
+import java.util.Arrays;
+import java.util.Set;
+public class ShutdownModuleFactory extends AbstractShutdownModuleFactory {
public ShutdownModule instantiateModule(String instanceName, DependencyResolver dependencyResolver,
ShutdownModule oldModule, AutoCloseable oldInstance,
Bundle systemBundle = bundleContext.getBundle(0);
return new ShutdownModule(new ModuleIdentifier(NAME, instanceName), systemBundle);
}
+
+ @Override
+ public Set<ShutdownModule> getDefaultModules(DependencyResolverFactory dependencyResolverFactory, BundleContext bundleContext) {
+ ModuleIdentifier id = new ModuleIdentifier(NAME, NAME);
+ DependencyResolver dependencyResolver = dependencyResolverFactory.createDependencyResolver(id);
+ ShutdownModule shutdownModule = instantiateModule(NAME, dependencyResolver, bundleContext);
+ return new java.util.HashSet<>(Arrays.asList(shutdownModule));
+ }
}
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+
public class ShutdownServiceImpl implements ShutdownService, AutoCloseable {
private final ShutdownService impl;
private final ShutdownRuntimeRegistration registration;
}
@Override
- public void shutdown(String inputSecret, Optional<String> reason) {
- impl.shutdown(inputSecret, reason);
+ public void shutdown(String inputSecret, Long maxWaitTime, Optional<String> reason) {
+ impl.shutdown(inputSecret, maxWaitTime, reason);
}
@Override
}
@Override
- public void shutdown(String inputSecret, Optional<String> reason) {
+ public void shutdown(String inputSecret, Long maxWaitTime, Optional<String> reason) {
logger.warn("Shutdown issued with secret {} and reason {}", inputSecret, reason);
try {
Thread.sleep(1000); // prevent brute force attack
if (this.secret.equals(inputSecret)) {
logger.info("Server is shutting down");
- Thread stopSystemBundle = new Thread() {
- @Override
- public void run() {
- try {
- // wait so that JMX response is received
- Thread.sleep(1000);
- systemBundle.stop();
- } catch (BundleException e) {
- logger.warn("Can not stop OSGi server", e);
- } catch (InterruptedException e) {
- logger.warn("Shutdown process interrupted", e);
- }
- }
- };
- stopSystemBundle.start();
-
+ // actual work:
+ Thread stopSystemBundleThread = new StopSystemBundleThread(systemBundle);
+ stopSystemBundleThread.start();
+ if (maxWaitTime != null && maxWaitTime > 0) {
+ Thread systemExitThread = new CallSystemExitThread(maxWaitTime);
+ logger.debug("Scheduling {}", systemExitThread);
+ systemExitThread.start();
+ }
+ // end
} else {
logger.warn("Unauthorized attempt to shut down server");
throw new IllegalArgumentException("Invalid secret");
}
+class StopSystemBundleThread extends Thread {
+ private static final Logger logger = LoggerFactory.getLogger(StopSystemBundleThread.class);
+ private final Bundle systemBundle;
+
+ StopSystemBundleThread(Bundle systemBundle) {
+ super("stop-system-bundle");
+ this.systemBundle = systemBundle;
+ }
+
+ @Override
+ public void run() {
+ try {
+ // wait so that JMX response is received
+ Thread.sleep(1000);
+ systemBundle.stop();
+ } catch (BundleException e) {
+ logger.warn("Can not stop OSGi server", e);
+ } catch (InterruptedException e) {
+ logger.warn("Shutdown process interrupted", e);
+ }
+ }
+}
+
+class CallSystemExitThread extends Thread {
+ private static final Logger logger = LoggerFactory.getLogger(CallSystemExitThread.class);
+ private final long maxWaitTime;
+ CallSystemExitThread(long maxWaitTime) {
+ super("call-system-exit-daemon");
+ setDaemon(true);
+ if (maxWaitTime <= 0){
+ throw new IllegalArgumentException("Cannot schedule to zero or negative time:" + maxWaitTime);
+ }
+ this.maxWaitTime = maxWaitTime;
+ }
+
+ @Override
+ public String toString() {
+ return "CallSystemExitThread{" +
+ "maxWaitTime=" + maxWaitTime +
+ '}';
+ }
+
+ @Override
+ public void run() {
+ try {
+ // wait specified time
+ Thread.sleep(maxWaitTime);
+ logger.error("Since some threads are still running, server is going to shut down via System.exit(1) !");
+ // do a thread dump
+ ThreadInfo[] threads = ManagementFactory.getThreadMXBean().dumpAllThreads(true, true);
+ StringBuffer sb = new StringBuffer();
+ for(ThreadInfo info : threads) {
+ sb.append(info);
+ sb.append("\n");
+ }
+ logger.warn("Thread dump:{}", sb);
+ System.exit(1);
+ } catch (InterruptedException e) {
+ logger.info("Interrupted, not going to call System.exit(1)");
+ }
+ }
+}
+
+
class MXBeanImpl implements ShutdownRuntimeMXBean {
private final ShutdownService impl;
}
@Override
- public void shutdown(String inputSecret, String nullableReason) {
+ public void shutdown(String inputSecret, Long maxWaitTime, String nullableReason) {
Optional<String> optionalReason;
if (nullableReason == null) {
optionalReason = Optional.absent();
} else {
optionalReason = Optional.of(nullableReason);
}
- impl.shutdown(inputSecret, optionalReason);
+ impl.shutdown(inputSecret, maxWaitTime, optionalReason);
}
}
type string;
default "";
}
- leaf old-secret {
- type string;
- default "";
- }
}
}
leaf input-secret {
type string;
}
+ leaf max-wait-time {
+ type uint32;
+ description "Maximum time in milliseconds before process is forcibly exited. Zero or null cancels this functionality.";
+ }
leaf reason {
type string;
}
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
-import org.opendaylight.controller.config.api.ValidationException;
-import org.opendaylight.controller.config.api.ValidationException.ExceptionMessageWithStackTrace;
import org.opendaylight.controller.config.api.jmx.ObjectNameUtil;
import org.opendaylight.controller.config.manager.impl.AbstractConfigTest;
import org.opendaylight.controller.config.manager.impl.factoriesresolver.HardcodedModuleFactoriesResolver;
import javax.management.JMX;
import javax.management.ObjectName;
import java.util.Collections;
-import java.util.Map;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
doReturn(mockedSysBundle).when(mockedContext).getBundle(0);
mockedContext.getBundle(0);
doNothing().when(mockedSysBundle).stop();
+ ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction();
+ // initialize default instance
+ transaction.commit();
}
@Test
}
}
+ private static final ObjectName runtimeON = ObjectNameUtil.createRuntimeBeanName(NAME, NAME, Collections.<String, String>emptyMap());
+
@Test
public void testWithoutSecret() throws Exception {
- ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction();
- transaction.createModule(NAME, NAME);
- transaction.commit();
// test JMX rpc
- ObjectName runtimeON = ObjectNameUtil.createRuntimeBeanName(NAME, NAME, Collections.<String, String>emptyMap());
+
ShutdownRuntimeMXBean runtime = configRegistryClient.newMXBeanProxy(runtimeON, ShutdownRuntimeMXBean.class);
try {
- runtime.shutdown("foo", null);
+ runtime.shutdown("foo", 60000L, null);
fail();
} catch (IllegalArgumentException e) {
assertEquals("Invalid secret", e.getMessage());
}
- runtime.shutdown("", null);
+ runtime.shutdown("", 60000L, null);
assertStopped();
}
@Test
public void testWithSecret() throws Exception {
ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction();
- ObjectName on = transaction.createModule(NAME, NAME);
+ ObjectName on = transaction.lookupConfigBean(NAME, NAME);
ShutdownModuleMXBean proxy = transaction.newMXBeanProxy(on, ShutdownModuleMXBean.class);
String secret = "secret";
proxy.setSecret(secret);
transaction.commit();
shutdownViaRuntimeJMX(secret);
-
- // test old secret
- transaction = configRegistryClient.createTransaction();
- on = transaction.lookupConfigBean(NAME, NAME);
- proxy = transaction.newMXBeanProxy(on, ShutdownModuleMXBean.class);
- try {
- rethrowCause(proxy).getOldSecret();
- fail();
- } catch (UnsupportedOperationException e) {
- }
try {
- rethrowCause(proxy).getSecret();
+ ShutdownRuntimeMXBean runtime = JMX.newMXBeanProxy(platformMBeanServer, runtimeON, ShutdownRuntimeMXBean.class);
+ runtime.shutdown("foo", 60000L, null);
fail();
- } catch (UnsupportedOperationException e) {
- }
- // set secret to nothing
- String newSecret = "newSecret";
- proxy.setSecret(newSecret);
- try {
- transaction.commit();
- fail("Old secret not provided - should fail validation");
- } catch (ValidationException e) {
- Map<String, Map<String, ExceptionMessageWithStackTrace>> failedValidations = e.getFailedValidations();
- assertTrue(failedValidations.containsKey(NAME));
- ExceptionMessageWithStackTrace exceptionMessageWithStackTrace = failedValidations.get(NAME).get(NAME);
- assertNotNull(exceptionMessageWithStackTrace);
- assertEquals("OldSecret Invalid old secret", exceptionMessageWithStackTrace.getMessage());
-
+ } catch (IllegalArgumentException e) {
+ assertEquals("Invalid secret", e.getMessage());
}
- proxy.setOldSecret(secret);
- transaction.commit();
- shutdownViaRuntimeJMX(newSecret);
}
private void shutdownViaRuntimeJMX(String secret) throws Exception {
// test JMX rpc
- ObjectName runtimeON = ObjectNameUtil.createRuntimeBeanName(NAME, NAME, Collections.<String, String>emptyMap());
ShutdownRuntimeMXBean runtime = JMX.newMXBeanProxy(platformMBeanServer, runtimeON, ShutdownRuntimeMXBean.class);
try {
- runtime.shutdown("", null);
+ runtime.shutdown("", 60000L, null);
fail();
} catch (IllegalArgumentException e) {
assertEquals("Invalid secret", e.getMessage());
}
- runtime.shutdown(secret, null);
+ runtime.shutdown(secret, 60000L, null);
assertStopped();
}
import com.google.common.base.Preconditions;
import org.opendaylight.controller.netconf.util.xml.XmlElement;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.List;
public class SimpleAttributeReadingStrategy extends AbstractAttributeReadingStrategy {
+ private static final Logger logger = LoggerFactory.getLogger(SimpleAttributeReadingStrategy.class);
+
public SimpleAttributeReadingStrategy(String nullableDefault) {
super(nullableDefault);
Preconditions.checkState(configNodes.size() == 1, "This element should be present only once " + xmlElement
+ " but was " + configNodes.size());
- String textContent = xmlElement.getTextContent();
+ String textContent = "";
+ try{
+ textContent = xmlElement.getTextContent();
+ }catch(IllegalStateException | NullPointerException e) {
+ // yuma sends <attribute /> for empty value instead of <attribute></attribute>
+ logger.warn("Ignoring exception caused by failure to read text element", e);
+ }
Preconditions.checkNotNull(textContent, "This element should contain text %s", xmlElement);
return AttributeConfigElement.create(postprocessNullableDefault(getNullableDefault()),
public String getTextContent() {
Node textChild = element.getFirstChild();
- Preconditions.checkState(textChild instanceof Text, getName() + " should contain text");
+ Preconditions.checkNotNull(textChild, "Child node expected, got null for " + getName() + " : " + element);
+ Preconditions.checkState(textChild instanceof Text, getName() + " should contain text." +
+ Text.class.getName() + " expected, got " + textChild);
String content = textChild.getTextContent();
// Trim needed
return content.trim();
Code Review / controller.git / commitdiff