install: split the upgrade_common function

The upgrade_common function implements a non trivial logic that defines
how overrides are applied to the install.upgrade task, as well as the
way upgrades are applied to the desired targets.

The function is split in two:

  upgrade_common which remains the entry point
  upgrade_remote_to_config which encapsulates the logic

This allows other parts of teuthology to obey the same logic by calling
the function instead of replicating it.

Signed-off-by: Loic Dachary <loic@dachary.org>

diff --git a/teuthology/task/install.py b/teuthology/task/install.py
index a2a5106bcc74bbd55861984398c7417e23f3fabf..058a42a91602742c8536127b379acca781b7619b 100644 (file)
--- a/teuthology/task/install.py
+++ b/teuthology/task/install.py
@@ -908,11 +908,7 @@ def upgrade_with_ceph_deploy(ctx, node, remote, pkgs, sys_type):
     subprocess.call(['ceph-deploy', 'install'] + params)
     remote.run(args=['sudo', 'restart', 'ceph-all'])
 
-
-def upgrade_common(ctx, config, deploy_style):
-    """
-    Common code for upgrading
-    """
+def upgrade_remote_to_config(ctx, config):
     assert config is None or isinstance(config, dict), \
         "install.upgrade only supports a dictionary for configuration"
 
@@ -925,10 +921,6 @@ def upgrade_common(ctx, config, deploy_style):
     log.info('project %s config %s overrides %s', project, config,
              install_overrides)
 
-    # FIXME: extra_pkgs is not distro-agnostic
-    extra_pkgs = config.get('extra_packages', [])
-    log.info('extra packages: {packages}'.format(packages=extra_pkgs))
-
     # build a normalized remote -> config dict
     remotes = {}
     if 'all' in config:
@@ -946,6 +938,7 @@ def upgrade_common(ctx, config, deploy_style):
                 continue
             remotes[remote] = config.get(role)
 
+    result = {}
     for remote, node in remotes.iteritems():
         if not node:
             node = {}
@@ -958,6 +951,24 @@ def upgrade_common(ctx, config, deploy_style):
             this_overrides.pop('branch', None)
         teuthology.deep_merge(node, this_overrides)
         log.info('remote %s config %s', remote, node)
+        node['project'] = project
+
+        result[remote] = node
+
+    return result
+
+def upgrade_common(ctx, config, deploy_style):
+    """
+    Common code for upgrading
+    """
+    remotes = upgrade_remote_to_config(ctx, config)
+    project = config.get('project', 'ceph')
+
+    # FIXME: extra_pkgs is not distro-agnostic
+    extra_pkgs = config.get('extra_packages', [])
+    log.info('extra packages: {packages}'.format(packages=extra_pkgs))
+
+    for remote, node in remotes.iteritems():
 
         system_type = teuthology.get_system_type(remote)
         assert system_type in ('deb', 'rpm')
@@ -968,11 +979,10 @@ def upgrade_common(ctx, config, deploy_style):
             proj=project, system_type=system_type, pkgs=', '.join(pkgs)))
             # FIXME: again, make extra_pkgs distro-agnostic
         pkgs += extra_pkgs
-        node['project'] = project
 
         deploy_style(ctx, node, remote, pkgs, system_type)
         verify_package_version(ctx, node, remote)
