You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Sheng Yang <sh...@yasker.org> on 2014/07/18 02:09:04 UTC

VirtualRoutingResource bugs CLOUDSTACK-7111 after refactor

Hi Hugo,

I saw you refactor the VirtualRoutingResource, but it introduced
https://issues.apache.org/jira/browse/CLOUDSTACK-7111 .

Here are the lines you deleted.

-            ExecutionResult result = new ExecutionResult(true, "No
configure to be applied");
-            for (ConfigItem c : cfg) {
-                result = applyConfigToVR(cmd, c);
-                if (!result.isSuccess()) {
-                    break;
-                }
-            }
-            return new Answer(cmd, result.isSuccess(),
result.getDetails());
-        }

The result is true by default, because in some cases, configuration won't
be generated since VR determined it's not necessary, for example when VPN
is created first time, there is no VPN user available. So cfg can be null
and success.

And here is the new code.

        boolean finalResult = false;
        for (ConfigItem configItem : cfg) {
            ExecutionResult result =
applyConfigToVR(cmd.getRouterAccessIp(), configItem);
            if (result == null) {
                result = new ExecutionResult(false, "null execution
result");
            }
            results.add(result);
            details.add(configItem.getInfo() + (result.isSuccess() ? " -
success: " : " - failed: ") + result.getDetails());
            finalResult = result.isSuccess();
        }

        // Not sure why this matters, but log it anyway
        if (cmd.getAnswersCount() != results.size()) {
            s_logger.warn("Expected " + cmd.getAnswersCount() + " answers
while executing " + cmd.getClass().getSimpleName() + " but received " +
results.size());
        }

If cfg is null, then you would have less Answers than cfg.

Then finalResult is false in the new code, which caused bug on create VPN
failure.

Could you check on it?

Thanks.

--Sheng