[GitHub] nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart
nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114:
Reduce VR downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#discussion_r181347015
##
File path: ui/scripts/network.js
##
@@ -1100,11 +1100,23 @@
});
args.$form.find('.form-item[rel=cleanup]').find('input').attr('checked',
'checked'); //checked
args.$form.find('.form-item[rel=cleanup]').css('display', 'inline-block');
//shown
+
args.$form.find('.form-item[rel=makeredundant]').find('input').attr('checked',
'checked'); //checked
+
args.$form.find('.form-item[rel=makeredundant]').css('display',
'inline-block'); //shown
+
+if
(Boolean(args.context.networks[0].redundantrouter)) {
Review comment:
@rhtyd, As I understand, this functionality is not applicable to RVRs? For
RVRs, the upgrade procedure is same as the old way. (Reboot the back-up and
then reboot the master). But from the snapshots, I see, it creates a new router
for each restart. (r-46-VM creates r-47-VM) and (r-45-VM creates r-48-VM).
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart
nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114:
Reduce VR downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#discussion_r181297798
##
File path: ui/scripts/network.js
##
@@ -1100,11 +1100,23 @@
});
args.$form.find('.form-item[rel=cleanup]').find('input').attr('checked',
'checked'); //checked
args.$form.find('.form-item[rel=cleanup]').css('display', 'inline-block');
//shown
+
args.$form.find('.form-item[rel=makeredundant]').find('input').attr('checked',
'checked'); //checked
+
args.$form.find('.form-item[rel=makeredundant]').css('display',
'inline-block'); //shown
+
+if
(Boolean(args.context.networks[0].redundantrouter)) {
Review comment:
@rhtyd, When we say redundant networks, that means it already contains the
new upgraded VR(after Blue-Green deployment), we created after a restart? If
that is the case, I think it would be great, if we give a different name
instead of redundant. Because there is a chance we will misinterpret as RVR.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart
nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114:
Reduce VR downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#discussion_r181044136
##
File path: ui/scripts/network.js
##
@@ -1100,11 +1100,23 @@
});
args.$form.find('.form-item[rel=cleanup]').find('input').attr('checked',
'checked'); //checked
args.$form.find('.form-item[rel=cleanup]').css('display', 'inline-block');
//shown
+
args.$form.find('.form-item[rel=makeredundant]').find('input').attr('checked',
'checked'); //checked
+
args.$form.find('.form-item[rel=makeredundant]').css('display',
'inline-block'); //shown
+
+if
(Boolean(args.context.networks[0].redundantrouter)) {
Review comment:
As I understand, for RVR also the rolling upgrade will be applicable, any
reason we hide the button.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart
nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114:
Reduce VR downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#discussion_r181043216
##
File path:
engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##
@@ -2868,6 +2849,89 @@ public boolean restartNetwork(final Long networkId,
final Account callerAccount,
}
}
+@Override
+public void destroyExpendableRouters(final List
routers, final ReservationContext context) throws ResourceUnavailableException {
+final List remainingRouters = new ArrayList<>();
+// Purge invalid routers
+for (final VirtualRouter router : routers) {
+if (router.getState() == VirtualMachine.State.Stopped ||
+router.getState() == VirtualMachine.State.Error ||
+router.getState() == VirtualMachine.State.Shutdowned ||
+router.getState() == VirtualMachine.State.Unknown) {
+s_logger.debug("Destroying old router " + router);
+_routerService.destroyRouter(router.getId(),
context.getAccount(), context.getCaller().getId());
+} else {
+remainingRouters.add(router);
+}
+}
+
+if (remainingRouters.size() < 2) {
+return;
+}
+
+// Purge any backup router
+VirtualRouter backupRouter = null;
+for (final VirtualRouter router : remainingRouters) {
+if (router.getRedundantState() ==
VirtualRouter.RedundantState.BACKUP) {
+backupRouter = router;
+}
+}
+if (backupRouter == null) {
+backupRouter = routers.get(routers.size() - 1);
+}
+if (backupRouter != null) {
+_routerService.destroyRouter(backupRouter.getId(),
context.getAccount(), context.getCaller().getId());
+}
+}
+
+@Override
+public boolean validateNewRouters(final List
routers, final boolean isRedundant) {
+for (final VirtualRouter router : routers) {
+if (router.getState() != VirtualMachine.State.Running) {
+s_logger.debug("Found new router " + router.getInstanceName()
+ " to be in non-Running state: " + router.getState() + ". Please try
restarting network again.");
+return false;
+}
+if (!isRedundant) {
+
router.setRedundantState(VirtualRouter.RedundantState.REDUNDANT_CAPABLE);
+_routerDao.update(router.getId(), (DomainRouterVO) router);
+}
+}
+return true;
+}
+
+private boolean rollingRestartRouters(final NetworkVO network, final
NetworkOffering offering, final DeployDestination dest, final
ReservationContext context) throws ResourceUnavailableException,
ConcurrentOperationException, InsufficientCapacityException {
+s_logger.debug("Performing rolling restart of routers of network " +
network);
+destroyExpendableRouters(_routerDao.findByNetwork(network.getId()),
context);
+
+final List providersToImplement =
getNetworkProviders(network.getId());
+final List oldRouters =
_routerDao.findByNetwork(network.getId());
+
+// Deploy a new router
+final boolean originalRedundancy = network.isRedundant();
+network.setRedundant(true);
+implementNetworkElements(dest, context, network, offering,
providersToImplement);
+network.setRedundant(originalRedundancy);
+
+// For redundant network wait for 3*advert_int+skew_seconds for VRRP
to kick in
+if (network.isRedundant() || (oldRouters.size() == 1 &&
oldRouters.get(0).getIsRedundantRouter())) {
+try {
+Thread.sleep(1L);
+} catch (final InterruptedException ignored) {}
+}
+
+// Destroy old routers
+for (final DomainRouterVO oldRouter : oldRouters) {
+_routerService.destroyRouter(oldRouter.getId(),
context.getAccount(), context.getCaller().getId());
Review comment:
Here, before validating the new routers, we are destroying the old ones. If
the new routers don't come up properly, we won't have old routers to roll back.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart
nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114:
Reduce VR downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#discussion_r181038001
##
File path:
api/src/org/apache/cloudstack/api/command/user/network/RestartNetworkCmd.java
##
@@ -77,6 +80,13 @@ public Boolean getCleanup() {
return true;
}
+public Boolean getMakeRedundant() {
+if (makeRedundant != null) {
+return makeRedundant;
+}
+return true;
Review comment:
If we don't pass anything, I think it should be false, it would continue
with normal restart instead of this blue-green deployment.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart
nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart URL: https://github.com/apache/cloudstack/pull/2508#discussion_r180965420 ## File path: api/src/org/apache/cloudstack/api/command/user/network/RestartNetworkCmd.java ## @@ -57,6 +57,9 @@ @Parameter(name = ApiConstants.CLEANUP, type = CommandType.BOOLEAN, required = false, description = "If cleanup old network elements") private Boolean cleanup; +@Parameter(name = ApiConstants.MAKEREDUNDANTE, type = CommandType.BOOLEAN, required = false, description = "Turn the network into a network with redundant routers.", since = "4.11.1") Review comment: @rhtyd, Thanks for adding this functionality. I think there is a typo in ApiConstants name, E at the end. Would be great if we change the name of existing name to MAKE_REDUNDANT, I just checked the usage is also very less. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart
nitin-maharana commented on a change in pull request #2508: CLOUDSTACK-9114: Reduce VR downtime during network restart URL: https://github.com/apache/cloudstack/pull/2508#discussion_r180965420 ## File path: api/src/org/apache/cloudstack/api/command/user/network/RestartNetworkCmd.java ## @@ -57,6 +57,9 @@ @Parameter(name = ApiConstants.CLEANUP, type = CommandType.BOOLEAN, required = false, description = "If cleanup old network elements") private Boolean cleanup; +@Parameter(name = ApiConstants.MAKEREDUNDANTE, type = CommandType.BOOLEAN, required = false, description = "Turn the network into a network with redundant routers.", since = "4.11.1") Review comment: @rhtyd, Thanks for adding this functionality. I think there is a typo in ApiConstants name, E at the end. Would be great if we change the name of existing name to MAKE_RENDUNDANT, I just checked the usage is also very less. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
