OS-5716: avoid unnecessary sudo calls in test-runner

Details

Issue Type:Improvement
Priority:4 - Normal
Status:Resolved
Created at:2016-10-13T17:21:41.000Z
Updated at:2019-10-15T09:55:35.161Z

People

Created by:Ryan Zezeski [X]
Reported by:Ryan Zezeski [X]
Assigned to:Ryan Zezeski [X]

Resolution

Fixed: A fix for this issue is checked into the tree and tested.
(Resolution Date: 2017-02-07T16:26:19.000Z)

Fix Versions

2017-02-16 JAMAICA DECOY (Release Date: 2017-02-16)

Description

The gate repo comes with some built-in tests which use a python script as a driver (previously it was named run.py}):

usr/src/test/test-runner/cmd/run

Part of the runner's job is to read a test's configuration file, e.g. os-tests/runfiles/default.run and set various properties, like the user, if specified. Up until commit 9f9230833b50b8271840dc2c12bd1e94d9df7d12 there was a bug which caused all properties not in the DEFAULT section to not be set. E.g., if you set user in the test's section but not in DEFAULT then user wouldn't actually bet set. If no user is set then there is no user to verify and verify_user is never called.

        for user in [user for user in users if len(user)]:
            if not verify_user(user, logger):

Now that the bug is fixed, the runner is setting the user and verify_user is getting called. The verify_user function relies on sudo to do its job and thus breaks on SmartOS since we don't provide sudo.

This is the change that fixed the property setting bug:

-                    try:
-                        setattr(test, prop, config.get('DEFAULT', prop))
-                        setattr(test, prop, config.get(section, prop))
-                    except ConfigParser.NoOptionError:
-                        pass
+                    for sect in ['DEFAULT', section]:
+                        if config.has_option(sect, prop):
+                            setattr(test, prop, config.get(sect, prop))
+

We should see if we can find a way to perform the same job without relying on sudo.

Comments

Comment by Bot Bot [X]
Created at 2017-02-07T16:23:12.000Z

illumos-joyent commit 3faf446 (branch master, by Ryan Zezeski)

OS-5716 avoid unnecessary sudo calls in test-runner
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Jerry Jelinek <jerry.jelinek@joyent.com>


Comment by Tim Foster
Created at 2019-10-15T09:55:35.161Z

As part of https://www.illumos.org/issues/11814 I'd like to upstream this fix. With the changes in OS-7943, the smartos-test wrapper will configure a system so that we're more likely to actually have sudo, but

  1. we shouldn't need to rely on that script being run
  2. using fewer sudo calls is still a good thing, so this is worth upstreaming