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

[Guillermo Gonzalez]

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)

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.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 400
Branch: lp:~verterok/landscape-charm/support-postgresql-charm-v2-protocol
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3974/

review: Needs Fixing (test results)

[Guillermo Gonzalez (verterok)]

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

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 401
Branch: lp:~verterok/landscape-charm/support-postgresql-charm-v2-protocol
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3983/

review: Approve (test results)

[lp:~verterok/landscape-charm/support-postgresql-charm-v2-protocol]

402. By Guillermo Gonzalez on 2019-07-16

add test for PostgreSQLProvider

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 402
Branch: lp:~verterok/landscape-charm/support-postgresql-charm-v2-protocol
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3984/

review: Approve (test results)

[Simon Poirier (simpoir)]

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

review: Approve

=== modified file 'lib/apt.py'
--- lib/apt.py	2019-05-28 20:36:35 +0000
+++ lib/apt.py	2019-07-16 16:09:37 +0000
@@ -145,7 +145,7 @@
         if install_sources and source:
             raise SourceConflictError(install_sources, source)
         if install_sources:
-            self._fetch.configure_sources()
+            self._fetch.configure_sources(update=True)
             return
         if not source:
             raise AptNoSourceConfigError()
 
=== modified file 'lib/callbacks/scripts.py'
--- lib/callbacks/scripts.py	2016-09-20 08:18:25 +0000
+++ lib/callbacks/scripts.py	2019-07-16 16:09:37 +0000
@@ -12,7 +12,7 @@
     "source", "key", "ssl-cert", "ssl-key", "smtp-relay-host"}
 
 # Database relation keys for which, in case of change, a restart is not needed.
-NO_RESTART_DB_RELATION_KEYS = {"allowed-units", "state"}
+NO_RESTART_DB_RELATION_KEYS = {"allowed-units"}
 
 
 class ScriptCallback(ManagerCallback):
 
=== modified file 'lib/callbacks/tests/test_scripts.py'
--- lib/callbacks/tests/test_scripts.py	2016-09-16 10:23:48 +0000
+++ lib/callbacks/tests/test_scripts.py	2019-07-16 16:09:37 +0000
@@ -244,6 +244,17 @@
         The 'lsctl' script is invoked if db connection details have changed.
         """
         old = SAMPLE_DB_UNIT_DATA.copy()
+        # update sample data
+        rel_data = {}
+        for item in old["master"].split(' '):
+            key, value = item.split('=')
+            if key == 'host':
+                assert value != "9.9.9.9"
+                rel_data[key] = "9.9.9.9"
+            else:
+                rel_data[key] = value
+        old["master"] = " ".join("{}={}".format(k, v)
+                                 for k, v in rel_data.items())
         assert old["host"] != "9.9.9.9"
         old["host"] = "9.9.9.9"
         update_persisted_data("db", old, hookenv=self.hookenv)
 
=== modified file 'lib/relations/postgresql.py'
--- lib/relations/postgresql.py	2015-02-03 10:26:59 +0000
+++ lib/relations/postgresql.py	2019-07-16 16:09:37 +0000
@@ -4,6 +4,18 @@
 from lib.relations.errors import UnitDataNotReadyError
 
 
+class PostgreSQLProvider(RelationContext):
+    name = "db"
+    interface = "pgsql"
+
+    def __init__(self, database=None):
+        super(PostgreSQLProvider, self).__init__()
+        self.database = database
+
+    def provide_data(self):
+        return {"database": self.database}
+
+
 class PostgreSQLRequirer(RelationContext):
     """
     Relation context for the `pgsql` interface.
@@ -11,13 +23,8 @@
     name = "db"
     interface = "pgsql"
     required_keys = [
-        "host",
-        "port",
-        'user',
-        "password",
-        "database",
-        "allowed-units",
-        "state"]
+        "master",
+        "allowed-units"]
 
     def __init__(self, hookenv=hookenv):
         self._hookenv = hookenv
@@ -37,7 +44,8 @@
 
         try:
             self._check_allowed_units(unit_data)
