mirror of https://github.com/apache/cloudstack.git
Fixes required for regressions found running automated tests. (1) method signatures that weren't properly refactored to new API framework (2) API request parameter types should always be specified lest they default to String which may or may not be desired (3) better exception handling with regard to Queued commands and generic exceptions (4) fix eventing to pass the proper accountId where neccessary
This commit is contained in:
parent
5f2acc8cdd
commit
41a16a478a
|
|
@ -31,6 +31,7 @@ import org.apache.log4j.Logger;
|
|||
|
||||
import com.cloud.agent.manager.AgentManager;
|
||||
import com.cloud.api.BaseCmd.CommandType;
|
||||
import com.cloud.async.AsyncCommandQueued;
|
||||
import com.cloud.configuration.ConfigurationManager;
|
||||
import com.cloud.consoleproxy.ConsoleProxyManager;
|
||||
import com.cloud.exception.InvalidParameterValueException;
|
||||
|
|
@ -156,9 +157,6 @@ public class ApiDispatcher {
|
|||
} catch (IllegalArgumentException iArgEx) {
|
||||
s_logger.warn("Exception executing method " + methodName + " for command " + cmd.getClass().getSimpleName(), iArgEx);
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Unable to execute method " + methodName + " for command " + cmd.getClass().getSimpleName() + ", internal error in the implementation.");
|
||||
} catch (Exception ex) {
|
||||
s_logger.error("Unhandled exception invoking method " + methodName + " for command " + cmd.getClass().getSimpleName(), ex);
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Unable to execute method " + methodName + " for command " + cmd.getClass().getSimpleName() + ", internal error in the implementation.");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -213,8 +211,11 @@ public class ApiDispatcher {
|
|||
s_logger.warn("Exception executing method " + methodName + " for command " + cmd.getClass().getSimpleName(), nsme);
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Unable to execute method " + methodName + " for command " + cmd.getClass().getSimpleName() + ", unable to find implementation.");
|
||||
} catch (InvocationTargetException ite) {
|
||||
s_logger.warn("Exception executing method " + methodName + " for command " + cmd.getClass().getSimpleName(), ite);
|
||||
Throwable cause = ite.getCause();
|
||||
if (cause instanceof AsyncCommandQueued) {
|
||||
throw (AsyncCommandQueued)cause;
|
||||
}
|
||||
s_logger.warn("Exception executing method " + methodName + " for command " + cmd.getClass().getSimpleName(), ite);
|
||||
if (cause instanceof InvalidParameterValueException) {
|
||||
throw new ServerApiException(BaseCmd.PARAM_ERROR, cause.getMessage());
|
||||
} else if (cause instanceof PermissionDeniedException) {
|
||||
|
|
@ -227,9 +228,6 @@ public class ApiDispatcher {
|
|||
} catch (IllegalArgumentException iArgEx) {
|
||||
s_logger.warn("Exception executing method " + methodName + " for command " + cmd.getClass().getSimpleName(), iArgEx);
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Unable to execute method " + methodName + " for command " + cmd.getClass().getSimpleName() + ", internal error in the implementation.");
|
||||
} catch (Exception ex) {
|
||||
s_logger.error("Unhandled exception invoking method " + methodName + " for command " + cmd.getClass().getSimpleName(), ex);
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Unable to execute method " + methodName + " for command " + cmd.getClass().getSimpleName() + ", internal error in the implementation.");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -350,6 +350,7 @@ public class ApiServer implements HttpRequestHandler {
|
|||
BaseAsyncCreateCmd createCmd = (BaseAsyncCreateCmd)cmdObj;
|
||||
objectId = _dispatcher.dispatchCreateCmd(createCmd, params);
|
||||
createCmd.setId(objectId);
|
||||
params.put("id", objectId.toString());
|
||||
}
|
||||
BaseAsyncCmd asyncCmd = (BaseAsyncCmd)cmdObj;
|
||||
|
||||
|
|
@ -366,8 +367,15 @@ public class ApiServer implements HttpRequestHandler {
|
|||
}
|
||||
|
||||
AsyncJobVO job = new AsyncJobVO();
|
||||
job.setUserId(UserContext.current().getUserId());
|
||||
job.setAccountId(ctx.getAccountId());
|
||||
job.setUserId(userId);
|
||||
if (account != null) {
|
||||
job.setAccountId(ctx.getAccountId());
|
||||
} else {
|
||||
// Just have SYSTEM own the job for now. Users won't be able to see this job,
|
||||
// but in an admin case (like domain admin) they won't be able to see it anyway
|
||||
// so no loss of service.
|
||||
job.setAccountId(1L);
|
||||
}
|
||||
job.setCmd(cmdObj.getClass().getName());
|
||||
job.setCmdInfo(gson.toJson(params));
|
||||
long jobId = _asyncMgr.submitAsyncJob(job);
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import com.cloud.api.response.ApiResponseSerializer;
|
|||
import com.cloud.api.response.CreateCmdResponse;
|
||||
|
||||
public abstract class BaseAsyncCreateCmd extends BaseAsyncCmd {
|
||||
@Parameter(name="id")
|
||||
@Parameter(name="id", type=CommandType.LONG)
|
||||
private Long id;
|
||||
|
||||
public Long getId() {
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ public class CreatePortForwardingServiceRuleCmd extends BaseAsyncCreateCmd {
|
|||
@Parameter(name="privateport", type=CommandType.STRING, required=true)
|
||||
private String privatePort;
|
||||
|
||||
@Parameter(name="protocol", type=CommandType.STRING, required=true)
|
||||
@Parameter(name="protocol", type=CommandType.STRING)
|
||||
private String protocol;
|
||||
|
||||
@Parameter(name="publicport", type=CommandType.STRING, required=true)
|
||||
|
|
|
|||
|
|
@ -1503,7 +1503,7 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager
|
|||
}
|
||||
|
||||
|
||||
@Override
|
||||
@Override @DB
|
||||
public IPAddressVO associateIP(AssociateIPAddrCmd cmd) throws ResourceAllocationException, InsufficientAddressCapacityException, InvalidParameterValueException, InternalErrorException, PermissionDeniedException {
|
||||
String accountName = cmd.getAccountName();
|
||||
Long domainId = cmd.getDomainId();
|
||||
|
|
@ -1962,7 +1962,7 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager
|
|||
return _rulesDao.listIPForwarding(cmd.getIpAddress(), true);
|
||||
}
|
||||
|
||||
@Override
|
||||
@Override @DB
|
||||
public void assignToLoadBalancer(AssignToLoadBalancerRuleCmd cmd) throws NetworkRuleConflictException, InternalErrorException,
|
||||
PermissionDeniedException, InvalidParameterValueException {
|
||||
Long loadBalancerId = cmd.getLoadBalancerId();
|
||||
|
|
@ -2195,7 +2195,7 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager
|
|||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
@Override @DB
|
||||
public LoadBalancerVO createLoadBalancerRule(CreateLoadBalancerRuleCmd cmd) throws InvalidParameterValueException, PermissionDeniedException {
|
||||
String publicIp = cmd.getPublicIp();
|
||||
|
||||
|
|
@ -3304,6 +3304,12 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager
|
|||
throw new ServerApiException(BaseCmd.PARAM_ERROR, "No virtual machine id specified.");
|
||||
}
|
||||
|
||||
// if a single instanceId was given, add it to the list so we can always just process the list if instanceIds
|
||||
if (instanceIds == null) {
|
||||
instanceIds = new ArrayList<Long>();
|
||||
instanceIds.add(vmInstanceId);
|
||||
}
|
||||
|
||||
if (userId == null) {
|
||||
userId = Long.valueOf(1);
|
||||
}
|
||||
|
|
@ -3381,7 +3387,7 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager
|
|||
description = "deleted load balancer rule [" + updatedRule.getPublicIpAddress() + ":" + updatedRule.getPublicPort() + "]->["
|
||||
+ updatedRule.getPrivateIpAddress() + ":" + updatedRule.getPrivatePort() + "]" + " " + updatedRule.getProtocol();
|
||||
|
||||
EventUtils.saveEvent(userId, account.getId(), level, type, description);
|
||||
EventUtils.saveEvent(userId, loadBalancer.getAccountId(), level, type, description);
|
||||
}
|
||||
}
|
||||
txn.commit();
|
||||
|
|
|
|||
|
|
@ -773,10 +773,10 @@ public interface ManagementServer {
|
|||
|
||||
/**
|
||||
* Apply a port forwarding service rule to all VMs that have the port forwarding service applied
|
||||
* @param ruleId the id of the created rule to apply
|
||||
* @param cmd the command object that wraps the id of the created rule to apply
|
||||
* @return the updated rule if successful, null otherwise
|
||||
*/
|
||||
NetworkRuleConfigVO applyPortForwardingServiceRule(Long ruleId) throws NetworkRuleConflictException;
|
||||
NetworkRuleConfigVO applyPortForwardingServiceRule(CreatePortForwardingServiceRuleCmd cmd) throws NetworkRuleConflictException;
|
||||
|
||||
ConsoleProxyInfo getConsoleProxy(long dataCenterId, long userVmId);
|
||||
ConsoleProxyVO startConsoleProxy(long instanceId, long startEventId) throws InternalErrorException;
|
||||
|
|
|
|||
|
|
@ -4549,7 +4549,8 @@ public class ManagementServerImpl implements ManagementServer {
|
|||
}
|
||||
|
||||
@Override
|
||||
public NetworkRuleConfigVO applyPortForwardingServiceRule(Long ruleId) throws NetworkRuleConflictException {
|
||||
public NetworkRuleConfigVO applyPortForwardingServiceRule(CreatePortForwardingServiceRuleCmd cmd) throws NetworkRuleConflictException {
|
||||
Long ruleId = cmd.getId();
|
||||
NetworkRuleConfigVO netRule = null;
|
||||
if (ruleId != null) {
|
||||
Long userId = UserContext.current().getUserId();
|
||||
|
|
@ -5476,30 +5477,28 @@ public class ManagementServerImpl implements ManagementServer {
|
|||
Long accountId = null;
|
||||
String portForwardingServiceName = cmd.getPortForwardingServiceName();
|
||||
|
||||
if (account != null) {
|
||||
if (isAdmin(account.getType())) {
|
||||
if ((accountName != null) && (domainId != null)) {
|
||||
if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) {
|
||||
throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to create port forwarding service in domain " + domainId + ", permission denied.");
|
||||
}
|
||||
if ((account == null) || isAdmin(account.getType())) {
|
||||
if ((accountName != null) && (domainId != null)) {
|
||||
if ((account != null) && !_domainDao.isChildDomain(account.getDomainId(), domainId)) {
|
||||
throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to create port forwarding service in domain " + domainId + ", permission denied.");
|
||||
}
|
||||
|
||||
Account userAccount = findActiveAccount(accountName, domainId);
|
||||
if (userAccount != null) {
|
||||
accountId = userAccount.getId();
|
||||
} else {
|
||||
throw new InvalidParameterValueException("Unable to create port forwarding service " + portForwardingServiceName + ", could not find account " + accountName + " in domain " + domainId);
|
||||
}
|
||||
Account userAccount = findActiveAccount(accountName, domainId);
|
||||
if (userAccount != null) {
|
||||
accountId = userAccount.getId();
|
||||
} else {
|
||||
// the admin must be creating the security group
|
||||
if (account != null) {
|
||||
accountId = account.getId();
|
||||
domainId = account.getDomainId();
|
||||
}
|
||||
throw new InvalidParameterValueException("Unable to create port forwarding service " + portForwardingServiceName + ", could not find account " + accountName + " in domain " + domainId);
|
||||
}
|
||||
} else {
|
||||
accountId = account.getId();
|
||||
domainId = account.getDomainId();
|
||||
// the admin must be creating the security group
|
||||
if (account != null) {
|
||||
accountId = account.getId();
|
||||
domainId = account.getDomainId();
|
||||
}
|
||||
}
|
||||
} else {
|
||||
accountId = account.getId();
|
||||
domainId = account.getDomainId();
|
||||
}
|
||||
|
||||
if (accountId == null) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue