autoserv: append profile selection to hostname parameter

I spent much time going back and forth trying to find the smallest
impact way to present installer server profiles to autoserv for control
file usage. Eventually, I decided that appending it to the hostname
makes sense, as it's a parameter of the host selection -- i.e., I want
UserW on HostX installed with ProfileY.

'#' is not a valid character in hostnames, afaik, so I think it should
be safe to use as a separator. Update the parse_machine to know about
this separator. Now, create_host() automatically puts in a profile
object, if the host is of type InstallableHost.

Signed-off-by: Nishanth Aravamudan <[email protected]>

scheduler/monitor_db.py

@@ -68,23 +68,40 @@ def _get_pidfile_timeout_secs():
     return pidfile_timeout_mins * 60
 
 
-def _autoserv_command_line(machines, extra_args, job=None,
+def _autoserv_command_line(machines, profiles, extra_args, job=None,
                            queue_entry=None, verbose=True):
     """
-    @returns The autoserv command line as a list of executable + parameters.
-
-    @param machines - string - A machine or comma separated list of machines
-            for the (-m) flag.
-    @param extra_args - list - Additional arguments to pass to autoserv.
-    @param job - Job object - If supplied, -u owner and -l name parameters
-            will be added.
-    @param queue_entry - A HostQueueEntry object - If supplied and no Job
-            object was supplied, this will be used to lookup the Job object.
+    Builds an autoserv command line composed of the executable and parameters
+
+    @type machines: list
+    @param machines: List of machines for the (-m) flag
+
+    @type profiles: list
+    @param profiles: List of profiles to set for machines (will be added to
+                     machine names with the machine#host syntax)
+
+    @type extra_args: list
+    @param extra_args: Additional arguments to pass to autoserv.
+
+    @type job: Job object
+    @param job: If supplied, -u owner and -l name parameters will be added.
+
+    @type queue_entry: HostQueueEntry object
+    @param queue_entry: If supplied and no Job object was supplied, this will
+                        be used to lookup the Job object.
+
+    @type verbose: boolean
+    @param verbose: Add the '--verbose' argument to the autoserv command line
+
+    @returns: The autoserv command line as a list of executable + parameters.
     """
     autoserv_argv = [_autoserv_path, '-p',
                      '-r', drone_manager.WORKING_DIRECTORY]
     if machines:
-        autoserv_argv += ['-m', machines]
+        if profiles:
+            # add profiles using the host#profile notation
+            machines = ['%s#%s' % (m, p) for (m, p) in zip(machines, profiles)]
+        autoserv_argv += ['-m', ','.join(machines)]
     if job or queue_entry:
         if not job:
             job = queue_entry.job
@@ -1450,7 +1467,7 @@ def _keyval_path(self):
 
 
     def _command_line(self):
-        return _autoserv_command_line(self.host.hostname,
+        return _autoserv_command_line([self.host.hostname], [],
                                       self._extra_command_args,
                                       queue_entry=self.queue_entry)
 
@@ -1714,13 +1731,17 @@ def _write_control_file(self, execution_path):
     def _command_line(self):
         execution_path = self.queue_entries[0].execution_path()
         control_path = self._write_control_file(execution_path)
-        hostnames = ','.join(entry.host.hostname
+        hostnames = [entry.host.hostname
+                             for entry in self.queue_entries
+                             if not entry.is_hostless()]
+        profiles = [entry.profile
                              for entry in self.queue_entries
-                             if not entry.is_hostless())
+                             if not entry.is_hostless() and entry.profile]
 
         execution_tag = self.queue_entries[0].execution_tag()
         params = _autoserv_command_line(
             hostnames,
+            profiles,
             ['-P', execution_tag, '-n',
              _drone_manager.absolute_path(control_path)],
             job=self.job, verbose=False)

scheduler/monitor_db_unittest.py

@@ -1338,31 +1338,51 @@ def tearDown(self):
 
 
     def test_autoserv_command_line(self):
-        machines = 'abcd12,efgh34'
+        machines_s = 'abcd12,efgh34'
+        machines = ('abcd12', 'efgh34')
+        profiles = ('fedora17', 'fedora18')
+        machines_profiles = 'abcd12#fedora17,efgh34#fedora18'
         extra_args = ['-Z', 'hello']
-        expected_command_line_base = set((monitor_db._autoserv_path, '-p',
-                                          '-m', machines, '-r',
-                                          drone_manager.WORKING_DIRECTORY))
 
-        expected_command_line = expected_command_line_base.union(
-                ['--verbose']).union(extra_args)
-        command_line = set(
-                monitor_db._autoserv_command_line(machines, extra_args))
-        self.assertEqual(expected_command_line, command_line)
+        # ecl is our expected command line
+        # cl is our actual command line
+        ecl_base = set((monitor_db._autoserv_path, '-p', '-m', machines_s, '-r',
+                        drone_manager.WORKING_DIRECTORY))
+        ecl_profiles = set((monitor_db._autoserv_path, '-p', '-m',
+                            machines_profiles, '-r',
+                            drone_manager.WORKING_DIRECTORY))
+        ecl = ecl_base.union(['--verbose']).union(extra_args)
+        ecl_profiles = ecl_profiles.union(['--verbose']).union(extra_args)
+        cl = set(monitor_db._autoserv_command_line(machines, [], extra_args))
+        cl_profiles = set(
+              monitor_db._autoserv_command_line(machines, profiles, extra_args))
+
+        self.assertEqual(ecl, cl)
+        self.assertEqual(ecl_profiles, cl_profiles)
 
         class FakeJob(object):
             owner = 'Bob'
             name = 'fake job name'
-            id = 1337
 
         class FakeHQE(object):
             job = FakeJob
 
-        expected_command_line = expected_command_line_base.union(
-                ['-u', FakeJob.owner, '-l', FakeJob.name])
-        command_line = set(monitor_db._autoserv_command_line(
-                machines, extra_args=[], queue_entry=FakeHQE, verbose=False))
-        self.assertEqual(expected_command_line, command_line)
+        ecl = ecl_base.union(['-u', FakeJob.owner, '-l', FakeJob.name])
+        ecl_profiles = ecl_profiles.union(['-u', FakeJob.owner, '-l',
+                                           FakeJob.name])
+        cl = set(monitor_db._autoserv_command_line(machines, profiles=[],
+                                                   extra_args=[], job=FakeJob,
+                                                   queue_entry=FakeHQE,
+                                                   verbose=False))
+        cl_profiles = set(monitor_db._autoserv_command_line(machines,
+                                                            profiles=profiles,
+                                                            extra_args=extra_args,
+                                                            job=FakeJob,
+                                                            queue_entry=FakeHQE,
+                                                            verbose=True))
+
+        self.assertEqual(ecl, cl)
+        self.assertEqual(ecl_profiles, cl_profiles)
 
 
 class AgentTaskTest(unittest.TestCase,

server/base_utils.py

@@ -244,13 +244,12 @@ def form_ntuples_from_machines(machines, n=2, mapping_func=default_mappings):
     return (ntuples, failures)
 
 
-def parse_machine(machine, user='root', password='', port=22):
+def parse_machine(machine, user='root', password='', port=22, profile=''):
     """
     Parse the machine string user:pass@host:port and return it separately,
     if the machine string is not complete, use the default parameters
     when appropriate.
     """
-
     if '@' in machine:
         user, machine = machine.split('@', 1)
 
@@ -259,12 +258,19 @@ def parse_machine(machine, user='root', password='', port=22):
 
     if ':' in machine:
         machine, port = machine.split(':', 1)
-        port = int(port)
+        try:
+            port = int(port)
+        except ValueError:
+            port, profile = port.split('#', 1)
+            port = int(port)
+
+    if '#' in machine:
+        machine, profile = machine.split('#', 1)
 
     if not machine or not user:
         raise ValueError
 
-    return machine, user, password, port
+    return machine, user, password, port, profile
 
 
 def get_public_key():

server/base_utils_unittest.py

@@ -32,20 +32,29 @@ def test_form_cell_mappings(self):
     # parse_machine() test cases
     def test_parse_machine_good(self):
         '''test that parse_machine() is outputting the correct data'''
-        gooddata = (('host',                ('host', 'root', '', 22)),
-                    ('host:21',             ('host', 'root', '', 21)),
-                    ('user@host',           ('host', 'user', '', 22)),
-                    ('user:pass@host',      ('host', 'user', 'pass', 22)),
-                    ('user:pass@host:1234', ('host', 'user', 'pass', 1234)),
+        gooddata = (('host',                ('host', 'root', '', 22, '')),
+                    ('host:21',             ('host', 'root', '', 21, '')),
+                    ('user@host',           ('host', 'user', '', 22, '')),
+                    ('user:pass@host',      ('host', 'user', 'pass', 22, '')),
+                    ('user:pass@host:1234', ('host', 'user', 'pass', 1234, '')),
+                    ('user:pass@host:1234#fedora16',
+                     ('host', 'user', 'pass', 1234, 'fedora16')),
+                    ('user:pass@host:1234#fedora16',
+                     ('host', 'user', 'pass', 1234, 'fedora16')),
+                    ('user:pass@host#fedora16',
+                     ('host', 'user', 'pass', 22, 'fedora16')),
+                    ('user@host#fedora16',
+                     ('host', 'user', '', 22, 'fedora16')),
                    )
+
         for machine, result in gooddata:
             self.assertEquals(utils.parse_machine(machine), result)
 
 
     def test_parse_machine_override(self):
         '''Test that parse_machine() defaults can be overridden'''
-        self.assertEquals(utils.parse_machine('host', 'bob', 'foo', 1234),
-                          ('host', 'bob', 'foo', 1234))
+        self.assertEquals(utils.parse_machine('host', 'bob', 'foo', 1234, 'rhel6'),
+                          ('host', 'bob', 'foo', 1234, 'rhel6'))
 
 
     def test_parse_machine_bad(self):
@@ -56,6 +65,7 @@ def test_parse_machine_bad(self):
                    ('user@', ValueError),       # neglect to pass a hostname #2
                    ('user@:22', ValueError),    # neglect to pass a hostname #3
                    (':pass@host', ValueError),  # neglect to pass a username
+                   (':pass@host#fedora16', ValueError),  # neglect to pass a username
                   )
         for machine, exception in baddata:
             self.assertRaises(exception, utils.parse_machine, machine)

server/hosts/factory.py

@@ -15,6 +15,11 @@
 def create_host(
     hostname, auto_monitor=True, follow_paths=None, pattern_paths=None,
     netconsole=False, **args):
+    # parse out the profile up-front, if it's there, or else console monitoring
+    # will not work
+    hostname, args['user'], args['password'], args['port'], args['profile'] = \
+            server_utils.parse_machine(hostname, ssh_user, ssh_pass, ssh_port)
+
     # by default assume we're using SSH support
     if SSH_ENGINE == 'paramiko':
         from autotest.server.hosts import paramiko_host
@@ -68,9 +73,6 @@ def create_host(
     site_factory.postprocess_classes(classes, hostname,
                                      auto_monitor=auto_monitor, **args)
 
-    hostname, args['user'], args['password'], args['port'], args['profile'] = \
-            server_utils.parse_machine(hostname, ssh_user, ssh_pass, ssh_port)
-
     # create a custom host class for this machine and return an instance of it
     host_class = type("%s_host" % hostname, tuple(classes), {})
     host_instance = host_class(hostname, **args)