From 449b05495711a6fd7a89d6c1de0ee64124a1f734 Mon Sep 17 00:00:00 2001 From: Paul Breaux Date: Mon, 1 Aug 2016 23:57:10 -0600 Subject: [PATCH 1/2] During functional testing against 11.5.4, test/functional/tm/ltm/test_virtual.py test fails Issues: Fixes #589 Problem: A profile cannot be created or loaded on 11.5.4 under a virtual without specifying the partition. Analysis: Added a tmos version check in the __init__ of the Profiles resource under Virtual. If the tmos version is less than 11.6.0, the partition is required for creation and load. Tests: Virtual tests pass against 11.5.4 and 11.6.0 --- f5/bigip/tm/ltm/virtual.py | 10 ++++ test/functional/tm/ltm/test_virtual.py | 72 +++++++++++++++++++++----- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/f5/bigip/tm/ltm/virtual.py b/f5/bigip/tm/ltm/virtual.py index 290ccde35..3a028a1a4 100644 --- a/f5/bigip/tm/ltm/virtual.py +++ b/f5/bigip/tm/ltm/virtual.py @@ -30,6 +30,8 @@ from f5.bigip.resource import Collection from f5.bigip.resource import Resource +from distutils.version import LooseVersion + class Virtuals(Collection): """BIG-IP® LTM virtual collection""" @@ -55,6 +57,14 @@ class Profiles(Resource): def __init__(self, Profiles_s): '''Autogenerated constructor.''' super(Profiles, self).__init__(Profiles_s) + if LooseVersion(self._meta_data['bigip']._meta_data['tmos_version']) \ + < LooseVersion('11.6.0'): + self._meta_data['required_creation_parameters'].update( + ('partition',) + ) + self._meta_data['required_load_parameters'].update( + ('partition',) + ) self._meta_data['template_generated'] = True self._meta_data['required_json_kind'] =\ u"tm:ltm:virtual:profiles:profilesstate" diff --git a/test/functional/tm/ltm/test_virtual.py b/test/functional/tm/ltm/test_virtual.py index 90bdaf1e7..ca2d20548 100644 --- a/test/functional/tm/ltm/test_virtual.py +++ b/test/functional/tm/ltm/test_virtual.py @@ -13,6 +13,9 @@ # limitations under the License. # +from f5.bigip.resource import MissingRequiredCreationParameter +from f5.bigip.resource import MissingRequiredReadParameter + from distutils.version import LooseVersion from pprint import pprint as pp import pytest @@ -25,19 +28,23 @@ def delete_resource(resources): resource.delete() -def setup_virtual_test(request, bigip, partition, name): +def setup_virtual_test(request, mgmt_root, partition, name): def teardown(): delete_resource(vc1) request.addfinalizer(teardown) - vc1 = bigip.ltm.virtuals + vc1 = mgmt_root.tm.ltm.virtuals pp('****') virtual1 = vc1.virtual.create(name=name, partition=partition) return virtual1, vc1 class TestVirtual(object): - def test_virtual_create_refresh_update_delete_load(self, request, bigip): - virtual1, vc1 = setup_virtual_test(request, bigip, 'Common', 'vstest1') + def test_virtual_create_refresh_update_delete_load( + self, request, mgmt_root, setup_device_snapshot + ): + virtual1, vc1 = setup_virtual_test( + request, mgmt_root, 'Common', 'vstest1' + ) assert virtual1.name == 'vstest1' virtual1.description = TESTDESCRIPTION virtual1.update() @@ -49,19 +56,20 @@ def test_virtual_create_refresh_update_delete_load(self, request, bigip): assert virtual2.selfLink == virtual1.selfLink -@pytest.mark.skipif(LooseVersion(pytest.config.getoption('--release')) < - LooseVersion('11.6.0'), - reason='This test fails in 11.5.4. Will ' - 'revert this change in next PR.' - ) -def test_profiles_CE(bigip, opt_release): - v1 = bigip.ltm.virtuals.virtual.create(name="tv1", partition="Common") +@pytest.mark.skipif( + LooseVersion(pytest.config.getoption('--release')) + < LooseVersion('11.6.0'), + reason='Profiles do not need a partition in 11.6.0 and up.' +) +def test_profiles_CE_and_greater( + mgmt_root, opt_release, setup_device_snapshot +): + v1 = mgmt_root.tm.ltm.virtuals.virtual.create( + name="tv1", partition="Common" + ) p1 = v1.profiles_s.profiles.create(name="http") - pp(p1.raw) test_profiles_s = v1.profiles_s - pp(test_profiles_s.raw) test_profiles_s.context = 'all' - pp(test_profiles_s.raw) assert p1.selfLink ==\ u"https://localhost/mgmt/tm/ltm/virtual/"\ "~Common~tv1/profiles/http?ver="+opt_release @@ -70,3 +78,39 @@ def test_profiles_CE(bigip, opt_release): assert p2.exists(name='http') v1.delete() + + +@pytest.mark.skipif( + LooseVersion(pytest.config.getoption('--release')) + >= LooseVersion('11.6.0'), + reason='Profiles are created with a partition in 11.5.4.' +) +def test_profiles_CE_11_5_4_and_less( + mgmt_root, opt_release, setup_device_snapshot +): + v1 = mgmt_root.tm.ltm.virtuals.virtual.create( + name="tv2", partition="Common" + ) + # Ensure we cannot create a profile without partition in 11.5.4 + with pytest.raises(MissingRequiredCreationParameter) as ex: + v1.profiles_s.profiles.create(name="http") + assert "Missing required params: ['partition']" in ex.value.message + + # Create the profile with the partition given + p1 = v1.profiles_s.profiles.create(name="http", partition="Common") + test_profiles_s = v1.profiles_s + test_profiles_s.context = 'all' + assert p1.selfLink ==\ + u"https://localhost/mgmt/tm/ltm/virtual/"\ + "~Common~tv2/profiles/~Common~http?ver="+opt_release + + p2 = v1.profiles_s.profiles + # Ensure we cannot check for existence without partition in 11.5.4 + with pytest.raises(MissingRequiredReadParameter) as ex: + assert p2.exists(name='http') + assert "Missing required params: ['partition']" in ex.value.message + + # Check for existence with partition given + p2.exists(name='http', partition='Common') + + v1.delete() From ab98333cce19a60ed03949e6815c11b1aa61fb20 Mon Sep 17 00:00:00 2001 From: Paul Breaux Date: Tue, 2 Aug 2016 09:25:49 -0600 Subject: [PATCH 2/2] Address code review comments from @zancas. Requiring partition for any TMOS version for virtual profiles. --- f5/bigip/tm/ltm/virtual.py | 12 ++----- test/functional/tm/ltm/test_virtual.py | 44 +++++++++----------------- 2 files changed, 17 insertions(+), 39 deletions(-) diff --git a/f5/bigip/tm/ltm/virtual.py b/f5/bigip/tm/ltm/virtual.py index 3a028a1a4..3062cab60 100644 --- a/f5/bigip/tm/ltm/virtual.py +++ b/f5/bigip/tm/ltm/virtual.py @@ -30,8 +30,6 @@ from f5.bigip.resource import Collection from f5.bigip.resource import Resource -from distutils.version import LooseVersion - class Virtuals(Collection): """BIG-IP® LTM virtual collection""" @@ -57,14 +55,8 @@ class Profiles(Resource): def __init__(self, Profiles_s): '''Autogenerated constructor.''' super(Profiles, self).__init__(Profiles_s) - if LooseVersion(self._meta_data['bigip']._meta_data['tmos_version']) \ - < LooseVersion('11.6.0'): - self._meta_data['required_creation_parameters'].update( - ('partition',) - ) - self._meta_data['required_load_parameters'].update( - ('partition',) - ) + self._meta_data['required_creation_parameters'].update(('partition',)) + self._meta_data['required_load_parameters'].update(('partition',)) self._meta_data['template_generated'] = True self._meta_data['required_json_kind'] =\ u"tm:ltm:virtual:profiles:profilesstate" diff --git a/test/functional/tm/ltm/test_virtual.py b/test/functional/tm/ltm/test_virtual.py index ca2d20548..302b26af6 100644 --- a/test/functional/tm/ltm/test_virtual.py +++ b/test/functional/tm/ltm/test_virtual.py @@ -16,7 +16,6 @@ from f5.bigip.resource import MissingRequiredCreationParameter from f5.bigip.resource import MissingRequiredReadParameter -from distutils.version import LooseVersion from pprint import pprint as pp import pytest @@ -56,61 +55,48 @@ def test_virtual_create_refresh_update_delete_load( assert virtual2.selfLink == virtual1.selfLink -@pytest.mark.skipif( - LooseVersion(pytest.config.getoption('--release')) - < LooseVersion('11.6.0'), - reason='Profiles do not need a partition in 11.6.0 and up.' -) -def test_profiles_CE_and_greater( +def test_profiles_CE( mgmt_root, opt_release, setup_device_snapshot ): v1 = mgmt_root.tm.ltm.virtuals.virtual.create( name="tv1", partition="Common" ) - p1 = v1.profiles_s.profiles.create(name="http") + p1 = v1.profiles_s.profiles.create(name="http", partition='Common') test_profiles_s = v1.profiles_s test_profiles_s.context = 'all' assert p1.selfLink ==\ u"https://localhost/mgmt/tm/ltm/virtual/"\ - "~Common~tv1/profiles/http?ver="+opt_release + "~Common~tv1/profiles/~Common~http?ver="+opt_release p2 = v1.profiles_s.profiles - assert p2.exists(name='http') + assert p2.exists(name='http', partition='Common') v1.delete() -@pytest.mark.skipif( - LooseVersion(pytest.config.getoption('--release')) - >= LooseVersion('11.6.0'), - reason='Profiles are created with a partition in 11.5.4.' -) -def test_profiles_CE_11_5_4_and_less( - mgmt_root, opt_release, setup_device_snapshot -): +def test_profiles_CE_check_create_params(mgmt_root, setup_device_snapshot): v1 = mgmt_root.tm.ltm.virtuals.virtual.create( name="tv2", partition="Common" ) - # Ensure we cannot create a profile without partition in 11.5.4 with pytest.raises(MissingRequiredCreationParameter) as ex: v1.profiles_s.profiles.create(name="http") assert "Missing required params: ['partition']" in ex.value.message + v1.delete() - # Create the profile with the partition given + +def test_profiles_CE_check_load_params(mgmt_root, setup_device_snapshot): + v1 = mgmt_root.tm.ltm.virtuals.virtual.create( + name="tv3", partition="Common" + ) p1 = v1.profiles_s.profiles.create(name="http", partition="Common") - test_profiles_s = v1.profiles_s - test_profiles_s.context = 'all' - assert p1.selfLink ==\ - u"https://localhost/mgmt/tm/ltm/virtual/"\ - "~Common~tv2/profiles/~Common~http?ver="+opt_release - p2 = v1.profiles_s.profiles - # Ensure we cannot check for existence without partition in 11.5.4 with pytest.raises(MissingRequiredReadParameter) as ex: - assert p2.exists(name='http') + assert v1.profiles_s.profiles.load(name='http') assert "Missing required params: ['partition']" in ex.value.message + v1.profiles_s.profiles.load(name="http", partition="Common") + # Check for existence with partition given - p2.exists(name='http', partition='Common') + p1.exists(name='http', partition='Common') v1.delete()