MAAS

Merge ~ack/maas:postgres-notifier-1817484 into maas:master

  1. Git
  2. lp:~ack/maas
  3. postgres-notifier-1817484
  4. Merge into master
Proposed by Alberto Donato
Status: Merged
Approved by: Alberto Donato
Approved revision: 2c7acda9b451d0adf21a5e8ae01a49b29ba28c9e
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ack/maas:postgres-notifier-1817484
Merge into: maas:master
Diff against target: 265 lines (+81/-36)
8 files modified
src/maasserver/config.py (+16/-0)
src/maasserver/djangosettings/settings.py (+6/-0)
src/maasserver/listener.py (+4/-0)
src/maasserver/management/commands/tests/test_config.py (+7/-2)
src/maasserver/region_controller.py (+9/-5)
src/maasserver/tests/test_config.py (+12/-1)
src/maasserver/tests/test_listener.py (+17/-0)
src/maasserver/tests/test_region_controller.py (+10/-28)
Reviewer Review Type Date Requested Status
MAAS Lander Needs Fixing
Blake Rouse (community) Approve
Newell Jensen (community) Approve
Review via email: mp+363768@code.launchpad.net

Commit message

LP: #1817484 - add keepalive for Postgres connections and connect/disconnect events for the listener

To post a comment you must log in.
Revision history for this message
Newell Jensen (newell-jensen) wrote :

LGTM syntax wise. Did not test it myself.

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b postgres-notifier-1817484 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: b705ee247aafa8ba6d3b891a5c5e1ab6490d8d90

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b postgres-notifier-1817484 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5231/console
COMMIT: 4c1b3df86c1d00efeb63a5c816078e7417099467

review: Needs Fixing
Revision history for this message
Blake Rouse (blake-rouse) wrote :

Looks good.

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b postgres-notifier-1817484 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5236/consoleText

Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b postgres-notifier-1817484 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5239/consoleText

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b postgres-notifier-1817484 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5240/console
COMMIT: fc8d35a8cbb3dff7b175e2ffc9833082702b9e9b

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b postgres-notifier-1817484 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5248/consoleText

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b postgres-notifier-1817484 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5249/console
COMMIT: 2c7acda9b451d0adf21a5e8ae01a49b29ba28c9e

review: Needs Fixing

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/maasserver/config.py b/src/maasserver/config.py
index 0b613bd..1eabe0d 100644
--- a/src/maasserver/config.py
+++ b/src/maasserver/config.py
@@ -58,6 +58,22 @@ class RegionConfiguration(Configuration, metaclass=RegionConfigurationMeta):
58 "database_conn_max_age",58 "database_conn_max_age",
59 "The lifetime of a database connection, in seconds.",59 "The lifetime of a database connection, in seconds.",
60 Int(if_missing=(5 * 60), accept_python=False, min=0))60 Int(if_missing=(5 * 60), accept_python=False, min=0))
61 database_keepalive = ConfigurationOption(
62 "database_keepalive",
63 "Whether keepalive for database connections is enabled.",
64 StringBool(if_missing=True))
65 database_keepalive_idle = ConfigurationOption(
66 "database_keepalive_idle",
67 "Time (in seconds) after which keepalives will be started.",
68 Int(if_missing=15))
69 database_keepalive_interval = ConfigurationOption(
70 "database_keepalive_interval",
71 "Interval (in seconds) between keepaliveds.",
72 Int(if_missing=15))
73 database_keepalive_count = ConfigurationOption(
74 "database_keepalive_count",
75 "Number of keeaplives that can be lost before connection is reset.",
76 Int(if_missing=2))
6177
62 # Worker options.78 # Worker options.
63 num_workers = ConfigurationOption(79 num_workers = ConfigurationOption(
diff --git a/src/maasserver/djangosettings/settings.py b/src/maasserver/djangosettings/settings.py
index 28a0308..172cbaf 100644
--- a/src/maasserver/djangosettings/settings.py
+++ b/src/maasserver/djangosettings/settings.py
@@ -115,6 +115,12 @@ try:
115 'HOST': config.database_host,115 'HOST': config.database_host,
116 'PORT': str(config.database_port),116 'PORT': str(config.database_port),
117 'CONN_MAX_AGE': config.database_conn_max_age,117 'CONN_MAX_AGE': config.database_conn_max_age,
118 'OPTIONS': {
119 'keepalives': int(config.database_keepalive),
120 'keepalives_idle': config.database_keepalive_idle,
121 'keepalives_interval': config.database_keepalive_interval,
122 'keepalives_count': config.database_keepalive_count
123 }
118 }124 }
119 }125 }
120 DEBUG = config.debug126 DEBUG = config.debug
diff --git a/src/maasserver/listener.py b/src/maasserver/listener.py
index 1e4d669..e30b1c9 100644
--- a/src/maasserver/listener.py
+++ b/src/maasserver/listener.py
@@ -15,6 +15,7 @@ from errno import ENOENT
15from django.db import connections15from django.db import connections
16from django.db.utils import load_backend16from django.db.utils import load_backend
17from provisioningserver.utils.enum import map_enum17from provisioningserver.utils.enum import map_enum
18from provisioningserver.utils.events import EventGroup
18from provisioningserver.utils.twisted import (19from provisioningserver.utils.twisted import (
19 callOut,20 callOut,
20 suppress,21 suppress,
@@ -98,6 +99,7 @@ class PostgresListenerService(Service, object):
98 self.disconnecting = None99 self.disconnecting = None
99 self.registeredChannels = False100 self.registeredChannels = False
100 self.log = Logger(__name__, self)101 self.log = Logger(__name__, self)
102 self.events = EventGroup("connected", "disconnected")
101103
102 def startService(self):104 def startService(self):
103 """Start the listener."""105 """Start the listener."""
@@ -300,6 +302,7 @@ class PostgresListenerService(Service, object):
300 def connect(interval=self.HANDLE_NOTIFY_DELAY):302 def connect(interval=self.HANDLE_NOTIFY_DELAY):
301 d = deferToThread(self.startConnection)303 d = deferToThread(self.startConnection)
302 d.addCallback(callOut, deferToThread, self.registerChannels)304 d.addCallback(callOut, deferToThread, self.registerChannels)
305 d.addCallback(callOut, self.events.connected.fire)
303 d.addCallback(callOut, self.startReading)306 d.addCallback(callOut, self.startReading)
304 d.addCallback(callOut, self.runHandleNotify, interval)307 d.addCallback(callOut, self.runHandleNotify, interval)
305 # On failure ensure that the database connection is stopped.308 # On failure ensure that the database connection is stopped.
@@ -350,6 +353,7 @@ class PostgresListenerService(Service, object):
350 self.log.failure("Connection lost.", reason)353 self.log.failure("Connection lost.", reason)
351 if self.autoReconnect:354 if self.autoReconnect:
352 reactor.callLater(3, self.tryConnection)355 reactor.callLater(3, self.tryConnection)
356 self.events.disconnected.fire(reason)
353357
354 def registerChannel(self, channel):358 def registerChannel(self, channel):
355 """Register the channel."""359 """Register the channel."""
diff --git a/src/maasserver/management/commands/tests/test_config.py b/src/maasserver/management/commands/tests/test_config.py
index 63dc5ea..e738265 100644
--- a/src/maasserver/management/commands/tests/test_config.py
+++ b/src/maasserver/management/commands/tests/test_config.py
@@ -116,11 +116,16 @@ class TestConfigurationSet(MAASTestCase):
116 # Set the option to a random value.116 # Set the option to a random value.
117 if self.option == "database_port":117 if self.option == "database_port":
118 value = factory.pick_port()118 value = factory.pick_port()
119 elif self.option == "database_conn_max_age":119 elif self.option in (
120 "database_conn_max_age", "database_keepalive_count",
121 "database_keepalive_interval",
122 "database_keepalive_idle"):
120 value = random.randint(0, 60)123 value = random.randint(0, 60)
121 elif self.option == "num_workers":124 elif self.option == "num_workers":
122 value = random.randint(1, 16)125 value = random.randint(1, 16)
123 elif self.option in ["debug", "debug_queries", "debug_http"]:126 elif self.option in [
127 "debug", "debug_queries", "debug_http",
128 "database_keepalive"]:
124 value = random.choice(['true', 'false'])129 value = random.choice(['true', 'false'])
125 else:130 else:
126 value = factory.make_name("foobar")131 value = factory.make_name("foobar")
diff --git a/src/maasserver/region_controller.py b/src/maasserver/region_controller.py
index 4087a73..9a41c72 100644
--- a/src/maasserver/region_controller.py
+++ b/src/maasserver/region_controller.py
@@ -119,16 +119,15 @@ class RegionControllerService(Service):
119 self.postgresListener.register("sys_dns", self.markDNSForUpdate)119 self.postgresListener.register("sys_dns", self.markDNSForUpdate)
120 self.postgresListener.register("sys_proxy", self.markProxyForUpdate)120 self.postgresListener.register("sys_proxy", self.markProxyForUpdate)
121 self.postgresListener.register("sys_rbac", self.markRBACForUpdate)121 self.postgresListener.register("sys_rbac", self.markRBACForUpdate)
122122 self.postgresListener.events.connected.registerHandler(
123 # Update DNS and proxy on first start.123 self.markAllForUpdate)
124 self.markDNSForUpdate(None, None)
125 self.markProxyForUpdate(None, None)
126 self.markRBACForUpdate(None, None)
127124
128 @asynchronous(timeout=FOREVER)125 @asynchronous(timeout=FOREVER)
129 def stopService(self):126 def stopService(self):
130 """Close the controller."""127 """Close the controller."""
131 super(RegionControllerService, self).stopService()128 super(RegionControllerService, self).stopService()
129 self.postgresListener.events.connected.unregisterHandler(
130 self.markAllForUpdate)
132 self.postgresListener.unregister("sys_dns", self.markDNSForUpdate)131 self.postgresListener.unregister("sys_dns", self.markDNSForUpdate)
133 self.postgresListener.unregister("sys_proxy", self.markProxyForUpdate)132 self.postgresListener.unregister("sys_proxy", self.markProxyForUpdate)
134 self.postgresListener.unregister("sys_rbac", self.markRBACForUpdate)133 self.postgresListener.unregister("sys_rbac", self.markRBACForUpdate)
@@ -137,6 +136,11 @@ class RegionControllerService(Service):
137 self.processing.stop()136 self.processing.stop()
138 return d137 return d
139138
139 def markAllForUpdate(self):
140 self.markDNSForUpdate(None, None)
141 self.markProxyForUpdate(None, None)
142 self.markRBACForUpdate(None, None)
143
140 def markDNSForUpdate(self, channel, message):144 def markDNSForUpdate(self, channel, message):
141 """Called when the `sys_dns` message is received."""145 """Called when the `sys_dns` message is received."""
142 self.needsDNSUpdate = True146 self.needsDNSUpdate = True
diff --git a/src/maasserver/tests/test_config.py b/src/maasserver/tests/test_config.py
index 19edf83..1878e10 100644
--- a/src/maasserver/tests/test_config.py
+++ b/src/maasserver/tests/test_config.py
@@ -74,6 +74,10 @@ class TestRegionConfigurationDatabaseOptions(MAASTestCase):
74 "database_user": "maas",74 "database_user": "maas",
75 "database_pass": "",75 "database_pass": "",
76 "database_conn_max_age": 5 * 60,76 "database_conn_max_age": 5 * 60,
77 "database_keepalive": True,
78 "database_keepalive_idle": 15,
79 "database_keepalive_interval": 15,
80 "database_keepalive_count": 2
77 }81 }
7882
79 scenarios = tuple(83 scenarios = tuple(
@@ -89,6 +93,8 @@ class TestRegionConfigurationDatabaseOptions(MAASTestCase):
89 config = RegionConfiguration({})93 config = RegionConfiguration({})
90 if isinstance(getattr(config, self.option), str):94 if isinstance(getattr(config, self.option), str):
91 example_value = factory.make_name(self.option)95 example_value = factory.make_name(self.option)
96 elif self.option == 'database_keepalive':
97 example_value = random.choice(['true', 'false'])
92 else:98 else:
93 example_value = factory.pick_port()99 example_value = factory.pick_port()
94 # Argument values will most often be passed in from the command-line,100 # Argument values will most often be passed in from the command-line,
@@ -96,7 +102,12 @@ class TestRegionConfigurationDatabaseOptions(MAASTestCase):
96 setattr(config, self.option, str(example_value))102 setattr(config, self.option, str(example_value))
97 self.assertEqual(example_value, getattr(config, self.option))103 self.assertEqual(example_value, getattr(config, self.option))
98 # It's also stored in the configuration database.104 # It's also stored in the configuration database.
99 self.assertEqual({self.option: example_value}, config.store)105 expected_value = example_value
106 if example_value == 'true':
107 expected_value = True
108 elif expected_value == 'false':
109 expected_value = False
110 self.assertEqual({self.option: expected_value}, config.store)
100111
101112
102class TestRegionConfigurationWorkerOptions(MAASTestCase):113class TestRegionConfigurationWorkerOptions(MAASTestCase):
diff --git a/src/maasserver/tests/test_listener.py b/src/maasserver/tests/test_listener.py
index e58118c..fdbbb10 100644
--- a/src/maasserver/tests/test_listener.py
+++ b/src/maasserver/tests/test_listener.py
@@ -111,6 +111,23 @@ class TestPostgresListenerService(MAASServerTestCase):
111111
112 @wait_for_reactor112 @wait_for_reactor
113 @inlineCallbacks113 @inlineCallbacks
114 def test_event_on_connection(self):
115 listener = PostgresListenerService()
116 start_calls = []
117 stop_calls = []
118 listener.events.connected.registerHandler(
119 lambda: start_calls.append(True))
120 listener.events.disconnected.registerHandler(
121 lambda reason: stop_calls.append(reason))
122 yield listener.startService()
123 self.assertEqual(len(start_calls), 1)
124 yield listener.stopService()
125 self.assertEqual(len(stop_calls), 1)
126 [failure] = stop_calls
127 self.assertIsInstance(failure.value, error.ConnectionDone)
128
129 @wait_for_reactor
130 @inlineCallbacks
114 def test__handles_missing_system_handler_on_notification(self):131 def test__handles_missing_system_handler_on_notification(self):
115 # Captured notifications from the database will go here.132 # Captured notifications from the database will go here.
116 notices = DeferredQueue()133 notices = DeferredQueue()
diff --git a/src/maasserver/tests/test_region_controller.py b/src/maasserver/tests/test_region_controller.py
index 6b67be8..2c109db 100644
--- a/src/maasserver/tests/test_region_controller.py
+++ b/src/maasserver/tests/test_region_controller.py
@@ -43,6 +43,7 @@ from maastesting.matchers import (
43 MockCallsMatch,43 MockCallsMatch,
44 MockNotCalled,44 MockNotCalled,
45)45)
46from provisioningserver.utils.events import Event
46from testtools import ExpectedException47from testtools import ExpectedException
47from testtools.matchers import MatchesStructure48from testtools.matchers import MatchesStructure
48from twisted.internet import reactor49from twisted.internet import reactor
@@ -92,37 +93,18 @@ class TestRegionControllerService(MAASServerTestCase):
92 call("sys_proxy", service.markProxyForUpdate),93 call("sys_proxy", service.markProxyForUpdate),
93 call("sys_rbac", service.markRBACForUpdate)))94 call("sys_rbac", service.markRBACForUpdate)))
9495
95 @wait_for_reactor96 def test_startService_markAllForUpdate_on_connect(self):
96 @inlineCallbacks
97 def test_startService_calls_markDNSForUpdate(self):
98 listener = MagicMock()
99 service = self.make_service(listener)
100 mock_markDNSForUpdate = self.patch(service, "markDNSForUpdate")
101 service.startService()
102 yield service.processingDefer
103 self.assertThat(mock_markDNSForUpdate, MockCalledOnceWith(None, None))
104
105 @wait_for_reactor
106 @inlineCallbacks
107 def test_startService_calls_markProxyForUpdate(self):
108 listener = MagicMock()97 listener = MagicMock()
98 listener.events.connected = Event()
109 service = self.make_service(listener)99 service = self.make_service(listener)
110 mock_markProxyForUpdate = self.patch(service, "markProxyForUpdate")100 mock_mark_dns_for_update = self.patch(service, 'markDNSForUpdate')
101 mock_mark_rbac_for_update = self.patch(service, 'markRBACForUpdate')
102 mock_mark_proxy_for_update = self.patch(service, 'markProxyForUpdate')
111 service.startService()103 service.startService()
112 yield service.processingDefer104 service.postgresListener.events.connected.fire()
113 self.assertThat(105 mock_mark_dns_for_update.assert_called_once()
114 mock_markProxyForUpdate, MockCalledOnceWith(None, None))106 mock_mark_rbac_for_update.assert_called_once()
115107 mock_mark_proxy_for_update.assert_called_once()
116 @wait_for_reactor
117 @inlineCallbacks
118 def test_startService_calls_markRBACForUpdate(self):
119 listener = MagicMock()
120 service = self.make_service(listener)
121 mock_markRBACForUpdate = self.patch(service, "markRBACForUpdate")
122 service.startService()
123 yield service.processingDefer
124 self.assertThat(
125 mock_markRBACForUpdate, MockCalledOnceWith(None, None))
126108
127 def test_stopService_calls_unregister_on_the_listener(self):109 def test_stopService_calls_unregister_on_the_listener(self):
128 listener = MagicMock()110 listener = MagicMock()

Subscribers

People subscribed via source and target branches