Integrate your charm with PostgreSQL

I think what I found was that it would deploy, but end up at a “blocked” state. It wasn’t until I dived into the logs that I saw it trying to do things with service accounts and then I found the postgresql-k8s charm page itself (Charmhub | Deploy Charmed PostgreSQL K8s using Charmhub - The Open Operator Collection) had a --trust in its tutorial.

1 Like

@tmihoc if psql itself points to be trusted, then we need to add trust here

1 Like

Done, thanks!

key is never used in the loop of the above code. If there is no reason to evaluate the key attribute we may want to make the code easier to understand, e.g. like this:

def fetch_postgres_relation_data(self) -> dict:
    relations = self.database.fetch_relation_data()
    logger.debug("Got following database data: %s", relations)
    for data in relations.values():
        if not data:
            continue
        logger.info("New PSQL database endpoint is %s", data["endpoints"])
        ...

true! thanks

@tmihoc can you please update the post and git repo ?

2 Likes

Done, thanks!

1 Like

The method fetch_postgres_relation_data in the charms.py is broken from lection 4 on. https://github.com/canonical/juju-sdk-tutorial-k8s/blob/04_integrate_with_psql/src/charm.py#L107C1-L131C28

    def fetch_postgres_relation_data(self) -> dict:
        """Fetch postgres relation data.

        This function retrieves relation data from a postgres database using
        the `fetch_relation_data` method of the `database` object. The retrieved data is
        then logged for debugging purposes, and any non-empty data is processed to extract
        endpoint information, username, and password. This processed data is then returned as
        a dictionary. If no data is retrieved, the unit is set to waiting status and
        the program exits with a zero status code."""
        relations = self.database.fetch_relation_data()
        logger.debug("Got following database data: %s", relations)
        for data in relations.values():
            if not data:
                continue
            logger.info("New PSQL database endpoint is %s", data["endpoints"])
            host, port = val["endpoints"].split(":")
            db_data = {
                "db_host": host,
                "db_port": port,
                "db_username": val["username"],
                "db_password": val["password"],
            }
            return db_data
        self.unit.status = WaitingStatus("Waiting for database relation")
        raise SystemExit(0)

should be

    def fetch_postgres_relation_data(self) -> dict:
        """Fetch postgres relation data.

        This function retrieves relation data from a postgres database using
        the `fetch_relation_data` method of the `database` object. The retrieved data is
        then logged for debugging purposes, and any non-empty data is processed to extract
        endpoint information, username, and password. This processed data is then returned as
        a dictionary. If no data is retrieved, the unit is set to waiting status and
        the program exits with a zero status code."""
        relations = self.database.fetch_relation_data()
        logger.debug("Got following database data: %s", relations)
        for data in relations.values():
            if not data:
                continue
            logger.info("New PSQL database endpoint is %s", data["endpoints"])
            host, port = data["endpoints"].split(":")
            db_data = {
                "db_host": host,
                "db_port": port,
                "db_username": data["username"],
                "db_password": data["password"],
            }
            return db_data
        self.unit.status = WaitingStatus("Waiting for database relation")
        raise SystemExit(0)

(Variable name val is not defined and should be data)

I already changed that in the doc post. Should I do a PR for the Github repo or directly pushing it to the branch?

1 Like

@tmihoc please suggest how to go with it on GitHub

@bschimke95 The process we’ve followed so far is direct push to the branch + updates to the subsequent branches by successive merge (switch to the next branch, merge the previous branch into it, repeat).

Done, thanks!

M1/M2 mac users, arm64 users might get the following error:

sqlalchemy.exc.OperationalError: (psycopg2.OperationalError) SCRAM authentication requires libpq version 10 or above

This is because of a bug mentioned here: https://github.com/psycopg/psycopg2/issues/1360.

I believe the docker image used in the tutorial is from this repo: https://github.com/canonical/api_demo_server, and it’s using a relatively older version of the psycopg2, see here: https://github.com/canonical/api_demo_server/blob/master/pyproject.toml#L17.

The bug is fixed from 2.9.5.

A workaround would be to clone the above repo, change the version of psycopg2 to the latest (I used 2.9.9) and build the image.

I’ll submit a PR to fix this issue in the original repo as well.

2 Likes

The docstring in the function below is over-indented and produces an error when I use it.

def fetch_postgres_relation_data(self) -> dict:
        """Fetch postgres relation data.

        This function retrieves relation data from a postgres database using
        the `fetch_relation_data` method of the `database` object. The retrieved data is
        then logged for debugging purposes, and any non-empty data is processed to extract
        endpoint information, username, and password. This processed data is then returned as
        a dictionary. If no data is retrieved, the unit is set to waiting status and
        the program exits with a zero status code."""
    relations = self.database.fetch_relation_data()
    logger.debug("Got following database data: %s", relations)
    for data in relations.values():
        if not data:
            continue
        logger.info("New PSQL database endpoint is %s", data["endpoints"])
        host, port = data["endpoints"].split(":")
        db_data = {
            "db_host": host,
            "db_port": port,
            "db_username": data["username"],
            "db_password": data["password"],
        }
        return db_data
    raise DatabaseNotReady()

I also notice in that same function in the tutorial we use a previously-created Exception class called DatabaseNotReady(), but in the provided code, that class is not used and the line self.unit.status = WaitingStatus("Waiting for Pebble in the workload container) is used instead. I have seen WaitingStatus() used in other instructional material so it may best to keep things consistent unless there was another reason we have the Exception class in the tutorial.

Thanks - fixed!

The status is still set to Waiting (code higher up catches the exception and sets the status). The older version of the code did a system exit in the handler, which isn’t something that we recommend.

The code’s a little behind the docs - there was a bit of a mixup with handling changes to the branches, but we’re getting that fixed this week.

1 Like

Why the status is waiting? what charm is waiting for? if there is no relation, then it should be blocked status signaling to the admin that we need a relation

Did something change since then? https://github.com/canonical/operator/issues/839#issuecomment-1325648179

I was more saying that the non-active waiting set is still there, not saying that waiting was the best choice. Exactly when to use which status type is actually being discussed at the moment, because the various docs (in Juju | Documentation plus the ops API docs plus the Juju code docs) aren’t super consistent at the moment (and are also inconsistent with how we’re seeing people using them). We’re planning on having a Charm-Tech + Juju discussion in Madrid to nail this down.

I wasn’t aware of that discussion, but it was @benhoyt that originally suggested removing the sys.exit, so I guess Ben’s opinion changed? And we expanded the Charm-Tech team, and so added in new opinions, which also prefer raising up an (non SystemExit) exception (which is suggested as a possibility in that thread).

Docs are actually quite explicit. If we have a relation but charm waits for data, then it is a waiting status

If admin has to relate charms, and in meantime charm cannot proceed, then it is a blocked state

That means that we either change status to blocked or change the message that we wait for data, not for relation

The docs are explicit, but they are not consistent. For example, compare:

It would probably be nice if the tutorial distinguished between “I don’t have a DB relation yet” (blocked) and “the relation exists but doesn’t have the data I need yet” (waiting). But the OP wasn’t really asking about changing which type of status is used. We could clean this up after we’ve clarified the exact usage (particularly for some maintenance vs. waiting cases).

This is probably getting a bit tangential for this topic? Happy to pick this up in Charm Development (or Juju or Ops Library Development) in Matrix!

2 Likes

I see references to the metadata.yaml file here:

Next, open the metadata.yaml file of your charm and, before the containers section, define a relation endpoint using a requires block, as below. This endpoint says that our charm is requesting a relation called database over an interface called postgresql_client with a maximum number of supported connections of 1. (Note: Here, database is a custom relation name, though in general we recommend sticking to default recommended names for each charm.)

requires:
  database:
    interface: postgresql_client
    limit: 1

In the first page of this tutorial it states that the containers should be put in the charmcraft.yaml file:

Note: In the past, the containers were specified in a metadata.yaml file, but the modern practice is that all charm specification is in a single charmcraft.yaml file.

At no point in this tutorial do we create a metadata.yaml file but I still see references to it throughout. Is the current paradigm to put everything in charmcraft.yaml instead? If so, it looks like there were a few spots where that change wasn’t made and the tutorial still references metadata.yaml. Either way we want to do it, it looks like it isn’t consistent across pages.

1 Like