Merge lp:~martin-hilton/charm-haproxy/alias-all_services-services into lp:charm-haproxy

Proposed by Martin Hilton
Status: Merged
Approved by: Stuart Bishop
Approved revision: 118
Merged at revision: 130
Proposed branch: lp:~martin-hilton/charm-haproxy/alias-all_services-services
Merge into: lp:charm-haproxy
Diff against target: 157 lines (+67/-14)
4 files modified
Makefile (+1/-1)
hooks/hooks.py (+17/-10)
hooks/tests/test_install.py (+1/-2)
hooks/tests/test_reverseproxy_hooks.py (+48/-1)
To merge this branch: bzr merge lp:~martin-hilton/charm-haproxy/alias-all_services-services
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Canonical IS Reviewers Pending
Review via email: mp+367560@code.launchpad.net

Commit message

Description of the change

This is the change originally proposed in https://code.launchpad.net/~evarlast/charm-haproxy/alias-all_services-services/+merge/317254 just updated to be based on the latest version and have test/lint pass.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Stuart Bishop (stub) wrote :

Looks fine, apart from formatting. One pep8 issue that flake8 likely ignores, and some triple quoted string indentation that can be made easier to read. Best if these get fixed before landing.

review: Approve
Revision history for this message
Stuart Bishop (stub) :
review: Needs Fixing
118. By Martin Hilton

review updates

Revision history for this message
Stuart Bishop (stub) wrote :

Yup, ta muchly

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 130

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2019-03-21 11:33:58 +0000
3+++ Makefile 2019-05-17 12:11:53 +0000
4@@ -24,7 +24,7 @@
5
6 lint: .venv
7 @echo Checking for Python syntax...
8- @python -m flake8 $(HOOKS_DIR) --extend-ignore=E123 --exclude=$(HOOKS_DIR)/charmhelpers
9+ @python -m flake8 $(HOOKS_DIR) --ignore=E123 --max-line-length=120 --exclude=$(HOOKS_DIR)/charmhelpers
10
11 sourcedeps:
12 @echo Updating source dependencies...
13
14=== modified file 'hooks/hooks.py'
15--- hooks/hooks.py 2019-03-21 15:39:53 +0000
16+++ hooks/hooks.py 2019-05-17 12:11:53 +0000
17@@ -253,9 +253,8 @@
18 # bind 1.2.3.5:234
19 # bind 2001:db8::1:80
20 # bind 1.2.3.4:123 ssl crt /foo/bar
21- bind_stanzas = re.findall(
22- r"\s+bind\s+([a-fA-F0-9\.:\*]+):(\d+).*\n\s+default_backend\s+([^\s]+)",
23- haproxy_config, re.M)
24+ bind_stanzas = re.findall(r"\s+bind\s+([a-fA-F0-9\.:\*]+):(\d+).*\n\s+default_backend\s+([^\s]+)",
25+ haproxy_config, re.M)
26 return (tuple(((service, addr, int(port))
27 for service, addr, port in listen_stanzas)) +
28 tuple(((service, addr, int(port))
29@@ -614,8 +613,19 @@
30 # Handle relations which specify their own services clauses
31 for relation_info in relation_data:
32 if "services" in relation_info:
33+ services_dict = parse_services_yaml(services_dict, relation_info['services'])
34+ # apache2 charm uses "all_services" key instead of "services".
35+ if "all_services" in relation_info and "services" not in relation_info:
36 services_dict = parse_services_yaml(services_dict,
37- relation_info['services'])
38+ relation_info['all_services'])
39+ # Replace the backend server(2hops away) with the private-address.
40+ for service_name in services_dict.keys():
41+ if service_name == 'service' or 'servers' not in services_dict[service_name]:
42+ continue
43+ servers = services_dict[service_name]['servers']
44+ for i in range(len(servers)):
45+ servers[i][1] = relation_info['private-address']
46+ servers[i][2] = str(services_dict[service_name]['service_port'])
47
48 if len(services_dict) == 0:
49 log("No services configured, exiting.")
50@@ -636,8 +646,7 @@
51 relation_ok = True
52 for required in ("port", "private-address"):
53 if required not in relation_info:
54- log("No %s in relation data for '%s', skipping." %
55- (required, unit))
56+ log("No %s in relation data for '%s', skipping." % (required, unit))
57 relation_ok = False
58 break
59
60@@ -655,8 +664,7 @@
61 if relation_info['service_name'] in services_dict:
62 service_names.add(relation_info['service_name'])
63 else:
64- log("Service '%s' does not exist." %
65- relation_info['service_name'])
66+ log("Service '%s' does not exist." % relation_info['service_name'])
67 continue
68
69 if 'sitenames' in relation_info:
70@@ -927,8 +935,7 @@
71 if release == 'trusty':
72 pkgs.append('python-ipaddr')
73 apt_install(filter_installed_packages(pkgs), fatal=False)
74- ensure_package_status(service_affecting_packages,
75- config_data['package_status'])
76+ ensure_package_status(service_affecting_packages, config_data['package_status'])
77 enable_haproxy()
78
79
80
81=== modified file 'hooks/tests/test_install.py'
82--- hooks/tests/test_install.py 2019-03-21 10:47:31 +0000
83+++ hooks/tests/test_install.py 2019-05-17 12:11:53 +0000
84@@ -50,8 +50,7 @@
85 for package in calls[1][0][0]:
86 self.assertIn(
87 package,
88- (['python-pyasn1', 'python-pyasn1-modules', 'python-apt',
89- 'python-openssl'],))
90+ ['python-pyasn1', 'python-pyasn1-modules', 'python-apt', 'python-openssl'])
91 self.assertEqual({'fatal': False}, calls[1][1])
92
93 def test_add_source(self):
94
95=== modified file 'hooks/tests/test_reverseproxy_hooks.py'
96--- hooks/tests/test_reverseproxy_hooks.py 2015-09-01 13:41:44 +0000
97+++ hooks/tests/test_reverseproxy_hooks.py 2019-05-17 12:11:53 +0000
98@@ -503,7 +503,8 @@
99 'service': {
100 'service_name': 'service',
101 'service_host': '0.0.0.0',
102- 'service_port': 10002,
103+ 'service_options': ['balance leastconn'],
104+ 'service_port': 4242,
105 'servers': [
106 ['foo-0', '1.2.3.4', 4242, ["maxconn 4"]],
107 ]
108@@ -648,3 +649,49 @@
109 relation_settings={
110 "public-address": "1.2.3.4",
111 "ssl_cert": ssl_cert})
112+
113+ def test_relation_all_services(self):
114+ self.get_config_services.return_value = {
115+ None: {
116+ "service_name": "service",
117+ },
118+ "service": {
119+ "service_name": "service",
120+ },
121+ }
122+ self.relations_of_type.return_value = [
123+ {"port": 80,
124+ "hostname": "blah.internal",
125+ "private-address": "1.2.3.4",
126+ "__unit__": "foo/0",
127+ "all_services": '''
128+ - server_options: &id001 [check inter 2000 rise 2 fall 5 maxconn 500]
129+ servers:
130+ - - my-service-2-8082
131+ - 10.142.0.16
132+ - '8082'
133+ - *id001
134+ service_host: 0.0.0.0
135+ service_name: my-service
136+ service_options: [mode http, balance url_param waitid]
137+ service_port: 83
138+ '''
139+ },
140+ ]
141+
142+ expected = {
143+ 'my-service': {
144+ 'server_options': ['check inter 2000 rise 2 fall 5 maxconn 500'],
145+ 'servers': [['my-service-2-8082', '1.2.3.4', '83', ['check inter 2000 rise 2 fall 5 maxconn 500']]],
146+ 'service_host': '0.0.0.0',
147+ 'service_name': 'my-service',
148+ 'service_options': ['mode http', 'balance url_param waitid'],
149+ 'service_port': 83},
150+ 'service': {
151+ 'servers': [('foo-0-80', '1.2.3.4', 80, [])],
152+ 'service_host': '0.0.0.0',
153+ 'service_name': 'service',
154+ 'service_port': 85}}
155+
156+ self.assertEqual(expected, hooks.create_services())
157+ self.write_service_config.assert_called_with(expected)

Subscribers

People subscribed via source and target branches

to all changes: