Skip to content

Commit

Permalink
[service discovery] Improve logging, test coverage and template varia…
Browse files Browse the repository at this point in the history 
…ble interpolation (#2573)

* Improve logging, tests, and interpolation of template variables
in particular, add the ability to specify keys on top of indexes
for template variables (eg: host_bridge will return the IP
address of the host on its bridge network).
  • Loading branch information
@hkaj @gmmeyer
hkaj authored and gmmeyer committed Jun 27, 2016
1 parent eafec6b commit 4c13bbc
Show file tree
Hide file tree
Showing 5 changed files with 360 additions and 108 deletions.
10 changes: 8 additions & 2 deletions config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -829,19 +829,25 @@ def _service_disco_configs(agentConfig):
""" Retrieve all the service disco configs and return their conf dicts
"""
if agentConfig.get('service_discovery') and agentConfig.get('service_discovery_backend') in SD_BACKENDS:
sd_backend = get_sd_backend(agentConfig=agentConfig)
service_disco_configs = sd_backend.get_configs()
try:
log.info("Fetching service discovery check configurations.")
sd_backend = get_sd_backend(agentConfig=agentConfig)
service_disco_configs = sd_backend.get_configs()
except Exception:
log.exception("Loading service discovery configurations failed.")
else:
service_disco_configs = {}

return service_disco_configs


def _conf_path_to_check_name(conf_path):
f = os.path.splitext(os.path.split(conf_path)[1])
if f[1] and f[1] == ".default":
f = os.path.splitext(f[0])
return f[0]


def get_checks_places(osname, agentConfig):
""" Return a list of methods which, when called with a check name, will each return a check path to inspect
"""
Expand Down
272 changes: 226 additions & 46 deletions tests/core/test_service_discovery.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def raise_for_status(self):

def _get_container_inspect(c_id):
"""Return a mocked container inspect dict from self.container_inspects."""
for co, _, _ in TestServiceDiscovery.container_inspects:
for co, _, _, _ in TestServiceDiscovery.container_inspects:
if co.get('Id') == c_id:
return co
return None
Expand Down Expand Up @@ -81,9 +81,9 @@ class TestServiceDiscovery(unittest.TestCase):
}
container_inspects = [
# (inspect_dict, expected_ip, expected_port)
(docker_container_inspect, '172.17.0.21', ['80', '443']),
(kubernetes_container_inspect, None, ['6379']), # arbitrarily defined in the mocked pod_list
(malformed_container_inspect, None, KeyError)
(docker_container_inspect, '172.17.0.21', 'port', '443'),
(kubernetes_container_inspect, None, 'port', '6379'), # arbitrarily defined in the mocked pod_list
(malformed_container_inspect, None, 'port', KeyError)
]

# templates with variables already extracted
Expand Down Expand Up @@ -152,7 +152,7 @@ def setUp(self):

@mock.patch('utils.http.requests.get')
@mock.patch('utils.kubeutil.check_yaml')
def test_get_host(self, mock_check_yaml, mock_get):
def test_get_host_address(self, mock_check_yaml, mock_get):
kubernetes_config = {'instances': [{'kubelet_port': 1337}]}
pod_list = {
'items': [{
Expand All @@ -165,34 +165,73 @@ def test_get_host(self, mock_check_yaml, mock_get):
}]
}

# (inspect, tpl_var, expected_result)
ip_address_inspects = [
({'NetworkSettings': {}}, 'host', None),
({'NetworkSettings': {'IPAddress': ''}}, 'host', None),

({'NetworkSettings': {'IPAddress': '127.0.0.1'}}, 'host', '127.0.0.1'),
({'NetworkSettings': {'IPAddress': '127.0.0.1', 'Networks': {}}}, 'host', '127.0.0.1'),
({'NetworkSettings': {
'IPAddress': '127.0.0.1',
'Networks': {'bridge': {'IPAddress': '127.0.0.1'}}}},
'host', '127.0.0.1'),
({'NetworkSettings': {
'IPAddress': '',
'Networks': {'bridge': {'IPAddress': '127.0.0.1'}}}},
'host_bridge', '127.0.0.1'),
({'NetworkSettings': {
'IPAddress': '127.0.0.1',
'Networks': {
'bridge': {'IPAddress': '172.17.0.2'},
'foo': {'IPAddress': '192.168.0.2'}}}},
'host', '127.0.0.1'),

({'NetworkSettings': {'Networks': {}}}, 'host', None),
({'NetworkSettings': {'Networks': {}}}, 'host_bridge', None),
({'NetworkSettings': {'Networks': {'bridge': {}}}}, 'host', None),
({'NetworkSettings': {'Networks': {'bridge': {}}}}, 'host_bridge', None),
({'NetworkSettings': {
'Networks': {
'bridge': {'IPAddress': '172.17.0.2'}
}}},
'host_bridge', '172.17.0.2'),
({'NetworkSettings': {
'Networks': {
'bridge': {'IPAddress': '172.17.0.2'},
'foo': {'IPAddress': '192.168.0.2'}
}}},
'host_foo', '192.168.0.2')
]

mock_check_yaml.return_value = kubernetes_config
mock_get.return_value = Response(pod_list)

for c_ins, expected_ip, _ in self.container_inspects:
for c_ins, tpl_var, expected_ip in ip_address_inspects:
with mock.patch.object(AbstractConfigStore, '__init__', return_value=None):
with mock.patch('utils.dockerutil.DockerUtil.client', return_value=None):
with mock.patch('utils.kubeutil.get_conf_path', return_value=None):
sd_backend = get_sd_backend(agentConfig=self.auto_conf_agentConfig)
self.assertEqual(sd_backend._get_host(c_ins), expected_ip)
self.assertEquals(sd_backend._get_host_address(c_ins, tpl_var), expected_ip)
clear_singletons(self.auto_conf_agentConfig)

def test_get_ports(self):
def test_get_port(self):
with mock.patch('utils.dockerutil.DockerUtil.client', return_value=None):
for c_ins, _, expected_ports in self.container_inspects:
for c_ins, _, var_tpl, expected_ports in self.container_inspects:
sd_backend = get_sd_backend(agentConfig=self.auto_conf_agentConfig)
if isinstance(expected_ports, list):
self.assertEqual(sd_backend._get_ports(c_ins), expected_ports)
if isinstance(expected_ports, str):
self.assertEquals(sd_backend._get_port(c_ins, var_tpl), expected_ports)
else:
self.assertRaises(expected_ports, sd_backend._get_ports, c_ins)
self.assertRaises(expected_ports, sd_backend._get_port, c_ins, var_tpl)
clear_singletons(self.auto_conf_agentConfig)

@mock.patch('docker.Client.inspect_container', side_effect=_get_container_inspect)
@mock.patch.object(SDDockerBackend, '_get_config_templates', side_effect=_get_conf_tpls)
def test_get_check_configs(self, mock_inspect_container, mock_get_conf_tpls):
"""Test get_check_config with mocked container inspect and config template"""
with mock.patch('utils.dockerutil.DockerUtil.client', return_value=None):
with mock.patch.object(SDDockerBackend, '_get_host', return_value='127.0.0.1'):
with mock.patch.object(SDDockerBackend, '_get_ports', return_value=['1337']):
with mock.patch.object(SDDockerBackend, '_get_host_address', return_value='127.0.0.1'):
with mock.patch.object(SDDockerBackend, '_get_port', return_value='1337'):
c_id = self.docker_container_inspect.get('Id')
for image in self.mock_templates.keys():
sd_backend = get_sd_backend(agentConfig=self.auto_conf_agentConfig)
Expand Down Expand Up @@ -239,29 +278,78 @@ def test_render_template(self):
]

with mock.patch('utils.dockerutil.DockerUtil.client', return_value=None):
for agentConfig in self.agentConfigs:
sd_backend = get_sd_backend(agentConfig=agentConfig)
for tpl, res in valid_configs:
init, instance, variables = tpl
config = sd_backend._render_template(init, instance, variables)
self.assertEquals(config, res)
for init, instance, variables in invalid_configs:
config = sd_backend._render_template(init, instance, variables)
self.assertEquals(config, None)
clear_singletons(agentConfig)
with mock.patch.object(EtcdStore, 'get_client', return_value=None):
with mock.patch.object(ConsulStore, 'get_client', return_value=None):
for agentConfig in self.agentConfigs:
sd_backend = get_sd_backend(agentConfig=agentConfig)
for tpl, res in valid_configs:
init, instance, variables = tpl
config = sd_backend._render_template(init, instance, variables)
self.assertEquals(config, res)
for init, instance, variables in invalid_configs:
config = sd_backend._render_template(init, instance, variables)
self.assertEquals(config, None)
clear_singletons(agentConfig)

def test_fill_tpl(self):
"""Test _fill_tpl with mock _get_ports"""
"""Test _fill_tpl with mocked docker client"""

valid_configs = [
# ((inspect, instance_tpl, variables, tags), (expected_instance_tpl, expected_var_values))
(({}, {'host': 'localhost'}, [], None), ({'host': 'localhost'}, {})),
(
({}, {'host': 'localhost'}, [], None),
({'NetworkSettings': {'IPAddress': ''}}, {'host': 'localhost'}, [], None),
({'host': 'localhost'}, {})
),
(
({'NetworkSettings': {'Networks': {}}}, {'host': 'localhost'}, [], None),
({'host': 'localhost'}, {})
),
(
({'NetworkSettings': {'Networks': {'bridge': {}}}}, {'host': 'localhost'}, [], None),
({'host': 'localhost'}, {})
),
(
({'NetworkSettings': {'IPAddress': '127.0.0.1'}},
{'host': '%%host%%', 'port': 1337}, ['host'], ['foo', 'bar:baz']),
({'host': '%%host%%', 'port': 1337, 'tags': ['foo', 'bar:baz']}, {'host': '127.0.0.1'})
({'host': '%%host%%', 'port': 1337, 'tags': ['foo', 'bar:baz']}, {'host': '127.0.0.1'}),
),
(
({'NetworkSettings': {'IPAddress': '127.0.0.1', 'Networks': {}}},
{'host': '%%host%%', 'port': 1337}, ['host'], ['foo', 'bar:baz']),
({'host': '%%host%%', 'port': 1337, 'tags': ['foo', 'bar:baz']}, {'host': '127.0.0.1'}),
),
(
({'NetworkSettings': {
'IPAddress': '127.0.0.1',
'Networks': {'bridge': {'IPAddress': '172.17.0.2'}}}
},
{'host': '%%host%%', 'port': 1337}, ['host'], ['foo', 'bar:baz']),
({'host': '%%host%%', 'port': 1337, 'tags': ['foo', 'bar:baz']}, {'host': '127.0.0.1'}),
),
(
({'NetworkSettings': {
'IPAddress': '',
'Networks': {
'bridge': {'IPAddress': '172.17.0.2'},
'foo': {'IPAddress': '192.168.0.2'}
}}
},
{'host': '%%host_bridge%%', 'port': 1337}, ['host_bridge'], ['foo', 'bar:baz']),
({'host': '%%host_bridge%%', 'port': 1337, 'tags': ['foo', 'bar:baz']},
{'host_bridge': '172.17.0.2'}),
),
(
({'NetworkSettings': {
'IPAddress': '',
'Networks': {
'bridge': {'IPAddress': '172.17.0.2'},
'foo': {'IPAddress': '192.168.0.2'}
}}
},
{'host': '%%host_foo%%', 'port': 1337}, ['host_foo'], ['foo', 'bar:baz']),
({'host': '%%host_foo%%', 'port': 1337, 'tags': ['foo', 'bar:baz']},
{'host_foo': '192.168.0.2'}),
),
(
({'NetworkSettings': {'IPAddress': '127.0.0.1', 'Ports': {'42/tcp': None, '22/tcp': None}}},
Expand All @@ -271,33 +359,125 @@ def test_fill_tpl(self):
{'host': '127.0.0.1', 'port_1': '42'})
)
]

# should not fail but return something specific
edge_cases = [
# ((inspect, instance_tpl, variables, tags), (expected_instance_tpl, expected_var_values))

# specify bridge but there is also a default IPAddress (networks should be preferred)
(
({'NetworkSettings': {
'IPAddress': '127.0.0.1',
'Networks': {'bridge': {'IPAddress': '172.17.0.2'}}}},
{'host': '%%host_bridge%%', 'port': 1337}, ['host_bridge'], ['foo', 'bar:baz']),
({'host': '%%host_bridge%%', 'port': 1337, 'tags': ['foo', 'bar:baz']},
{'host_bridge': '172.17.0.2'})
),
# specify index but there is a default IPAddress (there's a specifier, even if it's wrong, walking networks should be preferred)
(
({'NetworkSettings': {
'IPAddress': '127.0.0.1',
'Networks': {'bridge': {'IPAddress': '172.17.0.2'}}}},
{'host': '%%host_0%%', 'port': 1337}, ['host_0'], ['foo', 'bar:baz']),
({'host': '%%host_0%%', 'port': 1337, 'tags': ['foo', 'bar:baz']}, {'host_0': '172.17.0.2'}),
),
# missing key for host, bridge network should be preferred
(
({'NetworkSettings': {'Networks': {
'bridge': {'IPAddress': '127.0.0.1'},
'foo': {'IPAddress': '172.17.0.2'}}}},
{'host': '%%host_bar%%', 'port': 1337}, ['host_bar'], []),
({'host': '%%host_bar%%', 'port': 1337}, {'host_bar': '127.0.0.1'}),
),
# missing index for port
(
({'NetworkSettings': {'IPAddress': '127.0.0.1', 'Ports': {'42/tcp': None, '22/tcp': None}}},
{'host': '%%host%%', 'port': '%%port_2%%', 'tags': ['env:test']},
['host', 'port_2'], ['foo', 'bar:baz']),
({'host': '%%host%%', 'port': '%%port_2%%', 'tags': ['env:test', 'foo', 'bar:baz']},
{'host': '127.0.0.1', 'port_2': '42'})
)
]

# should raise
invalid_config = [
# ((inspect, instance_tpl, variables, tags), expected_exception)

# template variable but no IPAddress available
(
({'NetworkSettings': {'Networks': {}}},
{'host': '%%host%%', 'port': 1337}, ['host'], ['foo', 'bar:baz']),
Exception,
),
# index but no IPAddress available
(
({'NetworkSettings': {'Networks': {}}},
{'host': '%%host_0%%', 'port': 1337}, ['host_0'], ['foo', 'bar:baz']),
Exception,
),
# key but no IPAddress available
(
({'NetworkSettings': {'Networks': {}}},
{'host': '%%host_foo%%', 'port': 1337}, ['host_foo'], ['foo', 'bar:baz']),
Exception,
),

# template variable but no port available
(
({'NetworkSettings': {'Networks': {}}},
{'host': 'localhost', 'port': '%%port%%'}, ['port'], []),
Exception,
),
# index but no port available
(
({'NetworkSettings': {'Networks': {}}},
{'host': 'localhost', 'port_0': '%%port%%'}, ['port_0'], []),
Exception,
),
# key but no port available
(
({'NetworkSettings': {'Networks': {}}},
{'host': 'localhost', 'port': '%%port_foo%%'}, ['port_foo'], []),
Exception,
)
]

with mock.patch('utils.dockerutil.DockerUtil.client', return_value=None):
for ac in self.agentConfigs:
sd_backend = get_sd_backend(agentConfig=ac)
try:
for co in valid_configs:
inspect, tpl, variables, tags = co[0]
instance_tpl, var_values = sd_backend._fill_tpl(inspect, tpl, variables, tags)
for key in instance_tpl.keys():
if isinstance(instance_tpl[key], list):
self.assertEquals(len(instance_tpl[key]), len(co[1][0].get(key)))
for elem in instance_tpl[key]:
self.assertTrue(elem in co[1][0].get(key))
else:
self.assertEquals(instance_tpl[key], co[1][0].get(key))
self.assertEquals(var_values, co[1][1])
clear_singletons(ac)
except Exception:
clear_singletons(ac)
raise
with mock.patch.object(EtcdStore, 'get_client', return_value=None):
with mock.patch.object(ConsulStore, 'get_client', return_value=None):
for ac in self.agentConfigs:
sd_backend = get_sd_backend(agentConfig=ac)
try:
for co in valid_configs + edge_cases:
inspect, tpl, variables, tags = co[0]
instance_tpl, var_values = sd_backend._fill_tpl(inspect, tpl, variables, tags)
for key in instance_tpl.keys():
if isinstance(instance_tpl[key], list):
self.assertEquals(len(instance_tpl[key]), len(co[1][0].get(key)))
for elem in instance_tpl[key]:
self.assertTrue(elem in co[1][0].get(key))
else:
self.assertEquals(instance_tpl[key], co[1][0].get(key))
self.assertEquals(var_values, co[1][1])

for co in invalid_config:
inspect, tpl, variables, tags = co[0]
self.assertRaises(co[1], sd_backend._fill_tpl(inspect, tpl, variables, tags))

clear_singletons(ac)
except Exception:
clear_singletons(ac)
raise

# config_stores tests

def test_get_auto_config(self):
"""Test _get_auto_config"""
expected_tpl = {
'redis': ('redisdb', None, {"host": "%%host%%", "port": "%%port%%"}),
'consul': ('consul', None, {"url": "http://%%host%%:%%port%%", "catalog_checks": True, "new_leader_checks": True}),
'consul': ('consul', None, {
"url": "http://%%host%%:%%port%%", "catalog_checks": True, "new_leader_checks": True
}),
'foobar': None
}

Expand Down
13 changes: 0 additions & 13 deletions utils/service_discovery/abstract_config_store.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,3 @@ def crawl_config_template(self):
self.previous_config_index = config_index
return True
return False


class StubStore(AbstractConfigStore):
"""Used when no valid config store was found. Allow to use auto_config."""
def _extract_settings(self, config):
pass

def get_client(self):
pass

def crawl_config_template(self):
# There is no user provided templates in auto_config mode
return False
Loading

0 comments on commit 4c13bbc

Please sign in to comment.