cleanup

config.yaml

@@ -121,13 +121,14 @@ options:
     description: |
       YAML-formatted associative array of sysctl key/value pairs to be set
       persistently e.g. '{ kernel.pid_max : 4194303 }'.
-  # Legacy HA
+  # Legacy (Icehouse) HA
   ha-legacy-mode:
     type: boolean
     default: False
     description: |
-      Support HA ACTIVE/PASSIVE mode with pacemaker and corosync before neutron
+      If True will enable ACTIVE/PASSIVE HA mode for neutron agents using
-      native HA feature landed to Juno.
+      Pacemaker and Corosync. This is intended for < Juno which natively
+      supports HA in Neutron itself.
   ha-bindiface:
     type: string
     default: eth0

hooks/quantum_hooks.py

@@ -24,7 +24,7 @@ from charmhelpers.core.host import (
 )
 from charmhelpers.contrib.hahelpers.cluster import(
     eligible_leader,
-    get_hacluster_config
+    get_hacluster_config,
 )
 from charmhelpers.contrib.hahelpers.apache import(
     install_ca_cert
@@ -52,7 +52,7 @@ from quantum_utils import (
     update_legacy_ha_files,
     remove_legacy_ha_files,
     delete_legacy_resources,
-    add_hostname_to_hosts
+    add_hostname_to_hosts,
 )
 
 hooks = Hooks()
@@ -124,7 +124,7 @@ def config_changed():
 def upgrade_charm():
     install()
     config_changed()
-    update_legacy_ha_files(update=True)
+    update_legacy_ha_files(force=True)
 
 
 @hooks.hook('shared-db-relation-joined')
@@ -237,7 +237,7 @@ def stop():
 def ha_relation_joined():
     if config('ha-legacy-mode'):
         cache_env_data()
-        cluster_config = get_hacluster_config(excludes_key=['vip'])
+        cluster_config = get_hacluster_config(exclude_keys=['vip'])
         resources = {
             'res_monitor': 'ocf:canonical:NeutronAgentMon',
         }
@@ -257,6 +257,8 @@ def ha_relation_joined():
 
 @hooks.hook('ha-relation-departed')
 def ha_relation_destroyed():
+    # If e.g. we want to upgrade to Juno and use native Neutron HA support then
+    #  we need to un-corosync-cluster to enable the transition.
     if config('ha-legacy-mode'):
         delete_legacy_resources()
         remove_legacy_ha_files()

hooks/quantum_utils.py

@@ -4,14 +4,16 @@ import stat
 import subprocess
 from shutil import copy2
 from charmhelpers.core.host import (
+    mkdir,
     service_running,
     service_stop,
     service_restart,
     lsb_release,
-    mkdir
 )
 from charmhelpers.core.hookenv import (
     log,
+    DEBUG,
+    INFO,
     ERROR,
     config,
     relations_of_type,
@@ -603,50 +605,47 @@ def configure_ovs():
                             promisc=True)
 
 
-def copy_file(source_dir, des_dir, f, f_mod=None, update=False):
+def copy_file(src, dst, perms=None, force=False):
-    if not os.path.isdir(des_dir):
+    if not os.path.isdir(dst):
-        mkdir(des_dir)
+        log('Creating directory %s' % dst, level=DEBUG)
-        log('Directory created at: %s' % des_dir)
+        mkdir(dst)
 
-    if not os.path.isfile(os.path.join(des_dir, f)) or update:
+    fdst = os.path.join(dst, os.path.basename(src))
+    if not os.path.isfile(fdst) or force:
         try:
-            source_f = os.path.join(source_dir, f)
+            copy2(src, fdst)
-            des_f = os.path.join(des_dir, f)
+            if perms:
-            copy2(source_f, des_dir)
+                os.chmod(fdst, perms)
-            if f_mod:
-                os.chmod(des_f, f_mod)
         except IOError:
-            log('Failed to copy file from %s to %s.' %
+            log('Failed to copy file from %s to %s.' % (src, dst), level=ERROR)
-                (source_f, des_dir), level=ERROR)
             raise
 
 
-def remove_file(des_dir, f):
+def remove_file(path):
-    if not os.path.isdir(des_dir):
+    if not os.path.isfile(path):
-        log('Directory %s already removed.' % des_dir)
+        log('File %s does not exist.' % path, level=INFO)
+        return
 
-    f = os.path.join(des_dir, f)
-    if os.path.isfile(f):
     try:
-            os.remove(f)
+        os.remove(path)
     except IOError:
-            log('Failed to remove file %s.' % f, level=ERROR)
+        log('Failed to remove file %s.' % path, level=ERROR)
 
 
-def install_legacy_ha_files(update=False):
+def install_legacy_ha_files(force=False):
     for f, p in LEGACY_FILES_MAP.iteritems():
-        copy_file(LEGACY_HA_TEMPLATE_FILES, p['path'], f,
+        copy_file(LEGACY_HA_TEMPLATE_FILES, p['path'], p['permission'],
-                  p['permission'], update=update)
+                  force=force)
 
 
 def remove_legacy_ha_files():
     for f, p in LEGACY_FILES_MAP.iteritems():
-        remove_file(p['path'], f)
+        remove_file(os.path.join(p['path'], f))
 
 
-def update_legacy_ha_files(update=False):
+def update_legacy_ha_files(force=False):
     if config('ha-legacy-mode'):
-        install_legacy_ha_files(update=update)
+        install_legacy_ha_files(force=force)
     else:
         remove_legacy_ha_files()
 
@@ -662,8 +661,8 @@ def cache_env_data():
     if os.path.isfile(envrc_f):
         with open(envrc_f, 'r') as f:
             data = f.read()
-        data = data.strip().split('\n')
 
+        data = data.strip().split('\n')
         diff = False
         for line in data:
             k = line.split('=')[0]
@@ -680,10 +679,12 @@ def cache_env_data():
                 f.write(''.join([k, '=', v, '\n']))
 
 
-def delete_legacy_resources():
 def crm_op(op, res):
     cmd = 'crm -w -F %s %s' % (op, res)
     subprocess.call(cmd.split())
+
+
+def delete_legacy_resources():
     for res in LEGACY_RES_MAP:
         crm_op('resource stop', res)
         crm_op('configure delete', res)
@@ -692,15 +693,12 @@ def delete_legacy_resources():
 def add_hostname_to_hosts():
     # To fix bug 1405588, ovsdb-server got error when
     # running ovsdb-client monitor command start with 'sudo'.
-    hosts_f = '/etc/hosts'
+    hostsfile = '/etc/hosts'
-    if not os.path.isfile(hosts_f):
-        mkdir(hosts_f)
-
     resolve_hostname = '127.0.0.1 %s' % socket.gethostname()
-    with open(hosts_f, 'r') as f:
+    with open(hostsfile, 'r') as f:
         for line in f:
             if resolve_hostname in line:
                 return
 
-    with open(hosts_f, 'a') as f:
+    with open(hostsfile, 'a') as f:
-        f.write('\n' + resolve_hostname + '\n')
+        f.write('\n%s\n' % resolve_hostname)

unit_tests/test_quantum_utils.py

@@ -363,39 +363,37 @@ class TestQuantumUtils(CharmTestCase):
         self.assertEquals(quantum_utils.get_common_package(), 'neutron-common')
 
     def test_copy_file_without_update(self):
-        source_dir = 'dummy_source_dir'
+        src = 'dummy_source_dir/dummy_file'
-        des_dir = 'dummy_des_dir'
+        dst = 'dummy_des_dir'
-        f = 'dummy_file'
+        quantum_utils.copy_file(src, dst)
-        quantum_utils.copy_file(source_dir, des_dir, f)
         self.assertTrue(self.mkdir.called)
         self.assertTrue(self.copy2.called)
 
     @patch('quantum_utils.os.path.isfile')
     def test_copy_file_with_update(self, _isfile):
-        source_dir = 'dummy_source_dir'
+        src = 'dummy_source_dir/dummy_file'
-        des_dir = 'dummy_des_dir'
+        dst = 'dummy_des_dir'
-        f = 'dummy_file'
         _isfile.return_value = False
-        quantum_utils.copy_file(source_dir, des_dir, f, update=True)
+        quantum_utils.copy_file(src, dst, force=True)
         self.assertTrue(self.mkdir.called)
         self.assertTrue(self.copy2.called)
 
+    @patch('quantum_utils.os.remove')
     @patch('quantum_utils.os.path.isfile')
-    def test_remove_file_exists(self, _isfile):
+    def test_remove_file_exists(self, _isfile, _remove):
-        des_dir = 'dummy_des_dir'
+        path = 'dummy_des_dir/dummy_file'
-        f = 'dummy_file'
+        _isfile.return_value = True
-        _isfile.return_value = False
+        quantum_utils.remove_file(path)
-        quantum_utils.remove_file(des_dir, f)
+        self.assertTrue(_remove.called)
-        self.assertTrue(self.log.called)
+        self.assertFalse(self.log.called)
 
     @patch('quantum_utils.os.remove')
     @patch('quantum_utils.os.path.isfile')
     def test_remove_file_non_exists(self, _isfile, _remove):
-        des_dir = 'dummy_des_dir'
+        path = 'dummy_des_dir/dummy_file'
-        f = 'dummy_file'
+        _isfile.return_value = False
-        _isfile.return_value = True
+        quantum_utils.remove_file(path)
-        _remove.return_value = MagicMock()
+        self.assertFalse(_remove.called)
-        quantum_utils.remove_file(des_dir, f)
         self.assertTrue(self.log.called)