diff --git a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py index 677d66c9..100205eb 100644 --- a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py +++ b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py @@ -87,6 +87,13 @@ class NsxPolicyLibTestCase(policy_testcase.TestPolicyApi): actual_dict = mock_api.call_args_list[call_num][0][0].body self.assertEqual(expected_dict, actual_dict) + def assert_called_with_def_and_kwargs(self, mock_api, expected_def, + call_num=0, **kwargs): + # verify the api & resource definition + self.assert_called_with_def(mock_api, expected_def, call_num=call_num) + actual_kwargs = mock_api.call_args_list[call_num][1] + self.assertDictEqual(actual_kwargs, kwargs) + def mock_get(self, obj_id, obj_name, **kwargs): obj_dict = { 'id': obj_id, @@ -3685,7 +3692,7 @@ class TestPolicyTier0Bgp(NsxPolicyLibTestCase): self.assert_called_with_def(api_call, expected_def) self.assertIsNone(result) - def test_update(self): + def test_update_patch(self): name = 'test' description = 'bgp' tier0_id = 't0' @@ -3740,6 +3747,66 @@ class TestPolicyTier0Bgp(NsxPolicyLibTestCase): self.assert_called_with_def(api_call, expected_def) self.assertIsNone(result) + def test_update_put(self): + name = 'test' + description = 'bgp' + tier0_id = 't0' + service_id = "default", + ecmp = True, + enabled = True, + graceful_restart_config = { + "mode": "DISABLE", + "timer": { + "restart_timer": 180, + "stale_route_timer": 600 + } + } + inter_sr_ibgp = False, + local_as_num = "65546", + multipath_relax = False, + route_aggregations = [{ + "prefix": "10.1.1.0/24"}, { + "prefix": "11.1.0.0/16", "summary_only": "false"}] + tags = [{"tag": "tag", "scope": "scope"}] + tenant = TEST_TENANT + with mock.patch.object(self.policy_api, "get", + return_value={}),\ + mock.patch.object(self.policy_api, + "update_with_put") as api_call: + result = self.resourceApi.update( + tier0_id, service_id, + name=name, + description=description, + ecmp=ecmp, + enabled=enabled, + graceful_restart_config=graceful_restart_config, + inter_sr_ibgp=inter_sr_ibgp, + local_as_num=local_as_num, + multipath_relax=multipath_relax, + route_aggregations=route_aggregations, + tags=tags, + tenant=tenant, + put=True, + revision=1) + expected_def = core_defs.BgpRoutingConfigDef( + tier0_id=tier0_id, + service_id=service_id, + name=name, + description=description, + ecmp=ecmp, + enabled=enabled, + graceful_restart_config=graceful_restart_config, + inter_sr_ibgp=inter_sr_ibgp, + local_as_num=local_as_num, + multipath_relax=multipath_relax, + route_aggregations=route_aggregations, + tags=tags, + tenant=tenant + ) + self.assert_called_with_def_and_kwargs( + api_call, expected_def, revision=1) + self.assertIsNotNone(result) + def test_get(self): tier0_id = 't0' service_id = 'default' diff --git a/vmware_nsxlib/v3/policy/core_defs.py b/vmware_nsxlib/v3/policy/core_defs.py index 505ec331..32a193a8 100644 --- a/vmware_nsxlib/v3/policy/core_defs.py +++ b/vmware_nsxlib/v3/policy/core_defs.py @@ -2485,6 +2485,21 @@ class NsxPolicyApi(object): def partial_updates_supported(self): return self.partial_updates + def update_with_put(self, resource_def, revision=None): + """Create or update a policy object with PUT + + During concurrent read-updates, using PUT can be safer as it requires + revision number in the object. Policy API rejects the request if + revision number is not latest + """ + path = resource_def.get_resource_path() + if resource_def.resource_use_cache(): + self.cache.remove(path) + body = resource_def.get_obj_dict() + if revision is not None: + body.update({"_revision": revision}) + return self.client.update(path, body) + def create_or_update(self, resource_def, partial_updates=False, force=False): """Create or update a policy object. diff --git a/vmware_nsxlib/v3/policy/core_resources.py b/vmware_nsxlib/v3/policy/core_resources.py index c95e144b..2a7c90e9 100644 --- a/vmware_nsxlib/v3/policy/core_resources.py +++ b/vmware_nsxlib/v3/policy/core_resources.py @@ -141,14 +141,19 @@ class NsxPolicyResourceBase(object, metaclass=abc.ABCMeta): return resource_def - def _update(self, allow_partial_updates=True, force=False, **kwargs): + def _update(self, allow_partial_updates=True, + force=False, put=False, revision=None, **kwargs): """Helper for update function - ignore attrs without explicit value""" + # DO NOT retry if caller specifies revision + max_attempts = (self.policy_api.client.max_attempts + if revision is None else 0) + @utils.retry_upon_exception( exceptions.StaleRevision, - max_attempts=self.policy_api.client.max_attempts) + max_attempts=max_attempts) def _do_update_with_retry(): if (allow_partial_updates and - self.policy_api.partial_updates_supported()): + self.policy_api.partial_updates_supported() and not put): policy_def = self._init_def(**kwargs) partial_updates = True else: @@ -158,8 +163,12 @@ class NsxPolicyResourceBase(object, metaclass=abc.ABCMeta): if policy_def.bodyless(): # Nothing to update - only keys provided in kwargs return - self.policy_api.create_or_update( - policy_def, partial_updates=partial_updates, force=force) + if put: + return self.policy_api.update_with_put( + policy_def, revision=revision) + else: + self.policy_api.create_or_update( + policy_def, partial_updates=partial_updates, force=force) return _do_update_with_retry() @@ -1774,20 +1783,24 @@ class NsxPolicyTier0BgpApi(NsxPolicyResourceBase): multipath_relax=IGNORE, route_aggregations=IGNORE, tags=IGNORE, - tenant=constants.POLICY_INFRA_TENANT): - self._update(name=name, - description=description, - tier0_id=tier0_id, - service_id=service_id, - ecmp=ecmp, - enabled=enabled, - graceful_restart_config=graceful_restart_config, - inter_sr_ibgp=inter_sr_ibgp, - local_as_num=local_as_num, - multipath_relax=multipath_relax, - route_aggregations=route_aggregations, - tags=tags, - tenant=tenant) + tenant=constants.POLICY_INFRA_TENANT, + put=False, + revision=None): + return self._update(name=name, + description=description, + tier0_id=tier0_id, + service_id=service_id, + ecmp=ecmp, + enabled=enabled, + graceful_restart_config=graceful_restart_config, + inter_sr_ibgp=inter_sr_ibgp, + local_as_num=local_as_num, + multipath_relax=multipath_relax, + route_aggregations=route_aggregations, + tags=tags, + tenant=tenant, + put=put, + revision=revision) class NsxPolicyTier0NatRuleApi(NsxPolicyResourceBase):