Merge ~ballot/charm-k8s-postgresql/+git/charm-k8s-postgresql:ballot_review into charm-k8s-postgresql:master

Proposed by Benjamin Allot
Status: Superseded
Proposed branch: ~ballot/charm-k8s-postgresql/+git/charm-k8s-postgresql:ballot_review
Merge into: charm-k8s-postgresql:master
Prerequisite: ~stub/charm-k8s-postgresql:fix-failover
Diff against target: 256 lines (+37/-72)
4 files modified
Dockerfile (+1/-1)
Makefile (+7/-16)
files/docker_entrypoint.py (+1/-0)
files/pgcharm.py (+28/-55)
Reviewer Review Type Date Requested Status
Canonical IS Reviewers Pending
PostgreSQL Charm Maintainers Pending
Review via email: mp+391211@code.launchpad.net

This proposal has been superseded by a proposal from 2020-09-23.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

Unmerged commits

db7d9b6... by Benjamin Allot

Remove useless makefile target

Also, allow variables to be passed as environment variables

b1e1ace... by Stuart Bishop

Merge branch 'missing-test' of git+ssh://git.launchpad.net/~mthaddon/charm-k8s-postgresql/+git/charm-k8s-postgresql into integration

e2bdc6d... by Stuart Bishop

Use a hack to enable DNS resolution of pods

Pods need a stable address to each other to register with
repmgr. However, pods get a new IP address each time they
get restarted. Register a k8s Headless Service for each
pod, using a selector to ensure connections are routed
correctly, allowing us to use the DNS name of the
service to connect to PostgreSQL on the pods.

1ac0c84... by Stuart Bishop

Run repmgrd as pid 1

0632041... by Stuart Bishop

No need for a custom readyness script

db91540... by Stuart Bishop

Readyness check and make code more robust

981719e... by Stuart Bishop

Note that custom services bypass 'juju expose'

bceb86a... by Stuart Bishop

Move k8s Service creation to podspec

Requires Juju 2.8.2

9a5144d... by Tom Haddon

Add test for missing image password, and include psycopg2-binary in requirements so tests run locally

c5ff3b8... by Tom Haddon

Make sure Dockerfile and docker related files won't be included in the built charm

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/Dockerfile b/Dockerfile
index f5910bc..2b8d6ef 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -52,7 +52,7 @@ ENV PGDATA="/srv/pgdata/${PG_MAJOR}/main" \
52 PATH="$PATH:/usr/lib/postgresql/${PG_MAJOR}/bin" \52 PATH="$PATH:/usr/lib/postgresql/${PG_MAJOR}/bin" \
53 PG_MAJOR="${PG_MAJOR}"53 PG_MAJOR="${PG_MAJOR}"
5454
55ARG PKGS_TO_INSTALL="postgresql postgresql-${PG_MAJOR}-repack repmgr python3 python3-psycopg2 python3-tenacity python3-yaml python3-pip less vim sudo"55ARG PKGS_TO_INSTALL="postgresql postgresql-${PG_MAJOR}-repack repmgr python3 python3-psycopg2 python3-yaml python3-pip less vim sudo"
5656
57RUN \57RUN \
58# Install remaining packages58# Install remaining packages
diff --git a/Makefile b/Makefile
index 40ffda9..1bfa93a 100644
--- a/Makefile
+++ b/Makefile
@@ -13,13 +13,13 @@
13# You should have received a copy of the GNU General Public License13# You should have received a copy of the GNU General Public License
14# along with this program. If not, see <http://www.gnu.org/licenses/>.14# along with this program. If not, see <http://www.gnu.org/licenses/>.
1515
16PG_MAJOR := 1216PG_MAJOR ?= 12
17DIST_RELEASE := focal17DIST_RELEASE ?= focal
1818
19IMAGE_REGISTRY :=19IMAGE_REGISTRY ?=
20IMAGE_NAME := pgcharm20IMAGE_NAME ?= pgcharm
21IMAGE_TAG := latest21IMAGE_TAG ?= latest
22NO_CACHE :=22NO_CACHE ?=
23# NO_CACHE := --no-cache23# NO_CACHE := --no-cache
2424
25REGISTRY_IMAGE := $(IMAGE_REGISTRY)/$(IMAGE_NAME):$(IMAGE_TAG)25REGISTRY_IMAGE := $(IMAGE_REGISTRY)/$(IMAGE_NAME):$(IMAGE_TAG)
@@ -47,16 +47,7 @@ clean:
47postgresql.charm: src/*.py requirements.txt *.yaml .jujuignore47postgresql.charm: src/*.py requirements.txt *.yaml .jujuignore
48 charmcraft build48 charmcraft build
4949
50image-deps:50image-build:
51 @echo "Checking shellcheck is present."
52 @command -v shellcheck >/dev/null || { echo "Please install shellcheck to continue ('sudo snap install shellcheck')" && false; }
53
54image-lint: image-deps
55 @echo "Running shellcheck."
56 shellcheck files/docker-entrypoint.sh
57 shellcheck files/docker-readyness.sh
58
59image-build: image-lint
60 @echo "Building the $(LOCAL_IMAGE) image"51 @echo "Building the $(LOCAL_IMAGE) image"
61 docker build $(NO_CACHE) -t $(LOCAL_IMAGE) --build-arg BUILD_DATE=$$(date -u +'%Y-%m-%dT%H:%M:%SZ') .52 docker build $(NO_CACHE) -t $(LOCAL_IMAGE) --build-arg BUILD_DATE=$$(date -u +'%Y-%m-%dT%H:%M:%SZ') .
6253
diff --git a/files/docker_entrypoint.py b/files/docker_entrypoint.py
index 95e5c0f..c1ac0cc 100644
--- a/files/docker_entrypoint.py
+++ b/files/docker_entrypoint.py
@@ -21,3 +21,4 @@ import pgcharm
2121
22if __name__ == "__main__":22if __name__ == "__main__":
23 pgcharm.docker_entrypoint()23 pgcharm.docker_entrypoint()
24 # pgcharm.debug_docker_entrypoint()
diff --git a/files/pgcharm.py b/files/pgcharm.py
index e82f242..8f03fb3 100644
--- a/files/pgcharm.py
+++ b/files/pgcharm.py
@@ -24,7 +24,6 @@ import time
2424
25import kubernetes25import kubernetes
26import psycopg226import psycopg2
27from tenacity import before_log, retry, retry_if_exception_type, stop_after_delay, wait_random_exponential
2827
2928
30PGDATA = os.environ["PGDATA"] # No underscore, PostgreSQL config29PGDATA = os.environ["PGDATA"] # No underscore, PostgreSQL config
@@ -45,7 +44,7 @@ JUJU_EXPECTED_UNITS = os.environ["JUJU_EXPECTED_UNITS"].split(" ")
45NAMESPACE = os.environ["JUJU_POD_NAMESPACE"]44NAMESPACE = os.environ["JUJU_POD_NAMESPACE"]
46HOSTNAME = os.environ["HOSTNAME"]45HOSTNAME = os.environ["HOSTNAME"]
4746
48AS_PG_CMD = ["sudo", "-u", "postgres", "-EH", "--"]47AS_PG_CMD = ["sudo", "-u", "postgres", "-H", "--"]
49REPMGR_CMD = AS_PG_CMD + ["repmgr", "-f", REPMGR_CONF]48REPMGR_CMD = AS_PG_CMD + ["repmgr", "-f", REPMGR_CONF]
5049
51log = logging.getLogger(__name__)50log = logging.getLogger(__name__)
@@ -68,7 +67,7 @@ def fix_mounts():
68 shutil.chown(pgpath, user="postgres", group="postgres")67 shutil.chown(pgpath, user="postgres", group="postgres")
6968
7069
71def db_exists() -> bool:70def db_exists():
72 return os.path.isdir(PGDATA)71 return os.path.isdir(PGDATA)
7372
7473
@@ -118,12 +117,15 @@ def initdb():
118 "--locale=en_US.UTF-8",117 "--locale=en_US.UTF-8",
119 "--port=5432",118 "--port=5432",
120 "--datadir=" + PGDATA,119 "--datadir=" + PGDATA,
121 "--",
122 "--auth-local=trust",120 "--auth-local=trust",
123 "--auth-host=scram-sha-256",121 "--auth-host=scram-sha-256",
124 ]122 ]
125 log.info(f"Running {' '.join(cmd)}")123 log.info(f"Running {' '.join(cmd)}")
126 subprocess.run(cmd, check=True, text=True)124 subprocess.run(
125 ["pg_createcluster", PG_MAJOR, "main", "--locale=en_US.UTF-8", "--port=5432", "--datadir=" + PGDATA],
126 check=True,
127 text=True,
128 )
127129
128130
129def start_db():131def start_db():
@@ -139,7 +141,7 @@ def update_postgresql_conf():
139 outf.write(141 outf.write(
140 dedent(142 dedent(
141 f"""\143 f"""\
142 # This file is maintained by the Juju PostgreSQL k8s charm144 # This file maintained by the Juju PostgreSQL k8s charm
143 listen_addresses = '*'145 listen_addresses = '*'
144 hot_standby = on146 hot_standby = on
145 wal_level = replica147 wal_level = replica
@@ -156,7 +158,7 @@ def update_postgresql_conf():
156 os.chmod(pgconf_override, 0o644)158 os.chmod(pgconf_override, 0o644)
157159
158 hba = open(PG_HBA_CONF, "r").readlines()160 hba = open(PG_HBA_CONF, "r").readlines()
159 marker = "# These rules are appended by Juju"161 marker = "# These rules appended by Juju"
160 if (marker + "\n") not in hba:162 if (marker + "\n") not in hba:
161 with open(PG_HBA_CONF, "a") as outf:163 with open(PG_HBA_CONF, "a") as outf:
162 outf.write("\n")164 outf.write("\n")
@@ -242,14 +244,6 @@ def update_repmgr_conf():
242 os.chmod(REPMGR_CONF, 0o644)244 os.chmod(REPMGR_CONF, 0o644)
243245
244246
245# Retry in case local PostgreSQL is still starting up.
246@retry(
247 retry=retry_if_exception_type(psycopg2.OperationalError),
248 stop=stop_after_delay(300),
249 wait=wait_random_exponential(multiplier=1, max=15),
250 reraise=True,
251 before=before_log(log, logging.DEBUG),
252)
253def update_repmgr_db():247def update_repmgr_db():
254 log.info(f"Resetting repmgr database user password")248 log.info(f"Resetting repmgr database user password")
255 con = psycopg2.connect("dbname=postgres user=postgres")249 con = psycopg2.connect("dbname=postgres user=postgres")
@@ -280,28 +274,10 @@ def register_repmgr_master():
280 subprocess.run(cmd, check=True, text=True)274 subprocess.run(cmd, check=True, text=True)
281275
282276
283# Retry in case the master is not running, or gets shut down midway.277def register_repmgr_standby():
284@retry(278 log.info(f"Registering PostgreSQL hot standby server with repmgr")
285 stop=stop_after_delay(300),
286 wait=wait_random_exponential(multiplier=1, max=15),
287 reraise=True,
288 before=before_log(log, logging.DEBUG),
289)
290def register_repmgr_standby(master):
291 log.info(f"Registering PostgreSQL hot standby server with {master}")
292 # Always reregister with force, as our IP address might have changed.279 # Always reregister with force, as our IP address might have changed.
293 cmd = REPMGR_CMD + [280 cmd = REPMGR_CMD + ["standby", "register", "--force", "--wait-sync=60"]
294 "standby",
295 "register",
296 "--force",
297 "--wait-sync=60",
298 "-h",
299 get_pod_hostname(master),
300 "-U",
301 "repmgr",
302 "-d",
303 "repmgr",
304 ]
305 log.info(f"Running {' '.join(cmd)}")281 log.info(f"Running {' '.join(cmd)}")
306 subprocess.run(cmd, check=True, text=True)282 subprocess.run(cmd, check=True, text=True)
307283
@@ -335,22 +311,12 @@ def get_master() -> str:
335 return None311 return None
336312
337313
338class NoMasterException(Exception):
339 pass
340
341
342@retry(
343 retry=retry_if_exception_type(NoMasterException),
344 stop=stop_after_delay(300),
345 wait=wait_random_exponential(max=12),
346 reraise=True,
347 before=before_log(log, logging.DEBUG),
348)
349def wait_master() -> str:314def wait_master() -> str:
350 master = get_master()315 while True:
351 if master:316 master = get_master()
352 return master317 if master:
353 raise NoMasterException()318 return master
319 time.sleep(5)
354320
355321
356def set_master():322def set_master():
@@ -389,6 +355,13 @@ def get_pod_hostname(name) -> str:
389 return f"{JUJU_APPLICATION}-{name}"355 return f"{JUJU_APPLICATION}-{name}"
390356
391357
358# def get_pod_ip(name) -> str:
359# cl = kubernetes.client.ApiClient()
360# api = kubernetes.client.CoreV1Api(cl)
361# pod = api.read_namespaced_pod(name, NAMESPACE)
362# return pod.status.pod_ip
363
364
392def init_logging():365def init_logging():
393 logging.basicConfig(format="%(asctime)-15s %(levelname)8s: %(message)s")366 logging.basicConfig(format="%(asctime)-15s %(levelname)8s: %(message)s")
394 log.setLevel(logging.DEBUG)367 log.setLevel(logging.DEBUG)
@@ -431,6 +404,7 @@ def docker_entrypoint():
431404
432 start_db() # TODO: Ensure DB shutdown cleanly405 start_db() # TODO: Ensure DB shutdown cleanly
433406
407 # TODO: Wait until DB is available, via DNS lookup.
434 if is_master():408 if is_master():
435 update_repmgr_db()409 update_repmgr_db()
436 register_repmgr_master()410 register_repmgr_master()
@@ -438,8 +412,7 @@ def docker_entrypoint():
438 # pods to continue.412 # pods to continue.
439 set_master()413 set_master()
440 else:414 else:
441 master = wait_master()415 register_repmgr_standby()
442 register_repmgr_standby(master)
443 set_standby()416 set_standby()
444417
445 exec_repmgrd() # Does not return418 exec_repmgrd() # Does not return
@@ -455,8 +428,8 @@ def exec_repmgrd():
455def promote_entrypoint():428def promote_entrypoint():
456 init_logging()429 init_logging()
457 configure_k8s_api()430 configure_k8s_api()
458 set_master() # First, lessening chance connections go to an existing master.
459 subprocess.check_call(["repmgr", "standby", "promote", "-v", "-f", REPMGR_CONF, "--log-to-file"], text=True)431 subprocess.check_call(["repmgr", "standby", "promote", "-v", "-f", REPMGR_CONF, "--log-to-file"], text=True)
432 set_master()
460433
461434
462def follow_entrypoint():435def follow_entrypoint():
@@ -477,7 +450,7 @@ def follow_entrypoint():
477 ],450 ],
478 text=True,451 text=True,
479 )452 )
480 set_standby()453 set_master()
481454
482455
483def hang_forever():456def hang_forever():

Subscribers

People subscribed via source and target branches