-            self._check_state(unit_data)
+            if not unit_data.get('master'):
+                return False
         except UnitDataNotReadyError:
             return False
 
@@ -57,30 +65,17 @@
                 local_unit, allowed_units))
             raise UnitDataNotReadyError()
 
-    def _check_state(self, unit_data):
-        """Check that the postgresql unit at hand is a master one.
-
-        From postgresql charm's README.md: "If there is more than one
-        PostgreSQL unit related, the client charm must only use units
-        with state set to 'master' or 'hot standby'.".
-        """
-        ignored_states = set(["hot standby", "failover"])
-
-        # XXX for now we support relating to at most one PostgreSQL service
-        #     when we'll support sharding we'll want to account for more
-        #     than one master.
-        relation_id = self._hookenv.relation_ids(self.name)[0]
-
-        units_count = len(self._hookenv.related_units(relation_id))
-        if units_count > 1:
-            self._hookenv.log(
-                "The postgresql service is clustered with %s units. "
-                "Ignoring any intermittent 'standalone' states." % units_count)
-            ignored_states.add("standalone")
-
-        state = unit_data["state"]
-        if state in ignored_states:
-            self._hookenv.log(
-                "Discarding postgresql unit with invalid state '%s' for "
-                "host %s." % (state, unit_data["host"]))
-            raise UnitDataNotReadyError()
+    def get_data(self):
+        super(PostgreSQLRequirer, self).get_data()
+        for n in self.get(self.name, []):
+            if n.get("master"):
+                n.update(self.parse_dsn(n["master"]))
+
+    def parse_dsn(self, dsn):
+        data = {}
+        for v in dsn.split(' '):
+            key, value = v.split("=")
+            if key == 'port':
+                value = int(value)
+            data[key] = value
+        return data
 
=== modified file 'lib/relations/tests/test_postgresql.py'
--- lib/relations/tests/test_postgresql.py	2015-02-02 13:59:10 +0000
+++ lib/relations/tests/test_postgresql.py	2019-07-16 16:09:37 +0000
@@ -1,6 +1,6 @@
 from lib.tests.helpers import HookenvTest
 from lib.tests.sample import SAMPLE_DB_UNIT_DATA
-from lib.relations.postgresql import PostgreSQLRequirer
+from lib.relations.postgresql import PostgreSQLRequirer, PostgreSQLProvider
 
 
 class PostgreSQLRequirerTest(HookenvTest):
@@ -14,8 +14,7 @@
         ready.
         """
         self.assertEqual(
-            ["host", "port", "user", "password", "database",
-             "allowed-units", "state"],
+            ["master", "allowed-units"],
             PostgreSQLRequirer.required_keys)
 
     def test_local_unit_in_allowed_units(self):
@@ -25,6 +24,7 @@
         """
         unit_data = SAMPLE_DB_UNIT_DATA.copy()
         unit_data["allowed-units"] = ""
+        unit_data["master"] = "host=10.0.3.169"
         self.hookenv.relations = {
             "db": {
                 "db:1": {
@@ -45,14 +45,12 @@
         is not a 'master'.
         """
         unit_data1 = SAMPLE_DB_UNIT_DATA.copy()
-        unit_data1["host"] = "10.0.3.170"
-        unit_data1["state"] = "hot standby"
+        unit_data1["standbys"] = "host=10.0.3.170\n"
+        unit_data1.pop('master')
         unit_data2 = SAMPLE_DB_UNIT_DATA.copy()
-        unit_data2["host"] = "10.0.3.169"
-        unit_data2["state"] = "master"
+        unit_data2["master"] = "host=10.0.3.169"
         unit_data3 = SAMPLE_DB_UNIT_DATA.copy()
-        unit_data3["host"] = "10.0.3.171"
-        unit_data3["state"] = "standalone"
+        unit_data3["master"] = "host=10.0.3.171"
         self.hookenv.relations = {
             "db": {
                 "db:1": {
@@ -64,12 +62,15 @@
         }
         relation = PostgreSQLRequirer(hookenv=self.hookenv)
         self.assertTrue(relation.is_ready())
-        self.assertIn(
-            ("Discarding postgresql unit with invalid state 'hot standby' "
-             "for host 10.0.3.170.", None),
-            self.hookenv.messages)
-        self.assertIn(
-            ("Discarding postgresql unit with invalid state 'standalone' "
-             "for host 10.0.3.171.", None),
-            self.hookenv.messages)
         self.assertEqual("10.0.3.169", relation["db"][0]["host"])
+
+
+class PostgreSQLProviderTest(HookenvTest):
+
+    with_hookenv_monkey_patch = True
+
+    def test_scafolding(self):
+        relation = PostgreSQLProvider(database="test")
+        self.assertEqual({"database": "test"}, relation.provide_data())
+        self.assertEqual("db", relation.name)
+        self.assertEqual("pgsql", relation.interface)
 
=== modified file 'lib/services.py'
--- lib/services.py	2016-11-03 11:28:38 +0000
+++ lib/services.py	2019-07-16 16:09:37 +0000
@@ -9,7 +9,7 @@
 
 from lib.hook import Hook
 from lib.paths import default_paths
-from lib.relations.postgresql import PostgreSQLRequirer
+from lib.relations.postgresql import PostgreSQLRequirer, PostgreSQLProvider
 from lib.relations.rabbitmq import RabbitMQRequirer, RabbitMQProvider
 from lib.relations.haproxy import HAProxyProvider, HAProxyRequirer
 from lib.relations.leader import LeaderProvider, LeaderRequirer
@@ -57,6 +57,7 @@
                 HAProxyProvider(
                     config_requirer, hosted_requirer, paths=self._paths),
                 RabbitMQProvider(),
+                PostgreSQLProvider(database="landscape"),
             ],
             # Required data is available to the render_template calls below.
             "required_data": [
 
=== modified file 'lib/tests/sample.py'
--- lib/tests/sample.py	2015-11-05 17:51:36 +0000
+++ lib/tests/sample.py	2019-07-16 16:09:37 +0000
@@ -2,13 +2,14 @@
 
 
 SAMPLE_DB_UNIT_DATA = {
-    "database": "all",
     "allowed-units": "landscape-server/0",
-    "state": "standalone",
+    "master": ("host=10.0.3.168 user=db_admin_1 password=sekret port=5432 "
+               "database=all"),
     "host": "10.0.3.168",
     "user": "db_admin_1",
     "password": "sekret",
-    "port": "5432"
+    "port": "5432",
+    "database": "all"
 }
 
 SAMPLE_LEADER_CONTEXT_DATA = {
 
=== modified file 'lib/tests/stubs.py'
--- lib/tests/stubs.py	2019-01-17 14:23:49 +0000
+++ lib/tests/stubs.py	2019-07-16 16:09:37 +0000
@@ -128,7 +128,7 @@
     def apt_install(self, packages, options=None, fatal=False):
         self.installed.append((packages, options, fatal))
 
-    def configure_sources(self):
+    def configure_sources(self, update=False):
         install_sources = self._config().get('install_sources')
         install_keys = self._config().get('install_keys')
         self.configured_sources.append((install_sources, install_keys))
 
=== modified file 'lib/tests/test_services.py'
--- lib/tests/test_services.py	2019-01-17 14:23:49 +0000
+++ lib/tests/test_services.py	2019-07-16 16:09:37 +0000
@@ -113,11 +113,19 @@
             "config": config_expected,
             "is_leader": True,
         }
-
         for render in self.renders:
             rendered_context = render[2]
             for key in context.keys():
-                self.assertEqual(context[key], rendered_context[key])
+                if key == 'db':
+                    # check that all keys are in the rendered_context
+                    expected = sorted(
+                        ['master', 'host', 'port', 'user', 'password',
+                         'database', 'allowed-units'])
+                    self.assertEqual(expected,
+                                     sorted(rendered_context[key][0].keys()))
+                else:
+                    self.assertEqual(context[key], rendered_context[key])
+
         self.assertEqual(
             ("service.conf", self.paths.service_conf()),
             self.renders[0][:2])