[feature] hard restarts upon ping-restart received

5 files changed, 67 insertions, 58 deletions

diff --git a/src/leap/bitmask/vpn/_control.py b/src/leap/bitmask/vpn/_control.py
index 31a12160..49d7acef 100644
--- a/src/leap/bitmask/vpn/_control.py
+++ b/src/leap/bitmask/vpn/_control.py
@@ -28,38 +28,34 @@ class VPNControl(object):
     """
     TERMINATE_MAXTRIES = 10
     TERMINATE_WAIT = 1  # secs
+    RESTART_WAIT = 2  # secs
 
     OPENVPN_VERB = "openvpn_verb"
 
-    def __init__(self, **kwargs):
-        # TODO what the fuck this is doing that is different from
-        # the manager?
+    def __init__(self, remotes, eipconfig,
+                 providerconfig, socket_host, socket_port):
         self._vpnproc = None
         self._pollers = []
 
-        # self._openvpn_verb = flags.OPENVPN_VERBOSITY
         self._openvpn_verb = None
         self._user_stopped = False
-        self._remotes = kwargs['remotes']
 
-    def start(self, *args, **kwargs):
-        """
-        Starts the openvpn subprocess.
-
-        :param args: args to be passed to the VPNProcess
-        :type args: tuple
+        self._remotes = remotes
+        self._eipconfig = eipconfig
+        self._providerconfig = providerconfig
+        self._host = socket_host
+        self._port = socket_port
 
-        :param kwargs: kwargs to be passed to the VPNProcess
-        :type kwargs: dict
-        """
+    def start(self):
         logger.debug('VPN: start')
+
         self._user_stopped = False
         self._stop_pollers()
-        kwargs['openvpn_verb'] = self._openvpn_verb
-        kwargs['remotes'] = self._remotes
 
-        # start the main vpn subprocess
-        vpnproc = VPNProcess(*args, **kwargs)
+        vpnproc = VPNProcess(
+            self._eipconfig, self._providerconfig, self._host,
+            self._port, openvpn_verb=7, remotes=self._remotes,
+            restartfun=self.restart)
 
         if vpnproc.get_openvpn_process():
             logger.info("Another vpn process is running. Will try to stop it.")
@@ -91,17 +87,12 @@ class VPNControl(object):
                      LoopingCall(vpnproc.pollState)]
         self._pollers.extend(poll_list)
         self._start_pollers()
+        return True
 
-    @property
-    def status(self):
-        if not self._vpnproc:
-            return 'OFFLINE'
-        return self._vpnproc.status
-
-    @property
-    def traffic_status(self):
-        return self._vpnproc.traffic_status
-
+    def restart(self):
+        self.stop(shutdown=False, restart=True)
+        reactor.callLater(
+            self.RESTART_WAIT, self.start)
 
     def stop(self, shutdown=False, restart=False):
         """
@@ -122,7 +113,7 @@ class VPNControl(object):
             # We assume that the only valid stops are initiated
             # by an user action, not hard restarts
             self._user_stopped = not restart
-            self._vpnproc.is_restart = restart
+            self._vpnproc.restarting = restart
 
             self._sentterm = True
             self._vpnproc.terminate_openvpn(shutdown=shutdown)
@@ -134,6 +125,10 @@ class VPNControl(object):
         else:
             logger.debug("VPN is not running.")
 
+        return True
+
+
+    # FIXME -- is this used from somewhere???
     def bitmask_root_vpn_down(self):
         """
         Bring openvpn down using the privileged wrapper.
@@ -148,6 +143,16 @@ class VPNControl(object):
                                     BM_ROOT, "openvpn", "stop"])
         return True if exitCode is 0 else False
 
+    @property
+    def status(self):
+        if not self._vpnproc:
+            return 'OFFLINE'
+        return self._vpnproc.status
+
+    @property
+    def traffic_status(self):
+        return self._vpnproc.traffic_status
+
     def _killit(self):
         """
         Sends a kill signal to the process.
@@ -159,8 +164,6 @@ class VPNControl(object):
             self._vpnproc.aborted = True
             self._vpnproc.killProcess()
 
-
-
     def _kill_if_left_alive(self, tries=0):
         """
         Check if the process is still alive, and send a
diff --git a/src/leap/bitmask/vpn/_status.py b/src/leap/bitmask/vpn/_status.py
index da8992ed..940c0cd6 100644
--- a/src/leap/bitmask/vpn/_status.py
+++ b/src/leap/bitmask/vpn/_status.py
@@ -20,8 +20,6 @@ class VPNStatus(object):
             'Network is unreachable (code=101)',),
         'PROCESS_RESTART_TLS': (
             "SIGTERM[soft,tls-error]",),
-        'PROCESS_RESTART_PING': (
-            "SIGTERM[soft,ping-restart]",),
         'INITIALIZATION_COMPLETED': (
             "Initialization Sequence Completed",),
     }
@@ -71,7 +69,6 @@ class VPNStatus(object):
         _table = {
             "network_unreachable": ('OFFLINE', 'network unreachable'),
             "process_restart_tls": ('RESTARTING', 'restart tls'),
-            "process_restart_ping": ('CONNECTING', None),
             "initialization_completed": ('ONLINE', None)
         }
         return _table.get(event.lower())
diff --git a/src/leap/bitmask/vpn/launcher.py b/src/leap/bitmask/vpn/launcher.py
index b8691326..5a904890 100644
--- a/src/leap/bitmask/vpn/launcher.py
+++ b/src/leap/bitmask/vpn/launcher.py
@@ -262,9 +262,13 @@ class VPNLauncher(object):
             '--ca', providerconfig.get_ca_cert_path()
         ]
 
-        #args += [
-        #    '--ping', '10',
-        #    '--ping-restart', '30']
+        args += [
+            '--ping', '5',
+            '--ping-restart', '10']
+
+        args += [
+                '--persist-key',
+                '--persist-local-ip', '--persist-remote-ip']
 
         command_and_args = [openvpn_path] + args
         return command_and_args
diff --git a/src/leap/bitmask/vpn/manager.py b/src/leap/bitmask/vpn/manager.py
index 8781603d..abadc00e 100644
--- a/src/leap/bitmask/vpn/manager.py
+++ b/src/leap/bitmask/vpn/manager.py
@@ -42,36 +42,33 @@ class VPNManager(object):
         :type remotes: tuple of tuple(str, int)
         """
         # TODO we can set all the needed ports, gateways and paths in here
+        # TODO need gateways here
+        # sorting them doesn't belong in here
+        # gateways = ((ip1, portA), (ip2, portB), ...)
+
         ports = []
 
-        # this seems to be obsolete, needed to get gateways
+        # TODO fix hardcoding
         domain = "demo.bitmask.net"
+
         self._remotes = remotes
 
         self._eipconfig = _TempEIPConfig(extra_flags, cert_path, ports)
         self._providerconfig = _TempProviderConfig(domain, ca_path)
-        self._vpn = VPNControl(remotes=remotes)
 
+        host, port = self._get_management_location()
+        self._vpn = VPNControl(remotes=remotes,
+                               eipconfig=self._eipconfig,
+                               providerconfig=self._providerconfig,
+                               socket_host=host, socket_port=port)
 
     def start(self):
         """
         Start the VPN process.
-
-        VPN needs:
-        * paths for: cert, key, ca
-        * gateway, port
-        * domain name
         """
-        host, port = self._get_management_location()
-
-        # TODO need gateways here
-        # sorting them doesn't belong in here
-        # gateways = ((ip1, portA), (ip2, portB), ...)
-
-        self._vpn.start(eipconfig=self._eipconfig,
-                        providerconfig=self._providerconfig,
-                        socket_host=host, socket_port=port)
-        return True
+        result = self._vpn.start()
+        print "RESULT START --->", result
+        return result
 
     def stop(self):
         """
@@ -81,8 +78,8 @@ class VPNManager(object):
         :rtype: bool
         """
         # TODO how to return False if this fails
-        self._vpn.stop(False, False)  # TODO review params
-        return True
+        result = self._vpn.stop(False, False)  # TODO review params
+        return result
 
 
     @property
diff --git a/src/leap/bitmask/vpn/process.py b/src/leap/bitmask/vpn/process.py
index 82b4e7c3..c2941d01 100644
--- a/src/leap/bitmask/vpn/process.py
+++ b/src/leap/bitmask/vpn/process.py
@@ -57,8 +57,10 @@ class VPNProcess(protocol.ProcessProtocol, _management.VPNManagement):
     programmatically.
     """
 
+    # TODO do we really need the eipconfig/providerconfig objects in here???
+
     def __init__(self, eipconfig, providerconfig, socket_host, socket_port,
-                 openvpn_verb, remotes):
+                 openvpn_verb, remotes, restartfun=None):
         """
         :param eipconfig: eip configuration object
         :type eipconfig: EIPConfig
@@ -93,9 +95,10 @@ class VPNProcess(protocol.ProcessProtocol, _management.VPNManagement):
         # XXX use flags, maybe, instead of passing
         # the parameter around.
         self._openvpn_verb = openvpn_verb
+        self._restartfun = restartfun
 
         self._status = _status.VPNStatus()
-        self.is_restart = False
+        self.restarting = False
 
         self._remotes = remotes
 
@@ -129,6 +132,8 @@ class VPNProcess(protocol.ProcessProtocol, _management.VPNManagement):
         """
         # truncate the newline
         line = data[:-1]
+        if 'SIGTERM[soft,ping-restart]' in line:
+            self.restarting = True
         logger.info(line)
         self._status.watch(line)
 
@@ -163,6 +168,9 @@ class VPNProcess(protocol.ProcessProtocol, _management.VPNManagement):
         exit_code = reason.value.exitCode
         if isinstance(exit_code, int):
             logger.debug("processEnded, status %d" % (exit_code,))
+            if self.restarting:
+                logger.debug('Restarting VPN process')
+                reactor.callLater(2, self._restartfun)
 
     # polling