-
+    return len(remotes)
 
 docstring_for_upgrade = """"
     Upgrades packages for a given project.

diff --git a/teuthology/test/task/test_install.py b/teuthology/test/task/test_install.py
index 178d4acfc979c5c7841a980fcb82199789993cda..f982d8c93cb5da4b055596a3972be7b94b20a3f9 100644 (file)
--- a/teuthology/test/task/test_install.py
+++ b/teuthology/test/task/test_install.py
@@ -62,3 +62,166 @@ class TestInstall(object):
             valgrind=True
         )
         assert install.get_flavor(config) == 'notcmalloc'
+
+    @patch("teuthology.misc.get_system_type")
+    @patch("teuthology.task.install.verify_package_version")
+    def test_upgrade_common(self,
+                            m_verify_package_version,
+                            m_get_system_type):
+        expected_system_type = 'deb'
+        def make_remote():
+            remote = Mock()
+            remote.arch = 'x86_64'
+            remote.os = Mock()
+            remote.os.name = 'ubuntu'
+            remote.os.version = '14.04'
+            remote.os.codename = 'trusty'
+            remote.system_type = expected_system_type
+            return remote
+        ctx = Mock()
+        class cluster:
+            remote1 = make_remote()
+            remote2 = make_remote()
+            remotes = {
+                remote1: ['client.0'],
+                remote2: ['mon.a','osd.0'],
+            }
+            def only(self, role):
+                result = Mock()
+                if role in ('client.0',):
+                    result.remotes = { cluster.remote1: None }
+                if role in ('osd.0', 'mon.a'):
+                    result.remotes = { cluster.remote2: None }
+                return result
+        ctx.cluster = cluster()
+        config = {
+            'client.0': {
+                'sha1': 'expectedsha1',
+            },
+        }
+        ctx.config = {
+            'roles': [ ['client.0'], ['mon.a','osd.0'] ],
+            'tasks': [
+                {
+                    'install.upgrade': config,
+                },
+            ],
+        }
+        m_get_system_type.return_value = "deb"
+        def upgrade(ctx, node, remote, pkgs, system_type):
+            assert system_type == expected_system_type
+        assert install.upgrade_common(ctx, config, upgrade) == 1
+        expected_config = {
+            'project': 'ceph',
+            'sha1': 'expectedsha1',
+        }
+        m_verify_package_version.assert_called_with(ctx,
+                                                    expected_config,
+                                                    cluster.remote1)
+    def test_upgrade_remote_to_config(self):
+        expected_system_type = 'deb'
+        def make_remote():
+            remote = Mock()
+            remote.arch = 'x86_64'
+            remote.os = Mock()
+            remote.os.name = 'ubuntu'
+            remote.os.version = '14.04'
+            remote.os.codename = 'trusty'
+            remote.system_type = expected_system_type
+            return remote
+        ctx = Mock()
+        class cluster:
+            remote1 = make_remote()
+            remote2 = make_remote()
+            remotes = {
+                remote1: ['client.0'],
+                remote2: ['mon.a','osd.0'],
+            }
+            def only(self, role):
+                result = Mock()
+                if role in ('client.0',):
+                    result.remotes = { cluster.remote1: None }
+                elif role in ('osd.0', 'mon.a'):
+                    result.remotes = { cluster.remote2: None }
+                else:
+                    result.remotes = None
+                return result
+        ctx.cluster = cluster()
+        ctx.config = {
+            'roles': [ ['client.0'], ['mon.a','osd.0'] ],
+        }
+
+        # nothing -> nothing
+        assert install.upgrade_remote_to_config(ctx, {}) == {}
+
+        # select the remote for the osd.0 role
+        # the 'ignored' role does not exist and is ignored
+        # the remote for mon.a is the same as for osd.0 and
+        # is silently ignored (actually it could be the other
+        # way around, depending on how the keys are hashed)
+        config = {
+            'osd.0': {
+                'sha1': 'expectedsha1',
+            },
+            'ignored': None,
+            'mon.a': {
+                'sha1': 'expectedsha1',
+            },
+        }
+        expected_config = {
+            cluster.remote2: {
+                'project': 'ceph',
+                'sha1': 'expectedsha1',
+            },
+        }
+        assert install.upgrade_remote_to_config(ctx, config) == expected_config
+
+        # select all nodes, regardless
+        config = {
+            'all': {
+                'sha1': 'expectedsha1',
+            },
+        }
+        expected_config = {
+            cluster.remote1: {
+                'project': 'ceph',
+                'sha1': 'expectedsha1',
+            },
+            cluster.remote2: {
+                'project': 'ceph',
+                'sha1': 'expectedsha1',
+            },
+        }
+        assert install.upgrade_remote_to_config(ctx, config) == expected_config
+
+        # verify that install overrides are used as default
+        # values for the upgrade task, not as override
+        ctx.config['overrides'] = {
+            'install': {
+                'ceph': {
+                    'sha1': 'overridesha1',
+                    'tag': 'overridetag',
+                    'branch': 'overridebranch',
+                },
+            },
+        }
+        config = {
+            'client.0': {
+                'sha1': 'expectedsha1',
+            },
+            'osd.0': {
+            },
+        }
+        expected_config = {
+            cluster.remote1: {
+                'project': 'ceph',
+                'sha1': 'expectedsha1',
+            },
+            cluster.remote2: {
+                'project': 'ceph',
+                'sha1': 'overridesha1',
+                'tag': 'overridetag',
+                'branch': 'overridebranch',
+            },
+        }
+        assert install.upgrade_remote_to_config(ctx, config) == expected_config