Revert "Add os_type and os_version args to lock_many()"

This reverts commit f39b6958c4dd94f7e682833f4cf3122c4537dcde.

diff --git a/teuthology/lock.py b/teuthology/lock.py
index 2ad0858bef759b2a23c7eab6fa3fc3eb66cce963..01755ede19b8736dd817dce828a445204866199a 100644 (file)
--- a/teuthology/lock.py
+++ b/teuthology/lock.py
@@ -24,7 +24,6 @@ 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.
@@ -60,7 +59,7 @@ def get_distro_from_downburst():
         return default_table
 
 
-def validate_os_type_and_version(ctx, machine_type):
+def vps_version_or_type_valid(machine_type, os_type, os_version):
     """
     Check os-type and os-version parameters when locking a vps.
     Os-type will always be set (defaults to ubuntu).
@@ -73,18 +72,15 @@ def validate_os_type_and_version(ctx, machine_type):
     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 '%s' is invalid", os_type)
+        log.error('os-type is invalid')
         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 '%s' is invalid", os_version)
         return False
     return True
 
-
 def validate_distro_version(version, supported_versions):
     """
     Return True if the version is valid.  For Ubuntu, possible
@@ -101,7 +97,6 @@ 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)
@@ -236,7 +231,8 @@ def main(ctx):
         return 0
 
     elif ctx.lock:
-        if not validate_os_type_and_version(ctx, ctx.machine_type):
+        if not vps_version_or_type_valid(ctx.machine_type, ctx.os_type,
+                                         ctx.os_version):
             log.error('Invalid os-type or version detected -- lock failed')
             return 1
         for machine in machines:
@@ -263,7 +259,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.os_type, ctx.os_version)
+                           ctx.desc)
         if not result:
             ret = 1
         else:
@@ -304,29 +300,22 @@ def main(ctx):
     return ret
 
 
-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):
+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):
         log.error('Invalid os-type or version detected -- lock failed')
         return
-    machinetypes = misc.get_multi_machine_types(machine_type)
+    machinetypes = misc.get_multi_machine_types(machinetype)
     if user is None:
         user = misc.get_user()
-    for machine_type in machinetypes:
+    for machinetype in machinetypes:
         uri = os.path.join(config.lock_server, 'nodes', 'lock_many', '')
         data = dict(
             locked_by=user,
             count=num,
-            machine_type=machine_type,
+            machine_type=machinetype,
             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),
@@ -337,7 +326,7 @@ def lock_many(ctx, num, machine_type, user=None, description=None,
                         machine['ssh_pub_key'] for machine in response.json()}
             log.debug('locked {machines}'.format(
                 machines=', '.join(machines.keys())))
-            if machine_type == 'vps':
+            if machinetype == 'vps':
                 ok_machs = {}
                 for machine in machines:
                     if provision.create_if_vm(ctx, machine):
@@ -350,11 +339,11 @@ def lock_many(ctx, num, machine_type, user=None, description=None,
             return machines
         elif response.status_code == 503:
             log.error('Insufficient nodes available to lock %d %s nodes.',
-                      num, machine_type)
+                      num, machinetype)
             log.error(response.text)
         else:
             log.error('Could not lock %d %s nodes, reason: unknown.',
-                      num, machine_type)
+                      num, machinetype)
     return []
 
 

diff --git a/teuthology/task/internal.py b/teuthology/task/internal.py
index ad6da25a5d88ce1883218e3fcd9d85d6217910e9..546821ca19c07e5ed5df59a1a6b0ddb262280643 100644 (file)
--- a/teuthology/task/internal.py
+++ b/teuthology/task/internal.py
@@ -95,10 +95,8 @@ 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, os_type, os_version)
+                                      ctx.archive)
         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 87514eac66926996b9343c385bbe6dbb98b09747..d3b5fe9cbe26215c06afe171fa32acab4a36471b 100644 (file)
--- a/teuthology/test/test_vps_os_vers_parameter_checking.py
+++ b/teuthology/test/test_vps_os_vers_parameter_checking.py
@@ -1,9 +1,6 @@
 from .. import lock
 
-
-class Mock:
-    pass
-
+class Mock: pass
 
 class TestVpsOsVersionParamCheck(object):
 
@@ -16,49 +13,55 @@ class TestVpsOsVersionParamCheck(object):
     def test_ubuntu_precise(self):
         self.fake_ctx.os_type = 'ubuntu'
         self.fake_ctx.os_version = 'precise'
-        check_value = lock.validate_os_type_and_version(
-            self.fake_ctx,
-            self.fake_ctx.machine_type)
-
+        check_value = lock.vps_version_or_type_valid(
+                      self.fake_ctx.machine_type,
+                      self.fake_ctx.os_type,
+                      self.fake_ctx.os_version)
+                            
         assert check_value
 
     def test_ubuntu_number(self):
         self.fake_ctx.os_type = 'ubuntu'
         self.fake_ctx.os_version = '12.04'
-        check_value = lock.validate_os_type_and_version(
-            self.fake_ctx,
-            self.fake_ctx.machine_type)
+        check_value = lock.vps_version_or_type_valid(
+                      self.fake_ctx.machine_type,
+                      self.fake_ctx.os_type,
+                      self.fake_ctx.os_version)
         assert check_value
 
     def test_rhel(self):
         self.fake_ctx.os_type = 'rhel'
         self.fake_ctx.os_version = '6.5'
-        check_value = lock.validate_os_type_and_version(
-            self.fake_ctx,
-            self.fake_ctx.machine_type)
+        check_value = lock.vps_version_or_type_valid(
+                      self.fake_ctx.machine_type,
+                      self.fake_ctx.os_type,
+                      self.fake_ctx.os_version)
         assert check_value
 
     def test_mixup(self):
         self.fake_ctx.os_type = '6.5'
         self.fake_ctx.os_version = 'rhel'
-        check_value = lock.validate_os_type_and_version(
-            self.fake_ctx,
-            self.fake_ctx.machine_type)
+        check_value = lock.vps_version_or_type_valid(
+                      self.fake_ctx.machine_type,
+                      self.fake_ctx.os_type,
+                      self.fake_ctx.os_version)
         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.validate_os_type_and_version(
-            self.fake_ctx,
-            self.fake_ctx.machine_type)
+        check_value = lock.vps_version_or_type_valid(
+                      self.fake_ctx.machine_type,
+                      self.fake_ctx.os_type,
+                      self.fake_ctx.os_version)
         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.validate_os_type_and_version(
-            self.fake_ctx,
-            self.fake_ctx.machine_type)
+        check_value = lock.vps_version_or_type_valid(
+                      self.fake_ctx.machine_type,
+                      self.fake_ctx.os_type,
+                      self.fake_ctx.os_version)
         assert not check_value