From 6778c62c6f9bfb7f6dc0123f268eb3f4e960b847 Mon Sep 17 00:00:00 2001 From: Tommy Carpenter Date: Wed, 19 Jun 2019 14:54:53 +0000 Subject: Move to k8s service names, exceptions Issue-ID: DCAEGEN2-1537 Change-Id: Ibb7bd0233b5de9eb853f3516ba0907e148eae21d Signed-off-by: Tommy Carpenter --- onap-dcae-cbs-docker-client/Changelog.md | 6 ++ onap-dcae-cbs-docker-client/MANIFEST.in | 1 - onap-dcae-cbs-docker-client/README.md | 38 +++++++--- .../onap_dcae_cbs_docker_client/__init__.py | 16 +++- .../onap_dcae_cbs_docker_client/client.py | 88 ++++++++-------------- .../onap_dcae_cbs_docker_client/exceptions.py | 45 +++++++++++ onap-dcae-cbs-docker-client/pom.xml | 2 +- onap-dcae-cbs-docker-client/setup.py | 8 +- onap-dcae-cbs-docker-client/tests/conftest.py | 83 ++++++++++++++++++++ onap-dcae-cbs-docker-client/tests/test_client.py | 71 ++++++++--------- onap-dcae-cbs-docker-client/tox-local.ini | 22 ------ onap-dcae-cbs-docker-client/tox.ini | 3 +- 12 files changed, 248 insertions(+), 135 deletions(-) delete mode 100644 onap-dcae-cbs-docker-client/MANIFEST.in create mode 100644 onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/exceptions.py create mode 100644 onap-dcae-cbs-docker-client/tests/conftest.py delete mode 100644 onap-dcae-cbs-docker-client/tox-local.ini (limited to 'onap-dcae-cbs-docker-client') diff --git a/onap-dcae-cbs-docker-client/Changelog.md b/onap-dcae-cbs-docker-client/Changelog.md index 354bc2a..d108e85 100644 --- a/onap-dcae-cbs-docker-client/Changelog.md +++ b/onap-dcae-cbs-docker-client/Changelog.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [2.0.0] - 6/19/2019 +* The env variable CONFIG_BINDING_SERVICE now has a different meaning per DCAEGEN2-1537. Specifically this variable now holds a resolvable hostname for the CBS, rather than a consul lookup key +* Since the API was broken anyway, the decision not to throw an exception was revisted and overturned. This was causing problems for some users, who were getting `{}` back in their configuration, but without knowing why; either the config wasn't set up, the config was set but as `{}`, or the CBS being unreachable altogether. This client library now throws native python exceptions, rather than logging and returning `{}`. The application client code can handle the exceptions, and retry if they choose. +* Add more tests, move fixtures to conftest +* Move exceptions to own module + ## [1.0.4] * Allow the CBS to be registered in Consul under a different name than "config_binding_service"; instead read from the already-set ENV variable CONFIG_BINDING_SERVICE diff --git a/onap-dcae-cbs-docker-client/MANIFEST.in b/onap-dcae-cbs-docker-client/MANIFEST.in deleted file mode 100644 index f9bd145..0000000 --- a/onap-dcae-cbs-docker-client/MANIFEST.in +++ /dev/null @@ -1 +0,0 @@ -include requirements.txt diff --git a/onap-dcae-cbs-docker-client/README.md b/onap-dcae-cbs-docker-client/README.md index 8466e50..5c5c986 100644 --- a/onap-dcae-cbs-docker-client/README.md +++ b/onap-dcae-cbs-docker-client/README.md @@ -1,21 +1,30 @@ # Python CBS Docker Client -Used for DCAE Dockerized microservices written in Python. Pulls your configuration from the config_binding_service. Expects that CONSUL_HOST, HOSTNAME, CONFIG_BINDING_SERVICE are set as env variables, which is true in DCAE. +Used for DCAE Dockerized microservices written in Python. Pulls your configuration from the Config Binding Service # Client Usage -## Development outside of Docker -To test your raw code without Docker, you will need to set the env variables CONSUL_HOST and HOSTNAME (name of your key to pull from) that are set in DCAEs Docker environment. -1. `CONSUL_HOST` is the hostname only of the Consul instance you are talking to -2. `HOSTNAME` is the name of your component in Consul -3. `CONFIG_BINDING_SERVICE` is the name under which the CBS is registered in Consul +The environment that this client runs in, whether it be in Docker or "natievely", needs to have the following env variables: +1. `HOSTNAME` is the name of your component in Consul +2. `CONFIG_BINDING_SERVICE` a resolvable hostname to the CBS ## Usage in your code -``` ->>> from onap_dcae_cbs_docker_client import client ->>> client.get_config() ->>> client.get_all() -``` + + >>> from onap_dcae_cbs_docker_client import client + >>> client.get_config() + >>> client.get_all() + + +If the CBS is reachable, but your configuration key is not there, you will get a CantGetConfig exception: + + onap_dcae_cbs_docker_client.exceptions.CantGetConfig + +You can access the original HTTP status code and text via the `code` and `text` attributes. + +If the CBS is unreachable, you will get an exception: + + onap_dcae_cbs_docker_client.exceptions.CBSUnreachable + # Installation @@ -28,3 +37,10 @@ pip install onap-dcae-cbs-docker-client ``` tox ``` + +# Version Changes +When changes are made, the versions to be bumped are in: + +1. setup.py +2. Changelog.md +3. pom.xml diff --git a/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/__init__.py b/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/__init__.py index e9d0246..ce9debe 100644 --- a/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/__init__.py +++ b/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/__init__.py @@ -1,5 +1,5 @@ # ================================================================================ -# Copyright (c) 2017-2018 AT&T Intellectual Property. All rights reserved. +# Copyright (c) 2017-2019 AT&T Intellectual Property. All rights reserved. # ================================================================================ # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -15,3 +15,17 @@ # ============LICENSE_END========================================================= # # ECOMP is a trademark and service mark of AT&T Intellectual Property. +import logging + + +def get_module_logger(mod_name): + """ + To use this, do logger = get_module_logger(__name__) + """ + logger = logging.getLogger(mod_name) + handler = logging.StreamHandler() + formatter = logging.Formatter("%(asctime)s [%(name)-12s] %(levelname)-8s %(message)s") + handler.setFormatter(formatter) + logger.addHandler(handler) + logger.setLevel(logging.DEBUG) + return logger diff --git a/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/client.py b/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/client.py index 80c9f75..ef0dfbc 100644 --- a/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/client.py +++ b/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/client.py @@ -1,5 +1,5 @@ # ================================================================================ -# Copyright (c) 2017-2018 AT&T Intellectual Property. All rights reserved. +# Copyright (c) 2017-2019 AT&T Intellectual Property. All rights reserved. # ================================================================================ # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -18,82 +18,53 @@ import json import os -import logging import requests +from onap_dcae_cbs_docker_client import get_module_logger +from onap_dcae_cbs_docker_client.exceptions import ENVsMissing, CantGetConfig, CBSUnreachable -LOGGER = logging.getLogger().getChild(__name__) - -class ENVsMissing(Exception): - """ - Exception to represent critical ENVs are missing - """ - pass +logger = get_module_logger(__name__) ######### # HELPERS - - -def _get_uri_from_consul(consul_url, name): - """ - Call consul's catalog - TODO: currently assumes there is only one service with this hostname - """ - url = "{0}/v1/catalog/service/{1}".format(consul_url, name) - LOGGER.debug("Trying to lookup service: {0}".format(url)) - res = requests.get(url) - res.raise_for_status() - services = res.json() - return "http://{0}:{1}".format(services[0]["ServiceAddress"], services[0]["ServicePort"]) - - -def _get_envs(): +def _get_path(path): """ - Returns hostname, consul_host. - If the necessary ENVs are not found, this is fatal, and raises an exception. + Try to get the config, and return appropriate exceptions otherwise """ try: hostname = os.environ["HOSTNAME"] # this is the name of the component itself - consul_host = os.environ["CONSUL_HOST"] # this is the host of consul itself - cbs_name = os.environ["CONFIG_BINDING_SERVICE"] # this is the name under which the CBS is registered in Consul. + # in most cases, this is the K8s service name which is a resolvable DNS name + # if running outside k8s, this name needs to be resolvable by DNS via other means. + cbs_resolvable_hostname = os.environ["CONFIG_BINDING_SERVICE"] except KeyError as e: raise ENVsMissing("Required ENV Variable {0} missing".format(e)) - return hostname, consul_host, cbs_name + # TODO: https + cbs_url = "http://{0}:10000".format(cbs_resolvable_hostname) -def _get_path(path): - """ - This call does not raise an exception if Consul or the CBS cannot complete the request. - It logs an error and returns {} if the config is not bindable. - It could be a temporary network outage. Call me again later. - - It will raise an exception if the necessary env parameters were not set because that is irrecoverable. - This function is called in my /heatlhcheck, so this will be caught early. - """ - - config = {} - - hostname, consul_host, cbs_name = _get_envs() - - # not sure how I as the component developer is supposed to know consul port - consul_url = "http://{0}:8500".format(consul_host) - + # get my config try: - # get my config - cbs_url = _get_uri_from_consul(consul_url, cbs_name) my_config_endpoint = "{0}/{1}/{2}".format(cbs_url, path, hostname) res = requests.get(my_config_endpoint) - res.raise_for_status() config = res.json() - LOGGER.info("get_config returned the following configuration: {0}".format( - json.dumps(config))) - except requests.exceptions.HTTPError: - LOGGER.error("in get_config, the config binding service endpoint %s was not reachable. Error code: %d, Error text: %s", my_config_endpoint, res.status_code, res.text) - except Exception as exc: - LOGGER.exception(exc) - return config + logger.debug( + "get_config returned the following configuration: %s using the config url %s", + json.dumps(config), + my_config_endpoint, + ) + return config + except requests.exceptions.HTTPError: # this is thrown by raise_for_status + logger.error( + "The config binding service endpoint %s returned a bad status. code: %d, text: %s", + my_config_endpoint, + res.status_code, + res.text, + ) + raise CantGetConfig(res.status_code, res.text) + except requests.exceptions.ConnectionError: # this is thrown if requests.get cant even connect to the endpoint + raise CBSUnreachable() ######### @@ -108,5 +79,8 @@ def get_all(): def get_config(): """ Hit the CBS service_component endpoint + + TODO: should we take in a "retry" boolean, and retry on behalf of the caller? + Currently, we return an exception and let the application decide how it wants to proceed (Crash, try again, etc). """ return _get_path("service_component") diff --git a/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/exceptions.py b/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/exceptions.py new file mode 100644 index 0000000..6b73b91 --- /dev/null +++ b/onap-dcae-cbs-docker-client/onap_dcae_cbs_docker_client/exceptions.py @@ -0,0 +1,45 @@ +# ================================================================================ +# Copyright (c) 2019 AT&T Intellectual Property. All rights reserved. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ============LICENSE_END========================================================= +# +# ECOMP is a trademark and service mark of AT&T Intellectual Property. + + +class ENVsMissing(Exception): + """ + Exception to represent critical ENVs are missing + """ + + pass + + +class CantGetConfig(Exception): + """ + Configuration could not be fetched likely due to the config not being in Consul + """ + + def __init__(self, code, text): + self.code = code + self.text = text + + pass + + +class CBSUnreachable(Exception): + """ + CBS was not reachable at all + """ + + pass diff --git a/onap-dcae-cbs-docker-client/pom.xml b/onap-dcae-cbs-docker-client/pom.xml index 0a2c763..f76b872 100644 --- a/onap-dcae-cbs-docker-client/pom.xml +++ b/onap-dcae-cbs-docker-client/pom.xml @@ -28,7 +28,7 @@ ECOMP is a trademark and service mark of AT&T Intellectual Property. org.onap.dcaegen2.utils onap-dcae-cbs-docker-client dcaegen2-utils-python-cbs-docker-client - 1.0.4-SNAPSHOT + 2.0.0-SNAPSHOT http://maven.apache.org diff --git a/onap-dcae-cbs-docker-client/setup.py b/onap-dcae-cbs-docker-client/setup.py index 3d4467c..2f04827 100644 --- a/onap-dcae-cbs-docker-client/setup.py +++ b/onap-dcae-cbs-docker-client/setup.py @@ -19,14 +19,12 @@ from setuptools import setup, find_packages setup( name="onap_dcae_cbs_docker_client", description="very lightweight client for a DCAE dockerized component to get it's config from the CBS", - version="1.0.4", + version="2.0.0", packages=find_packages(), author="Tommy Carpenter", author_email="tommy@research.att.com", - license='Apache 2', + license="Apache 2", keywords="", url="", - install_requires=[ - "requests>= 2.0.0, < 3.0.0" - ] + install_requires=["requests>= 2.0.0, < 3.0.0"], ) diff --git a/onap-dcae-cbs-docker-client/tests/conftest.py b/onap-dcae-cbs-docker-client/tests/conftest.py new file mode 100644 index 0000000..05d2eb5 --- /dev/null +++ b/onap-dcae-cbs-docker-client/tests/conftest.py @@ -0,0 +1,83 @@ +# ================================================================================ +# Copyright (c) 2019 AT&T Intellectual Property. All rights reserved. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ============LICENSE_END========================================================= +# +# ECOMP is a trademark and service mark of AT&T Intellectual Property. +import pytest +from requests.exceptions import HTTPError, ConnectionError + + +class FakeResponse: + def __init__(self, status_code, thejson): + self.status_code = status_code + self.thejson = thejson + self.text = "" + + def raise_for_status(self): + if self.status_code > 299: + raise HTTPError + + def json(self): + return self.thejson + + +@pytest.fixture +def monkeyed_requests_get(): + """ + mock for the CBS get + """ + + def _monkeyed_requests_get(url): + if url == "http://config_binding_service:10000/service_component_all/mybestfrienddotcom": + return FakeResponse( + status_code=200, + thejson={ + "config": {"key_to_your_heart": 666}, + "dti": {"some amazing": "dti stuff"}, + "policies": {"event": {"foo": "bar"}, "items": [{"foo2": "bar2"}]}, + "otherkey": {"foo3": "bar3"}, + }, + ) + + elif url == "http://config_binding_service:10000/service_component/mybestfrienddotcom": + return FakeResponse(status_code=200, thejson={"key_to_your_heart": 666}) + else: + raise Exception("Unexpected URL {0}!".format(url)) + + return _monkeyed_requests_get + + +@pytest.fixture +def monkeyed_requests_get_404(): + def _monkeyed_requests_get_404(url): + """ + get that pretends that key doesnt exist + """ + if url in [ + "http://config_binding_service:10000/service_component_all/mybestfrienddotcom", + "http://config_binding_service:10000/service_component/mybestfrienddotcom", + ]: + return FakeResponse(status_code=404, thejson={}) + raise Exception("Unexpected URL {0}!".format(url)) + + return _monkeyed_requests_get_404 + + +@pytest.fixture +def monkeyed_requests_get_unreachable(): + def _monkeyed_requests_get_unreachable(_url): + raise ConnectionError() + + return _monkeyed_requests_get_unreachable diff --git a/onap-dcae-cbs-docker-client/tests/test_client.py b/onap-dcae-cbs-docker-client/tests/test_client.py index c87f573..b1589c2 100644 --- a/onap-dcae-cbs-docker-client/tests/test_client.py +++ b/onap-dcae-cbs-docker-client/tests/test_client.py @@ -13,50 +13,51 @@ # See the License for the specific language governing permissions and # limitations under the License. # ============LICENSE_END========================================================= - +import pytest from onap_dcae_cbs_docker_client.client import get_config, get_all +from onap_dcae_cbs_docker_client.exceptions import CantGetConfig, CBSUnreachable, ENVsMissing -class FakeResponse: - def __init__(self, status_code, thejson): - self.status_code = status_code - self.thejson = thejson +def test_config(monkeypatch, monkeyed_requests_get): + monkeypatch.setattr("requests.get", monkeyed_requests_get) + assert get_config() == {"key_to_your_heart": 666} - def raise_for_status(self): - pass - def json(self): - return self.thejson +def test_all(monkeypatch, monkeyed_requests_get): + monkeypatch.setattr("requests.get", monkeyed_requests_get) + assert get_all() == { + "config": {"key_to_your_heart": 666}, + "dti": {"some amazing": "dti stuff"}, + "policies": {"event": {"foo": "bar"}, "items": [{"foo2": "bar2"}]}, + "otherkey": {"foo3": "bar3"}, + } -def monkeyed_requests_get(url): - # mock all the get calls for existent and non-existent - if url == "http://consuldotcom:8500/v1/catalog/service/config_binding_service": - return FakeResponse(status_code=200, - thejson=[{"ServiceAddress": "666.666.666.666", - "ServicePort": 8888}]) - elif url == "http://666.666.666.666:8888/service_component_all/mybestfrienddotcom": - return FakeResponse(status_code=200, - thejson={"config": {"key_to_your_heart": 666}, - "dti": {"some amazing": "dti stuff"}, - "policies": {"event": {"foo": "bar"}, - "items": [{"foo2": "bar2"}]}, - "otherkey": {"foo3": "bar3"}}) +def test_bad_hostname(monkeypatch, monkeyed_requests_get_404): + monkeypatch.setattr("requests.get", monkeyed_requests_get_404) + with pytest.raises(CantGetConfig): + get_config() + with pytest.raises(CantGetConfig): + get_all() - elif url == "http://666.666.666.666:8888/service_component/mybestfrienddotcom": - return FakeResponse(status_code=200, - thejson={"key_to_your_heart": 666}) + try: + get_config() + except CantGetConfig as e: + assert e.code == 404 + assert e.text == "" -def test_config(monkeypatch): - monkeypatch.setattr('requests.get', monkeyed_requests_get) - assert(get_config() == {"key_to_your_heart": 666}) +def test_unreachable(monkeypatch, monkeyed_requests_get_unreachable): + monkeypatch.setattr("requests.get", monkeyed_requests_get_unreachable) + with pytest.raises(CBSUnreachable): + get_config() + with pytest.raises(CBSUnreachable): + get_all() -def test_all(monkeypatch): - monkeypatch.setattr('requests.get', monkeyed_requests_get) - assert(get_all() == {"config": {"key_to_your_heart": 666}, - "dti": {"some amazing": "dti stuff"}, - "policies": {"event": {"foo": "bar"}, - "items": [{"foo2": "bar2"}]}, - "otherkey": {"foo3": "bar3"}}) +def test_badenv(monkeypatch): + monkeypatch.delenv("CONFIG_BINDING_SERVICE") + with pytest.raises(ENVsMissing): + get_config() + with pytest.raises(ENVsMissing): + get_all() diff --git a/onap-dcae-cbs-docker-client/tox-local.ini b/onap-dcae-cbs-docker-client/tox-local.ini deleted file mode 100644 index 7faf386..0000000 --- a/onap-dcae-cbs-docker-client/tox-local.ini +++ /dev/null @@ -1,22 +0,0 @@ -[tox] -envlist = py27,py36,flake8 - -[testenv] -deps= - pytest - coverage - pytest-cov -setenv = - CONSUL_HOST = consuldotcom - HOSTNAME = mybestfrienddotcom - CONFIG_BINDING_SERVICE = config_binding_service -commands=pytest --cov {envsitepackagesdir}/onap_dcae_cbs_docker_client --cov-report html - -[testenv:flake8] -basepython = python3.6 -skip_install = true -deps = flake8 -commands = flake8 setup.py onap_dcae_cbs_docker_client tests - -[flake8] -ignore = E501 diff --git a/onap-dcae-cbs-docker-client/tox.ini b/onap-dcae-cbs-docker-client/tox.ini index a8c9272..e5eeb43 100644 --- a/onap-dcae-cbs-docker-client/tox.ini +++ b/onap-dcae-cbs-docker-client/tox.ini @@ -1,6 +1,6 @@ # content of: tox.ini , put in same dir as setup.py [tox] -envlist = py27,py36,flake8 +envlist = py36,flake8 [testenv] deps= @@ -8,7 +8,6 @@ deps= coverage pytest-cov setenv = - CONSUL_HOST = consuldotcom HOSTNAME = mybestfrienddotcom CONFIG_BINDING_SERVICE = config_binding_service PYTHONPATH={toxinidir} -- cgit 1.2.3-korg