Add PostgreSQL 9.6 for CentOS7

[bmbouter]

Pulp is requiring PostgreSQL 9.6 with Django 2.2 (LTS). This upgrades
the version of PostgreSQL on CentOS 7 to 9.6 via software collections.

The systemd files can no longer depend on the postgresql service name to
restart them when postgresql restarts. Foremost because we can't be
guaranteed postgresql is a systemd unit on this system. Secondly due to
this PR that name is now changing on centos7.

This PR removes the systemd dependency on postgresql service units and
instead configures all Pulp services to automatically always restart
when it exits. Stopping the unit with systemctl still stops it.

https://pulp.plan.io/issues/5147
closes #5147

[bmbouter]

For testing you need to put the following vars in your playbook:

# centos postgreSQL vars
postgresql_packages: rh-postgresql96
postgresql_daemon: rh-postgresql96-postgresql
postgresql_bin_path: /opt/rh/rh-postgresql96/root/bin
postgresql_data_dir: /var/opt/rh/rh-postgresql96/lib/pgsql/data
postgresql_config_path: /var/opt/rh/rh-postgresql96/lib/pgsql/data

[bmbouter]

This is the error I get:

TASK [pulp-workers : Set states of pulp workers] *******************************
failed: [pulp3-source-centos7] (item={'value': {u'state': u'started', u'enabled': True}, 'key': 1}) => {
    "changed": false, 
    "item": {
        "key": 1, 
        "value": {
            "enabled": true, 
            "state": "started"
        }
    }
}

MSG:

Unable to start service pulp-worker@1.service: Failed to start pulp-worker@1.service: Unit not found.


failed: [pulp3-source-centos7] (item={'value': {u'state': u'started', u'enabled': True}, 'key': 2}) => {
    "changed": false, 
    "item": {
        "key": 2, 
        "value": {
            "enabled": true, 
            "state": "started"
        }
    }
}

MSG:

Unable to start service pulp-worker@2.service: Failed to start pulp-worker@2.service: Unit not found.

[mikedep333]

I think multiple changes of your's belong in the geerlingguy.postgresql role rather than in our pulp-database role. In fact, I think they are already implemented, we just need to call a different code path. See https://github.com/dralley/ansible-role-postgresql/tree/master/vars

As listed in https://github.com/pulp/ansible-pulp/blob/master/requirements.yml , we grab a fork of the role from https://github.com/dralley/ansible-role-postgresql . I've even sent daniel a Pull Request, which he accepted.

(It is unfortunate that we're in this situation. We should work with the original dev on geerlingguy/ansible-role-postgresql#54 .)

[mikedep333]

I think multiple changes of your's belong in the geerlingguy.postgresql role rather than in our pulp-database role. In fact, I think they are already implemented, we just need to call a different code path. See https://github.com/dralley/ansible-role-postgresql/tree/master/vars

We need to call the role with the variable postgresql_use_scl: True

dralley/ansible-role-postgresql@417a8d6

[bmbouter]

@mikedep333 I see what you are saying. I wasn't aware we were using a forked version. Let's not add them to @dralley's fork though since that's not going to be long-lived. We probably need to get them upstream. I actually got this config from someone working on the upstream bug already: geerlingguy/ansible-role-postgresql#65 (comment)

[mikedep333]

@bmbouter Well, it looks like we don't need to add to his fork. We just need to call the role with the var set.

[bmbouter]

@mikedep333 I wasn't involved as much when we switched to the fork so I'm fuzzy on why we did. If we use features that are exclusively in the fork and not in the upstream geerlingguy role that's concerning. To adopt usage of postgresql_use_scl that would take use further from the upstream too. The patch here works with the upstream geerlinguy so I'm hesitant to redo it.

One of my questions is how do we switch back to a vanilla geerlingguy role generally (outside of this issue)? Also what should we do?

[mikedep333]

@bmbouter We should work on ehelms's 2 existing Pull Requests, and then submit 2 more for Daniel's order of operations commit and the Fedora 30 commit.

Each of the 4 commits corresponds to a PR:
https://github.com/dralley/ansible-role-postgresql/commits/master

We are totally reliant on Daniel's branch for Fedora support, and I assume the order of operations commit is important as well.

This is the other existing PR I forgot to mention, Fedora <= 29 support:
geerlingguy/ansible-role-postgresql#53

[dralley]

@bmbouter I would hope it's not long-term, but geerlingguy has been incredibly non-responsive to requests to please merge the PRs.

I'm fuzzy on why we did.

That was why, several of us dropped by the PRs and mentioned him directly and asked to get the PRs merged and we received only total radio silence. So in practical terms we had to fork it in order to get anything done.

[bmbouter]

@mikedep333 thank you I had no idea about all of this. I agree we should work to get all of these into upstream, and I can see you all have been even though I'm just now realizing it. Yes let's do all that.

In terms of this work though, I think contributing it here is right for a few reasons. It works with the upstream geerlingguy role and dralley's. Also upstream isn't going to make 9.6 the default, 9.2 is since that's in epel so the recommended way AIUI is to set these variables which we would do since it's not the default.

[bmbouter]

@dralley I understand now why we're using the fork, it's the Fedora support. I didn't realize we weren't getting responses. Is the overall plan to switch back at some point though?

[dralley]

@bmbouter That would be great but it's still contingent on geerlinguy being an active maintainer and merging the patches. It's not even just us begging for these patches.

geerlingguy/ansible-role-postgresql#31

[bmbouter]

@dralley I agree we are beholden on those merging for us to switch back.

So that effort aside, I don't think the fork is where these changes should live because we only want things to go there that we can't get into upstream geerlingguy. In this case there are no changes needed in either fork to allow us to resolve this bug.

[mikedep333]

@bmbouter I think it's best to just use our fork's existing functionality and set that var to true. See:
https://github.com/pulp/ansible-pulp/pull/130/files

However, this is not a big deal either way. Not even a medium size deal.

[mikedep333]

This systemd unit file is already a template. We could just make the service name a variable.

I agree with not depending on postgres being a systemd unit, but I think we should add a comment about it.

[bmbouter]

My goal is to not have users have Pulp services depend on any other services other than network-online.target. I don't think we want to affect Pulp services when postgreSQL is on the same system in the same way we don't want to affect Pulp services when PostgreSQL is on a different system.

The reason this was here to start with was to handle cases where postgreSQL restarts. The issue is that what about Redis? What about when the OOM killer kills a process? What about when the network can't route to postgreSQL for 30 seconds and then resolves? We can resolve all of these cases by making pulp services Restart=true. With Pulp auto-restarting it no longer benefits from a systemd dependency since the services are now able to start/run independently.

This work could also be moved to its own ticket and probably be its own bug. What do you think about me moving this work to its own PR and filing an issue about Pulp service not auto-relaunching?

What do you think about this problem statement ^?

[mikedep333]

It would be a good idea to be a separate ticket, but I do not think it is a strictly necessary (use of your time.)

I also agree with Restart=true instead of a templated dependency (I was playing devil's advocate). You just provided additional reasons for me to agree with it.

However, I think we should put a comment about Restart=true. Most SysAdmins won't look through the history of the systemd unit file for rationale before they customize it; they will just look at the current version on disk.

[bmbouter]

I see what you mean about leaving a comment so admins understand its purpose. I'll add that now and push.

I'm going to not make the issue and also leave the changes in this PR because until the installer gets a towncrier changelog having a Redmine issue isn't that valuable since the change is needed along with this work anyway.

[mikedep333]

Looks good after our discussion, and adding the comment to the systemd service file.

[bmbouter]

Thanks @mikedep333 and @dralley !

[bmbouter]

Travis failures will be resolved when Pulpcore releases RC4. https://www.redhat.com/archives/pulp-dev/2019-July/msg00076.html

[jlsherrill]

This worked great for @Katello's use case