router: Persistent DHCP leases file on VRs and cleanup /etc/hosts on VM deletion (#3351)

Since the CloudStack virtual router was redesigned on version 4.6 it has been observed that the DHCP leases file is not persistent across network operations. This causes conflicts on guest VMs static IPs, causing these static IPs to not be renewed by the DHCP server running on isolated and VPC networks' virtual routers (dnsmasq). On stopping or destroying a VM, its dhcp/dns records are not removed from the virtual router causing ghost effects.

Fixes #3272
Fixes #3354

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

api/src/com/cloud/network/element/DhcpServiceProvider.java

@@ -37,4 +37,6 @@ public interface DhcpServiceProvider extends NetworkElement {
     boolean removeDhcpSupportForSubnet(Network network) throws ResourceUnavailableException;
 
     boolean setExtraDhcpOptions(Network network, long nicId, Map<Integer, String> dhcpOptions);
+
+    boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile vmProfile) throws ResourceUnavailableException;
 }

core/src/com/cloud/agent/api/routing/DhcpEntryCommand.java

@@ -35,6 +35,15 @@ public class DhcpEntryCommand extends NetworkElementCommand {
     String duid;
     private boolean isDefault;
     boolean executeInSequence = false;
+    boolean remove;
+
+    public boolean isRemove() {
+        return remove;
+    }
+
+    public void setRemove(boolean remove) {
+        this.remove = remove;
+    }
 
     protected DhcpEntryCommand() {
 

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

@@ -68,5 +68,4 @@ public class VRScripts {
     public static final String UPDATE_HOST_PASSWD = "update_host_passwd.sh";
 
     public static final String VR_CFG = "vr_cfg.sh";
-
 }

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

@@ -22,7 +22,6 @@ package com.cloud.agent.resource.virtualnetwork;
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.nio.channels.SocketChannel;
-import org.joda.time.Duration;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -41,6 +40,7 @@ import org.apache.cloudstack.ca.SetupKeyStoreCommand;
 import org.apache.cloudstack.ca.SetupKeystoreAnswer;
 import org.apache.cloudstack.utils.security.KeyStoreUtils;
 import org.apache.log4j.Logger;
+import org.joda.time.Duration;
 
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.CheckRouterAnswer;

core/src/com/cloud/agent/resource/virtualnetwork/facade/DhcpEntryConfigItem.java

@@ -35,7 +35,7 @@ public class DhcpEntryConfigItem extends AbstractConfigItemFacade {
         final DhcpEntryCommand command = (DhcpEntryCommand) cmd;
 
         final VmDhcpConfig vmDhcpConfig = new VmDhcpConfig(command.getVmName(), command.getVmMac(), command.getVmIpAddress(), command.getVmIp6Address(), command.getDuid(), command.getDefaultDns(),
-                command.getDefaultRouter(), command.getStaticRoutes(), command.isDefault());
+                command.getDefaultRouter(), command.getStaticRoutes(), command.isDefault(), command.isRemove());
 
         return generateConfigItems(vmDhcpConfig);
     }

core/src/com/cloud/agent/resource/virtualnetwork/model/VmDhcpConfig.java

@@ -30,12 +30,15 @@ public class VmDhcpConfig extends ConfigBase {
     private String staticRoutes;
     private boolean defaultEntry;
 
+    // Indicate if the entry should be removed when set to true
+    private boolean remove;
+
     public VmDhcpConfig() {
         super(VM_DHCP);
     }
 
     public VmDhcpConfig(String hostName, String macAddress, String ipv4Address, String ipv6Address, String ipv6Duid, String dnsAddresses, String defaultGateway,
-            String staticRoutes, boolean defaultEntry) {
+            String staticRoutes, boolean defaultEntry, boolean remove) {
         super(VM_DHCP);
         this.hostName = hostName;
         this.macAddress = macAddress;
@@ -46,6 +49,7 @@ public class VmDhcpConfig extends ConfigBase {
         this.defaultGateway = defaultGateway;
         this.staticRoutes = staticRoutes;
         this.defaultEntry = defaultEntry;
+        this.remove = remove;
     }
 
     public String getHostName() {
@@ -64,6 +68,14 @@ public class VmDhcpConfig extends ConfigBase {
         this.macAddress = macAddress;
     }
 
+    public boolean isRemove() {
+        return remove;
+    }
+
+    public void setRemove(boolean remove) {
+        this.remove = remove;
+    }
+
     public String getIpv4Address() {
         return ipv4Address;
     }

engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java

@@ -309,4 +309,8 @@ public interface NetworkOrchestrationService {
      */
     boolean areRoutersRunning(final List<? extends VirtualRouter> routers);
 
+    /**
+     * Remove entry from /etc/dhcphosts and /etc/hosts on virtual routers
+     */
+    void cleanupNicDhcpDnsEntry(Network network, VirtualMachineProfile vmProfile, NicProfile nicProfile);
 }

engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -2912,6 +2912,34 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra
         return true;
     }
 
+    /**
+     * Cleanup entry on VR file specified by type
+     */
+    @Override
+    public void cleanupNicDhcpDnsEntry(Network network, VirtualMachineProfile vmProfile, NicProfile nicProfile) {
+
+        final List<Provider> networkProviders = getNetworkProviders(network.getId());
+        for (final NetworkElement element : networkElements) {
+            if (networkProviders.contains(element.getProvider())) {
+                if (!_networkModel.isProviderEnabledInPhysicalNetwork(_networkModel.getPhysicalNetworkId(network), element.getProvider().getName())) {
+                    throw new CloudRuntimeException("Service provider " + element.getProvider().getName() + " either doesn't exist or is not enabled in physical network id: "
+                            + network.getPhysicalNetworkId());
+                }
+                if (vmProfile.getType() == Type.User && element.getProvider() != null) {
+                    if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)
+                            && _networkModel.isProviderSupportServiceInNetwork(network.getId(), Service.Dhcp, element.getProvider()) && element instanceof DhcpServiceProvider) {
+                        final DhcpServiceProvider sp = (DhcpServiceProvider) element;
+                        try {
+                            sp.removeDhcpEntry(network, nicProfile, vmProfile);
+                        } catch (ResourceUnavailableException e) {
+                            s_logger.error("Failed to remove dhcp-dns entry due to: ", e);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     /**
      * rollingRestartRouters performs restart of routers of a network by first
      * deploying a new VR and then destroying old VRs in rolling fashion. For

plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetalDhcpElement.java

@@ -185,4 +185,9 @@ public class BaremetalDhcpElement extends AdapterBase implements DhcpServiceProv
         return false;
     }
 
+    @Override
+    public boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile vmProfile) throws ResourceUnavailableException {
+        return false;
+    }
+
 }

plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailElementImpl.java

@@ -379,4 +379,9 @@ public class ContrailElementImpl extends AdapterBase
     public boolean setExtraDhcpOptions(Network network, long nicId, Map<Integer, String> dhcpOptions) {
         return false;
     }
+
+    @Override
+    public boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile vmProfile) {
+        return false;
+    }
 }

plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspElement.java

@@ -541,6 +541,11 @@ public class NuageVspElement extends AdapterBase implements ConnectivityProvider
         return true;
     }
 
+    @Override
+    public boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile vmProfile) {
+        return false;
+    }
+
     @Override
     public boolean applyStaticNats(Network config, List<? extends StaticNat> rules) throws ResourceUnavailableException {
         List<VspStaticNat> vspStaticNatDetails = new ArrayList<VspStaticNat>();

server/src/com/cloud/network/element/VirtualRouterElement.java

@@ -946,6 +946,34 @@ NetworkMigrationResponder, AggregatedCommandExecutor, RedundantResource, DnsServ
         return false;
     }
 
+    @Override
+    public boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile vmProfile) throws ResourceUnavailableException {
+        boolean result = true;
+        if (canHandle(network, Service.Dhcp)) {
+            if (vmProfile.getType() != VirtualMachine.Type.User) {
+                return false;
+            }
+
+            final List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER);
+
+            if (CollectionUtils.isEmpty(routers)) {
+                throw new ResourceUnavailableException("Can't find at least one router!", DataCenter.class, network.getDataCenterId());
+            }
+
+            final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
+            final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
+
+            for (final DomainRouterVO domainRouterVO : routers) {
+                if (domainRouterVO.getState() != VirtualMachine.State.Running) {
+                    continue;
+                }
+
+                result = result && networkTopology.removeDhcpEntry(network, nic, vmProfile, domainRouterVO);
+            }
+        }
+        return result;
+    }
+
     @Override
     public boolean removeDnsSupportForSubnet(Network network) throws ResourceUnavailableException {
         // Ignore if virtual router is already dhcp provider

server/src/com/cloud/network/router/CommandSetupHelper.java

@@ -211,7 +211,7 @@ public class CommandSetupHelper {
         cmds.addCommand("users", cmd);
     }
 
-    public void createDhcpEntryCommand(final VirtualRouter router, final UserVm vm, final NicVO nic, final Commands cmds) {
+    public void createDhcpEntryCommand(final VirtualRouter router, final UserVm vm, final NicVO nic, boolean remove, final Commands cmds) {
         final DhcpEntryCommand dhcpCommand = new DhcpEntryCommand(nic.getMacAddress(), nic.getIPv4Address(), vm.getHostName(), nic.getIPv6Address(),
                 _networkModel.getExecuteInSeqNtwkElmtCmd());
 
@@ -229,6 +229,7 @@ public class CommandSetupHelper {
         dhcpCommand.setDefaultDns(ipaddress);
         dhcpCommand.setDuid(NetUtils.getDuidLL(nic.getMacAddress()));
         dhcpCommand.setDefault(nic.isDefaultNic());
+        dhcpCommand.setRemove(remove);
 
         dhcpCommand.setAccessDetail(NetworkElementCommand.ROUTER_IP, _routerControlHelper.getRouterControlIp(router.getId()));
         dhcpCommand.setAccessDetail(NetworkElementCommand.ROUTER_NAME, router.getInstanceName());
@@ -622,7 +623,7 @@ public class CommandSetupHelper {
             final NicVO nic = _nicDao.findByNtwkIdAndInstanceId(guestNetworkId, vm.getId());
             if (nic != null) {
                 s_logger.debug("Creating dhcp entry for vm " + vm + " on domR " + router + ".");
-                createDhcpEntryCommand(router, vm, nic, cmds);
+                createDhcpEntryCommand(router, vm, nic, false, cmds);
             }
         }
     }

server/src/com/cloud/network/rules/DhcpEntryRules.java

@@ -36,6 +36,8 @@ public class DhcpEntryRules extends RuleApplier {
     private final VirtualMachineProfile _profile;
     private final DeployDestination _destination;
 
+    private boolean remove;
+
     private NicVO _nicVo;
     private UserVmVO _userVM;
 
@@ -77,4 +79,12 @@ public class DhcpEntryRules extends RuleApplier {
     public UserVmVO getUserVM() {
         return _userVM;
     }
+
+    public boolean isRemove() {
+        return remove;
+    }
+
+    public void setRemove(boolean remove) {
+        this.remove = remove;
+    }
 }

server/src/com/cloud/vm/UserVmManagerImpl.java

@@ -302,6 +302,7 @@ import com.cloud.vm.dao.VMInstanceDao;
 import com.cloud.vm.snapshot.VMSnapshotManager;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import com.google.common.base.Strings;
 
 
 public class UserVmManagerImpl extends ManagerBase implements UserVmManager, VirtualMachineGuru, UserVmService, Configurable {
@@ -4377,10 +4378,16 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             }
         }
 
-        List<NicVO> nics = _nicDao.listByVmId(vm.getId());
-        for (NicVO nic : nics) {
-            NetworkVO network = _networkDao.findById(nic.getNetworkId());
-            if (network.getTrafficType() == TrafficType.Guest) {
+        final List<NicVO> nics = _nicDao.listByVmId(vm.getId());
+        for (final NicVO nic : nics) {
+            final NetworkVO network = _networkDao.findById(nic.getNetworkId());
+            if (network != null && network.getTrafficType() == TrafficType.Guest) {
+                final String nicIp = Strings.isNullOrEmpty(nic.getIPv4Address()) ? nic.getIPv6Address() : nic.getIPv4Address();
+                if (!Strings.isNullOrEmpty(nicIp)) {
+                    NicProfile nicProfile = new NicProfile(nic.getIPv4Address(), nic.getIPv6Address(), nic.getMacAddress());
+                    nicProfile.setId(nic.getId());
+                    _networkMgr.cleanupNicDhcpDnsEntry(network, profile, nicProfile);
+                }
                 if (nic.getBroadcastUri() != null && nic.getBroadcastUri().getScheme().equals("pvlan")) {
                     NicProfile nicProfile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), 0, false, "pvlan-nic");
                     setupVmForPvlan(false, vm.getHostId(), nicProfile);

server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java

@@ -172,6 +172,21 @@ public class AdvancedNetworkTopology extends BasicNetworkTopology {
         return applyRules(network, router, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper<RuleApplier>(dhcpRules));
     }
 
+    @Override
+    public boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile profile, VirtualRouter virtualRouter) throws ResourceUnavailableException {
+        s_logger.debug("REMOVE VPC DHCP ENTRY RULES");
+
+        final String typeString = "dhcp entry";
+        final Long podId = null;
+        final boolean isPodLevelException = false;
+        final boolean failWhenDisconnect = false;
+
+        final DhcpEntryRules dhcpRules = new DhcpEntryRules(network, nic, profile, null);
+        dhcpRules.setRemove(true);
+
+        return applyRules(network, virtualRouter, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper<RuleApplier>(dhcpRules));
+    }
+
     @Override
     public boolean associatePublicIP(final Network network, final List<? extends PublicIpAddress> ipAddresses, final VirtualRouter router)
             throws ResourceUnavailableException {

server/src/org/apache/cloudstack/network/topology/AdvancedNetworkVisitor.java

@@ -80,8 +80,9 @@ public class AdvancedNetworkVisitor extends BasicNetworkVisitor {
         final Commands commands = new Commands(Command.OnError.Stop);
         final NicVO nicVo = dhcp.getNicVo();
         final UserVmVO userVM = dhcp.getUserVM();
+        final boolean remove = dhcp.isRemove();
 
-        _commandSetupHelper.createDhcpEntryCommand(router, userVM, nicVo, commands);
+        _commandSetupHelper.createDhcpEntryCommand(router, userVM, nicVo, remove, commands);
 
         return _networkGeneralHelper.sendCommandsToRouter(router, commands);
     }

server/src/org/apache/cloudstack/network/topology/BasicNetworkTopology.java

@@ -442,4 +442,25 @@ public class BasicNetworkTopology implements NetworkTopology {
         }
         return result;
     }
+
+    @Override
+    public boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile profile, VirtualRouter virtualRouter) throws ResourceUnavailableException {
+        s_logger.debug("REMOVING DHCP ENTRY RULE");
+
+        final String typeString = "dhcp entry";
+        final Long podId = profile.getVirtualMachine().getPodIdToDeployIn();
+        boolean isPodLevelException = false;
+
+        if (podId != null && profile.getVirtualMachine().getType() == VirtualMachine.Type.User && network.getTrafficType() == TrafficType.Guest
+                && network.getGuestType() == Network.GuestType.Shared) {
+            isPodLevelException = true;
+        }
+
+        final boolean failWhenDisconnect = false;
+
+        final DhcpEntryRules dhcpRules = new DhcpEntryRules(network, nic, profile, null);
+        dhcpRules.setRemove(true);
+
+        return applyRules(network, virtualRouter, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper<RuleApplier>(dhcpRules));
+    }
 }

server/src/org/apache/cloudstack/network/topology/BasicNetworkVisitor.java

@@ -196,9 +196,10 @@ public class BasicNetworkVisitor extends NetworkTopologyVisitor {
         final NicVO nicVo = dhcp.getNicVo();
         final UserVmVO userVM = dhcp.getUserVM();
         final DeployDestination destination = dhcp.getDestination();
+        final boolean remove = dhcp.isRemove();
 
         if (router.getPodIdToDeployIn().longValue() == destination.getPod().getId()) {
-            _commandSetupHelper.createDhcpEntryCommand(router, userVM, nicVo, commands);
+            _commandSetupHelper.createDhcpEntryCommand(router, userVM, nicVo, remove, commands);
 
             return _networkGeneralHelper.sendCommandsToRouter(router, commands);
         }

server/src/org/apache/cloudstack/network/topology/NetworkTopology.java

@@ -87,4 +87,6 @@ public interface NetworkTopology {
 
     boolean applyRules(final Network network, final VirtualRouter router, final String typeString, final boolean isPodLevelException, final Long podId,
             final boolean failWhenDisconnect, RuleApplierWrapper<RuleApplier> ruleApplier) throws ResourceUnavailableException;
+
+    boolean removeDhcpEntry(final Network network, final NicProfile nic, final VirtualMachineProfile profile, final VirtualRouter virtualRouter) throws ResourceUnavailableException;
 }

server/test/com/cloud/vpc/MockNetworkManagerImpl.java

@@ -924,6 +924,10 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches
         return false;
     }
 
+    @Override
+    public void cleanupNicDhcpDnsEntry(Network network, VirtualMachineProfile vmProfile, NicProfile nicProfile) {
+    }
+
     @Override
     public void finalizeUpdateInSequence(Network network, boolean success) {
         return;

systemvm/debian/opt/cloud/bin/cs/CsDhcp.py

@@ -16,6 +16,7 @@
 # under the License.
 import CsHelper
 import logging
+import os
 from netaddr import *
 from random import randint
 from CsGuestNetwork import CsGuestNetwork
@@ -46,8 +47,8 @@ class CsDhcp(CsDataBag):
         for item in self.dbag:
             if item == "id":
                 continue
+            if not self.dbag[item]['remove']:
                 self.add(self.dbag[item])
-        self.write_hosts()
 
         self.configure_server()
 
@@ -64,6 +65,8 @@ class CsDhcp(CsDataBag):
         if restart_dnsmasq:
             self.delete_leases()
 
+        self.write_hosts()
+
         if not self.cl.is_redundant() or self.cl.is_master():
             if restart_dnsmasq:
                 CsHelper.service("dnsmasq", "restart")
@@ -114,10 +117,28 @@ class CsDhcp(CsDataBag):
             idx += 1
 
     def delete_leases(self):
+        macs_dhcphosts = []
+        interfaces = filter(lambda x: x.startswith('eth'), os.listdir('/sys/class/net'))
         try:
-            open(LEASES, 'w').close()
-        except IOError:
-            return
+            logging.info("Attempting to delete entries from dnsmasq.leases file for VMs which are not on dhcphosts file")
+            for host in open(DHCP_HOSTS):
+                macs_dhcphosts.append(host.split(',')[0])
+
+            removed = 0
+            for leaseline in open(LEASES):
+                lease = leaseline.split(' ')
+                mac = lease[1]
+                ip = lease[2]
+                if mac not in macs_dhcphosts:
+                    for interface in interfaces:
+                        cmd = "dhcp_release %s %s %s" % (interface, ip, mac)
+                        logging.info(cmd)
+                        CsHelper.execute(cmd)
+                    removed = removed + 1
+                    self.del_host(ip)
+            logging.info("Deleted %s entries from dnsmasq.leases file" % str(removed))
+        except Exception as e:
+            logging.error("Caught error while trying to delete entries from dnsmasq.leases file: %s" % e)
 
     def preseed(self):
         self.add_host("127.0.0.1", "localhost %s" % CsHelper.get_hostname())
@@ -170,3 +191,7 @@ class CsDhcp(CsDataBag):
 
     def add_host(self, ip, hosts):
         self.hosts[ip] = hosts
+
+    def del_host(self, ip):
+        if ip in self.hosts:
+            self.hosts.pop(ip)

systemvm/debian/opt/cloud/bin/cs_dhcp.py

@@ -15,6 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import logging
 from netaddr import *
 
 
@@ -36,6 +37,9 @@ def merge(dbag, data):
                 remove_keys.add(key)
                 break
 
+        if data['remove'] and key not in remove_keys:
+            remove_keys.add(key)
+
         for remove_key in remove_keys:
             del(dbag[remove_key])