Prevent network disruption on adding a VPC tier for redundant VRs (#9251)

api/src/main/java/com/cloud/network/VpcVirtualNetworkApplianceService.java

@@ -29,7 +29,6 @@ public interface VpcVirtualNetworkApplianceService extends VirtualNetworkApplian
     /**
      * @param router
      * @param network
-     * @param isRedundant
      * @param params TODO
      * @return
      * @throws ConcurrentOperationException
@@ -42,11 +41,30 @@ public interface VpcVirtualNetworkApplianceService extends VirtualNetworkApplian
     /**
      * @param router
      * @param network
-     * @param isRedundant
      * @return
      * @throws ConcurrentOperationException
      * @throws ResourceUnavailableException
      */
     boolean removeVpcRouterFromGuestNetwork(VirtualRouter router, Network network) throws ConcurrentOperationException, ResourceUnavailableException;
 
+
+    /**
+     * @param router
+     * @param network
+     * @return
+     * @throws ConcurrentOperationException
+     * @throws ResourceUnavailableException
+     */
+    boolean stopKeepAlivedOnRouter(VirtualRouter router, Network network) throws ConcurrentOperationException, ResourceUnavailableException;
+
+
+    /**
+     * @param router
+     * @param network
+     * @return
+     * @throws ConcurrentOperationException
+     * @throws ResourceUnavailableException
+     */
+    boolean startKeepAlivedOnRouter(VirtualRouter router, Network network) throws ConcurrentOperationException, ResourceUnavailableException;
+
 }

core/src/main/java/com/cloud/agent/resource/virtualnetwork/VRScripts.java

@@ -81,4 +81,5 @@ public class VRScripts {
     public static final String VR_UPDATE_INTERFACE_CONFIG = "update_interface_config.sh";
 
     public static final String ROUTER_FILESYSTEM_WRITABLE_CHECK = "filesystem_writable_check.py";
+    public static final String MANAGE_SERVICE = "manage_service.sh";
 }

core/src/main/java/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java

@@ -34,6 +34,7 @@ import java.util.concurrent.locks.ReentrantLock;
 
 import javax.naming.ConfigurationException;
 
+import org.apache.cloudstack.agent.routing.ManageServiceCommand;
 import com.cloud.agent.api.routing.UpdateNetworkCommand;
 import com.cloud.agent.api.to.IpAddressTO;
 import com.cloud.network.router.VirtualRouter;
@@ -143,6 +144,10 @@ public class VirtualRoutingResource {
                 return execute((UpdateNetworkCommand) cmd);
             }
 
+            if (cmd instanceof ManageServiceCommand) {
+                return execute((ManageServiceCommand) cmd);
+            }
+
             if (_vrAggregateCommandsSet.containsKey(routerName)) {
                 _vrAggregateCommandsSet.get(routerName).add(cmd);
                 aggregated = true;
@@ -270,6 +275,20 @@ public class VirtualRoutingResource {
         return new Answer(cmd, new CloudRuntimeException("Failed to update interface mtu"));
     }
 
+    private Answer execute(ManageServiceCommand cmd) {
+        String routerIp = cmd.getAccessDetail(NetworkElementCommand.ROUTER_IP);
+        String args = cmd.getAction() + " " + cmd.getServiceName();
+        ExecutionResult result = _vrDeployer.executeInVR(routerIp, VRScripts.MANAGE_SERVICE, args);
+        if (result.isSuccess()) {
+            return new Answer(cmd, true,
+                    String.format("Successfully executed action: %s on service: %s. Details: %s",
+                            cmd.getAction(), cmd.getServiceName(), result.getDetails()));
+        } else {
+            return new Answer(cmd, false, String.format("Failed to execute action: %s on service: %s. Details: %s",
+                    cmd.getAction(), cmd.getServiceName(), result.getDetails()));
+        }
+    }
+
     private ExecutionResult applyConfigToVR(String routerAccessIp, ConfigItem c) {
         return applyConfigToVR(routerAccessIp, c, VRScripts.VR_SCRIPT_EXEC_TIMEOUT);
     }

core/src/main/java/org/apache/cloudstack/agent/routing/ManageServiceCommand.java

@@ -0,0 +1,49 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package org.apache.cloudstack.agent.routing;
+
+import com.cloud.agent.api.routing.NetworkElementCommand;
+
+public class ManageServiceCommand extends NetworkElementCommand {
+
+    String serviceName;
+    String action;
+
+    @Override
+    public boolean executeInSequence() {
+        return true;
+    }
+
+    protected ManageServiceCommand() {
+    }
+
+    public ManageServiceCommand(String serviceName, String action) {
+        this.serviceName = serviceName;
+        this.action = action;
+    }
+
+    public String getServiceName() {
+        return serviceName;
+    }
+
+    public String getAction() {
+        return action;
+    }
+}

server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java

@@ -223,24 +223,57 @@ public class VpcVirtualRouterElement extends VirtualRouterElement implements Vpc
         return true;
     }
 
