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

[Benjamin Allot]

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

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

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

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

Unmerged commits

db7d9b6... by Benjamin Allot on 2020-09-23

Remove useless makefile target

Also, allow variables to be passed as environment variables

b1e1ace... by Stuart Bishop on 2020-09-22

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

e2bdc6d... by Stuart Bishop on 2020-09-16

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 on 2020-09-15

Run repmgrd as pid 1

0632041... by Stuart Bishop on 2020-09-15

No need for a custom readyness script

db91540... by Stuart Bishop on 2020-09-14

Readyness check and make code more robust

981719e... by Stuart Bishop on 2020-09-14

Note that custom services bypass 'juju expose'

bceb86a... by Stuart Bishop on 2020-09-14

Move k8s Service creation to podspec

Requires Juju 2.8.2

9a5144d... by Tom Haddon on 2020-09-10

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

c5ff3b8... by Tom Haddon on 2020-09-09

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

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" \
     PATH="$PATH:/usr/lib/postgresql/${PG_MAJOR}/bin" \
     PG_MAJOR="${PG_MAJOR}"
 
-ARG PKGS_TO_INSTALL="postgresql postgresql-${PG_MAJOR}-repack repmgr python3 python3-psycopg2 python3-tenacity python3-yaml python3-pip less vim sudo"
+ARG PKGS_TO_INSTALL="postgresql postgresql-${PG_MAJOR}-repack repmgr python3 python3-psycopg2 python3-yaml python3-pip less vim sudo"
 
 RUN \
 # Install remaining packages
diff --git a/Makefile b/Makefile
index 40ffda9..1bfa93a 100644
--- a/Makefile
+++ b/Makefile
@@ -13,13 +13,13 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-PG_MAJOR := 12
-DIST_RELEASE := focal
+PG_MAJOR ?= 12
+DIST_RELEASE ?= focal
 
-IMAGE_REGISTRY :=
-IMAGE_NAME := pgcharm
-IMAGE_TAG := latest
-NO_CACHE :=
+IMAGE_REGISTRY ?=
+IMAGE_NAME ?= pgcharm
+IMAGE_TAG ?= latest
+NO_CACHE ?=
 # NO_CACHE := --no-cache
 
 REGISTRY_IMAGE := $(IMAGE_REGISTRY)/$(IMAGE_NAME):$(IMAGE_TAG)
