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.

Sign up for GitHub

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

Serve metrics under a dedicated port #48

Merged
merged 5 commits into from
Mar 7, 2018
Merged

Serve metrics under a dedicated port #48

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16f2411
feat: serve metrics under a dedicated port
linki Feb 28, 2018
7ab8bde
fix: enable metrics flag only when etcd is v3.3+
linki Feb 28, 2018
0a91415
fix: cleanup detection of etcd version
linki Mar 1, 2018
d8e6c10
fix: ensure that correct flags are used when upgrading as well
linki Mar 6, 2018
974f733
fix: remove superflous closing parenthesis
linki Mar 6, 2018
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
21 changes: 20 additions & 1 deletion etcd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class EtcdMember:
API_VERSION = '/v2/'
DEFAULT_CLIENT_PORT = 2379
DEFAULT_PEER_PORT = 2380
DEFAULT_METRICS_PORT = 2381
AG_TAG = 'aws:autoscaling:groupName'
CF_TAG = 'aws:cloudformation:stack-name'

Expand All @@ -56,6 +57,7 @@ def __init__(self, arg, region=None):

self.client_port = self.DEFAULT_CLIENT_PORT
self.peer_port = self.DEFAULT_PEER_PORT
self.metrics_port = self.DEFAULT_METRICS_PORT

self.client_urls = [] # these values could be assigned only from the running etcd
self.peer_urls = [] # cluster by performing http://addr:client_port/v2/members api call
Expand Down Expand Up @@ -242,7 +244,8 @@ def delete_member(self, member):
return result

def etcd_arguments(self, data_dir, initial_cluster, cluster_state):
return [
# common flags that always have to be set
arguments = [
'-name',
self.instance_id,
'--data-dir',
Expand All @@ -263,6 +266,22 @@ def etcd_arguments(self, data_dir, initial_cluster, cluster_state):
cluster_state
]

# this section handles etcd version specific flags
etcdversion = os.environ.get('ETCDVERSION')
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still one corner case left, it breaks upgrade from 3.2 to 3.3 :(
During the upgrade first it starts the old binary in order to get data, than it stops it and starts the new binary.
Current change will try to apply -listen-metrics-urls to the old binary and fail if the old version is 3.2

In order to solve it, let's introduce one more argument to this method, run_old and pass its value from https://github.com/linki/stups-etcd-cluster/blob/0a914150be28b93ff77b56747b0cf09a010138dc/etcd.py#L474:

    return self.me.etcd_arguments(self.DATA_DIR, peers, cluster_state, self.run_old)

And the line 270 has to be changed to:

    etcdversion = os.environ.get('ETCDVERSION_PREV' if run_old else 'ETCDVERSION')

if etcdversion:
etcdversion = list(map(lambda x: int(x), etcdversion.split('.')))
Copy link

@aermakov-zalando aermakov-zalando Mar 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

etcdversion = [int(x) for x in etcdversion.split('.')]

You could also use tuples which are immutable (tuple(int(x) for x in etcdversion.split('.'))), but don't forget to change the comparison to match.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I changed it.

# don't try to add additonal flags if we encounter an unexpected version format
if len(etcdversion) == 3:
Copy link

@aermakov-zalando aermakov-zalando Mar 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure if this is worth it TBH. If they decide to do a 3.3.1.2, this will suddenly break instead of continuing. I'd just check that the length is >= 2, or even drop the check altogether.

# etcd >= v3.3: serve metrics on an additonal port
if etcdversion[0] >= 3 and etcdversion[1] >= 3:
Copy link

Choose a reason for hiding this comment

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

etcdversion >= [3, 3]

arguments += [
'-listen-metrics-urls',
'http://0.0.0.0:{}'.format(self.metrics_port),
]

# return final list of arguments
return arguments


class EtcdCluster:
REGIONS = [] # more then one (1) Region if this a Multi-Region-Cluster
Expand Down