-    protected void configureGuestNetwork(final Network network, final List<DomainRouterVO> routers )
+    protected boolean configureGuestNetworkForRouter(final Network network,
-            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
+            final DomainRouterVO router) throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
+        if (!_networkMdl.isVmPartOfNetwork(router.getId(), network.getId())) {
+            final Map<VirtualMachineProfile.Param, Object> paramsForRouter = new HashMap<VirtualMachineProfile.Param, Object>(1);
+            if (network.getState() == State.Setup) {
+                paramsForRouter.put(VirtualMachineProfile.Param.ReProgramGuestNetworks, true);
+            }
+            if (!_vpcRouterMgr.addVpcRouterToGuestNetwork(router, network, paramsForRouter)) {
+                s_logger.error("Failed to add VPC router " + router + " to guest network " + network);
+                return false;
+            } else {
+                s_logger.debug("Successfully added VPC router " + router + " to guest network " + network);
+                return true;
+            }
+        }
+        return true;
+    }
 
+    protected void configureGuestNetwork(final Network network, final List<DomainRouterVO> routers)
+            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
         s_logger.info("Adding VPC routers to Guest Network: " + routers.size() + " to be added!");
 
-        for (final DomainRouterVO router : routers) {
+        List<DomainRouterVO> backupRouters = new ArrayList<>();
+        List<DomainRouterVO> remainingRouters = new ArrayList<>();
+        for (DomainRouterVO router : routers) {
             if (!_networkMdl.isVmPartOfNetwork(router.getId(), network.getId())) {
-                final Map<VirtualMachineProfile.Param, Object> paramsForRouter = new HashMap<VirtualMachineProfile.Param, Object>(1);
+                if (router.getRedundantState().equals(DomainRouterVO.RedundantState.BACKUP)) {
-                if (network.getState() == State.Setup) {
+                    backupRouters.add(router);
-                    paramsForRouter.put(VirtualMachineProfile.Param.ReProgramGuestNetworks, true);
-                }
-                if (!_vpcRouterMgr.addVpcRouterToGuestNetwork(router, network, paramsForRouter)) {
-                    s_logger.error("Failed to add VPC router " + router + " to guest network " + network);
                 } else {
-                    s_logger.debug("Successfully added VPC router " + router + " to guest network " + network);
+                    remainingRouters.add(router);
                 }
             }
         }
+
+        for (final DomainRouterVO router : backupRouters) {
+            if (network.getState() != State.Setup) {
+                if (!_vpcRouterMgr.stopKeepAlivedOnRouter(router, network)) {
+                    s_logger.error("Failed to stop keepalived on VPC router " + router + " to guest network " + network);
+                } else {
+                    s_logger.debug("Successfully stopped keepalived on VPC router " + router + " to guest network " + network);
+                }
+            }
+        }
+        for (final DomainRouterVO router : remainingRouters) {
+            configureGuestNetworkForRouter(network, router);
+        }
+        for (final DomainRouterVO router : backupRouters) {
+            if (!configureGuestNetworkForRouter(network, router) && !_vpcRouterMgr.startKeepAlivedOnRouter(router, network)) {
+                s_logger.error("Failed to start keepalived on VPC router " + router + " to guest network " + network);
+            }
+        }
     }
 
     @Override
@@ -285,30 +318,7 @@ public class VpcVirtualRouterElement extends VirtualRouterElement implements Vpc
 
     @Override
     public boolean shutdown(final Network network, final ReservationContext context, final boolean cleanup) throws ConcurrentOperationException, ResourceUnavailableException {
-        final Long vpcId = network.getVpcId();
+        return destroy(network, context);
-        if (vpcId == null) {
-            s_logger.debug("Network " + network + " doesn't belong to any vpc, so skipping unplug nic part");
-            return true;
-        }
-
-        boolean success = true;
-        final List<? extends VirtualRouter> routers = _routerDao.listByVpcId(vpcId);
-        for (final VirtualRouter router : routers) {
-            // 1) Check if router is already a part of the network
-            if (!_networkMdl.isVmPartOfNetwork(router.getId(), network.getId())) {
-                s_logger.debug("Router " + router + " is not a part the network " + network);
-                continue;
-            }
-            // 2) Call unplugNics in the network service
-            success = success && _vpcRouterMgr.removeVpcRouterFromGuestNetwork(router, network);
-            if (!success) {
-                s_logger.warn("Failed to unplug nic in network " + network + " for virtual router " + router);
-            } else {
-                s_logger.debug("Successfully unplugged nic in network " + network + " for virtual router " + router);
-            }
-        }
-
-        return success;
     }
 
     @Override

server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java

@@ -27,6 +27,8 @@ import java.util.Map;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import org.apache.cloudstack.agent.routing.ManageServiceCommand;
+import com.cloud.agent.api.routing.NetworkElementCommand;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -226,6 +228,53 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
         return result;
     }
 