@@ -47,16 +47,7 @@ clean:
 postgresql.charm: src/*.py requirements.txt *.yaml .jujuignore
 	charmcraft build
 
-image-deps:
-	@echo "Checking shellcheck is present."
-	@command -v shellcheck >/dev/null || { echo "Please install shellcheck to continue ('sudo snap install shellcheck')" && false; }
-
-image-lint: image-deps
-	@echo "Running shellcheck."
-	shellcheck files/docker-entrypoint.sh
-	shellcheck files/docker-readyness.sh
-
-image-build: image-lint
+image-build:
 	@echo "Building the $(LOCAL_IMAGE) image"
 	docker build $(NO_CACHE) -t $(LOCAL_IMAGE) --build-arg BUILD_DATE=$$(date -u +'%Y-%m-%dT%H:%M:%SZ') .
 
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
 
 if __name__ == "__main__":
     pgcharm.docker_entrypoint()
+    # 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
 
 import kubernetes
 import psycopg2
-from tenacity import before_log, retry, retry_if_exception_type, stop_after_delay, wait_random_exponential
 
 
 PGDATA = os.environ["PGDATA"]  # No underscore, PostgreSQL config
@@ -45,7 +44,7 @@ JUJU_EXPECTED_UNITS = os.environ["JUJU_EXPECTED_UNITS"].split(" ")
 NAMESPACE = os.environ["JUJU_POD_NAMESPACE"]
 HOSTNAME = os.environ["HOSTNAME"]
 
-AS_PG_CMD = ["sudo", "-u", "postgres", "-EH", "--"]
+AS_PG_CMD = ["sudo", "-u", "postgres", "-H", "--"]
 REPMGR_CMD = AS_PG_CMD + ["repmgr", "-f", REPMGR_CONF]
 
 log = logging.getLogger(__name__)
@@ -68,7 +67,7 @@ def fix_mounts():
         shutil.chown(pgpath, user="postgres", group="postgres")
 
 
-def db_exists() -> bool:
+def db_exists():
     return os.path.isdir(PGDATA)
 
 
@@ -118,12 +117,15 @@ def initdb():
         "--locale=en_US.UTF-8",
         "--port=5432",
         "--datadir=" + PGDATA,
-        "--",
         "--auth-local=trust",
         "--auth-host=scram-sha-256",
     ]
     log.info(f"Running {' '.join(cmd)}")
-    subprocess.run(cmd, check=True, text=True)
+    subprocess.run(
+        ["pg_createcluster", PG_MAJOR, "main", "--locale=en_US.UTF-8", "--port=5432", "--datadir=" + PGDATA],
+        check=True,
+        text=True,
+    )
 
 
 def start_db():
@@ -139,7 +141,7 @@ def update_postgresql_conf():
         outf.write(
             dedent(
                 f"""\
-                # This file is maintained by the Juju PostgreSQL k8s charm
+                # This file maintained by the Juju PostgreSQL k8s charm
                 listen_addresses = '*'
                 hot_standby = on
                 wal_level = replica
@@ -156,7 +158,7 @@ def update_postgresql_conf():
     os.chmod(pgconf_override, 0o644)
 
     hba = open(PG_HBA_CONF, "r").readlines()
-    marker = "# These rules are appended by Juju"
+    marker = "# These rules appended by Juju"
     if (marker + "\n") not in hba:
         with open(PG_HBA_CONF, "a") as outf:
             outf.write("\n")
@@ -242,14 +244,6 @@ def update_repmgr_conf():
     os.chmod(REPMGR_CONF, 0o644)
 
 
-# Retry in case local PostgreSQL is still starting up.
-@retry(
-    retry=retry_if_exception_type(psycopg2.OperationalError),
-    stop=stop_after_delay(300),
-    wait=wait_random_exponential(multiplier=1, max=15),
-    reraise=True,
-    before=before_log(log, logging.DEBUG),
-)
 def update_repmgr_db():
     log.info(f"Resetting repmgr database user password")
     con = psycopg2.connect("dbname=postgres user=postgres")
@@ -280,28 +274,10 @@ def register_repmgr_master():
     subprocess.run(cmd, check=True, text=True)
 
 
-# Retry in case the master is not running, or gets shut down midway.
-@retry(
-    stop=stop_after_delay(300),
-    wait=wait_random_exponential(multiplier=1, max=15),
-    reraise=True,
-    before=before_log(log, logging.DEBUG),
-)
-def register_repmgr_standby(master):
-    log.info(f"Registering PostgreSQL hot standby server with {master}")
+def register_repmgr_standby():
+    log.info(f"Registering PostgreSQL hot standby server with repmgr")
     # Always reregister with force, as our IP address might have changed.
-    cmd = REPMGR_CMD + [
-        "standby",
-        "register",
-        "--force",
-        "--wait-sync=60",
-        "-h",
-        get_pod_hostname(master),
-        "-U",
-        "repmgr",
-        "-d",
-        "repmgr",
-    ]
+    cmd = REPMGR_CMD + ["standby", "register", "--force", "--wait-sync=60"]
     log.info(f"Running {' '.join(cmd)}")
     subprocess.run(cmd, check=True, text=True)
 
@@ -335,22 +311,12 @@ def get_master() -> str:
     return None
 
 
-class NoMasterException(Exception):
-    pass
-
-
-@retry(
-    retry=retry_if_exception_type(NoMasterException),
-    stop=stop_after_delay(300),
-    wait=wait_random_exponential(max=12),
-    reraise=True,
-    before=before_log(log, logging.DEBUG),
-)
 def wait_master() -> str:
-    master = get_master()
-    if master:
-        return master
-    raise NoMasterException()
+    while True:
+        master = get_master()
+        if master:
+            return master
+        time.sleep(5)
 
 
 def set_master():
@@ -389,6 +355,13 @@ def get_pod_hostname(name) -> str:
     return f"{JUJU_APPLICATION}-{name}"
 
 
+# def get_pod_ip(name) -> str:
+#     cl = kubernetes.client.ApiClient()
+#     api = kubernetes.client.CoreV1Api(cl)
+#     pod = api.read_namespaced_pod(name, NAMESPACE)
+#     return pod.status.pod_ip
+
+
 def init_logging():
     logging.basicConfig(format="%(asctime)-15s %(levelname)8s: %(message)s")
     log.setLevel(logging.DEBUG)
@@ -431,6 +404,7 @@ def docker_entrypoint():
 
     start_db()  # TODO: Ensure DB shutdown cleanly
 
+    # TODO: Wait until DB is available, via DNS lookup.
     if is_master():
         update_repmgr_db()
         register_repmgr_master()
@@ -438,8 +412,7 @@ def docker_entrypoint():
         # pods to continue.
         set_master()
     else:
-        master = wait_master()
-        register_repmgr_standby(master)
+        register_repmgr_standby()
         set_standby()
 
     exec_repmgrd()  # Does not return
@@ -455,8 +428,8 @@ def exec_repmgrd():
 def promote_entrypoint():
     init_logging()
     configure_k8s_api()
-    set_master()  # First, lessening chance connections go to an existing master.
     subprocess.check_call(["repmgr", "standby", "promote", "-v", "-f", REPMGR_CONF, "--log-to-file"], text=True)
+    set_master()
 
 
 def follow_entrypoint():
@@ -477,7 +450,7 @@ def follow_entrypoint():
         ],
         text=True,
     )
-    set_standby()
+    set_master()
 
 
 def hang_forever():