Merge lp:~johnsca/charms/trusty/haproxy/verterok-restart-rsyslog-after-install into lp:~verterok/charms/trusty/haproxy/restart-rsyslog-after-install

Proposed by Cory Johns
Status: Merged
Approved by: Guillermo Gonzalez
Approved revision: 106
Merged at revision: 106
Proposed branch: lp:~johnsca/charms/trusty/haproxy/verterok-restart-rsyslog-after-install
Merge into: lp:~verterok/charms/trusty/haproxy/restart-rsyslog-after-install
Diff against target: 53 lines (+10/-7)
3 files modified
hooks/hooks.py (+6/-6)
hooks/tests/test_website_hooks.py (+0/-1)
tests/10_deploy_test.py (+4/-0)
To merge this branch: bzr merge lp:~johnsca/charms/trusty/haproxy/verterok-restart-rsyslog-after-install
Reviewer Review Type Date Requested Status
Guillermo Gonzalez Approve
Review via email: mp+273867@code.launchpad.net

Description of the change

Fixed config-changed hook failure, test not catching aforementioned error, and minor lint issue.

To post a comment you must log in.
Revision history for this message
Guillermo Gonzalez (verterok) wrote :

apologize for the delay.

Thanks for fixing this. I worked on a different approach, which is to move the "if config_data.changed("global_log")" outside the if/else.

Will land this and merge my fix later on.

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py 2015-09-22 21:10:38 +0000
+++ hooks/hooks.py 2015-10-08 16:31:46 +0000
@@ -957,12 +957,12 @@
957 if not (get_listen_stanzas() == old_stanzas):957 if not (get_listen_stanzas() == old_stanzas):
958 notify_website()958 notify_website()
959 notify_peer()959 notify_peer()
960 if config_data.changed("global_log") or config_data.changed("source"):960 if config_data.changed("global_log") or config_data.changed("source"):
961 # restart rsyslog to pickup haproxy rsyslog config961 # restart rsyslog to pickup haproxy rsyslog config
962 # This could be removed once the following bug is fixed in the haproxy962 # This could be removed once the following bug is fixed in the
963 # package:963 # haproxy package:
964 # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=790871964 # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=790871
965 service_restart("rsyslog")965 service_restart("rsyslog")
966 else:966 else:
967 # XXX Ideally the config should be restored to a working state if the967 # XXX Ideally the config should be restored to a working state if the
968 # check fails, otherwise an inadvertent reload will cause the service968 # check fails, otherwise an inadvertent reload will cause the service
969969
=== modified file 'hooks/tests/test_website_hooks.py'
--- hooks/tests/test_website_hooks.py 2015-09-04 19:17:57 +0000
+++ hooks/tests/test_website_hooks.py 2015-10-08 16:31:46 +0000
@@ -147,4 +147,3 @@
147 all_services=""),147 all_services=""),
148 ])148 ])
149 self.log.assert_has_calls([call("No services configured, exiting.")])149 self.log.assert_has_calls([call("No services configured, exiting.")])
150
151150
=== modified file 'tests/10_deploy_test.py'
--- tests/10_deploy_test.py 2015-03-18 20:28:29 +0000
+++ tests/10_deploy_test.py 2015-10-08 16:31:46 +0000
@@ -105,6 +105,8 @@
105 'crts': ['DEFAULT'],105 'crts': ['DEFAULT'],
106 'servers': [['apache', apache_private, 80, 'maxconn 50']]}])106 'servers': [['apache', apache_private, 80, 'maxconn 50']]}])
107})107})
108time.sleep(10)
109d.sentry.wait(seconds)
108110
109# We need a retry loop here, since there's no way to tell when the new111# We need a retry loop here, since there's no way to tell when the new
110# configuration is in place.112# configuration is in place.
@@ -151,6 +153,8 @@
151 ['apache2', apache_private2, 80, 'maxconn 50']]}153 ['apache2', apache_private2, 80, 'maxconn 50']]}
152 ]}])154 ]}])
153})155})
156time.sleep(10)
157d.sentry.wait(seconds)
154158
155# Let's exercise our URL-based routing by trying to fetch a URL that will159# Let's exercise our URL-based routing by trying to fetch a URL that will
156# only work for the second apache unit (which is configured as server160# only work for the second apache unit (which is configured as server

Subscribers

People subscribed via source and target branches

to all changes: