Merge ~barryprice/charm-canonical-livepatch/+git/canonical-livepatch-charm:master into ~livepatch-charmers/charm-canonical-livepatch:master

[Barry Price]

Use livepatch's native config for proxies - we no longer need to write our own YAML or restart the service. Also import os into its own namespace, will cover other imports in a future MP

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Paul Gear (paulgear)]

LGTM, but if you have a plan to switch to importing modules into separate namespaces, then now's the time to do it. At least time.sleep and yaml.load are trivial to implement now.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 74b40b32f58e96cf431425f07e6f166a53e281c0

diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py
index de797ec..21a0f4d 100644
--- a/reactive/canonical_livepatch.py
+++ b/reactive/canonical_livepatch.py
@@ -6,8 +6,8 @@ from charmhelpers.core import hookenv
 from charmhelpers.contrib.charmsupport import nrpe
 from subprocess import check_call, check_output, CalledProcessError
 from time import sleep
-from os import path, uname
-from yaml import load, dump
+from os import environ, path, uname
+from yaml import load
 from distutils.version import LooseVersion
 
 
@@ -83,69 +83,16 @@ def get_patch_details():
     return kernel, patch_state
 
 
-def get_yaml_if_exists(path_to_yaml):
-    if path.exists(path_to_yaml):
-        with open(path_to_yaml, 'r') as stream:
-            try:
-                conf_yaml = load(stream)
-            except Exception:
-                hookenv.log('Unable to load YAML from {}'.format(path_to_yaml), hookenv.ERROR)
-    else:
-        conf_yaml = {}
-
-    return conf_yaml
-
-
-def configure_proxies(http_proxy=None, https_proxy=None, no_proxy=None):
-    config_path = '/var/snap/canonical-livepatch/common/config'
-    conf_yaml = get_yaml_if_exists(config_path)
-
-    if conf_yaml.get('http-proxy'):
-        if http_proxy is None:
-            del conf_yaml['http-proxy']
-        else:
-            conf_yaml['http-proxy'] = http_proxy
-    elif http_proxy:
-        conf_yaml['http-proxy'] = http_proxy
-
-    if conf_yaml.get('https-proxy'):
-        if https_proxy is None:
-            del conf_yaml['https-proxy']
-        else:
-            conf_yaml['https-proxy'] = https_proxy
-    elif https_proxy:
-        conf_yaml['https-proxy'] = https_proxy
-
-    if conf_yaml.get('no-proxy'):
-        if no_proxy is None:
-            del conf_yaml['no-proxy']
-        else:
-            conf_yaml['no-proxy'] = no_proxy
-    elif no_proxy:
-        conf_yaml['no-proxy'] = no_proxy
-
-    stream = open(config_path, 'w')
-    dump(conf_yaml, stream)
-
-
-def restart_livepatch():
-    # do a clean stop of the service first, 'restart' seems fragile right now
-    cmd = ['/bin/systemctl', 'stop', 'snap.canonical-livepatch.canonical-livepatchd.service']
-    hookenv.log('Stopping canonical-livepatch service')
-    try:
-        check_call(cmd, universal_newlines=True)
-    except CalledProcessError:
-        hookenv.log('Failed to stop the service', hookenv.ERROR)
-
-    # and now try to start it again, it may fail the first time!
-    cmd = ['/bin/systemctl', 'start', 'snap.canonical-livepatch.canonical-livepatchd.service']
-    hookenv.log('Starting canonical-livepatch service')
-    try:
-        check_call(cmd, universal_newlines=True)
-    except CalledProcessError:
-        hookenv.log('Failed to start the service - waiting 5s and then trying again', hookenv.ERROR)
-        sleep(5)
-        check_call(cmd, universal_newlines=True)
+def configure_proxies(proxy=None):
+    for proto in ('http-proxy', 'https-proxy'):
+        if proxy is None:
+            proxy = ''
+        cmd = ['/snap/bin/canonical-livepatch', 'config', '{}={}'.format(proto, proxy)]
+        hookenv.log('Configuring {} as {}'.format(proto, proxy))
+        try:
+            check_call(cmd, universal_newlines=True)
+        except CalledProcessError:
+            hookenv.log('Failed to set proxy', hookenv.ERROR)
 
 
 def activate_livepatch():
@@ -172,7 +119,6 @@ def activate_livepatch():
             unit_update('blocked', 'Activation failed')
         else:
             set_flag('canonical-livepatch.active')
-            clear_flag('canonical-livepatch.restart-needed')
             unit_update()
     else:
         hookenv.log('Unable to activate canonical-livepatch as no key has been set', hookenv.ERROR)
@@ -197,16 +143,31 @@ def livepatch_supported():
     else:
         set_flag('canonical-livepatch.supported')
 
+
 @when('canonical-livepatch.supported')
 @when_not('livepatch-proxy.configured')
-def proxy_configure():
+def proxy_settings():
     # Configure proxies early, if required - LP#1761661
+
+    preferred_proxy = None  # default to None
+
+    for key, value in environ.items():
+        # use environment's https_proxy or http_proxy if either are set
+        if key == 'https_proxy':
+            preferred_proxy = value
+        elif key == 'http_proxy':
+            preferred_proxy = value
+
+    # but if we have a charm configured proxy, prefer that
     config = hookenv.config()
-    proxy_url = config.get('livepatch_proxy')
-    if proxy_url:
-        configure_proxies(http_proxy=proxy_url, https_proxy=proxy_url)
+    charm_config_proxy = config.get('livepatch_proxy')
+    if charm_config_proxy:
+        preferred_proxy = charm_config_proxy
+
+    # whatever we prefer (or None), configure it!
+    configure_proxies(preferred_proxy)
+
     set_flag('livepatch-proxy.configured')
-    set_flag('canonical-livepatch.restart-needed')
 
 
 @when('snap.installed.canonical-livepatch')
@@ -247,20 +208,6 @@ def update_key():
     activate_livepatch()
 
 
-@when('canonical-livepatch.connected', 'canonical-livepatch.restart-needed')
-def handle_restart():
-    unit_update('maintenance', 'Restarting client')
-
-    # restart the system service to pick up the new config
-    restart_livepatch()
-
-    # Make sure the service is ready for us
-    wait_for_livepatch()
-
-    # force a key update, we may have been blocked before proxy was set
-    update_key()
-
-
 @when('nrpe-external-master.available')
 def configure_nagios(nagios):
     if hookenv.hook_name() == 'update-status':