[FEATURE REQUEST] Add parameter with explicit Postgres bindir for various postgres states

[dkacar-oradian]

Is your feature request related to a problem? Please describe.
Various Postgres states (like postgres_initdb.present) require postgres.bins_dir configuration directive to be set. That's OK because that's the path where they can find command line utilities they need (like psql or initdb) to do the job.

It isn't OK that this value is retreived by config.option only because that disables the possibility of having different Postgres versions installed on the same OS. Having different versions is rather important because it is a very common scenario. There is usually one DB version per OS on a machine which is usually referred to as a "production database server" and this is where such feature isn't important. But all supporting databases need to have the ability to run multiple versions.

Supporting databases examples (that I actually have):

• Postgres slave from which masters are created via file system snapshot. Used by developers and customer support for debugging.
• Postgres master which is used to accept the results of pg_dump from the production database server. Used to generate data dumps for customers (they call it backups).

The need for different versions on the support servers comes because of the upgrade procedure for the production database servers. If you have more than one production DB server then it's wise to upgrade them one by one and leave some time (days) between upgrades to resolve problems if they appear. That way the problems won't hit all customers at once and we won't be swamped with support requests.

During the production database upgrade time all support database servers must have multiple Postgres versions installed and running. That can go on like that for days or weeks.

Production database server (with only one Postgres version per OS) is basically an anomaly. But there is only one such anomaly, as far as server configuration is concerned. But there are multiple types of supporting servers and each of them needs its own Salt configuration.

I just want a normal way to maintain multiple Postgres versions with Salt.

Describe the solution you'd like
To each Postgres state which is currently checking postgres.bins_dir configuration option add an explicit parameter which could contain that value. If the parameter is not specified then fall back to the current method of obtaining it.

For example (if you call that additional parameter bins_dir):

# This will work on RHEL family with packages from postgres.org.
initialize_postgres_12:
  postgres_initdb.present:
    - name: /data/pg_data/12
    - user: postgres
    - runas: postgres
    - locale: en_US.UTF-8
    # Just need to add this one:
    - bins_dir: /usr/pgsql-12/bin

Describe alternatives you've considered
It is possible to work around this problem because grain can be used to set postgres.bins_dir value, but that's really not the greatest way. For example, I'm currently using this for one of the servers mentioned above:

{% set dbdir_base = '/data/pg_data/ubackupdb' %}

# Ensure that postgres.bins_dir is not in the minion config file because
# that config file is checked before grains by config.option. Setting there
# would override our grain, so the code wouldn't work correctly.
{% set pbins_dir = salt.config.option('postgres.bins_dir') %}

postgres_ubackupdb_config_ok:
  test.configurable_test_state:
    - name: postgres.bins_dir
    - changes: False
{% if pbins_dir|length %}
    - result: False
    - comment: "postgres.bins_dir configuration option must not be set"
{% else %}
    - result: True
    - comment: "Minion configuration OK"
{% endif %}

{% for pgver in salt.pillar.get('postgres_versions') %}

{%   set pgbin = '/usr/pgsql-' ~ pgver ~ '/bin' %}
{%   set dbdir = dbdir_base ~ '/' ~ pgver %}

ubackupdb{{ pgver }}_set_grain:
  grains.present:
    - name: postgres.bins_dir
    - value: {{ pgbin }}
    - force: True
    - require:
      - postgres_ubackupdb_config_ok
    # This is utterly stupid, but that's the only way to avoid state changes
    # when there's nothing to do.
    - unless: "{{ pgbin }}/postgresql-{{ pgver }}-check-db-dir {{ dbdir }}"

ubackupdb{{ pgver }}:
  postgres_initdb.present:
    - name: {{ dbdir }}
    - user: postgres
    - runas: postgres
    - auth: ident
    - locale: en_US.UTF-8
    - checksums: True
    - onchanges:
      - ubackupdb{{ pgver }}_set_grain

ubackupdb{{ pgver }}_remove_grain:
  grains.absent:
    - name: postgres.bins_dir
    - destructive: True
    - force: True
    - onchanges:
      - ubackupdb{{ pgver }}_set_grain

{% endfor %}

That works, but it's clunky. And the unless clause for grains.present is particularly frustrating. It has to reimplement the main state's (here postgres_initdb.present) functionality which checks if there's a need for changes. The above code will work for Postgres versions starting from 10, I think. Older versions had a slightly different utility name, so I'd actually have to write a script to check that properly. Furtunately, I have no need for Postgres versions which are older than 10, so I can get away with what I wrote above.

But I'm illustrating that reimplementing that functionality isn't easy and since it already exists in your Python code I'd really like to be able to use it instead of having to write my own in a shell one-liner.

Additional context
Add any other context or screenshots about the feature request here.

Please Note
If this feature request would be considered a substantial change or addition, this should go through a SEP process here https://github.com/saltstack/salt-enhancement-proposals, instead of a feature request.

[DmitryKuzmenko]

@dkacar-oradian your explanation makes sense. Thank you for the feature request.