From 03dd34039a9cd867ee95b58139323de46dc7b195 Mon Sep 17 00:00:00 2001 From: wilderrodrigues Date: Wed, 24 Jun 2015 13:59:50 +0200 Subject: [PATCH] Decouple the use of updateHostPassword - The code was hard to maintain because updating a host or all the hosts in a cluster was handled in the same method - Created updateHost and updateCluster password in both ResourceManager and ManagementServer interfaces/classes - The chck for whihc method to use is done in the API level - Started adding the support for KVM host passwd update No API changes are needed and it will be backwards compatible. Signed-off-by: wilderrodrigues --- .../com/cloud/resource/ResourceService.java | 2 + .../com/cloud/server/ManagementService.java | 2 + .../admin/host/UpdateHostPasswordCmd.java | 10 +- .../cloud/resource/ResourceManagerImpl.java | 54 +++++---- .../cloud/server/ManagementServerImpl.java | 112 ++++++++++++------ 5 files changed, 113 insertions(+), 67 deletions(-) diff --git a/api/src/com/cloud/resource/ResourceService.java b/api/src/com/cloud/resource/ResourceService.java index f1080b82488..7461455aaee 100644 --- a/api/src/com/cloud/resource/ResourceService.java +++ b/api/src/com/cloud/resource/ResourceService.java @@ -84,6 +84,8 @@ public interface ResourceService { */ boolean deleteHost(long hostId, boolean isForced, boolean isForceDeleteStorage); + boolean updateClusterPassword(UpdateHostPasswordCmd upasscmd); + boolean updateHostPassword(UpdateHostPasswordCmd upasscmd); Host getHost(long hostId); diff --git a/api/src/com/cloud/server/ManagementService.java b/api/src/com/cloud/server/ManagementService.java index 7bf6154df15..7f3141bc418 100644 --- a/api/src/com/cloud/server/ManagementService.java +++ b/api/src/com/cloud/server/ManagementService.java @@ -300,6 +300,8 @@ public interface ManagementService { boolean updateHostPassword(UpdateHostPasswordCmd cmd); + boolean updateClusterPassword(UpdateHostPasswordCmd cmd); + InstanceGroup updateVmGroup(UpdateVMGroupCmd cmd); Map listCapabilities(ListCapabilitiesCmd cmd); diff --git a/api/src/org/apache/cloudstack/api/command/admin/host/UpdateHostPasswordCmd.java b/api/src/org/apache/cloudstack/api/command/admin/host/UpdateHostPasswordCmd.java index 91f47dadfc1..74b06d7dce6 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/host/UpdateHostPasswordCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/host/UpdateHostPasswordCmd.java @@ -86,8 +86,14 @@ public class UpdateHostPasswordCmd extends BaseCmd { @Override public void execute() { - _mgr.updateHostPassword(this); - _resourceService.updateHostPassword(this); + if (getClusterId() == null) { + _mgr.updateHostPassword(this); + _resourceService.updateHostPassword(this); + } else { + _mgr.updateClusterPassword(this); + _resourceService.updateClusterPassword(this); + } + setResponseObject(new SuccessResponse(getCommandName())); } } diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index 3d8f86ce2dc..53a6b1969db 100644 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -2239,39 +2239,41 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, } @Override - public boolean updateHostPassword(final UpdateHostPasswordCmd cmd) { - if (cmd.getClusterId() == null) { - // update agent attache password + public boolean updateClusterPassword(final UpdateHostPasswordCmd command) { + // get agents for the cluster + final List hosts = listAllHostsInCluster(command.getClusterId()); + for (final HostVO h : hosts) { try { - final Boolean result = propagateResourceEvent(cmd.getHostId(), ResourceState.Event.UpdatePassword); + /* + * FIXME: this is a buggy logic, check with alex. Shouldn't + * return if propagation return non null + */ + final Boolean result = propagateResourceEvent(h.getId(), ResourceState.Event.UpdatePassword); if (result != null) { return result; } } catch (final AgentUnavailableException e) { + s_logger.error("Agent is not availbale!", e); } - - return doUpdateHostPassword(cmd.getHostId()); - } else { - // get agents for the cluster - final List hosts = listAllHostsInCluster(cmd.getClusterId()); - for (final HostVO h : hosts) { - try { - /* - * FIXME: this is a buggy logic, check with alex. Shouldn't - * return if propagation return non null - */ - final Boolean result = propagateResourceEvent(h.getId(), ResourceState.Event.UpdatePassword); - if (result != null) { - return result; - } - - doUpdateHostPassword(h.getId()); - } catch (final AgentUnavailableException e) { - } - } - - return true; + doUpdateHostPassword(h.getId()); } + + return true; + } + + @Override + public boolean updateHostPassword(final UpdateHostPasswordCmd cmd) { + // update agent attache password + try { + final Boolean result = propagateResourceEvent(cmd.getHostId(), ResourceState.Event.UpdatePassword); + if (result != null) { + return result; + } + } catch (final AgentUnavailableException e) { + s_logger.error("Agent is not availbale!", e); + } + + return doUpdateHostPassword(cmd.getHostId()); } public String getPeerName(final long agentHostId) { diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 78f0f5a6552..8774f70b2a4 100644 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -3739,50 +3739,84 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe return password; } + private boolean updateHostsInCluster(final UpdateHostPasswordCmd command) { + // get all the hosts in this cluster + final List hosts = _resourceMgr.listAllHostsInCluster(command.getClusterId()); + + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(final TransactionStatus status) { + for (final HostVO h : hosts) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Changing password for host name = " + h.getName()); + } + // update password for this host + final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME); + if (nv.getValue().equals(command.getUsername())) { + final DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.PASSWORD); + nvp.setValue(DBEncryptionUtil.encrypt(command.getPassword())); + _detailsDao.persist(nvp); + } else { + // if one host in the cluster has diff username then + // rollback to maintain consistency + throw new InvalidParameterValueException("The username is not same for all hosts, please modify passwords for individual hosts."); + } + } + } + }); + return true; + } + + /** + * This method updates the password of all hosts in a given cluster. + */ + @Override + @DB + public boolean updateClusterPassword(final UpdateHostPasswordCmd command) { + if (command.getClusterId() == null) { + throw new InvalidParameterValueException("You should provide a cluster id."); + } + + final ClusterVO cluster = ApiDBUtils.findClusterById(command.getClusterId()); + if (cluster == null || cluster.getHypervisorType() != HypervisorType.XenServer || cluster.getHypervisorType() != HypervisorType.KVM) { + throw new InvalidParameterValueException("This operation is not supported for this hypervisor type"); + } + return updateHostsInCluster(command); + } + @Override @DB public boolean updateHostPassword(final UpdateHostPasswordCmd cmd) { - if (cmd.getClusterId() == null && cmd.getHostId() == null) { - throw new InvalidParameterValueException("You should provide one of cluster id or a host id."); - } else if (cmd.getClusterId() == null) { - final HostVO host = _hostDao.findById(cmd.getHostId()); - if (host != null && host.getHypervisorType() == HypervisorType.XenServer) { - throw new InvalidParameterValueException("You should provide cluster id for Xenserver cluster."); - } else { - throw new InvalidParameterValueException("This operation is not supported for this hypervisor type"); - } - } else { - - final ClusterVO cluster = ApiDBUtils.findClusterById(cmd.getClusterId()); - if (cluster == null || cluster.getHypervisorType() != HypervisorType.XenServer) { - throw new InvalidParameterValueException("This operation is not supported for this hypervisor type"); - } - // get all the hosts in this cluster - final List hosts = _resourceMgr.listAllHostsInCluster(cmd.getClusterId()); - - Transaction.execute(new TransactionCallbackNoReturn() { - @Override - public void doInTransactionWithoutResult(final TransactionStatus status) { - for (final HostVO h : hosts) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Changing password for host name = " + h.getName()); - } - // update password for this host - final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME); - if (nv.getValue().equals(cmd.getUsername())) { - final DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.PASSWORD); - nvp.setValue(DBEncryptionUtil.encrypt(cmd.getPassword())); - _detailsDao.persist(nvp); - } else { - // if one host in the cluster has diff username then - // rollback to maintain consistency - throw new InvalidParameterValueException("The username is not same for all hosts, please modify passwords for individual hosts."); - } - } - } - }); + if (cmd.getHostId() == null) { + throw new InvalidParameterValueException("You should provide an host id."); } + final HostVO host = _hostDao.findById(cmd.getHostId()); + + if (host.getHypervisorType() != HypervisorType.XenServer || host.getHypervisorType() != HypervisorType.KVM) { + throw new InvalidParameterValueException("This operation is not supported for this hypervisor type"); + } + + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(final TransactionStatus status) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Changing password for host name = " + host.getName()); + } + // update password for this host + final DetailVO nv = _detailsDao.findDetail(host.getId(), ApiConstants.USERNAME); + if (nv.getValue().equals(cmd.getUsername())) { + final DetailVO nvp = _detailsDao.findDetail(host.getId(), ApiConstants.PASSWORD); + nvp.setValue(DBEncryptionUtil.encrypt(cmd.getPassword())); + _detailsDao.persist(nvp); + } else { + // if one host in the cluster has diff username then + // rollback to maintain consistency + throw new InvalidParameterValueException("The username is not same for the hosts.."); + } + } + }); + return true; }