Merge lp:~aglenyoung/charms/precise/haproxy/haproxy-stats-socket into lp:charms/haproxy

Proposed by Andrew Glen-Young on 2014-01-16
Status: Merged
Merged at revision: 83
Proposed branch: lp:~aglenyoung/charms/precise/haproxy/haproxy-stats-socket
Merge into: lp:charms/haproxy
Prerequisite: lp:~aglenyoung/charms/precise/haproxy/haproxy-fix-tests
Diff against target: 60 lines (+12/-0)
3 files modified
config.yaml (+5/-0)
hooks/hooks.py (+3/-0)
hooks/tests/test_helpers.py (+4/-0)
To merge this branch: bzr merge lp:~aglenyoung/charms/precise/haproxy/haproxy-stats-socket
Reviewer Review Type Date Requested Status
Tim Van Steenburgh Approve on 2014-09-08
Marco Ceppi 2014-01-16 Pending
Review via email: mp+201980@code.launchpad.net

This proposal supersedes a proposal from 2014-01-15.

To post a comment you must log in.
Marco Ceppi (marcoceppi) wrote : Posted in a previous version of this proposal

Hi Andrew, thanks for taking the time to add this option! Unfortunately your changes do not pass the haproxy unit test.

You can verify this by running `make build` in the charm's root. Please update the tests to reflect your changes.

Barring the tests passing, this LGTM. However, because of the test fails I have to decline this merge. Once you've addressed this issue please select "Request another review" and enter "charmers". This will place your update back in our charm review queue so we can re-review it again.

review: Needs Fixing
Andrew Glen-Young (aglenyoung) wrote :

I've added a prerequisite branch which cleans up the tests a little before submitting my branch.

The functionality stays the same but I've done the following:
- Updated the config variable name to be consistent with the current convention.
- Added the required tests.

Tim Van Steenburgh (tvansteenburgh) wrote :

+1 LGTM.

This still fails charm-proof and `make test`, but I've pushed a MP that fixes that: https://code.launchpad.net/~tvansteenburgh/charms/precise/haproxy/fix-proof-and-tests/+merge/233784

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'config.yaml'
--- config.yaml 2014-01-07 18:44:15 +0000
+++ config.yaml 2014-01-16 19:00:01 +0000
@@ -34,6 +34,11 @@
34 the same physical server. With the help of this parameter, it becomes 34 the same physical server. With the help of this parameter, it becomes
35 possible to add some randomness in the check interval between 0 and 35 possible to add some randomness in the check interval between 0 and
36 +/- 50%. A value between 2 and 5 seems to show good results.36 +/- 50%. A value between 2 and 5 seems to show good results.
37 global_stats_socket:
38 default: False
39 type: boolean
40 description: |
41 Whether to enable the stats UNIX socket.
37 default_log:42 default_log:
38 default: "global"43 default: "global"
39 type: string44 type: string
4045
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py 2014-01-16 19:00:01 +0000
+++ hooks/hooks.py 2014-01-16 19:00:01 +0000
@@ -125,6 +125,9 @@
125 haproxy_globals.append(" quiet")125 haproxy_globals.append(" quiet")
126 haproxy_globals.append(" spread-checks %d" %126 haproxy_globals.append(" spread-checks %d" %
127 config_data['global_spread_checks'])127 config_data['global_spread_checks'])
128 if config_data['global_stats_socket'] is True:
129 sock_path = "/var/run/haproxy/haproxy.sock"
130 haproxy_globals.append(" stats socket %s mode 0600" % sock_path)
128 return '\n'.join(haproxy_globals)131 return '\n'.join(haproxy_globals)
129132
130133
131134
=== modified file 'hooks/tests/test_helpers.py'
--- hooks/tests/test_helpers.py 2013-12-17 16:00:29 +0000
+++ hooks/tests/test_helpers.py 2014-01-16 19:00:01 +0000
@@ -23,9 +23,11 @@
23 'global_spread_checks': 234,23 'global_spread_checks': 234,
24 'global_debug': False,24 'global_debug': False,
25 'global_quiet': False,25 'global_quiet': False,
26 'global_stats_socket': True,
26 }27 }
27 result = hooks.create_haproxy_globals()28 result = hooks.create_haproxy_globals()
2829
30 sock_path = "/var/run/haproxy/haproxy.sock"
29 expected = '\n'.join([31 expected = '\n'.join([
30 'global',32 'global',
31 ' log foo-log',33 ' log foo-log',
@@ -34,6 +36,7 @@
34 ' user foo-user',36 ' user foo-user',
35 ' group foo-group',37 ' group foo-group',
36 ' spread-checks 234',38 ' spread-checks 234',
39 ' stats socket %s mode 0600' % sock_path,
37 ])40 ])
38 self.assertEqual(result, expected)41 self.assertEqual(result, expected)
3942
@@ -47,6 +50,7 @@
47 'global_spread_checks': 234,50 'global_spread_checks': 234,
48 'global_debug': True,51 'global_debug': True,
49 'global_quiet': True,52 'global_quiet': True,
53 'global_stats_socket': False,
50 }54 }
51 result = hooks.create_haproxy_globals()55 result = hooks.create_haproxy_globals()
5256

Subscribers

People subscribed via source and target branches