Merge pull request #4758 from rhcarvalho/bz1465882-package_version-ovs2.7-on-ocp3.6

Allow Open vSwitch 2.7 on OpenShift 3.5 and 3.6

4 files changed, 114 insertions, 85 deletions

diff --git a/roles/openshift_health_checker/library/aos_version.py b/roles/openshift_health_checker/library/aos_version.py
index 4c205e48c..4f43ee751 100755
--- a/roles/openshift_health_checker/library/aos_version.py
+++ b/roles/openshift_health_checker/library/aos_version.py
@@ -19,6 +19,10 @@ the inventory, the version comparison checks just pass.
 '''
 
 from ansible.module_utils.basic import AnsibleModule
+# NOTE: because of the dependency on yum (Python 2-only), this module does not
+# work under Python 3. But since we run unit tests against both Python 2 and
+# Python 3, we use six for cross compatibility in this module alone:
+from ansible.module_utils.six import string_types
 
 IMPORT_EXCEPTION = None
 try:
@@ -122,12 +126,15 @@ def _check_precise_version_found(pkgs, expected_pkgs_dict):
     for pkg in pkgs:
         if pkg.name not in expected_pkgs_dict:
             continue
-        # does the version match, to the precision requested?
-        # and, is it strictly greater, at the precision requested?
-        expected_pkg_version = expected_pkgs_dict[pkg.name]["version"]
-        match_version = '.'.join(pkg.version.split('.')[:expected_pkg_version.count('.') + 1])
-        if match_version == expected_pkg_version:
-            pkgs_precise_version_found.add(pkg.name)
+        expected_pkg_versions = expected_pkgs_dict[pkg.name]["version"]
+        if isinstance(expected_pkg_versions, string_types):
+            expected_pkg_versions = [expected_pkg_versions]
+        for expected_pkg_version in expected_pkg_versions:
+            # does the version match, to the precision requested?
+            # and, is it strictly greater, at the precision requested?
+            match_version = '.'.join(pkg.version.split('.')[:expected_pkg_version.count('.') + 1])
+            if match_version == expected_pkg_version:
+                pkgs_precise_version_found.add(pkg.name)
 
     not_found = []
     for name, pkg in expected_pkgs_dict.items():
@@ -157,8 +164,13 @@ def _check_higher_version_found(pkgs, expected_pkgs_dict):
     for pkg in pkgs:
         if pkg.name not in expected_pkg_names:
             continue
-        expected_pkg_version = expected_pkgs_dict[pkg.name]["version"]
-        req_release_arr = [int(segment) for segment in expected_pkg_version.split(".")]
+        expected_pkg_versions = expected_pkgs_dict[pkg.name]["version"]
+        if isinstance(expected_pkg_versions, string_types):
+            expected_pkg_versions = [expected_pkg_versions]
+        # NOTE: the list of versions is assumed to be sorted so that the highest
+        # desirable version is the last.
+        highest_desirable_version = expected_pkg_versions[-1]
+        req_release_arr = [int(segment) for segment in highest_desirable_version.split(".")]
         version = [int(segment) for segment in pkg.version.split(".")]
         too_high = version[:len(req_release_arr)] > req_release_arr
         higher_than_seen = version > higher_version_for_pkg.get(pkg.name, [])
diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py
index 6a76bb93d..204752bd0 100644
--- a/roles/openshift_health_checker/openshift_checks/package_version.py
+++ b/roles/openshift_health_checker/openshift_checks/package_version.py
@@ -10,8 +10,8 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck):
     tags = ["preflight"]
 
     openshift_to_ovs_version = {
-        "3.6": "2.6",
-        "3.5": "2.6",
+        "3.6": ["2.6", "2.7"],
+        "3.5": ["2.6", "2.7"],
         "3.4": "2.4",
     }
 
diff --git a/roles/openshift_health_checker/test/aos_version_test.py b/roles/openshift_health_checker/test/aos_version_test.py
index 697805dd2..4100f6c70 100644
--- a/roles/openshift_health_checker/test/aos_version_test.py
+++ b/roles/openshift_health_checker/test/aos_version_test.py
@@ -18,7 +18,43 @@ expected_pkgs = {
 }
 
 
-@pytest.mark.parametrize('pkgs, expect_not_found', [
+@pytest.mark.parametrize('pkgs,expected_pkgs_dict', [
+    (
+        # all found
+        [Package('spam', '3.2.1'), Package('eggs', '3.2.1')],
+        expected_pkgs,
+    ),
+    (
+        # found with more specific version
+        [Package('spam', '3.2.1'), Package('eggs', '3.2.1.5')],
+        expected_pkgs,
+    ),
+    (
+        [Package('ovs', '2.6'), Package('ovs', '2.4')],
+        {
+            "ovs": {
+                "name": "ovs",
+                "version": ["2.6", "2.7"],
+                "check_multi": False,
+            }
+        },
+    ),
+    (
+        [Package('ovs', '2.7')],
+        {
+            "ovs": {
+                "name": "ovs",
+                "version": ["2.6", "2.7"],
+                "check_multi": False,
+            }
+        },
+    ),
+])
+def test_check_precise_version_found(pkgs, expected_pkgs_dict):
+    aos_version._check_precise_version_found(pkgs, expected_pkgs_dict)
+
+
+@pytest.mark.parametrize('pkgs,expect_not_found', [
     (
         [],
         {
@@ -55,14 +91,6 @@ expected_pkgs = {
         },  # not the right version
     ),
     (
-        [Package('spam', '3.2.1'), Package('eggs', '3.2.1')],
-        {},  # all found
-    ),
-    (
-        [Package('spam', '3.2.1'), Package('eggs', '3.2.1.5')],
-        {},  # found with more specific version
-    ),
-    (
         [Package('eggs', '1.2.3'), Package('eggs', '3.2.1.5')],
         {
             "spam": {
@@ -73,64 +101,86 @@ expected_pkgs = {
         },  # eggs found with multiple versions
     ),
 ])
-def test_check_pkgs_for_precise_version(pkgs, expect_not_found):
-    if expect_not_found:
-        with pytest.raises(aos_version.PreciseVersionNotFound) as e:
-            aos_version._check_precise_version_found(pkgs, expected_pkgs)
-
-        assert list(expect_not_found.values()) == e.value.problem_pkgs
-    else:
+def test_check_precise_version_found_fail(pkgs, expect_not_found):
+    with pytest.raises(aos_version.PreciseVersionNotFound) as e:
         aos_version._check_precise_version_found(pkgs, expected_pkgs)
+    assert list(expect_not_found.values()) == e.value.problem_pkgs
 
 
-@pytest.mark.parametrize('pkgs, expect_higher', [
+@pytest.mark.parametrize('pkgs,expected_pkgs_dict', [
     (
         [],
-        [],
+        expected_pkgs,
     ),
     (
+        # more precise but not strictly higher
         [Package('spam', '3.2.1.9')],
-        [],  # more precise but not strictly higher
+        expected_pkgs,
     ),
     (
+        [Package('ovs', '2.7')],
+        {
+            "ovs": {
+                "name": "ovs",
+                "version": ["2.6", "2.7"],
+                "check_multi": False,
+            }
+        },
+    ),
+])
+def test_check_higher_version_found(pkgs, expected_pkgs_dict):
+    aos_version._check_higher_version_found(pkgs, expected_pkgs_dict)
+
+
+@pytest.mark.parametrize('pkgs,expected_pkgs_dict,expect_higher', [
+    (
         [Package('spam', '3.3')],
+        expected_pkgs,
         ['spam-3.3'],  # lower precision, but higher
     ),
     (
         [Package('spam', '3.2.1'), Package('eggs', '3.3.2')],
+        expected_pkgs,
         ['eggs-3.3.2'],  # one too high
     ),
     (
         [Package('eggs', '1.2.3'), Package('eggs', '3.2.1.5'), Package('eggs', '3.4')],
+        expected_pkgs,
         ['eggs-3.4'],  # multiple versions, one is higher
     ),
     (
         [Package('eggs', '3.2.1'), Package('eggs', '3.4'), Package('eggs', '3.3')],
+        expected_pkgs,
         ['eggs-3.4'],  # multiple versions, two are higher
     ),
+    (
+        [Package('ovs', '2.8')],
+        {
+            "ovs": {
+                "name": "ovs",
+                "version": ["2.6", "2.7"],
+                "check_multi": False,
+            }
+        },
+        ['ovs-2.8'],
+    ),
 ])
-def test_check_pkgs_for_greater_version(pkgs, expect_higher):
-    if expect_higher:
-        with pytest.raises(aos_version.FoundHigherVersion) as e:
-            aos_version._check_higher_version_found(pkgs, expected_pkgs)
-        assert set(expect_higher) == set(e.value.problem_pkgs)
-    else:
-        aos_version._check_higher_version_found(pkgs, expected_pkgs)
+def test_check_higher_version_found_fail(pkgs, expected_pkgs_dict, expect_higher):
+    with pytest.raises(aos_version.FoundHigherVersion) as e:
+        aos_version._check_higher_version_found(pkgs, expected_pkgs_dict)
+    assert set(expect_higher) == set(e.value.problem_pkgs)
 
 
-@pytest.mark.parametrize('pkgs, expect_to_flag_pkgs', [
-    (
-        [],
-        [],
-    ),
-    (
-        [Package('spam', '3.2.1')],
-        [],
-    ),
-    (
-        [Package('spam', '3.2.1'), Package('eggs', '3.2.2')],
-        [],
-    ),
+@pytest.mark.parametrize('pkgs', [
+    [],
+    [Package('spam', '3.2.1')],
+    [Package('spam', '3.2.1'), Package('eggs', '3.2.2')],
+])
+def test_check_multi_minor_release(pkgs):
+    aos_version._check_multi_minor_release(pkgs, expected_pkgs)
+
+
+@pytest.mark.parametrize('pkgs,expect_to_flag_pkgs', [
     (
         [Package('spam', '3.2.1'), Package('spam', '3.3.2')],
         ['spam'],
@@ -140,10 +190,7 @@ def test_check_pkgs_for_greater_version(pkgs, expect_higher):
         ['eggs'],
     ),
 ])
-def test_check_pkgs_for_multi_release(pkgs, expect_to_flag_pkgs):
-    if expect_to_flag_pkgs:
-        with pytest.raises(aos_version.FoundMultiRelease) as e:
-            aos_version._check_multi_minor_release(pkgs, expected_pkgs)
-        assert set(expect_to_flag_pkgs) == set(e.value.problem_pkgs)
-    else:
+def test_check_multi_minor_release_fail(pkgs, expect_to_flag_pkgs):
+    with pytest.raises(aos_version.FoundMultiRelease) as e:
         aos_version._check_multi_minor_release(pkgs, expected_pkgs)
+    assert set(expect_to_flag_pkgs) == set(e.value.problem_pkgs)
diff --git a/roles/openshift_health_checker/test/package_version_test.py b/roles/openshift_health_checker/test/package_version_test.py
index 91eace512..1bb6371ae 100644
--- a/roles/openshift_health_checker/test/package_version_test.py
+++ b/roles/openshift_health_checker/test/package_version_test.py
@@ -72,36 +72,6 @@ def test_package_version(openshift_release):
     assert result is return_value
 
 
-@pytest.mark.parametrize('deployment_type,openshift_release,expected_ovs_version', [
-    ("openshift-enterprise", "3.5", "2.6"),
-    ("origin", "3.6", "2.6"),
-    ("openshift-enterprise", "3.4", "2.4"),
-    ("origin", "3.3", "2.4"),
-])
-def test_ovs_package_version(deployment_type, openshift_release, expected_ovs_version):
-    task_vars = dict(
-        openshift=dict(common=dict(service_type='origin')),
-        openshift_release=openshift_release,
-        openshift_image_tag='v' + openshift_release,
-        openshift_deployment_type=deployment_type,
-    )
-    return_value = object()
-
-    def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
-        assert module_name == 'aos_version'
-        assert "package_list" in module_args
-
-        for pkg in module_args["package_list"]:
-            if pkg["name"] == "openvswitch":
-                assert pkg["version"] == expected_ovs_version
-
-        return return_value
-
-    check = PackageVersion(execute_module=execute_module)
-    result = check.run(tmp=None, task_vars=task_vars)
-    assert result is return_value
-
-
 @pytest.mark.parametrize('deployment_type,openshift_release,expected_docker_version', [
     ("origin", "3.5", "1.12"),
     ("openshift-enterprise", "3.4", "1.12"),