Replace os_is_* functions

The `pulp_smash.utils.os_is_f2{6,7}` functions are not suitable for
inclusion within a top-level toolkit. There's an enormous number of
combinations of system facts that one might want to discover, and having
`os_is_*` functions is a recipe for an explosion in number of functions.
A framework should provide more generalized tools that individual users
can adapt to their needs.

Replace these functions with functions that report information from
`/etc/os-release`. This is more suitable for use within a toolkit: there
is a much more constrained set of values that one may wish to get from
`/etc/os-release`.

These functions are still somewhat susceptible to a surface-area
explosion. If users wish to get more and more information from
`/etc/os-release`, then it would be best to create a function which
returns a dict where the keys and values correspond to the keys and
values in `/etc/os-release`. Thankfully, the new functions which were
just added are forward compatible with such a design. If these happens,
the just-added functions could be rewritten like so:

```python
def get_os_release_id(cfg, pulp_host=None):
    raise DeprecationWarning(...)
    return get_os_release(cfg, pulp_host)['ID']
```

Fix: #1100

pulp_smash/pulp2/utils.py

@@ -328,7 +328,7 @@ def require_issue_3159(exc):
     """
     cfg = config.get_config()
     if (not selectors.bug_is_fixed(3159, cfg.pulp_version) and
-            utils.os_is_f27(cfg)):
+            _os_is_f27(cfg)):
         raise exc('https://pulp.plan.io/issues/3159')
 
 
@@ -342,7 +342,7 @@ def require_issue_3687(exc):
     """
     cfg = config.get_config()
     if (not selectors.bug_is_fixed(3687, cfg.pulp_version) and
-            utils.os_is_f27(cfg)):
+            _os_is_f27(cfg)):
         raise exc('https://pulp.plan.io/issues/3687')
 
 
@@ -570,3 +570,9 @@ def upload_import_unit(cfg, unit, import_params, repo):
     call_report = client.post(path, body)
     client.delete(malloc['_href'])
     return call_report
+
+
+def _os_is_f27(cfg, pulp_host=None):
+    """Tell whether the given Pulp host's OS is F27."""
+    return (utils.get_os_release_id(cfg, pulp_host) == 'fedora' and
+            utils.get_os_release_version_id(cfg, pulp_host) == '27')

pulp_smash/tests/pulp2/docker/api_v2/test_sync_publish.py

@@ -14,7 +14,11 @@
     gen_distributor,
     gen_repo,
 )
-from pulp_smash.tests.pulp2.docker.utils import get_upstream_name, skip_if
+from pulp_smash.tests.pulp2.docker.utils import (
+    get_upstream_name,
+    os_is_f26,
+    skip_if,
+)
 from pulp_smash.tests.pulp2.docker.utils import set_up_module as setUpModule  # pylint:disable=unused-import
 
 # Variable name derived from HTTP content-type.
@@ -167,7 +171,7 @@ def setUpClass(cls):
         """Create class-wide variables."""
         super().setUpClass()
         cls.cfg = config.get_config()
-        if (utils.os_is_f26(cls.cfg) and
+        if (os_is_f26(cls.cfg) and
                 not selectors.bug_is_fixed(3036, cls.cfg.pulp_version)):
             raise unittest.SkipTest('https://pulp.plan.io/issues/3036')
 
@@ -225,7 +229,7 @@ def setUpClass(cls):
         """Create class-wide variables."""
         super().setUpClass()
         cls.cfg = config.get_config()
-        if (utils.os_is_f26(cls.cfg) and
+        if (os_is_f26(cls.cfg) and
                 not selectors.bug_is_fixed(3036, cls.cfg.pulp_version)):
             raise unittest.SkipTest('https://pulp.plan.io/issues/3036')
         for issue_id in (2287, 2384):
@@ -421,7 +425,7 @@ def setUpClass(cls):
         super().setUpClass()
         cls.cfg = config.get_config()
         cls.repo = {}
-        if (utils.os_is_f26(cls.cfg) and
+        if (os_is_f26(cls.cfg) and
                 not selectors.bug_is_fixed(3036, cls.cfg.pulp_version)):
             raise unittest.SkipTest('https://pulp.plan.io/issues/3036')
         if not selectors.bug_is_fixed(2384, cls.cfg.pulp_version):

pulp_smash/tests/pulp2/docker/api_v2/test_upload.py

@@ -4,12 +4,13 @@
 import json
 import unittest
 
-from pulp_smash import api, config, selectors, utils
+from pulp_smash import api, config, selectors
 from pulp_smash.constants import DOCKER_V2_FEED_URL
 from pulp_smash.pulp2.utils import pulp_admin_login, upload_import_unit
 from pulp_smash.tests.pulp2.docker.api_v2.utils import SyncPublishMixin
 from pulp_smash.tests.pulp2.docker.utils import (
     get_upstream_name,
+    os_is_f26,
     set_up_module,
 )
 
@@ -28,7 +29,7 @@ def setUpClass(cls):
         """Create class-wide variables."""
         super().setUpClass()
         cls.cfg = config.get_config()
-        if (utils.os_is_f26(cls.cfg) and
+        if (os_is_f26(cls.cfg) and
                 not selectors.bug_is_fixed(3036, cls.cfg.pulp_version)):
             raise unittest.SkipTest('https://pulp.plan.io/issues/3036')
         for issue_id in (2287, 2384, 2993):

pulp_smash/tests/pulp2/docker/utils.py

@@ -61,6 +61,12 @@ def write_manifest_list(cfg, manifest_list):
     return file_path, dir_path
 
 
+def os_is_f26(cfg, pulp_host=None):
+    """Tell whether the given Pulp host's OS is F26."""
+    return (utils.get_os_release_id(cfg, pulp_host) == 'fedora' and
+            utils.get_os_release_version_id(cfg, pulp_host) == '26')
+
+
 skip_if = partial(selectors.skip_if, exc=SkipTest)  # pylint:disable=invalid-name
 """The ``@skip_if`` decorator, customized for unittest.
 

pulp_smash/tests/pulp2/puppet/api_v2/test_install_distributor.py

@@ -16,6 +16,7 @@
     publish_repo,
     upload_import_unit,
 )
+from pulp_smash.tests.pulp2.puppet.utils import os_is_f27
 from pulp_smash.tests.pulp2.puppet.api_v2.utils import (
     gen_install_distributor,
     gen_repo,
@@ -37,7 +38,7 @@ def test_all(self):
         4. Check if the puppet_install_distributor config was properly used
         """
         if (not selectors.bug_is_fixed(3314, self.cfg.pulp_version) and
-                utils.os_is_f27(self.cfg)):
+                os_is_f27(self.cfg)):
             self.skipTest('https://pulp.plan.io/issues/3314')
         cli_client = cli.Client(self.cfg)
         sudo = () if cli.is_root(self.cfg) else ('sudo',)

pulp_smash/tests/pulp2/puppet/utils.py

@@ -2,12 +2,24 @@
 """Utilities for Puppet tests."""
 from unittest import SkipTest
 
-from pulp_smash.pulp2 import utils
+from pulp_smash import utils
+from pulp_smash.pulp2.utils import (
+    require_issue_3159,
+    require_issue_3687,
+    require_pulp_2,
+    require_unit_types,
+)
 
 
 def set_up_module():
     """Skip tests if Pulp 2 isn't under test or if Puppet isn't installed."""
-    utils.require_pulp_2(SkipTest)
-    utils.require_issue_3159(SkipTest)
-    utils.require_issue_3687(SkipTest)
-    utils.require_unit_types({'puppet_module'}, SkipTest)
+    require_pulp_2(SkipTest)
+    require_issue_3159(SkipTest)
+    require_issue_3687(SkipTest)
+    require_unit_types({'puppet_module'}, SkipTest)
+
+
+def os_is_f27(cfg, pulp_host=None):
+    """Tell whether the given Pulp host's OS is F27."""
+    return (utils.get_os_release_id(cfg, pulp_host) == 'fedora' and
+            utils.get_os_release_version_id(cfg, pulp_host) == '27')

pulp_smash/tests/pulp2/rpm/api_v2/test_download_policies.py

@@ -29,6 +29,7 @@
     check_issue_2387,
     check_issue_2798,
     check_issue_3104,
+    os_is_f26,
     os_is_rhel6,
     set_up_module,
 )
@@ -40,8 +41,7 @@ def setUpModule():  # pylint:disable=invalid-name
     cfg = config.get_config()
     if cfg.pulp_version < Version('2.8'):
         raise unittest.SkipTest('This module requires Pulp 2.8 or greater.')
-    if (utils.os_is_f26(cfg) and
-            not selectors.bug_is_fixed(3036, cfg.pulp_version)):
+    if os_is_f26(cfg) and not selectors.bug_is_fixed(3036, cfg.pulp_version):
         raise unittest.SkipTest('https://pulp.plan.io/issues/3036')
     if check_issue_2798(cfg):
         raise unittest.SkipTest('https://pulp.plan.io/issues/2798')

pulp_smash/tests/pulp2/rpm/api_v2/test_iso_sync_publish.py

@@ -19,6 +19,7 @@
     set_pulp_manage_rsync,
 )
 from pulp_smash.tests.pulp2.rpm.utils import set_up_module as setUpModule  # pylint:disable=unused-import
+from pulp_smash.tests.pulp2.rpm.utils import os_is_f27
 
 
 class ServeHttpsFalseTestCase(TemporaryUserMixin, unittest.TestCase):
@@ -56,7 +57,7 @@ def test_all(self):
         if not selectors.bug_is_fixed(2657, self.cfg.pulp_version):
             self.skipTest('https://pulp.plan.io/issues/2657')
         if (not selectors.bug_is_fixed(3313, self.cfg.pulp_version) and
-                utils.os_is_f27(self.cfg)):
+                os_is_f27(self.cfg)):
             self.skipTest('https://pulp.plan.io/issues/3313')
 
         # Create a user with which to rsync files

pulp_smash/tests/pulp2/rpm/api_v2/test_package_paths.py

@@ -31,7 +31,7 @@
 """
 import unittest
 
-from pulp_smash import api, config, selectors, utils
+from pulp_smash import api, config, selectors
 from pulp_smash.constants import (
     RPM,
     RPM_ALT_LAYOUT_FEED_URL,
@@ -48,6 +48,7 @@
     check_issue_2354,
     check_issue_2798,
     check_issue_3104,
+    os_is_f26,
 )
 from pulp_smash.tests.pulp2.rpm.utils import set_up_module as setUpModule  # pylint:disable=unused-import
 
@@ -82,7 +83,7 @@ def test_all(self):
             self.skipTest('https://pulp.plan.io/issues/2798')
         if check_issue_2354(cfg):
             self.skipTest('https://pulp.plan.io/issues/2354')
-        if (utils.os_is_f26(cfg) and
+        if (os_is_f26(cfg) and
                 not selectors.bug_is_fixed(3036, cfg.pulp_version)):
             # Here, the calls to get_unit() cause pulp_streamer.service to die
             # without logging out anything. In Pulp #3036, certain actions

pulp_smash/tests/pulp2/rpm/api_v2/test_rsync_distributor.py

@@ -75,7 +75,11 @@
     get_dists_by_type_id,
     set_pulp_manage_rsync,
 )
-from pulp_smash.tests.pulp2.rpm.utils import check_issue_2844, set_up_module
+from pulp_smash.tests.pulp2.rpm.utils import (
+    check_issue_2844,
+    os_is_f27,
+    set_up_module,
+)
 
 
 def _split_path(path):
@@ -103,8 +107,7 @@ def setUpModule():  # pylint:disable=invalid-name
     cfg = config.get_config()
     if not selectors.bug_is_fixed(1759, cfg.pulp_version):
         raise unittest.SkipTest('https://pulp.plan.io/issues/1759')
-    if (not selectors.bug_is_fixed(3313, cfg.pulp_version) and
-            utils.os_is_f27(cfg)):
+    if not selectors.bug_is_fixed(3313, cfg.pulp_version) and os_is_f27(cfg):
         raise unittest.SkipTest('https://pulp.plan.io/issues/3313')
     if cfg.pulp_selinux_enabled:
         set_pulp_manage_rsync(cfg, True)

pulp_smash/tests/pulp2/rpm/utils.py

@@ -7,7 +7,7 @@
 
 from packaging.version import Version
 
-from pulp_smash import cli, selectors
+from pulp_smash import cli, selectors, utils
 from pulp_smash.pulp2 import utils as pulp2_utils
 
 
@@ -104,6 +104,18 @@ def check_issue_3104(cfg):
             not selectors.bug_is_fixed(3104, cfg.pulp_version))
 
 
+def os_is_f26(cfg, pulp_host=None):
+    """Tell whether the given Pulp host's OS is F26."""
+    return (utils.get_os_release_id(cfg, pulp_host) == 'fedora' and
+            utils.get_os_release_version_id(cfg, pulp_host) == '26')
+
+
+def os_is_f27(cfg, pulp_host=None):
+    """Tell whether the given Pulp host's OS is F27."""
+    return (utils.get_os_release_id(cfg, pulp_host) == 'fedora' and
+            utils.get_os_release_version_id(cfg, pulp_host) == '27')
+
+
 def os_is_rhel6(cfg):
     """Return ``True`` if the server runs RHEL 6, or ``False`` otherwise.
 

pulp_smash/utils.py

@@ -16,6 +16,42 @@
 _CHECKSUM_CACHE = {}
 
 
+def get_os_release_id(cfg, pulp_host=None):
+    """Get ``ID`` from ``/etc/os-release``.
+
+    :param pulp_smash.config.PulpSmashConfig cfg: Information about the system
+        being targeted.
+    :param pulp_host: A :class:`pulp_smash.config.PulpHost` to target,
+        instead of the default chosen by :class:`pulp_smash.cli.Client`.
+    :returns: A string such as "rhel," "fedora," or "arch." (These values come
+        from Red Hat Enterprise Linux, Fedora, and Arch Linux respectively.)
+    """
+    return cli.Client(cfg, pulp_host=pulp_host).run((
+        'bash',
+        '-c',
+        '(source /etc/os-release && echo "$ID")',
+    )).stdout.strip()
+
+
+def get_os_release_version_id(cfg, pulp_host=None):
+    """Get ``VERSION_ID`` from ``/etc/os-release``.
+
+    :param pulp_smash.config.PulpSmashConfig cfg: Information about the system
+        being targeted.
+    :param pulp_host: A :class:`pulp_smash.config.PulpHost` to target,
+        instead of the default chosen by :class:`pulp_smash.cli.Client`.
+    :returns: A string such as "7.5" or "27". (These values come from RHEL 7.5
+        and Fedora 27, respectively.) Make sure to convert this string to an
+        actual version object if doing version number comparisons.
+        ``packaging.version.Version`` can be used for this purpose.
+    """
+    return cli.Client(cfg, pulp_host=pulp_host).run((
+        'bash',
+        '-c',
+        '(source /etc/os-release && echo "$VERSION_ID")',
+    )).stdout.strip()
+
+
 def get_sha256_checksum(url):
     """Return the sha256 checksum of the file at the given URL.
 
@@ -50,42 +86,6 @@ def http_get(url, **kwargs):
     return response.content
 
 
-def os_is_f26(cfg, pulp_host=None):
-    """Return ``True`` if the server runs Fedora 26, or ``False`` otherwise.
-
-    :param pulp_smash.config.PulpSmashConfig cfg: Information about the system
-        being targeted.
-    :param pulp_host: A :class:`pulp_smash.config.PulpHost` to target,
-        instead of the default chosen by :class:`pulp_smash.cli.Client`.
-    :returns: True or false.
-    """
-    response = cli.Client(cfg, cli.echo_handler, pulp_host).run((
-        'grep',
-        '-i',
-        'fedora release 26',
-        '/etc/redhat-release',
-    ))
-    return response.returncode == 0
-
-
-def os_is_f27(cfg, pulp_host=None):
-    """Return ``True`` if the server runs Fedora 27, or ``False`` otherwise.
-
-    :param pulp_smash.config.PulpSmashConfig cfg: Information about the system
-        being targeted.
-    :param pulp_host: A :class:`pulp_smash.config.PulpHost` to target,
-        instead of the default chosen by :class:`pulp_smash.cli.Client`.
-    :returns: True or false.
-    """
-    response = cli.Client(cfg, cli.echo_handler, pulp_host).run((
-        'grep',
-        '-i',
-        'fedora release 27',
-        '/etc/redhat-release',
-    ))
-    return response.returncode == 0
-
-
 def fips_is_supported(cfg, pulp_host=None):
     """Return ``True`` if the server supports Fips, or ``False`` otherwise.
 

tests/test_config.py

@@ -417,9 +417,8 @@ def pulp_smash_config_load(config_str):
         from the configuration file.
     """
     with mock.patch.object(
-        builtins,
-        'open',
-        mock.mock_open(read_data=config_str),
-    ):
+            builtins,
+            'open',
+            mock.mock_open(read_data=config_str)):
         with mock.patch.object(config.PulpSmashConfig, 'get_load_path'):
             return config.PulpSmashConfig.load()

tests/test_utils.py

@@ -58,19 +58,23 @@ def test_all(self):
         self.assertEqual(checksums[0], checksums[2])
 
 
-class OsIsF26TestCase(unittest.TestCase):
-    """Test :func:`pulp_smash.utils.os_is_f26`."""
+class GetOsReleaseTestCase(unittest.TestCase):
+    """Test the ``get_os_release_*`` functions.
 
-    def test_returncode_zero(self):
-        """Assert true is returned if the CLI command returns zero."""
+    These tests are very simple: they just make sure that the string returned
+    by the used :class:`pulp_smash.cli.Client` object is stripped and returned.
+    """
+
+    def test_get_os_release_id(self):
+        """Test :func:`pulp_smash.utils.get_os_release_id`."""
         with mock.patch.object(cli, 'Client') as client:
-            client.return_value.run.return_value.returncode = 0
-            response = utils.os_is_f26(mock.Mock())
-        self.assertTrue(response)
+            client.return_value.run.return_value.stdout = ' fedora '
+            response = utils.get_os_release_id(mock.Mock())
+        self.assertEqual(response, 'fedora')
 
-    def test_returncode_nonzero(self):
-        """Assert false is returned if the CLI command returns non-zero."""
+    def test_get_os_release_version_id(self):
+        """Test :func:`pulp_smash.utils.get_os_release_version_id`."""
         with mock.patch.object(cli, 'Client') as client:
-            client.return_value.run.return_value.returncode = 1
-            response = utils.os_is_f26(mock.Mock())
-        self.assertFalse(response)
+            client.return_value.run.return_value.stdout = ' 27 '
+            response = utils.get_os_release_version_id(mock.Mock())
+        self.assertEqual(response, '27')