+    @Override
+    public boolean stopKeepAlivedOnRouter(VirtualRouter router,
+            Network network) throws ConcurrentOperationException, ResourceUnavailableException {
+        return manageKeepalivedServiceOnRouter(router, network, "stop");
+    }
+
+    @Override
+    public boolean startKeepAlivedOnRouter(VirtualRouter router,
+            Network network) throws ConcurrentOperationException, ResourceUnavailableException {
+        return manageKeepalivedServiceOnRouter(router, network, "start");
+    }
+
+    private boolean manageKeepalivedServiceOnRouter(VirtualRouter router,
+            Network network, String action) throws ConcurrentOperationException, ResourceUnavailableException {
+        if (network.getTrafficType() != TrafficType.Guest) {
+            s_logger.warn("Network " + network + " is not of type " + TrafficType.Guest);
+            return false;
+        }
+        boolean result = true;
+        try {
+            if (router.getState() == State.Running) {
+                final ManageServiceCommand stopCommand = new ManageServiceCommand("keepalived", action);
+                stopCommand.setAccessDetail(NetworkElementCommand.ROUTER_IP, _routerControlHelper.getRouterControlIp(router.getId()));
+
+                final Commands cmds = new Commands(Command.OnError.Stop);
+                cmds.addCommand("manageKeepalived", stopCommand);
+                _nwHelper.sendCommandsToRouter(router, cmds);
+
+                final Answer setupAnswer = cmds.getAnswer("manageKeepalived");
+                if (!(setupAnswer != null && setupAnswer.getResult())) {
+                    s_logger.warn("Unable to " + action + " keepalived on router " + router);
+                    result = false;
+                }
+            } else if (router.getState() == State.Stopped || router.getState() == State.Stopping) {
+                s_logger.debug("Router " + router.getInstanceName() + " is in " + router.getState() + ", so not sending command to the backend");
+            } else {
+                String message = "Unable to " + action + " keepalived on virtual router [" + router + "] is not in the right state " + router.getState();
+                s_logger.warn(message);
+                throw new ResourceUnavailableException(message, DataCenter.class, router.getDataCenterId());
+            }
+        } catch (final Exception ex) {
+            s_logger.warn("Failed to " + action + "  keepalived on router " + router + " to network " + network + " due to ", ex);
+            result = false;
+        }
+        return result;
+    }
+
     protected boolean setupVpcGuestNetwork(final Network network, final VirtualRouter router, final boolean add, final NicProfile guestNic) throws ConcurrentOperationException,
     ResourceUnavailableException {
 

server/src/test/java/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java

@@ -204,6 +204,20 @@ public class MockVpcVirtualNetworkApplianceManager extends ManagerBase implement
         return false;
     }
 
+    @Override
+    public boolean stopKeepAlivedOnRouter(VirtualRouter router,
+            Network network) throws ConcurrentOperationException, ResourceUnavailableException {
+        // TODO Auto-generated method stub
+        return false;
+    }
+
+    @Override
+    public boolean startKeepAlivedOnRouter(VirtualRouter router,
+            Network network) throws ConcurrentOperationException, ResourceUnavailableException {
+        // TODO Auto-generated method stub
+        return false;
+    }
+
     /* (non-Javadoc)
      * @see com.cloud.network.router.VpcVirtualNetworkApplianceManager#destroyPrivateGateway(com.cloud.network.vpc.PrivateGateway, com.cloud.network.router.VirtualRouter)
      */

systemvm/debian/opt/cloud/bin/manage_service.sh

@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+systemctl $1 $2