cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GabrielBrascher <...@git.apache.org>
Subject [GitHub] cloudstack issue #1278: CLOUDSTACK-9198: Virtual router gets deployed in dis...
Date Fri, 03 Mar 2017 21:54:13 GMT
Github user GabrielBrascher commented on the issue:

    https://github.com/apache/cloudstack/pull/1278
  
    @anshul1886 I would like to raise the point previously discussed by me and @rafaelweingartner.
I think that we should pay attention if the change of user and caller will really do the job.
So far I do not see how this PR changes the behavior.
    
    Basically this code changes two parameters in startVirtualRouter [_callerUser_ and _caller_
when calling startVirtualRouter(router, callerUser, caller, routerDeploymentDefinition.getParams())].
However, those parameters are only used inside startVirtualRouter when calling the method
start(router, user, caller, params, null).
    
    ```
    if (router.getRole() != Role.VIRTUAL_ROUTER || !router.getIsRedundantRouter()) {
                return start(router, user, caller, params, null);
    }
    ```
    
    The problem is that the method **start** does not use either the _user_ and the _caller_
parameters in the overridden implementation (the one that you are using).
    
    ```
        protected DomainRouterVO start(DomainRouterVO router, final User user, final Account
caller, final Map<Param, Object> params, final DeploymentPlan planToDeploy)
                throws StorageUnavailableException, InsufficientCapacityException, ConcurrentOperationException,
ResourceUnavailableException {
            s_logger.debug("Starting router " + router);
            try {
                _itMgr.advanceStart(router.getUuid(), params, planToDeploy, null);
            } catch (final OperationTimedoutException e) {
                throw new ResourceUnavailableException("Starting router " + router + " failed!
" + e.toString(), DataCenter.class, router.getDataCenterId());
            }
            if (router.isStopPending()) {
                s_logger.info("Clear the stop pending flag of router " + router.getHostName()
+ " after start router successfully!");
                router.setStopPending(false);
                router = _routerDao.persist(router);
            }
            // We don't want the failure of VPN Connection affect the status of
            // router, so we try to make connection
            // only after router start successfully
            final Long vpcId = router.getVpcId();
            if (vpcId != null) {
                _s2sVpnMgr.reconnectDisconnectedVpnByVpc(vpcId);
            }
            return _routerDao.findById(router.getId());
    }
    ```
    
    Sorry, but I can't see how your code alters the behavior as intended. Can you please show
that by changing the parameters _user_ and _caller_ you are changing the behavior?
    
    Thanks in advance.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message