Add os_type and os_version args to lock_many()

Signed-off-by: Zack Cerza <zack.cerza@inktank.com>

diff --git a/teuthology/lock.py b/teuthology/lock.py
index dcdd824d5a7d151f90591dea810fabe361810135..fc07fdb113d6af6151fc989d4de738aa94b7dae5 100644 (file)
--- a/teuthology/lock.py
+++ b/teuthology/lock.py
@@ -24,6 +24,7 @@ logging.getLogger("requests.packages.urllib3.connectionpool").setLevel(
 
 is_vpm = lambda name: 'vpm' in name
 
+
 def get_distro_from_downburst():
     """
     Return a table of valid distros.
@@ -59,7 +60,7 @@ def get_distro_from_downburst():
         return default_table
 
 
-def vps_version_or_type_valid(machine_type, os_type, os_version):
+def validate_os_type_and_version(ctx, machine_type):
     """
     Check os-type and os-version parameters when locking a vps.
     Os-type will always be set (defaults to ubuntu).
@@ -72,15 +73,18 @@ def vps_version_or_type_valid(machine_type, os_type, os_version):
     if not machine_type == 'vps':
         return True
     valid_os_and_version = get_distro_from_downburst()
+    os_type = misc.get_distro(ctx)
     if os_type not in valid_os_and_version:
-        log.error('os-type is invalid')
+        log.error("os-type '%s' is invalid", os_type)
         return False
+    os_version = misc.get_distro_version(ctx)
     if not validate_distro_version(os_version,
                                    valid_os_and_version[os_type]):
         log.error('os-version is invalid')
         return False
     return True
 
+
 def validate_distro_version(version, supported_versions):
     """
     Return True if the version is valid.  For Ubuntu, possible
@@ -97,6 +101,7 @@ def validate_distro_version(version, supported_versions):
             if version == part[1][0:len(part[1])-1]:
                 return True
 
+
 def main(ctx):
     if ctx.verbose:
         teuthology.log.setLevel(logging.DEBUG)
@@ -231,8 +236,7 @@ def main(ctx):
         return 0
 
     elif ctx.lock:
-        if not vps_version_or_type_valid(ctx.machine_type, ctx.os_type,
-                                         ctx.os_version):
+        if not validate_os_type_and_version(ctx, ctx.machine_type):
             log.error('Invalid os-type or version detected -- lock failed')
             return 1
         for machine in machines:
@@ -259,7 +263,7 @@ def main(ctx):
                 machines_to_update.append(machine)
     elif ctx.num_to_lock:
         result = lock_many(ctx, ctx.num_to_lock, ctx.machine_type, user,
-                           ctx.desc)
+                           ctx.desc, ctx.os_type, ctx.os_version)
         if not result:
             ret = 1
         else:
@@ -300,22 +304,29 @@ def main(ctx):
     return ret
 
 
-def lock_many(ctx, num, machinetype, user=None, description=None):
-    if not vps_version_or_type_valid(ctx.machine_type, ctx.os_type,
-                                     ctx.os_version):
+def lock_many(ctx, num, machine_type, user=None, description=None,
+              os_type=None, os_version=None):
+    if not validate_os_type_and_version(ctx, machine_type):
         log.error('Invalid os-type or version detected -- lock failed')
         return
-    machinetypes = misc.get_multi_machine_types(machinetype)
+    machinetypes = misc.get_multi_machine_types(machine_type)
     if user is None:
         user = misc.get_user()
-    for machinetype in machinetypes:
+    for machine_type in machinetypes:
         uri = os.path.join(config.lock_server, 'nodes', 'lock_many', '')
         data = dict(
             locked_by=user,
             count=num,
-            machine_type=machinetype,
+            machine_type=machine_type,
             description=description,
         )
+        # Only query for os_type/os_version if non-vps, since in that case we
+        # just create them.
+        if machine_type != 'vps':
+            if os_type:
+                data['os_type'] = os_type
+            if os_version:
+                data['os_version'] = os_version
         response = requests.post(
             uri,
             data=json.dumps(data),
@@ -326,7 +337,7 @@ def lock_many(ctx, num, machinetype, user=None, description=None):
                         machine['ssh_pub_key'] for machine in response.json()}
             log.debug('locked {machines}'.format(
                 machines=', '.join(machines.keys())))
-            if machinetype == 'vps':
+            if machine_type == 'vps':
                 ok_machs = {}
                 for machine in machines:
                     if provision.create_if_vm(ctx, machine):
@@ -339,11 +350,11 @@ def lock_many(ctx, num, machinetype, user=None, description=None):
             return machines
         elif response.status_code == 503:
             log.error('Insufficient nodes available to lock %d %s nodes.',
-                      num, machinetype)
+                      num, machine_type)
             log.error(response.text)
         else:
             log.error('Could not lock %d %s nodes, reason: unknown.',
-                      num, machinetype)
+                      num, machine_type)
     return []
 
 

diff --git a/teuthology/task/internal.py b/teuthology/task/internal.py
index 546821ca19c07e5ed5df59a1a6b0ddb262280643..ad6da25a5d88ce1883218e3fcd9d85d6217910e9 100644 (file)
--- a/teuthology/task/internal.py
+++ b/teuthology/task/internal.py
@@ -95,8 +95,10 @@ def lock_machines(ctx, config):
             else:
                 assert 0, 'not enough machines free'
 
+        os_type = misc.get_distro(ctx)
+        os_version = misc.get_distro_version(ctx)
         newly_locked = lock.lock_many(ctx, how_many, machine_type, ctx.owner,
-                                      ctx.archive)
+                                      ctx.archive, os_type, os_version)
         if not newly_locked and not isinstance(newly_locked, list):
             raise RuntimeError('Invalid parameters specified')
         if len(newly_locked) == how_many:

diff --git a/teuthology/test/test_vps_os_vers_parameter_checking.py b/teuthology/test/test_vps_os_vers_parameter_checking.py
index d3b5fe9cbe26215c06afe171fa32acab4a36471b..87514eac66926996b9343c385bbe6dbb98b09747 100644 (file)
--- a/teuthology/test/test_vps_os_vers_parameter_checking.py
+++ b/teuthology/test/test_vps_os_vers_parameter_checking.py
@@ -1,6 +1,9 @@
 from .. import lock
 
-class Mock: pass
+
+class Mock:
+    pass
+
 
 class TestVpsOsVersionParamCheck(object):
 
@@ -13,55 +16,49 @@ class TestVpsOsVersionParamCheck(object):
     def test_ubuntu_precise(self):
         self.fake_ctx.os_type = 'ubuntu'
         self.fake_ctx.os_version = 'precise'
-        check_value = lock.vps_version_or_type_valid(
-                      self.fake_ctx.machine_type,
-                      self.fake_ctx.os_type,
-                      self.fake_ctx.os_version)
-                            
+        check_value = lock.validate_os_type_and_version(
+            self.fake_ctx,
+            self.fake_ctx.machine_type)
+
         assert check_value
 
     def test_ubuntu_number(self):
         self.fake_ctx.os_type = 'ubuntu'
         self.fake_ctx.os_version = '12.04'
-        check_value = lock.vps_version_or_type_valid(
-                      self.fake_ctx.machine_type,
-                      self.fake_ctx.os_type,
-                      self.fake_ctx.os_version)
+        check_value = lock.validate_os_type_and_version(
+            self.fake_ctx,
+            self.fake_ctx.machine_type)
         assert check_value
 
     def test_rhel(self):
         self.fake_ctx.os_type = 'rhel'
         self.fake_ctx.os_version = '6.5'
-        check_value = lock.vps_version_or_type_valid(
-                      self.fake_ctx.machine_type,
-                      self.fake_ctx.os_type,
-                      self.fake_ctx.os_version)
+        check_value = lock.validate_os_type_and_version(
+            self.fake_ctx,
+            self.fake_ctx.machine_type)
         assert check_value
 
     def test_mixup(self):
         self.fake_ctx.os_type = '6.5'
         self.fake_ctx.os_version = 'rhel'
-        check_value = lock.vps_version_or_type_valid(
-                      self.fake_ctx.machine_type,
-                      self.fake_ctx.os_type,
-                      self.fake_ctx.os_version)
+        check_value = lock.validate_os_type_and_version(
+            self.fake_ctx,
+            self.fake_ctx.machine_type)
         assert not check_value
 
     def test_bad_type(self):
         self.fake_ctx.os_type = 'aardvark'
         self.fake_ctx.os_version = '6.5'
-        check_value = lock.vps_version_or_type_valid(
-                      self.fake_ctx.machine_type,
-                      self.fake_ctx.os_type,
-                      self.fake_ctx.os_version)
+        check_value = lock.validate_os_type_and_version(
+            self.fake_ctx,
+            self.fake_ctx.machine_type)
         assert not check_value
 
     def test_bad_version(self):
         self.fake_ctx.os_type = 'rhel'
         self.fake_ctx.os_version = 'vampire_bat'
-        check_value = lock.vps_version_or_type_valid(
-                      self.fake_ctx.machine_type,
-                      self.fake_ctx.os_type,
-                      self.fake_ctx.os_version)
+        check_value = lock.validate_os_type_and_version(
+            self.fake_ctx,
+            self.fake_ctx.machine_type)
         assert not check_value