From 69db9a91674a8b9485e7b1dcf47a90f712ef00d5 Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Wed, 8 Apr 2020 15:39:44 -0700 Subject: [PATCH] Refactor keepalive to execute node health check Before this change, keepalive probe consisted of two separate configurable roundrip - one based on keepalive_section attribute, and one on validation_method. The recommended way to probe NSX appliance is using node/health API, and tests show that it has best roundtrip time. This nsxlib will switch to this healthcheck, and not expose keepalive methology to clients any longer. Change-Id: Ia972ef3d087fd01fa18d5a4e9dc9c32fbed0eb40 --- vmware_nsxlib/tests/unit/v3/test_cluster.py | 23 ++------------ vmware_nsxlib/v3/__init__.py | 4 --- vmware_nsxlib/v3/cluster.py | 34 ++++++--------------- vmware_nsxlib/v3/config.py | 14 +++++---- vmware_nsxlib/v3/lib.py | 13 +------- vmware_nsxlib/v3/policy/__init__.py | 20 ------------ 6 files changed, 20 insertions(+), 88 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/test_cluster.py b/vmware_nsxlib/tests/unit/v3/test_cluster.py index 22b27b1a..5647137f 100644 --- a/vmware_nsxlib/tests/unit/v3/test_cluster.py +++ b/vmware_nsxlib/tests/unit/v3/test_cluster.py @@ -171,7 +171,6 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_cluster = mock.Mock() mock_cluster.nsxlib_config = mock.Mock() mock_cluster.nsxlib_config.url_base = 'abc' - mock_cluster.nsxlib_config.keepalive_section = 'transport-zones' provider = cluster.NSXRequestsHTTPProvider() with mock.patch.object(client.JSONRESTClient, "get", @@ -181,26 +180,10 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_cluster, mock_ep, mock_conn) with mock.patch.object(client.JSONRESTClient, "get", - return_value={'result_count': 1}): + return_value={'healthy': True}): provider.validate_connection(mock_cluster, mock_ep, mock_conn) - def test_validate_connection_no_keep_alive(self): - mock_conn = mocks.MockRequestSessionApi() - mock_conn.default_headers = {} - mock_ep = mock.Mock() - mock_ep.provider.url = 'https://1.2.3.4' - mock_cluster = mock.Mock() - mock_cluster.nsxlib_config = mock.Mock() - mock_cluster.nsxlib_config.url_base = 'abc' - mock_cluster.nsxlib_config.keepalive_section = None - provider = cluster.NSXRequestsHTTPProvider() - - with mock.patch.object(client.JSONRESTClient, "get") as mock_get: - provider.validate_connection(mock_cluster, mock_ep, mock_conn) - mock_get.assert_not_called() - - def _validate_con_mocks(self, nsx_version, - keepalive_section='transport-zones'): + def _validate_con_mocks(self, nsx_version): nsxlib_config = nsxlib_testcase.get_default_nsxlib_config() nsxlib = v3.NsxLib(nsxlib_config) nsxlib.nsx_version = nsx_version @@ -210,8 +193,6 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_ep.provider.url = 'https://1.2.3.4' conf = mock.Mock() conf.url_base = 'abc' - conf.keepalive_section = keepalive_section - conf.validate_connection_method = nsxlib.validate_connection_method mock_cluster = mock.Mock() mock_cluster.nsxlib_config = conf return (mock_cluster, mock_ep, mock_conn) diff --git a/vmware_nsxlib/v3/__init__.py b/vmware_nsxlib/v3/__init__.py index f55c0f16..de65dd73 100644 --- a/vmware_nsxlib/v3/__init__.py +++ b/vmware_nsxlib/v3/__init__.py @@ -118,10 +118,6 @@ class NsxLib(lib.NsxLibBase): self.tag_limits = self.get_tag_limits() utils.update_tag_limits(self.tag_limits) - @property - def keepalive_section(self): - return 'transport-zones' - @property def validate_connection_method(self): """Return a method that will validate the NSX manager status""" diff --git a/vmware_nsxlib/v3/cluster.py b/vmware_nsxlib/v3/cluster.py index 43228f57..39d09d24 100644 --- a/vmware_nsxlib/v3/cluster.py +++ b/vmware_nsxlib/v3/cluster.py @@ -67,11 +67,6 @@ class AbstractHTTPProvider(object): """A unique string name for this provider.""" pass - @abc.abstractmethod - def validate_connection(self, cluster_api, endpoint, conn): - """Validate the said connection for the given endpoint and cluster.""" - pass - @abc.abstractmethod def new_connection(self, cluster_api, provider): """Create a new http connection. @@ -191,29 +186,18 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): # nsxlib 'retries' and 'http_timeout' parameters. client = nsx_client.NSX3Client( conn, url_prefix=endpoint.provider.url, - url_path_base=cluster_api.nsxlib_config.url_base, + url_path_base=nsx_client.NSX3Client.NSX_V1_API_PREFIX, default_headers=conn.default_headers, max_attempts=1) - if cluster_api.nsxlib_config.validate_connection_method: - cluster_api.nsxlib_config.validate_connection_method( - client, endpoint.provider.url) - - # If keeplive section returns a list, it is assumed to be non-empty - keepalive_section = cluster_api.nsxlib_config.keepalive_section - # When validate connection also has the effect of keep-alive, - # keepalive_section can be disabled by passing in an empty value - if keepalive_section: - result = client.get(keepalive_section, - silent=True, - with_retries=False) - if not result or result.get('result_count', 1) <= 0: - msg = _("No %(section)s found " - "for '%(url)s'") % {'section': keepalive_section, - 'url': endpoint.provider.url} - LOG.warning(msg) - raise exceptions.ResourceNotFound( - manager=endpoint.provider.url, operation=msg) + # Try to get the status silently and with no retries + status = client.get('reverse-proxy/node/health', + silent=True, with_retries=False) + if not status or not status.get('healthy', False): + msg = _("NSX Node is not healthy, reported status: %s") % status + LOG.warning(msg) + raise exceptions.ResourceNotFound( + manager=endpoint.provider.url, operation=msg) def new_connection(self, cluster_api, provider): config = cluster_api.nsxlib_config diff --git a/vmware_nsxlib/v3/config.py b/vmware_nsxlib/v3/config.py index c6ba896e..7fe9969d 100644 --- a/vmware_nsxlib/v3/config.py +++ b/vmware_nsxlib/v3/config.py @@ -157,9 +157,9 @@ class NsxLibConfig(object): self.realization_wait_sec = realization_wait_sec if len(nsx_api_managers) == 1 and not self.cluster_unavailable_retry: - LOG.warning("When only one endpoint is provided, keepalive probes " - " are disabled. For the system to be able to recover " - " from DOWN state, cluster_unavailable_retry is set " + LOG.warning("When only one endpoint is provided, keepalive probes" + " are disabled. For the system to be able to recover" + " from DOWN state, cluster_unavailable_retry is set" " to True, overriding provided configuration") self.cluster_unavailable_retry = True @@ -172,9 +172,11 @@ class NsxLibConfig(object): def extend(self, keepalive_section, validate_connection_method=None, url_base=None): - """Called by library code to initialize application-specific data""" - self.keepalive_section = keepalive_section - self.validate_connection_method = validate_connection_method + if keepalive_section or validate_connection_method: + LOG.warning("keepalive_section and validate_connection_method are" + " no longer used to conduct keepalive probes. For" + " most efficient keepalive roundtrip, proxy health" + " API is always used.") self.url_base = url_base def _attribute_by_index(self, scalar_or_list, index): diff --git a/vmware_nsxlib/v3/lib.py b/vmware_nsxlib/v3/lib.py index 033e28c4..5deae540 100644 --- a/vmware_nsxlib/v3/lib.py +++ b/vmware_nsxlib/v3/lib.py @@ -61,10 +61,7 @@ class NsxLibBase(object): def set_config(self, nsxlib_config): """Set config user provided and extend it according to application""" self.nsxlib_config = nsxlib_config - self.nsxlib_config.extend( - keepalive_section=self.keepalive_section, - validate_connection_method=self.validate_connection_method, - url_base=self.client_url_prefix) + self.nsxlib_config.extend(None, url_base=self.client_url_prefix) def set_default_headers(self, nsxlib_config): """Set the default headers with token information""" @@ -82,14 +79,6 @@ class NsxLibBase(object): def client_url_prefix(self): pass - @abc.abstractproperty - def keepalive_section(self): - pass - - @abc.abstractproperty - def validate_connection_method(self): - pass - @abc.abstractmethod def init_api(self): pass diff --git a/vmware_nsxlib/v3/policy/__init__.py b/vmware_nsxlib/v3/policy/__init__.py index 85db4ff0..d4ab9544 100644 --- a/vmware_nsxlib/v3/policy/__init__.py +++ b/vmware_nsxlib/v3/policy/__init__.py @@ -132,26 +132,6 @@ class NsxPolicyLib(lib.NsxLibBase): self.ipsec_vpn = ipsec_vpn_resources.NsxPolicyIpsecVpnApi(*args) self.global_config = core_resources.NsxPolicyGlobalConfig(*args) - @property - def keepalive_section(self): - return 'infra' - - @property - def validate_connection_method(self): - """Return a method that will validate the NSX policy status""" - - def check_policy_status(client, url): - # Try to get the status silently and with no retries - infra = client.get('infra', - silent=True, with_retries=False) - if not infra or not infra.get('id'): - msg = _("Policy health check failed") - LOG.warning(msg) - raise exceptions.ResourceNotFound( - manager=url, operation=msg) - - return check_policy_status - def get_version(self): """Get the NSX Policy manager version