launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29979
[Merge] ~cjwatson/launchpad-layers:db-layer into launchpad-layers:main
Colin Watson has proposed merging ~cjwatson/launchpad-layers:db-layer into launchpad-layers:main with ~cjwatson/launchpad-layers:payload-layer as a prerequisite.
Commit message:
Split out a launchpad-db layer
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-layers/+git/launchpad-layers/+merge/442155
`launchpad-loggerhead` needs most of the `launchpad-base` layer, but doesn't have any direct database connectivity, so we don't want to require it to have a `db` relation before it will do anything. Split out database interaction into a separate layer.
This will require applications to change: they will need to import some functions from `charms.launchpad.db` rather than `charms.launchpad.base`, they will need to react to `launchpad.db.configured` rather than `launchpad.base.configured`, and their LAZR configuration files will need to extend `launchpad-db-lazr.conf` rather than `launchpad-base-lazr.conf`.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-layers:db-layer into launchpad-layers:main.
diff --git a/launchpad-base/config.yaml b/launchpad-base/config.yaml
index b804392..3263531 100644
--- a/launchpad-base/config.yaml
+++ b/launchpad-base/config.yaml
@@ -43,17 +43,6 @@ options:
type: string
description: URL of file used to control whether cron scripts run.
default: "file:cronscripts.ini"
- databases:
- type: string
- description: >
- YAML-encoded information about database relations, overriding the
- defaults. For example, setting this to "db: {name: launchpad_prod}"
- changes the name of the database used by the "db" relation.
- default: ""
- db_statement_timeout:
- type: int
- description: SQL statement timeout in milliseconds.
- default:
default_batch_size:
type: int
description: The default size used in a batched listing of results.
diff --git a/launchpad-base/layer.yaml b/launchpad-base/layer.yaml
index 5abc6ac..ed3e8dc 100644
--- a/launchpad-base/layer.yaml
+++ b/launchpad-base/layer.yaml
@@ -1,6 +1,5 @@
includes:
- layer:basic
- - layer:ols-pg
- interface:rabbitmq
- layer:launchpad-payload
repo: https://git.launchpad.net/launchpad-layers
diff --git a/launchpad-base/lib/charms/launchpad/base.py b/launchpad-base/lib/charms/launchpad/base.py
index 2d441de..043cd4f 100644
--- a/launchpad-base/lib/charms/launchpad/base.py
+++ b/launchpad-base/lib/charms/launchpad/base.py
@@ -1,22 +1,16 @@
# Copyright 2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-import grp
import os.path
-import pwd
-import re
import subprocess
-from dataclasses import dataclass
from email.utils import parseaddr
from urllib.parse import urlparse
from charmhelpers.core import hookenv, host, templating
from charms.launchpad.payload import config_file_path
from charms.launchpad.payload import get_service_config as get_payload_config
-from charms.launchpad.payload import home_dir
from charms.reactive import endpoint_from_flag
from ols import base
-from psycopg2.extensions import make_dsn, parse_dsn
def oopses_dir():
@@ -136,107 +130,3 @@ def configure_rsync(config, template, name):
templating.render(template, rsync_path, config, perms=0o644)
elif os.path.exists(rsync_path):
os.unlink(rsync_path)
-
-
-@dataclass
-class PgPassLine:
- hostname: str
- port: str
- database: str
- username: str
- password: str
-
-
-def update_pgpass(dsn):
- # See https://www.postgresql.org/docs/current/libpq-pgpass.html.
-
- def unescape(entry):
- return re.sub(r"\\(.)", r"\1", entry)
-
- def escape(entry):
- return re.sub(r"([:\\])", r"\\\1", entry)
-
- parsed_dsn = parse_dsn(dsn)
- pgpass_path = os.path.join(home_dir(), ".pgpass")
- pgpass = []
- try:
- with open(pgpass_path) as f:
- for line in f:
- if line.startswith("#"):
- continue
- match = re.match(
- r"""
- ^
- (?P<hostname>(?:[^:\\]|\\.)*):
- (?P<port>(?:[^:\\]|\\.)*):
- (?P<database>(?:[^:\\]|\\.)*):
- (?P<username>(?:[^:\\]|\\.)*):
- (?P<password>(?:[^:\\]|\\.)*)
- $
- """,
- line.rstrip("\n"),
- flags=re.X,
- )
- if match is None:
- continue
- pgpass.append(
- PgPassLine(
- hostname=unescape(match.group("hostname")),
- port=unescape(match.group("port")),
- database=unescape(match.group("database")),
- username=unescape(match.group("username")),
- password=unescape(match.group("password")),
- )
- )
- except OSError:
- pass
-
- modified = False
- for line in pgpass:
- if (
- line.hostname in ("*", parsed_dsn["host"])
- and line.port in ("*", parsed_dsn["port"])
- and line.database in ("*", parsed_dsn["dbname"])
- and line.username in ("*", parsed_dsn["user"])
- ):
- if line.password != parsed_dsn["password"]:
- line.password = parsed_dsn["password"]
- modified = True
- break
- else:
- pgpass.append(
- PgPassLine(
- hostname=parsed_dsn["host"],
- port=parsed_dsn["port"],
- database=parsed_dsn["dbname"],
- username=parsed_dsn["user"],
- password=parsed_dsn["password"],
- )
- )
- modified = True
-
- if modified:
- uid = pwd.getpwnam(base.user()).pw_uid
- gid = grp.getgrnam(base.user()).gr_gid
- with open(pgpass_path, "w") as f:
- for line in pgpass:
- print(
- ":".join(
- [
- escape(line.hostname),
- escape(line.port),
- escape(line.database),
- escape(line.username),
- escape(line.password),
- ]
- ),
- file=f,
- )
- os.fchown(f.fileno(), uid, gid)
- os.fchmod(f.fileno(), 0o600)
-
-
-def strip_dsn_authentication(dsn):
- parsed_dsn = parse_dsn(dsn)
- parsed_dsn.pop("password", None)
- return make_dsn(**parsed_dsn)
diff --git a/launchpad-base/metadata.yaml b/launchpad-base/metadata.yaml
index eafdcc0..f2480ad 100644
--- a/launchpad-base/metadata.yaml
+++ b/launchpad-base/metadata.yaml
@@ -1,5 +1,3 @@
requires:
- db:
- interface: pgsql
rabbitmq:
interface: rabbitmq
diff --git a/launchpad-base/reactive/launchpad-base.py b/launchpad-base/reactive/launchpad-base.py
index 186e7df..d8e3376 100644
--- a/launchpad-base/reactive/launchpad-base.py
+++ b/launchpad-base/reactive/launchpad-base.py
@@ -7,8 +7,6 @@ from charms.launchpad.base import (
configure_rsync,
ensure_lp_directories,
get_service_config,
- strip_dsn_authentication,
- update_pgpass,
)
from charms.launchpad.payload import configure_lazr
from charms.reactive import (
@@ -22,7 +20,7 @@ from charms.reactive import (
when_not,
when_not_all,
)
-from ols import base, postgres
+from ols import base
@when("rabbitmq.connected")
@@ -59,30 +57,15 @@ def rabbitmq_unavailable():
clear_flag("launchpad.rabbitmq.available")
-@when(
- "launchpad.payload.configured",
- "db.master.available",
- "launchpad.rabbitmq.available",
-)
+@when("launchpad.payload.configured", "launchpad.rabbitmq.available")
@when_not("launchpad.base.configured")
def configure():
- db = endpoint_from_flag("db.master.available")
rabbitmq = endpoint_from_flag("rabbitmq.available")
# Interactive use shouldn't be frequent, but it's still needed
# sometimes, so make it less annoying.
change_shell(base.user(), "/bin/bash")
ensure_lp_directories()
config = get_service_config()
- db_primary, db_standby = postgres.get_db_uris(db)
- # XXX cjwatson 2022-09-23: Mangle the connection strings into forms
- # Launchpad understands. In the long term it would be better to have
- # Launchpad be able to consume unmodified connection strings.
- for dsn in [db_primary] + db_standby:
- update_pgpass(dsn)
- config["db_primary"] = strip_dsn_authentication(db_primary)
- config["db_standby"] = ",".join(
- strip_dsn_authentication(dsn) for dsn in db_standby
- )
config["rabbitmq_broker_urls"] = sorted(get_rabbitmq_uris(rabbitmq))
configure_lazr(
config,
@@ -102,30 +85,16 @@ def configure():
@when("launchpad.base.configured")
-@when_not_all(
- "launchpad.payload.configured",
- "db.master.available",
- "launchpad.rabbitmq.available",
-)
+@when_not_all("launchpad.payload.configured", "launchpad.rabbitmq.available")
def deconfigure():
clear_flag("launchpad.base.configured")
- clear_flag("service.configured")
@when("config.changed")
def config_changed():
clear_flag("launchpad.base.configured")
- clear_flag("service.configured")
-
-
-@when("db.database.changed", "launchpad.base.configured")
-def db_changed():
- clear_flag("launchpad.base.configured")
- clear_flag("service.configured")
- clear_flag("db.database.changed")
@hook("{requires:rabbitmq}-relation-changed")
def rabbitmq_relation_changed(*args):
clear_flag("launchpad.base.configured")
- clear_flag("service.configured")
diff --git a/launchpad-base/templates/launchpad-base-lazr.conf b/launchpad-base/templates/launchpad-base-lazr.conf
index b9707cb..a73f081 100644
--- a/launchpad-base/templates/launchpad-base-lazr.conf
+++ b/launchpad-base/templates/launchpad-base-lazr.conf
@@ -44,10 +44,6 @@ git_ssh_root: git+ssh://{{ domain_git }}/
{%- endif %}
[database]
-{{- opt("db_statement_timeout", db_statement_timeout) }}
-rw_main_primary: {{ db_primary }}
-rw_main_standby: {{ db_standby or db_primary }}
-set_role_after_connecting: True
{{- opt("soft_request_timeout", soft_request_timeout) }}
[error_reports]
diff --git a/launchpad-db/config.yaml b/launchpad-db/config.yaml
new file mode 100644
index 0000000..cd088e2
--- /dev/null
+++ b/launchpad-db/config.yaml
@@ -0,0 +1,12 @@
+options:
+ databases:
+ type: string
+ description: >
+ YAML-encoded information about database relations, overriding the
+ defaults. For example, setting this to "db: {name: launchpad_prod}"
+ changes the name of the database used by the "db" relation.
+ default: ""
+ db_statement_timeout:
+ type: int
+ description: SQL statement timeout in milliseconds.
+ default:
diff --git a/launchpad-db/layer.yaml b/launchpad-db/layer.yaml
new file mode 100644
index 0000000..3ea5d6a
--- /dev/null
+++ b/launchpad-db/layer.yaml
@@ -0,0 +1,4 @@
+includes:
+ - layer:launchpad-base
+ - layer:ols-pg
+repo: https://git.launchpad.net/launchpad-layers
diff --git a/launchpad-db/lib/charms/launchpad/db.py b/launchpad-db/lib/charms/launchpad/db.py
new file mode 100644
index 0000000..22e1ead
--- /dev/null
+++ b/launchpad-db/lib/charms/launchpad/db.py
@@ -0,0 +1,121 @@
+# Copyright 2022-2023 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import grp
+import os.path
+import pwd
+import re
+from dataclasses import dataclass
+
+from charms.launchpad.base import lazr_config_files as base_config_files
+from charms.launchpad.payload import config_file_path, home_dir
+from ols import base
+from psycopg2.extensions import make_dsn, parse_dsn
+
+
+def lazr_config_files():
+ return base_config_files() + [config_file_path("launchpad-db-lazr.conf")]
+
+
+@dataclass
+class PgPassLine:
+ hostname: str
+ port: str
+ database: str
+ username: str
+ password: str
+
+
+def update_pgpass(dsn):
+ # See https://www.postgresql.org/docs/current/libpq-pgpass.html.
+
+ def unescape(entry):
+ return re.sub(r"\\(.)", r"\1", entry)
+
+ def escape(entry):
+ return re.sub(r"([:\\])", r"\\\1", entry)
+
+ parsed_dsn = parse_dsn(dsn)
+ pgpass_path = os.path.join(home_dir(), ".pgpass")
+ pgpass = []
+ try:
+ with open(pgpass_path) as f:
+ for line in f:
+ if line.startswith("#"):
+ continue
+ match = re.match(
+ r"""
+ ^
+ (?P<hostname>(?:[^:\\]|\\.)*):
+ (?P<port>(?:[^:\\]|\\.)*):
+ (?P<database>(?:[^:\\]|\\.)*):
+ (?P<username>(?:[^:\\]|\\.)*):
+ (?P<password>(?:[^:\\]|\\.)*)
+ $
+ """,
+ line.rstrip("\n"),
+ flags=re.X,
+ )
+ if match is None:
+ continue
+ pgpass.append(
+ PgPassLine(
+ hostname=unescape(match.group("hostname")),
+ port=unescape(match.group("port")),
+ database=unescape(match.group("database")),
+ username=unescape(match.group("username")),
+ password=unescape(match.group("password")),
+ )
+ )
+ except OSError:
+ pass
+
+ modified = False
+ for line in pgpass:
+ if (
+ line.hostname in ("*", parsed_dsn["host"])
+ and line.port in ("*", parsed_dsn["port"])
+ and line.database in ("*", parsed_dsn["dbname"])
+ and line.username in ("*", parsed_dsn["user"])
+ ):
+ if line.password != parsed_dsn["password"]:
+ line.password = parsed_dsn["password"]
+ modified = True
+ break
+ else:
+ pgpass.append(
+ PgPassLine(
+ hostname=parsed_dsn["host"],
+ port=parsed_dsn["port"],
+ database=parsed_dsn["dbname"],
+ username=parsed_dsn["user"],
+ password=parsed_dsn["password"],
+ )
+ )
+ modified = True
+
+ if modified:
+ uid = pwd.getpwnam(base.user()).pw_uid
+ gid = grp.getgrnam(base.user()).gr_gid
+ with open(pgpass_path, "w") as f:
+ for line in pgpass:
+ print(
+ ":".join(
+ [
+ escape(line.hostname),
+ escape(line.port),
+ escape(line.database),
+ escape(line.username),
+ escape(line.password),
+ ]
+ ),
+ file=f,
+ )
+ os.fchown(f.fileno(), uid, gid)
+ os.fchmod(f.fileno(), 0o600)
+
+
+def strip_dsn_authentication(dsn):
+ parsed_dsn = parse_dsn(dsn)
+ parsed_dsn.pop("password", None)
+ return make_dsn(**parsed_dsn)
diff --git a/launchpad-db/metadata.yaml b/launchpad-db/metadata.yaml
new file mode 100644
index 0000000..6de4a93
--- /dev/null
+++ b/launchpad-db/metadata.yaml
@@ -0,0 +1,3 @@
+requires:
+ db:
+ interface: pgsql
diff --git a/launchpad-db/reactive/launchpad-db.py b/launchpad-db/reactive/launchpad-db.py
new file mode 100644
index 0000000..b83a99f
--- /dev/null
+++ b/launchpad-db/reactive/launchpad-db.py
@@ -0,0 +1,57 @@
+# Copyright 2022-2023 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from charms.launchpad.base import get_service_config
+from charms.launchpad.db import strip_dsn_authentication, update_pgpass
+from charms.launchpad.payload import configure_lazr
+from charms.reactive import (
+ clear_flag,
+ endpoint_from_flag,
+ hook,
+ set_flag,
+ when,
+ when_not,
+ when_not_all,
+)
+from ols import postgres
+
+
+@when("launchpad.base.configured", "db.master.available")
+@when_not("launchpad.db.configured")
+def configure():
+ db = endpoint_from_flag("db.master.available")
+ config = get_service_config()
+ db_primary, db_standby = postgres.get_db_uris(db)
+ # XXX cjwatson 2022-09-23: Mangle the connection strings into forms
+ # Launchpad understands. In the long term it would be better to have
+ # Launchpad be able to consume unmodified connection strings.
+ for dsn in [db_primary] + db_standby:
+ update_pgpass(dsn)
+ config["db_primary"] = strip_dsn_authentication(db_primary)
+ config["db_standby"] = ",".join(
+ strip_dsn_authentication(dsn) for dsn in db_standby
+ )
+ configure_lazr(config, "launchpad-db-lazr.conf", "launchpad-db-lazr.conf")
+ set_flag("launchpad.db.configured")
+
+
+@when("launchpad.db.configured")
+@when_not_all("launchpad.base.configured", "db.master.available")
+def deconfigure():
+ clear_flag("launchpad.db.configured")
+
+
+@when("config.changed")
+def config_changed():
+ clear_flag("launchpad.db.configured")
+
+
+@when("db.database.changed", "launchpad.db.configured")
+def db_changed():
+ clear_flag("launchpad.db.configured")
+ clear_flag("db.database.changed")
+
+
+@hook("{requires:rabbitmq}-relation-changed")
+def rabbitmq_relation_changed(*args):
+ clear_flag("launchpad.db.configured")
diff --git a/launchpad-db/templates/launchpad-db-lazr.conf b/launchpad-db/templates/launchpad-db-lazr.conf
new file mode 100644
index 0000000..e6f4f80
--- /dev/null
+++ b/launchpad-db/templates/launchpad-db-lazr.conf
@@ -0,0 +1,19 @@
+# Public configuration data. The contents of this file may be freely shared
+# with developers if needed for debugging.
+
+# A schema's sections, keys, and values are automatically inherited, except
+# for '.optional' sections. Update this config to override key values.
+# Values are strings, except for numbers that look like ints. The tokens
+# true, false, and none are treated as True, False, and None.
+
+{% from "macros.j2" import opt -%}
+
+[meta]
+extends: launchpad-base-lazr.conf
+
+[database]
+{{- opt("db_statement_timeout", db_statement_timeout) }}
+rw_main_primary: {{ db_primary }}
+rw_main_standby: {{ db_standby or db_primary }}
+set_role_after_connecting: True
+
