Merge lp:~verterok/landscape-charm/support-postgresql-charm-v2-protocol into lp:landscape-charm

Proposed by Guillermo Gonzalez
Status: Merged
Approved by: Guillermo Gonzalez
Approved revision: 402
Merged at revision: 399
Proposed branch: lp:~verterok/landscape-charm/support-postgresql-charm-v2-protocol
Merge into: lp:landscape-charm
Diff against target: 296 lines (+78/-61)
9 files modified
lib/apt.py (+1/-1)
lib/callbacks/scripts.py (+1/-1)
lib/callbacks/tests/test_scripts.py (+11/-0)
lib/relations/postgresql.py (+30/-35)
lib/relations/tests/test_postgresql.py (+18/-17)
lib/services.py (+2/-1)
lib/tests/sample.py (+4/-3)
lib/tests/stubs.py (+1/-1)
lib/tests/test_services.py (+10/-2)
To merge this branch: bzr merge lp:~verterok/landscape-charm/support-postgresql-charm-v2-protocol
Reviewer Review Type Date Requested Status
Simon Poirier Approve
Landscape Builder test results Approve
Review via email: mp+370150@code.launchpad.net

This proposal supersedes a proposal from 2019-07-10.

Commit message

use postgresql-charm v2 protocol, manually parse the dsn as the available psycopg2 (2.6.x) doesn't provide parse_dsn (added in 2.7.x)

Description of the change

use postgresql-charm v2 protocol.
As we are using system packages for python deps, manually parse the dsn as the available psycopg2 (2.6.x) doesn't provide parse_dsn (added in 2.7.x).

This change is to be able to replace postgresql unit in the juju deploy with a external-services charm which acts as a proxy to connect to metal/external deployments of postgresql.

To post a comment you must log in.
Revision history for this message
Landscape Builder (landscape-builder) : Posted in a previous version of this proposal
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote : Posted in a previous version of this proposal
review: Needs Fixing (test results)
Revision history for this message
Guillermo Gonzalez (verterok) wrote : Posted in a previous version of this proposal

Oh, unittests...fancy. will fix and re-submit.

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Approve (test results)
402. By Guillermo Gonzalez on 2019-07-16

add test for PostgreSQLProvider

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Approve (test results)
Revision history for this message
Simon Poirier (simpoir) wrote :

+1 verified with clean deployment, and attached another franken-landscape using external-services

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/apt.py'
2--- lib/apt.py 2019-05-28 20:36:35 +0000
3+++ lib/apt.py 2019-07-16 16:09:37 +0000
4@@ -145,7 +145,7 @@
5 if install_sources and source:
6 raise SourceConflictError(install_sources, source)
7 if install_sources:
8- self._fetch.configure_sources()
9+ self._fetch.configure_sources(update=True)
10 return
11 if not source:
12 raise AptNoSourceConfigError()
13
14=== modified file 'lib/callbacks/scripts.py'
15--- lib/callbacks/scripts.py 2016-09-20 08:18:25 +0000
16+++ lib/callbacks/scripts.py 2019-07-16 16:09:37 +0000
17@@ -12,7 +12,7 @@
18 "source", "key", "ssl-cert", "ssl-key", "smtp-relay-host"}
19
20 # Database relation keys for which, in case of change, a restart is not needed.
21-NO_RESTART_DB_RELATION_KEYS = {"allowed-units", "state"}
22+NO_RESTART_DB_RELATION_KEYS = {"allowed-units"}
23
24
25 class ScriptCallback(ManagerCallback):
26
27=== modified file 'lib/callbacks/tests/test_scripts.py'
28--- lib/callbacks/tests/test_scripts.py 2016-09-16 10:23:48 +0000
29+++ lib/callbacks/tests/test_scripts.py 2019-07-16 16:09:37 +0000
30@@ -244,6 +244,17 @@
31 The 'lsctl' script is invoked if db connection details have changed.
32 """
33 old = SAMPLE_DB_UNIT_DATA.copy()
34+ # update sample data
35+ rel_data = {}
36+ for item in old["master"].split(' '):
37+ key, value = item.split('=')
38+ if key == 'host':
39+ assert value != "9.9.9.9"
40+ rel_data[key] = "9.9.9.9"
41+ else:
42+ rel_data[key] = value
43+ old["master"] = " ".join("{}={}".format(k, v)
44+ for k, v in rel_data.items())
45 assert old["host"] != "9.9.9.9"
46 old["host"] = "9.9.9.9"
47 update_persisted_data("db", old, hookenv=self.hookenv)
48
49=== modified file 'lib/relations/postgresql.py'
50--- lib/relations/postgresql.py 2015-02-03 10:26:59 +0000
51+++ lib/relations/postgresql.py 2019-07-16 16:09:37 +0000
52@@ -4,6 +4,18 @@
53 from lib.relations.errors import UnitDataNotReadyError
54
55
56+class PostgreSQLProvider(RelationContext):
57+ name = "db"
58+ interface = "pgsql"
59+
60+ def __init__(self, database=None):
61+ super(PostgreSQLProvider, self).__init__()
62+ self.database = database
63+
64+ def provide_data(self):
65+ return {"database": self.database}
66+
67+
68 class PostgreSQLRequirer(RelationContext):
69 """
70 Relation context for the `pgsql` interface.
71@@ -11,13 +23,8 @@
72 name = "db"
73 interface = "pgsql"
74 required_keys = [
75- "host",
76- "port",
77- 'user',
78- "password",
79- "database",
80- "allowed-units",
81- "state"]
82+ "master",
83+ "allowed-units"]
84
85 def __init__(self, hookenv=hookenv):
86 self._hookenv = hookenv
87@@ -37,7 +44,8 @@
88
89 try:
90 self._check_allowed_units(unit_data)
91- self._check_state(unit_data)
92+ if not unit_data.get('master'):
93+ return False
94 except UnitDataNotReadyError:
95 return False
96
97@@ -57,30 +65,17 @@
98 local_unit, allowed_units))
99 raise UnitDataNotReadyError()
100
101- def _check_state(self, unit_data):
102- """Check that the postgresql unit at hand is a master one.
103-
104- From postgresql charm's README.md: "If there is more than one
105- PostgreSQL unit related, the client charm must only use units
106- with state set to 'master' or 'hot standby'.".
107- """
108- ignored_states = set(["hot standby", "failover"])
109-
110- # XXX for now we support relating to at most one PostgreSQL service
111- # when we'll support sharding we'll want to account for more
112- # than one master.
113- relation_id = self._hookenv.relation_ids(self.name)[0]
114-
115- units_count = len(self._hookenv.related_units(relation_id))
116- if units_count > 1:
117- self._hookenv.log(
118- "The postgresql service is clustered with %s units. "
119- "Ignoring any intermittent 'standalone' states." % units_count)
120- ignored_states.add("standalone")
121-
122- state = unit_data["state"]
123- if state in ignored_states:
124- self._hookenv.log(
125- "Discarding postgresql unit with invalid state '%s' for "
126- "host %s." % (state, unit_data["host"]))
127- raise UnitDataNotReadyError()
128+ def get_data(self):
129+ super(PostgreSQLRequirer, self).get_data()
130+ for n in self.get(self.name, []):
131+ if n.get("master"):
132+ n.update(self.parse_dsn(n["master"]))
133+
134+ def parse_dsn(self, dsn):
135+ data = {}
136+ for v in dsn.split(' '):
137+ key, value = v.split("=")
138+ if key == 'port':
139+ value = int(value)
140+ data[key] = value
141+ return data
142
143=== modified file 'lib/relations/tests/test_postgresql.py'
144--- lib/relations/tests/test_postgresql.py 2015-02-02 13:59:10 +0000
145+++ lib/relations/tests/test_postgresql.py 2019-07-16 16:09:37 +0000
146@@ -1,6 +1,6 @@
147 from lib.tests.helpers import HookenvTest
148 from lib.tests.sample import SAMPLE_DB_UNIT_DATA
149-from lib.relations.postgresql import PostgreSQLRequirer
150+from lib.relations.postgresql import PostgreSQLRequirer, PostgreSQLProvider
151
152
153 class PostgreSQLRequirerTest(HookenvTest):
154@@ -14,8 +14,7 @@
155 ready.
156 """
157 self.assertEqual(
158- ["host", "port", "user", "password", "database",
159- "allowed-units", "state"],
160+ ["master", "allowed-units"],
161 PostgreSQLRequirer.required_keys)
162
163 def test_local_unit_in_allowed_units(self):
164@@ -25,6 +24,7 @@
165 """
166 unit_data = SAMPLE_DB_UNIT_DATA.copy()
167 unit_data["allowed-units"] = ""
168+ unit_data["master"] = "host=10.0.3.169"
169 self.hookenv.relations = {
170 "db": {
171 "db:1": {
172@@ -45,14 +45,12 @@
173 is not a 'master'.
174 """
175 unit_data1 = SAMPLE_DB_UNIT_DATA.copy()
176- unit_data1["host"] = "10.0.3.170"
177- unit_data1["state"] = "hot standby"
178+ unit_data1["standbys"] = "host=10.0.3.170\n"
179+ unit_data1.pop('master')
180 unit_data2 = SAMPLE_DB_UNIT_DATA.copy()
181- unit_data2["host"] = "10.0.3.169"
182- unit_data2["state"] = "master"
183+ unit_data2["master"] = "host=10.0.3.169"
184 unit_data3 = SAMPLE_DB_UNIT_DATA.copy()
185- unit_data3["host"] = "10.0.3.171"
186- unit_data3["state"] = "standalone"
187+ unit_data3["master"] = "host=10.0.3.171"
188 self.hookenv.relations = {
189 "db": {
190 "db:1": {
191@@ -64,12 +62,15 @@
192 }
193 relation = PostgreSQLRequirer(hookenv=self.hookenv)
194 self.assertTrue(relation.is_ready())
195- self.assertIn(
196- ("Discarding postgresql unit with invalid state 'hot standby' "
197- "for host 10.0.3.170.", None),
198- self.hookenv.messages)
199- self.assertIn(
200- ("Discarding postgresql unit with invalid state 'standalone' "
201- "for host 10.0.3.171.", None),
202- self.hookenv.messages)
203 self.assertEqual("10.0.3.169", relation["db"][0]["host"])
204+
205+
206+class PostgreSQLProviderTest(HookenvTest):
207+
208+ with_hookenv_monkey_patch = True
209+
210+ def test_scafolding(self):
211+ relation = PostgreSQLProvider(database="test")
212+ self.assertEqual({"database": "test"}, relation.provide_data())
213+ self.assertEqual("db", relation.name)
214+ self.assertEqual("pgsql", relation.interface)
215
216=== modified file 'lib/services.py'
217--- lib/services.py 2016-11-03 11:28:38 +0000
218+++ lib/services.py 2019-07-16 16:09:37 +0000
219@@ -9,7 +9,7 @@
220
221 from lib.hook import Hook
222 from lib.paths import default_paths
223-from lib.relations.postgresql import PostgreSQLRequirer
224+from lib.relations.postgresql import PostgreSQLRequirer, PostgreSQLProvider
225 from lib.relations.rabbitmq import RabbitMQRequirer, RabbitMQProvider
226 from lib.relations.haproxy import HAProxyProvider, HAProxyRequirer
227 from lib.relations.leader import LeaderProvider, LeaderRequirer
228@@ -57,6 +57,7 @@
229 HAProxyProvider(
230 config_requirer, hosted_requirer, paths=self._paths),
231 RabbitMQProvider(),
232+ PostgreSQLProvider(database="landscape"),
233 ],
234 # Required data is available to the render_template calls below.
235 "required_data": [
236
237=== modified file 'lib/tests/sample.py'
238--- lib/tests/sample.py 2015-11-05 17:51:36 +0000
239+++ lib/tests/sample.py 2019-07-16 16:09:37 +0000
240@@ -2,13 +2,14 @@
241
242
243 SAMPLE_DB_UNIT_DATA = {
244- "database": "all",
245 "allowed-units": "landscape-server/0",
246- "state": "standalone",
247+ "master": ("host=10.0.3.168 user=db_admin_1 password=sekret port=5432 "
248+ "database=all"),
249 "host": "10.0.3.168",
250 "user": "db_admin_1",
251 "password": "sekret",
252- "port": "5432"
253+ "port": "5432",
254+ "database": "all"
255 }
256
257 SAMPLE_LEADER_CONTEXT_DATA = {
258
259=== modified file 'lib/tests/stubs.py'
260--- lib/tests/stubs.py 2019-01-17 14:23:49 +0000
261+++ lib/tests/stubs.py 2019-07-16 16:09:37 +0000
262@@ -128,7 +128,7 @@
263 def apt_install(self, packages, options=None, fatal=False):
264 self.installed.append((packages, options, fatal))
265
266- def configure_sources(self):
267+ def configure_sources(self, update=False):
268 install_sources = self._config().get('install_sources')
269 install_keys = self._config().get('install_keys')
270 self.configured_sources.append((install_sources, install_keys))
271
272=== modified file 'lib/tests/test_services.py'
273--- lib/tests/test_services.py 2019-01-17 14:23:49 +0000
274+++ lib/tests/test_services.py 2019-07-16 16:09:37 +0000
275@@ -113,11 +113,19 @@
276 "config": config_expected,
277 "is_leader": True,
278 }
279-
280 for render in self.renders:
281 rendered_context = render[2]
282 for key in context.keys():
283- self.assertEqual(context[key], rendered_context[key])
284+ if key == 'db':
285+ # check that all keys are in the rendered_context
286+ expected = sorted(
287+ ['master', 'host', 'port', 'user', 'password',
288+ 'database', 'allowed-units'])
289+ self.assertEqual(expected,
290+ sorted(rendered_context[key][0].keys()))
291+ else:
292+ self.assertEqual(context[key], rendered_context[key])
293+
294 self.assertEqual(
295 ("service.conf", self.paths.service_conf()),
296 self.renders[0][:2])

Subscribers

People subscribed via source and target branches