From ddda5fc4319692e275a4495746e34b345ff89ef4 Mon Sep 17 00:00:00 2001 From: Kelven Yang Date: Mon, 31 Jan 2011 17:40:07 -0800 Subject: [PATCH] Cleanup of worker VMs left over from previous session in a reliable way --- .../cloud/agent/manager/AgentManagerImpl.java | 10 ++++---- server/src/com/cloud/maid/StackMaid.java | 24 +++++++++++++++---- .../com/cloud/maid/StackMaidManagerImpl.java | 4 ++-- .../test/com/cloud/async/CleanupDelegate.java | 7 +++--- .../src/com/cloud/utils/CleanupDelegate.java | 5 ++++ 5 files changed, 35 insertions(+), 15 deletions(-) create mode 100644 utils/src/com/cloud/utils/CleanupDelegate.java diff --git a/server/src/com/cloud/agent/manager/AgentManagerImpl.java b/server/src/com/cloud/agent/manager/AgentManagerImpl.java index 7cd8e11979b..56eb4d4ffd8 100755 --- a/server/src/com/cloud/agent/manager/AgentManagerImpl.java +++ b/server/src/com/cloud/agent/manager/AgentManagerImpl.java @@ -612,7 +612,8 @@ public class AgentManagerImpl implements AgentManager, HandlerFactory, Hypervisor.HypervisorType hypervisorType = Hypervisor.HypervisorType.getType(cmd.getHypervisor()); if (hypervisorType == null) { - throw new InvalidParameterValueException("Please specify a valid hypervisor name"); + s_logger.error("Unable to resolve " + cmd.getHypervisor() + " to a valid supported hypervisor type"); + throw new InvalidParameterValueException("Unable to resolve " + cmd.getHypervisor() + " to a supported "); } Cluster.ClusterType clusterType = null; @@ -625,8 +626,8 @@ public class AgentManagerImpl implements AgentManager, HandlerFactory, Discoverer discoverer = getMatchingDiscover(hypervisorType); if (discoverer == null) { - throw new InvalidParameterValueException( - "Please specify a valid hypervisor"); + + throw new InvalidParameterValueException("Could not find corresponding resource manager for " + cmd.getHypervisor()); } List result = new ArrayList(); @@ -664,8 +665,7 @@ public class AgentManagerImpl implements AgentManager, HandlerFactory, uri = new URI(UriUtils.encodeURIComponent(url)); if (uri.getScheme() == null) { throw new InvalidParameterValueException( - "uri.scheme is null " + url - + ", add http:// as a prefix"); + "uri.scheme is null " + url + ", add http:// as a prefix"); } else if (uri.getScheme().equalsIgnoreCase("http")) { if (uri.getHost() == null || uri.getHost().equalsIgnoreCase("") diff --git a/server/src/com/cloud/maid/StackMaid.java b/server/src/com/cloud/maid/StackMaid.java index 3739b3c84e3..feddbf28c27 100644 --- a/server/src/com/cloud/maid/StackMaid.java +++ b/server/src/com/cloud/maid/StackMaid.java @@ -8,7 +8,8 @@ import org.apache.log4j.Logger; import com.cloud.maid.dao.StackMaidDao; import com.cloud.maid.dao.StackMaidDaoImpl; import com.cloud.serializer.SerializerHelper; -import com.cloud.utils.ActionDelegate; +import com.cloud.utils.CleanupDelegate; +import com.cloud.utils.db.Transaction; public class StackMaid { protected final static Logger s_logger = Logger.getLogger(StackMaid.class); @@ -43,6 +44,16 @@ public class StackMaid { public Object getContext(String key) { return context.get(key); } + + public void expungeMaidItem(long maidId) { + // this is a bit ugly, but when it is not loaded by component locator, this is just a workable way for now + Transaction txn = Transaction.open(Transaction.CLOUD_DB); + try { + maidDao.expunge(maidId); + } finally { + txn.close(); + } + } public int push(String delegateClzName, Object context) { assert(msid_setby_manager != 0) : "Fatal, make sure StackMaidManager is loaded"; @@ -98,13 +109,15 @@ public class StackMaid { context.clear(); } - public static void doCleanup(StackMaidVO maid) { + public static boolean doCleanup(StackMaidVO maid) { if(maid.getDelegate() != null) { try { Class clz = Class.forName(maid.getDelegate()); Object delegate = clz.newInstance(); - if(delegate instanceof ActionDelegate) { - ((ActionDelegate)delegate).action(SerializerHelper.fromSerializedString(maid.getContext())); + if(delegate instanceof CleanupDelegate) { + return ((CleanupDelegate)delegate).cleanup(SerializerHelper.fromSerializedString(maid.getContext()), maid); + } else { + assert(false); } } catch (final ClassNotFoundException e) { s_logger.error("Unable to load StackMaid delegate class: " + maid.getDelegate(), e); @@ -117,6 +130,9 @@ public class StackMaid { } catch (final IllegalAccessException e) { s_logger.error("Illegal access exception when loading resource: " + maid.getDelegate()); } + + return false; } + return true; } } diff --git a/server/src/com/cloud/maid/StackMaidManagerImpl.java b/server/src/com/cloud/maid/StackMaidManagerImpl.java index ab92cb2eaa6..6e6132df970 100644 --- a/server/src/com/cloud/maid/StackMaidManagerImpl.java +++ b/server/src/com/cloud/maid/StackMaidManagerImpl.java @@ -51,8 +51,8 @@ public class StackMaidManagerImpl implements StackMaidManager { private void cleanupLeftovers(List l) { for(StackMaidVO maid : l) { - StackMaid.doCleanup(maid); - _maidDao.expunge(maid.getId()); + if(StackMaid.doCleanup(maid)) + _maidDao.expunge(maid.getId()); } } diff --git a/server/test/com/cloud/async/CleanupDelegate.java b/server/test/com/cloud/async/CleanupDelegate.java index 7d5d38b47a7..5ce6b4baa7e 100644 --- a/server/test/com/cloud/async/CleanupDelegate.java +++ b/server/test/com/cloud/async/CleanupDelegate.java @@ -2,13 +2,12 @@ package com.cloud.async; import org.apache.log4j.Logger; -import com.cloud.utils.ActionDelegate; - -public class CleanupDelegate implements ActionDelegate { +public class CleanupDelegate implements com.cloud.utils.CleanupDelegate { private static final Logger s_logger = Logger.getLogger(CleanupDelegate.class); @Override - public void action(String param) { + public boolean cleanup(String param, Object managerContext) { s_logger.info("Action called with param: " + param); + return true; } } diff --git a/utils/src/com/cloud/utils/CleanupDelegate.java b/utils/src/com/cloud/utils/CleanupDelegate.java new file mode 100644 index 00000000000..545dcddfb9f --- /dev/null +++ b/utils/src/com/cloud/utils/CleanupDelegate.java @@ -0,0 +1,5 @@ +package com.cloud.utils; + +public interface CleanupDelegate { + boolean cleanup(T itemContext, M managerContext); +}