Skip to content

Commit 03dd340

Browse files
wilderrodrigueswilderrodrigues
committed
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 <wrodrigues@schubergphilis.com>
1 parent ac1b5e3 commit 03dd340

5 files changed

Lines changed: 109 additions & 63 deletions

File tree

‎api/src/com/cloud/resource/ResourceService.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ public interface ResourceService {
8484
*/
8585
boolean deleteHost(long hostId, boolean isForced, boolean isForceDeleteStorage);
8686

87+
boolean updateClusterPassword(UpdateHostPasswordCmd upasscmd);
88+
8789
boolean updateHostPassword(UpdateHostPasswordCmd upasscmd);
8890

8991
Host getHost(long hostId);

‎api/src/com/cloud/server/ManagementService.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,8 @@ public interface ManagementService {
300300

301301
boolean updateHostPassword(UpdateHostPasswordCmd cmd);
302302

303+
boolean updateClusterPassword(UpdateHostPasswordCmd cmd);
304+
303305
InstanceGroup updateVmGroup(UpdateVMGroupCmd cmd);
304306

305307
Map<String, Object> listCapabilities(ListCapabilitiesCmd cmd);

‎api/src/org/apache/cloudstack/api/command/admin/host/UpdateHostPasswordCmd.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,14 @@ public long getEntityOwnerId() {
8686

8787
@Override
8888
public void execute() {
89-
_mgr.updateHostPassword(this);
90-
_resourceService.updateHostPassword(this);
89+
if (getClusterId() == null) {
90+
_mgr.updateHostPassword(this);
91+
_resourceService.updateHostPassword(this);
92+
} else {
93+
_mgr.updateClusterPassword(this);
94+
_resourceService.updateClusterPassword(this);
95+
}
96+
9197
setResponseObject(new SuccessResponse(getCommandName()));
9298
}
9399
}

‎server/src/com/cloud/resource/ResourceManagerImpl.java‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2239,39 +2239,41 @@ private boolean doUpdateHostPassword(final long hostId) {
22392239
}
22402240

22412241
@Override
2242-
public boolean updateHostPassword(final UpdateHostPasswordCmd cmd) {
2243-
if (cmd.getClusterId() == null) {
2244-
// update agent attache password
2242+
public boolean updateClusterPassword(final UpdateHostPasswordCmd command) {
2243+
// get agents for the cluster
2244+
final List<HostVO> hosts = listAllHostsInCluster(command.getClusterId());
2245+
for (final HostVO h : hosts) {
22452246
try {
2246-
final Boolean result = propagateResourceEvent(cmd.getHostId(), ResourceState.Event.UpdatePassword);
2247+
/*
2248+
* FIXME: this is a buggy logic, check with alex. Shouldn't
2249+
* return if propagation return non null
2250+
*/
2251+
final Boolean result = propagateResourceEvent(h.getId(), ResourceState.Event.UpdatePassword);
22472252
if (result != null) {
22482253
return result;
22492254
}
22502255
} catch (final AgentUnavailableException e) {
2256+
s_logger.error("Agent is not availbale!", e);
22512257
}
2258+
doUpdateHostPassword(h.getId());
2259+
}
22522260

2253-
return doUpdateHostPassword(cmd.getHostId());
2254-
} else {
2255-
// get agents for the cluster
2256-
final List<HostVO> hosts = listAllHostsInCluster(cmd.getClusterId());
2257-
for (final HostVO h : hosts) {
2258-
try {
2259-
/*
2260-
* FIXME: this is a buggy logic, check with alex. Shouldn't
2261-
* return if propagation return non null
2262-
*/
2263-
final Boolean result = propagateResourceEvent(h.getId(), ResourceState.Event.UpdatePassword);
2264-
if (result != null) {
2265-
return result;
2266-
}
2261+
return true;
2262+
}
22672263

2268-
doUpdateHostPassword(h.getId());
2269-
} catch (final AgentUnavailableException e) {
2270-
}
2264+
@Override
2265+
public boolean updateHostPassword(final UpdateHostPasswordCmd cmd) {
2266+
// update agent attache password
2267+
try {
2268+
final Boolean result = propagateResourceEvent(cmd.getHostId(), ResourceState.Event.UpdatePassword);
2269+
if (result != null) {
2270+
return result;
22712271
}
2272-
2273-
return true;
2272+
} catch (final AgentUnavailableException e) {
2273+
s_logger.error("Agent is not availbale!", e);
22742274
}
2275+
2276+
return doUpdateHostPassword(cmd.getHostId());
22752277
}
22762278

22772279
public String getPeerName(final long agentHostId) {

‎server/src/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 72 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3739,50 +3739,84 @@ public String getVMPassword(final GetVMPasswordCmd cmd) {
37393739
return password;
37403740
}
37413741

3742+
private boolean updateHostsInCluster(final UpdateHostPasswordCmd command) {
3743+
// get all the hosts in this cluster
3744+
final List<HostVO> hosts = _resourceMgr.listAllHostsInCluster(command.getClusterId());
3745+
3746+
Transaction.execute(new TransactionCallbackNoReturn() {
3747+
@Override
3748+
public void doInTransactionWithoutResult(final TransactionStatus status) {
3749+
for (final HostVO h : hosts) {
3750+
if (s_logger.isDebugEnabled()) {
3751+
s_logger.debug("Changing password for host name = " + h.getName());
3752+
}
3753+
// update password for this host
3754+
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
3755+
if (nv.getValue().equals(command.getUsername())) {
3756+
final DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.PASSWORD);
3757+
nvp.setValue(DBEncryptionUtil.encrypt(command.getPassword()));
3758+
_detailsDao.persist(nvp);
3759+
} else {
3760+
// if one host in the cluster has diff username then
3761+
// rollback to maintain consistency
3762+
throw new InvalidParameterValueException("The username is not same for all hosts, please modify passwords for individual hosts.");
3763+
}
3764+
}
3765+
}
3766+
});
3767+
return true;
3768+
}
3769+
3770+
/**
3771+
* This method updates the password of all hosts in a given cluster.
3772+
*/
3773+
@Override
3774+
@DB
3775+
public boolean updateClusterPassword(final UpdateHostPasswordCmd command) {
3776+
if (command.getClusterId() == null) {
3777+
throw new InvalidParameterValueException("You should provide a cluster id.");
3778+
}
3779+
3780+
final ClusterVO cluster = ApiDBUtils.findClusterById(command.getClusterId());
3781+
if (cluster == null || cluster.getHypervisorType() != HypervisorType.XenServer || cluster.getHypervisorType() != HypervisorType.KVM) {
3782+
throw new InvalidParameterValueException("This operation is not supported for this hypervisor type");
3783+
}
3784+
return updateHostsInCluster(command);
3785+
}
3786+
37423787
@Override
37433788
@DB
37443789
public boolean updateHostPassword(final UpdateHostPasswordCmd cmd) {
3745-
if (cmd.getClusterId() == null && cmd.getHostId() == null) {
3746-
throw new InvalidParameterValueException("You should provide one of cluster id or a host id.");
3747-
} else if (cmd.getClusterId() == null) {
3748-
final HostVO host = _hostDao.findById(cmd.getHostId());
3749-
if (host != null && host.getHypervisorType() == HypervisorType.XenServer) {
3750-
throw new InvalidParameterValueException("You should provide cluster id for Xenserver cluster.");
3751-
} else {
3752-
throw new InvalidParameterValueException("This operation is not supported for this hypervisor type");
3753-
}
3754-
} else {
3790+
if (cmd.getHostId() == null) {
3791+
throw new InvalidParameterValueException("You should provide an host id.");
3792+
}
37553793

3756-
final ClusterVO cluster = ApiDBUtils.findClusterById(cmd.getClusterId());
3757-
if (cluster == null || cluster.getHypervisorType() != HypervisorType.XenServer) {
3758-
throw new InvalidParameterValueException("This operation is not supported for this hypervisor type");
3759-
}
3760-
// get all the hosts in this cluster
3761-
final List<HostVO> hosts = _resourceMgr.listAllHostsInCluster(cmd.getClusterId());
3762-
3763-
Transaction.execute(new TransactionCallbackNoReturn() {
3764-
@Override
3765-
public void doInTransactionWithoutResult(final TransactionStatus status) {
3766-
for (final HostVO h : hosts) {
3767-
if (s_logger.isDebugEnabled()) {
3768-
s_logger.debug("Changing password for host name = " + h.getName());
3769-
}
3770-
// update password for this host
3771-
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
3772-
if (nv.getValue().equals(cmd.getUsername())) {
3773-
final DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.PASSWORD);
3774-
nvp.setValue(DBEncryptionUtil.encrypt(cmd.getPassword()));
3775-
_detailsDao.persist(nvp);
3776-
} else {
3777-
// if one host in the cluster has diff username then
3778-
// rollback to maintain consistency
3779-
throw new InvalidParameterValueException("The username is not same for all hosts, please modify passwords for individual hosts.");
3780-
}
3781-
}
3782-
}
3783-
});
3794+
final HostVO host = _hostDao.findById(cmd.getHostId());
3795+
3796+
if (host.getHypervisorType() != HypervisorType.XenServer || host.getHypervisorType() != HypervisorType.KVM) {
3797+
throw new InvalidParameterValueException("This operation is not supported for this hypervisor type");
37843798
}
37853799

3800+
Transaction.execute(new TransactionCallbackNoReturn() {
3801+
@Override
3802+
public void doInTransactionWithoutResult(final TransactionStatus status) {
3803+
if (s_logger.isDebugEnabled()) {
3804+
s_logger.debug("Changing password for host name = " + host.getName());
3805+
}
3806+
// update password for this host
3807+
final DetailVO nv = _detailsDao.findDetail(host.getId(), ApiConstants.USERNAME);
3808+
if (nv.getValue().equals(cmd.getUsername())) {
3809+
final DetailVO nvp = _detailsDao.findDetail(host.getId(), ApiConstants.PASSWORD);
3810+
nvp.setValue(DBEncryptionUtil.encrypt(cmd.getPassword()));
3811+
_detailsDao.persist(nvp);
3812+
} else {
3813+
// if one host in the cluster has diff username then
3814+
// rollback to maintain consistency
3815+
throw new InvalidParameterValueException("The username is not same for the hosts..");
3816+
}
3817+
}
3818+
});
3819+
37863820
return true;
37873821
}
37883822

0 commit comments

Comments
 (0)