Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple endpoints #55

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
dc2407e
Add support for multiple endpoints
dariko Feb 25, 2019
4b1de04
python 3.7 compatibility
dariko Feb 25, 2019
e3defa3
Rework tests to run against an etcd cluster
dariko Feb 25, 2019
c744c1a
use etcd version from envs.py
dariko Feb 25, 2019
7a61e92
Only include async fixtures when running on python3
dariko Feb 25, 2019
7b33a71
clarify retry loop
dariko Feb 25, 2019
41492ff
iterate over a copy of the endpoint, preventing concurrent ops to cha…
dariko Feb 25, 2019
cad609a
allow EtcdCluster.etcdctl to failover to a working node
dariko Feb 25, 2019
4406119
more stable cluster, containers status detection
dariko Feb 25, 2019
ea4814f
more stable cluster and containers status detection
dariko Feb 25, 2019
a6c50b7
add delay before asserting callback was called
dariko Feb 25, 2019
cebbedb
allow test_snapshot to be self-consistent
dariko Feb 25, 2019
f4e46dd
write snapshot data in docker-shared directory
dariko Feb 25, 2019
2e38ccb
test watch util during etcd cluster rolling restart
dariko Feb 25, 2019
1951e1c
python 2 compat
dariko Feb 25, 2019
a66edf9
remove useless decorator
dariko Feb 25, 2019
6998a42
use first endpoint data as default
dariko Feb 25, 2019
78d0a65
create shared directory for containers with permissive mode
dariko Feb 25, 2019
16be75c
allow aioclient to work without valid certificates
dariko Feb 27, 2019
aead4ea
disable certificate validation on tests
dariko Feb 27, 2019
95c7d24
disable certificate validation on tests
dariko Feb 27, 2019
ba4ea4f
Construct cluster endpoints based on container addresses
dariko Feb 27, 2019
339f4aa
move retry_all_hosts to utils.py, initial whitelist support
dariko Feb 28, 2019
ae94b43
remove validation preventing minimal call format
dariko Feb 28, 2019
7a07e22
replace deprecated log.warn with log.warning
dariko Mar 2, 2019
ec49b1a
rework failover logic
dariko Mar 2, 2019
681d39c
add `status` to failover_waitlist
dariko Mar 2, 2019
0151244
cleanup watch util failover test
dariko Mar 2, 2019
d47f9ed
tests
dariko Mar 2, 2019
709f74e
python 2 compatibility
dariko Mar 2, 2019
3aa610e
prevent mutable in parameter default
dariko Mar 2, 2019
8206a66
remove unused imports
dariko Mar 2, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ before_install:
- sudo docker cp tmp:/usr/local/bin/etcdctl /usr/bin/etcdctl && sudo chmod 755 /usr/bin/etcdctl
- sudo docker rm tmp
- which etcdctl
- sudo mkdir /tmp/shared
- sudo chmod 777 /tmp/shared

# command to install dependencies, e.g. pip install -r requirements.txt --use-mirrors
install: pip install -U tox-travis codecov
Expand Down
22 changes: 13 additions & 9 deletions etcd3/aio_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import six
from aiohttp.client import _RequestContextManager

from .utils import retry_all_hosts
from .baseclient import BaseClient
from .baseclient import BaseModelizedStreamResponse
from .baseclient import DEFAULT_VERSION
Expand Down Expand Up @@ -73,7 +74,7 @@ async def __aenter__(self):
async def __aexit__(self, exc_type, exc_val, exc_tb):
self.close()

async def __aiter__(self):
def __aiter__(self):
dariko marked this conversation as resolved.
Show resolved Hide resolved
return self

async def __anext__(self):
Expand Down Expand Up @@ -111,7 +112,7 @@ def __init__(self, resp):
self.left_chunk = b''
self.i = 0

async def __aiter__(self):
def __aiter__(self):
return self

async def next(self):
Expand All @@ -133,16 +134,17 @@ async def next(self):


