Merge lp:~mwhudson/charms/trusty/openstack-dashboard/no-node into lp:~openstack-charmers-archive/charms/trusty/openstack-dashboard/next

[Michael Hudson-Doyle]

This branch attempts to avoid installing nodejs and related things when it's not needed (which would let us deploy it on arm64). I haven't tested this _at all_, is there some standard way of doing this?

[Liam Young (gnuoy)]

See inline comment

review: Needs Fixing

[lp:~mwhudson/charms/trusty/openstack-dashboard/no-node]

35. By Michael Hudson-Doyle on 2014-07-25

oops

[Michael Hudson-Doyle (mwhudson)]

Hi, fixed I think.

[Liam Young (gnuoy)]

Looks good except the test_install_hook unit test is failing because os_release needs mocking out. I'd recommend also adding two additional tests to check that the packages are being adjusted correctly, one for pre-icehouse and one for icehouse. The icehouse could look something like:

=== modified file 'unit_tests/test_horizon_hooks.py'
--- unit_tests/test_horizon_hooks.py 2014-04-10 16:19:57 +0000
+++ unit_tests/test_horizon_hooks.py 2014-07-25 09:26:32 +0000
@@ -28,6 +28,7 @@
     'unit_get',
     'log',
     'execd_preinstall',
+ 'os_release',
     'b64decode']

@@ -53,6 +54,15 @@
         self.apt_update.assert_called_with(fatal=True)
         self.apt_install.assert_called_with(['foo', 'bar'], fatal=True)

+ def test_install_hook_icehouse_pkgs(self):
+ self.os_release.return_value = 'icehouse'
+ self._call_hook('install')
+ self.configure_installation_source.assert_called_with('distro')
+ self.apt_update.assert_called_with(fatal=True)
+ for pkg in ['nodejs', 'node-less']:
+ self.assertFalse(pkg in self.filter_installed_packages.call_args)
+ self.apt_install.assert_called()
+
     @patch('charmhelpers.core.host.file_hash')
     @patch('charmhelpers.core.host.service')
     def test_upgrade_charm_hook(self, _service, _hash):

review: Needs Fixing

[lp:~mwhudson/charms/trusty/openstack-dashboard/no-node]

36. By Michael Hudson-Doyle on 2014-08-27

update tests

37. By Michael Hudson-Doyle on 2014-08-27

merge trunk

38. By Michael Hudson-Doyle on 2014-08-27

merge next

[Michael Hudson-Doyle (mwhudson)]

Hi, I finally got around to fixing this, I think. Please have another look!

[Liam Young (gnuoy)]

Approved and mwhudson has tested

review: Approve

=== modified file 'hooks/horizon_hooks.py'
--- hooks/horizon_hooks.py	2014-01-22 10:20:24 +0000
+++ hooks/horizon_hooks.py	2014-08-27 03:51:21 +0000
@@ -22,6 +22,7 @@
 from charmhelpers.contrib.openstack.utils import (
     configure_installation_source,
     openstack_upgrade_available,
+    os_release,
     save_script_rc
 )
 from horizon_utils import (
@@ -44,7 +45,10 @@
 def install():
     configure_installation_source(config('openstack-origin'))
     apt_update(fatal=True)
-    apt_install(filter_installed_packages(PACKAGES), fatal=True)
+    packages = PACKAGES[:]
+    if os_release('openstack-dashboard') < 'icehouse':
+        packages += ['nodejs', 'node-less']
+    apt_install(filter_installed_packages(packages), fatal=True)
 
 
 @hooks.hook('upgrade-charm')

=== modified file 'hooks/horizon_utils.py'
--- hooks/horizon_utils.py	2014-05-21 10:06:19 +0000
+++ hooks/horizon_utils.py	2014-08-27 03:51:21 +0000
@@ -26,7 +26,7 @@
 PACKAGES = [
     "openstack-dashboard", "python-keystoneclient", "python-memcache",
     "memcached", "haproxy", "python-novaclient",
-    "nodejs", "node-less", "openstack-dashboard-ubuntu-theme"
+    "openstack-dashboard-ubuntu-theme"
 ]
 
 APACHE_CONF_DIR = "/etc/apache2"

=== modified file 'unit_tests/test_horizon_hooks.py'
--- unit_tests/test_horizon_hooks.py	2014-04-10 16:19:57 +0000
+++ unit_tests/test_horizon_hooks.py	2014-08-27 03:51:21 +0000
@@ -28,7 +28,8 @@
     'unit_get',
     'log',
     'execd_preinstall',
-    'b64decode']
+    'b64decode',
+    'os_release']
 
 
 def passthrough(value):
@@ -48,11 +49,26 @@
 
     def test_install_hook(self):
         self.filter_installed_packages.return_value = ['foo', 'bar']
+        self.os_release.return_value = 'icehouse'
         self._call_hook('install')
         self.configure_installation_source.assert_called_with('distro')
         self.apt_update.assert_called_with(fatal=True)
         self.apt_install.assert_called_with(['foo', 'bar'], fatal=True)
 
+    def test_install_hook_icehouse_pkgs(self):
+        self.os_release.return_value = 'icehouse'
+        self._call_hook('install')
+        for pkg in ['nodejs', 'node-less']:
+            self.assertFalse(pkg in self.filter_installed_packages.call_args[0][0])
+        self.apt_install.assert_called()
+
+    def test_install_hook_pre_icehouse_pkgs(self):
+        self.os_release.return_value = 'grizzly'
+        self._call_hook('install')
+        for pkg in ['nodejs', 'node-less']:
+            self.assertTrue(pkg in self.filter_installed_packages.call_args[0][0])
+        self.apt_install.assert_called()
+
     @patch('charmhelpers.core.host.file_hash')
     @patch('charmhelpers.core.host.service')
     def test_upgrade_charm_hook(self, _service, _hash):