Feature/zero config pg autoctl

[DimCitus]

Implement support for --monitor option and PG_AUTOCTL_MONITOR environment variable in many commands that don't need to have a local node already created. Those commands are:

pg_autoctl create formation
pg_autoctl drop formation

pg_autoctl get|set formation settings
pg_autoctl get|set formation number-sync-standbys
pg_autoctl get|set node replication-quorum
pg_autoctl get|set node candidate-priority

pg_autoctl show uri
pg_autoctl show events
pg_autoctl show state
pg_autoctl show settings
pg_autoctl show standby-names

pg_autoctl perform failover
pg_autoctl perform switchover
pg_autoctl perform promotion

The make cluster is also changed to use the PG_AUTOCTL_MONITOR environment variable rather than the monitor's PGDATA in the interactive terminal/session.

Fixes #437.

[JelteF]

I played around with this a bit. Some issues:

1. The --monitor argument doesn't actually seem to work (setting PG_AUTOCTL_MONITOR does work):

pg_autoctl show state --monitor 'postgres://autoctl_node@localhost:5500/pg_auto_failover?sslmode=prefer'

2. When using --pgdata you still see this error in the output:

12:02:39 14800 ERROR Failed to get value for environment variable 'PG_AUTOCTL_MONITOR', which is unset

3. When not specifying either --pgdata or --monitor the final fatal log should include something about --monitor and PG_AUTOCTL_MONITOR too:

11:55:49 4723 ERROR Failed to get value for environment variable 'PG_AUTOCTL_MONITOR', which is unset
11:55:49 4723 ERROR Failed to get value for environment variable 'PGDATA', which is unset
11:55:49 4723 FATAL Failed to set PGDATA either from the environment or from --pgdata

[JelteF]

Some more issues:
1.

PG_AUTOCTL_MONITOR='postgres://autoctl_node@localhost:5500/pg_auto_failover?sslmode=require' pg_autoctl create formation --formation 'test' --kind pgsql
13:48:11 24106 ERROR BUG: keeper_config_set_pathnames_from_pgdata: empty pgdata

pg_autoctl create formation --formation 'test' --kind pgsql
13:51:00 30894 ERROR BUG: keeper_config_set_pathnames_from_pgdata: empty pgdata

pg_autoctl create formation --formation 'test' --kind pgsql --monitor 'postgres://autoctl_node@localhost:5500/pg_auto_failover?sslmode=require'
13:51:37 32484 ERROR Failed to get value for environment variable 'PGDATA', which is unset
13:51:37 32484 FATAL Failed to set PGDATA either from the environment or from --pgdata

4. drop formation doesn't say it can use --monitor in the error message if you supply neither --monitor or --pgdata.

PG_AUTOCTL_MONITOR='postgres://autoctl_node@localhost:5500/pg_auto_failover?sslmode=require' pg_autoctl drop formation --formation 'test'
13:53:27 3635 ERROR Failed to get value for environment variable 'PGDATA', which is unset
13:53:27 3635 FATAL Failed to set PGDATA either from the environment or from --pgdata

6. show standby-names --group 5 does not error when there's no group 5 (might be the case on master too)

[JelteF]

These ones are not necessary. These are postgresql.conf options, not connection options.

[DimCitus]

I see what you mean. But in case we would reuse the function parse_pguri_ssl_settings ini other parts of the code I think it makes sense to still correctly parse those connection string parameters. After all those parameters are listed in https://www.postgresql.org/docs/current/libpq-connect.html.

[JelteF]

Ah you are totally right. My mistake, I forgot these are used for TLS user certs.

[DimCitus]

Some more issues:

I have changed the way the --monitor and --pgdata and PG_AUTOCTL_MONITOR and PGDATA are understood in many places except for the formation code path, that I had done first. Sorry about that. I have now updated the code to match our current expectations.

1. show standby-names --group 5 does not error when there's no group 5 (might be the case on master too)

Yeah, so the server-side function was returning NULL, which we didn't test against in parseSingleValueResult. So I have added an explicit PQgetisnull check in there, and also changed the server-side function to report an error in the case where we find zero nodes.

[JelteF]

Works well now