class AioClient(BaseClient):
def __init__(self, host='127.0.0.1', port=2379, protocol='http',
def __init__(self, host=None, port=None, endpoints=None, protocol='http',
cert=(), verify=None,
timeout=None, headers=None, user_agent=None, pool_size=30,
username=None, password=None, token=None,
server_version=DEFAULT_VERSION, cluster_version=DEFAULT_VERSION):
super(AioClient, self).__init__(host=host, port=port, protocol=protocol,
cert=cert, verify=verify,
timeout=timeout, headers=headers, user_agent=user_agent, pool_size=pool_size,
username=username, password=password, token=token,
server_version=server_version, cluster_version=cluster_version)
super(AioClient, self).__init__(
host=host, port=port, endpoints=endpoints, protocol=protocol,
cert=cert, verify=verify, timeout=timeout, headers=headers,
user_agent=user_agent, pool_size=pool_size,
username=username, password=password, token=token,
server_version=server_version, cluster_version=cluster_version)
self.ssl_context = None
if self.cert:
if verify is False:
Expand All @@ -165,7 +167,8 @@ def __init__(self, host='127.0.0.1', port=2379, protocol='http',
warnings.warn(Etcd3Warning("the openssl version of your python is too old to support TLSv1.1+,"
"please upgrade you python"))
ssl_context.verify_mode = cert_reqs
ssl_context.load_verify_locations(cafile=cafile)
if cafile:
ssl_context.load_verify_locations(cafile=cafile)
ssl_context.load_cert_chain(*self.cert)
connector = aiohttp.TCPConnector(limit=pool_size, ssl=self.ssl_context)
self.session = aiohttp.ClientSession(connector=connector)
Expand Down Expand Up @@ -225,6 +228,7 @@ async def _raise_for_status(resp):
code = data.get('code')
raise get_client_error(error, code, status, resp)

@retry_all_hosts
def call_rpc(self, method, data=None, stream=False, encode=True, raw=False, **kwargs):
"""
call ETCDv3 RPC and return response object
Expand Down
58 changes: 53 additions & 5 deletions etcd3/baseclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import semantic_version as sem
from six.moves import urllib_parse
import copy

from .apis import AuthAPI
from .apis import ClusterAPI
Expand All @@ -17,6 +18,7 @@
from .apis import MaintenanceAPI
from .apis import WatchAPI
from .errors import UnsupportedServerVersion
from .errors import Etcd3Exception
from .stateful import Lease
from .stateful import Lock
from .stateful import Txn
Expand All @@ -25,6 +27,8 @@
from .swaggerdefs import get_spec
from .utils import Etcd3Warning
from .utils import log
from .utils import check_param
from .utils import EtcdEndpoint
from .version import __version__


Expand All @@ -48,15 +52,59 @@ def __iter__(self):
DEFAULT_VERSION = '3.3.0'


def retry_all_hosts(func):
def wrapper(self, *args, **kwargs):
errors = []
got_result = False
call_endpoints = copy.copy(self.endpoints)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a lock is better than copy

if we have 3 endpoints of which the initial 2 are invalid:

client.endpoints:      [invalid, invalid, valid]
and after 1 request:
without copy   ->      [valid, invalid, invalid]       the next request will access the valid endpoint directly   
with copy      ->      [invalid, invalid, valid]       the next request still have to try 3 times

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two problems to consider:

  1. the endpoint whose response is accepted should become the active one
  2. the api call should use the endpoints defined at the start of the call (to prevent problems with concurrent code overwriting .endpoints)

A lock could work, but to prevent 2 it should lock the whole while retries > 0 loop and so prevent concurrent calls.

It should work correctly if when the while loop operating on the copy exits correctly, I add a block:

if call_endpoints[0] in endpoints:
  # move around endpoints to have endpoints[0] == call_enpoints[0]
else:
  # .endpoints has been modified externally
  # leave it as it is, the next call will find a good one

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to lock the whole while retries > 0

in fact, atomicity seems not so important here

or, just lock self.endpoints = xxxxx

retries = len(call_endpoints)
while retries > 0:
retries -= 1
endpoint = call_endpoints.pop(0)
call_endpoints.append(endpoint)
self.host = endpoint.host
self.port = endpoint.port
try:
ret = func(self, *args, **kwargs)
got_result = True
break
except Exception as e:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only retry when connection fails

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just catch these errors aiohttp.ClientError requests.RequestException urllib3.exceptions.HTTPError

errors.append(e)
log.warning('Failed to call %s(args: %s, kwargs: %s) on '
'endpoint %s (%s)' %
(func.__name__, args, kwargs, endpoint, e))
if not got_result:
exception_types = [x.__class__ for x in errors]
if len(set(exception_types)) == 1:
log.error('Failed to call %s(args: %s, kwargs: %s) on all '
'endpoints: %s. Got errors: %s' %
(func.__name__, args, kwargs, call_endpoints, errors))
raise errors[0]
else:
raise Etcd3Exception(
'Failed to call %s(args: %s, kwargs: %s) on all '
'endpoints: %s. Got errors: %s' %
(func.__name__, args, kwargs, call_endpoints, errors))
# elif len(errors) > 0:
# log.warning('Got errors %s, retried successfully')
return ret
return wrapper


class BaseClient(AuthAPI, ClusterAPI, KVAPI, LeaseAPI, MaintenanceAPI,
WatchAPI, ExtraAPI, LockAPI):
def __init__(self, host='127.0.0.1', port=2379, protocol='http',
cert=(), verify=None,
timeout=None, headers=None, user_agent=None, pool_size=30,
@check_param(at_most_one_of=['port', 'endpoints'], at_least_one_of=['port', 'endpoints'])
@check_param(at_most_one_of=['host', 'endpoints'], at_least_one_of=['host', 'endpoints'])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with at_least_one_of
I can't create default client client=etcd3.Client() the old way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I'm gonna remove the decorators

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in ae94b43

def __init__(self, host=None, port=None, endpoints=None, protocol='http', cert=(),
verify=None, timeout=None, headers=None, user_agent=None, pool_size=30,
username=None, password=None, token=None,
server_version=DEFAULT_VERSION, cluster_version=DEFAULT_VERSION):
self.host = host
self.port = port
if host is not None:
self.endpoints = ([EtcdEndpoint(host, port)])
else:
self.endpoints = endpoints
self.host = self.endpoints[0].host
self.port =self.endpoints[0].port
self.cert = cert
self.protocol = protocol
if cert:
Expand Down
17 changes: 10 additions & 7 deletions etcd3/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import six

from .baseclient import BaseClient
from .baseclient import retry_all_hosts
from .baseclient import BaseModelizedStreamResponse
from .baseclient import DEFAULT_VERSION
from .errors import Etcd3Exception
Expand Down Expand Up @@ -86,8 +87,8 @@ def iter_response(resp):


class Client(BaseClient):
def __init__(self, host='127.0.0.1', port=2379, protocol='http',
cert=(), verify=None,
def __init__(self, host=None, port=None, endpoints=None,
protocol='http', cert=(), verify=None,
timeout=None, headers=None, user_agent=None, pool_size=30,
username=None, password=None, token=None, max_retries=0,
server_version=DEFAULT_VERSION, cluster_version=DEFAULT_VERSION):
Expand All @@ -100,11 +101,12 @@ def __init__(self, host='127.0.0.1', port=2379, protocol='http',
which we retry a request, import urllib3's ``Retry`` class and pass
that instead.
"""
super(Client, self).__init__(host=host, port=port, protocol=protocol,
cert=cert, verify=verify,
timeout=timeout, headers=headers, user_agent=user_agent, pool_size=pool_size,
username=username, password=password, token=token,
server_version=server_version, cluster_version=cluster_version)
super(Client, self).__init__(
host=host, port=port, endpoints=endpoints, protocol=protocol,
cert=cert, verify=verify, timeout=timeout, headers=headers,
user_agent=user_agent, pool_size=pool_size,
username=username, password=password, token=token,
server_version=server_version, cluster_version=cluster_version)
self._session = requests.session()
self._session.cert = self.cert
self._session.verify = self.verify
Expand Down Expand Up @@ -164,6 +166,7 @@ def _post(self, url, data=None, json=None, **kwargs):
"""
return self._session.post(url, data=data, json=json, **kwargs)

@retry_all_hosts
def call_rpc(self, method, data=None, stream=False, encode=True, raw=False, **kwargs): # TODO: add modelize param
"""
call ETCDv3 RPC and return response object
Expand Down
9 changes: 9 additions & 0 deletions etcd3/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,12 @@ def find_executable(executable, path=None): # pragma: no cover
f = os.path.join(p, execname)
if os.path.isfile(f):
return f


class EtcdEndpoint():
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this class contains only host and port but made creating a client less friendly

any further design on this?

Copy link
Owner

@Revolution1 Revolution1 Feb 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better put this into etcd3/__init__.py

def __init__(self, host='127.0.0.1', port=2379):
self.host = host
self.port = port

def __repr__(self):
return "EtcdEndpoint(host=%s, port=%s)" % (self.host, self.port)
1 change: 1 addition & 0 deletions requirements_dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ m2r==0.2.1
codecov>=1.4.0
codacy-coverage==1.3.11
twine==1.13.0
docker==3.7.0
76 changes: 76 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import six
from etcd3.client import Client
import pytest
from .etcd_cluster import EtcdTestCluster


@pytest.fixture(scope='session')
def etcd_cluster(request):
# function_name = request.function.__name__
# function_name = re.sub(r"[^a-zA-Z0-9]+", "", function_name)
cluster = EtcdTestCluster(ident='cleartext', size=3)

def fin():
cluster.down()
request.addfinalizer(fin)
cluster.up()
cluster.wait_ready()

return cluster


@pytest.fixture(scope='session')
def etcd_cluster_ssl(request):
# function_name = request.function.__name__
# function_name = re.sub(r"[^a-zA-Z0-9]+", "", function_name)
cluster = EtcdTestCluster(ident='ssl', size=3, ssl=True)

def fin():
cluster.down()
request.addfinalizer(fin)
cluster.up()
cluster.wait_ready()

return cluster


@pytest.fixture(scope='module')
def client(etcd_cluster):
"""
init Etcd3Client, close its connection-pool when teardown
"""
# _, p, _ = docker_run_etcd_main()
c = Client(endpoints=etcd_cluster.get_endpoints(),
protocol='https' if etcd_cluster.ssl else 'http')
yield c
c.close()


@pytest.fixture
def clear(etcd_cluster):
def _clear():
etcd_cluster.etcdctl('del --from-key ""')
return _clear


def teardown_auth(etcd_cluster): # pragma: no cover
"""
disable auth, delete all users and roles
"""
etcd_cluster.etcdctl('--user root:root auth disable')
etcd_cluster.etcdctl('--user root:changed auth disable')
for i in (etcd_cluster.etcdctl('role list') or '').splitlines():
if six.PY3: # pragma: no cover
i = six.text_type(i, encoding='utf-8')
etcd_cluster.etcdctl('role delete %s' % i)
for i in (etcd_cluster.etcdctl('user list') or '').splitlines():
if six.PY3: # pragma: no cover
i = six.text_type(i, encoding='utf-8')
etcd_cluster.etcdctl('user delete %s' % i)


def enable_auth(etcd_cluster): # pragma: no cover
etcd_cluster.etcdctl('user add root:root')
etcd_cluster.etcdctl('role add root')
etcd_cluster.etcdctl('user grant root root')
etcd_cluster.etcdctl('auth enable')
Loading