multiple host support part1 #77

Nastradamus · 2018-11-22T14:50:21Z

No description provided.

…ck_name', generate report with this filename, add 'check_name' into json report

NikolayS · 2018-11-24T03:51:05Z

check

  echo >&2
 }

 #######################################
 # Cleanup function: close ssh sockets, etc.
 # Globals:
-#   None
+#   HOST


oh, I see -- HOST

We decided to check only one host per single script run.

NikolayS · 2018-11-24T03:52:17Z

check

-  done
+  if [[ ! -z ${HOST+x} ]]; then
+    dbg "closing ssh conenction to host '$HOST' (if exists)"
+    ssh -O exit ${HOST} 2>/dev/null || true


is this for closing persistent ssh connection ?

NikolayS · 2018-11-24T03:55:23Z

check

+# Generate json report
+# Globals:
+#   CURRENT_CHECK, SCRIPT_DIR, PROJECT,
+#   HOST, 


NikolayS · 2018-11-24T03:57:41Z

check

+
+  local epoch=null
+
+  [[ -z ${3+x} ]] && err "function needs 3 arguments"


cool way to check it!

NikolayS · 2018-11-24T04:00:55Z

resources/checks/A004_cluster_info.sh

 #dbg "PSQL_CONN_OPTIONS: ${PSQL_CONN_OPTIONS}"
-pgver=$(psql ${PSQL_CONN_OPTIONS}  -c "SHOW server_version" -t -A)
+
+pgver=$(ssh ${HOST} "${_PSQL} -c \"SHOW server_version\"")


it's a bit easer to get show server_version_num here and the make interger comparison, w/o any regexp-replacement

Let's refactor checks separate from script logic?

NikolayS · 2018-11-24T04:01:34Z

resources/checks/A004_cluster_info.sh

@@ -127,7 +129,7 @@ EOF

 fi

-psql ${PSQL_CONN_OPTIONS} <<SQL
+ssh ${HOST} "${_PSQL} -f - " <<SQL


it's great that it works!

NikolayS · 2018-11-24T04:07:09Z

resources/cli.conf

@@ -10,7 +10,10 @@
 Connection options|p|port|PORT|number|optional|PostgreSQL database server port (default: "5432")
 Connection options|d|dbname|DBNAME|word|optional|database name to connect to (default: "postgres")
 Connection options|U|username|USERNAME|word|optional|database user name (default: current)
-Connection options|h|hostname|HOST|alnum|mandatory|database and ssh server host or socket directory (default: "local socket")
+Connection options|h|hostname|HOST|text|mandatory|database and ssh server host
+Connection options|s|pg-socket-dir|PGSOCKET|text|optional|PostgreSQL domain socket dir (default: psql's default)


there is light inconsistency in naming here -- various prefixes (DB..., PG..., and no prefix as in HOST)

Yes, you're right. I changed PORT to PGPORT. DBNAME: I've got from man psql.
HOST: I decided to keep "as is" 'cause it is slightly not the same as PGHOST - we use it for a ssh connection.

NikolayS · 2018-11-24T04:07:18Z

resources/cli.conf

-Connection options|h|hostname|HOST|alnum|mandatory|database and ssh server host or socket directory (default: "local socket")
+Connection options|h|hostname|HOST|text|mandatory|database and ssh server host
+Connection options|s|pg-socket-dir|PGSOCKET|text|optional|PostgreSQL domain socket dir (default: psql's default)
+Connection options|None|psql-binary|PGBINARY|text|optional|psql utility path (default: from $PATH)


PSQLBINARY ?

Sure, fixed. Thanks.

NikolayS · 2018-11-24T04:09:04Z

resources/templates/nodes.json

@@ -0,0 +1,12 @@
+{


is there any risk that we, working with some particular DB replicaset, will override this file and commit/push by mistake?

it is kind of config, right? it will be individual for every project? where can we keep it (may we keep it outside the repo?)?

Hm... ./resources/templates directory conceived as a place where we keep templates to generate another files.
It is a part of ./check. We don't write artifacts into ./resources dirs.

NikolayS

Awesome! Only a few of minor comments. Let's merge it

Nastradamus

I replied and will push fixes.

Nastradamus · 2018-11-26T08:07:53Z

check

  echo >&2
 }

 #######################################
 # Cleanup function: close ssh sockets, etc.
 # Globals:
-#   None
+#   HOST


We decided to check only one host per single script run.

Nastradamus · 2018-11-26T08:08:32Z

check

-  done
+  if [[ ! -z ${HOST+x} ]]; then
+    dbg "closing ssh conenction to host '$HOST' (if exists)"
+    ssh -O exit ${HOST} 2>/dev/null || true


Nastradamus · 2018-11-26T08:14:51Z

resources/checks/A004_cluster_info.sh

 #dbg "PSQL_CONN_OPTIONS: ${PSQL_CONN_OPTIONS}"
-pgver=$(psql ${PSQL_CONN_OPTIONS}  -c "SHOW server_version" -t -A)
+
+pgver=$(ssh ${HOST} "${_PSQL} -c \"SHOW server_version\"")


Let's refactor checks separate from script logic?

Nastradamus · 2018-11-26T08:21:36Z

resources/cli.conf

@@ -10,7 +10,10 @@
 Connection options|p|port|PORT|number|optional|PostgreSQL database server port (default: "5432")
 Connection options|d|dbname|DBNAME|word|optional|database name to connect to (default: "postgres")
 Connection options|U|username|USERNAME|word|optional|database user name (default: current)
-Connection options|h|hostname|HOST|alnum|mandatory|database and ssh server host or socket directory (default: "local socket")
+Connection options|h|hostname|HOST|text|mandatory|database and ssh server host
+Connection options|s|pg-socket-dir|PGSOCKET|text|optional|PostgreSQL domain socket dir (default: psql's default)


Yes, you're right. I changed PORT to PGPORT. DBNAME: I've got from man psql.
HOST: I decided to keep "as is" 'cause it is slightly not the same as PGHOST - we use it for a ssh connection.

Nastradamus · 2018-11-26T08:21:51Z

resources/cli.conf

-Connection options|h|hostname|HOST|alnum|mandatory|database and ssh server host or socket directory (default: "local socket")
+Connection options|h|hostname|HOST|text|mandatory|database and ssh server host
+Connection options|s|pg-socket-dir|PGSOCKET|text|optional|PostgreSQL domain socket dir (default: psql's default)
+Connection options|None|psql-binary|PGBINARY|text|optional|psql utility path (default: from $PATH)


Sure, fixed. Thanks.

Nastradamus · 2018-11-26T08:24:58Z

resources/templates/nodes.json

@@ -0,0 +1,12 @@
+{


Hm... ./resources/templates directory conceived as a place where we keep templates to generate another files.
It is a part of ./check. We don't write artifacts into ./resources dirs.

…tgres-health-check into 64-multiple-host-support

Nastradamus and others added 5 commits November 22, 2018 17:49

#64 rethink some concepts. Remove multiple host support in a single run.

7169118

#64 added new json - to keep general structure for a project

aa6ffef

Some fixed

4b8d244

#64 back-port jq array fix from @dmius

dfd2589

#64 massive rework of json generator, refactoring, add new value 'che…

ed0546d

…ck_name', generate report with this filename, add 'check_name' into json report

Nastradamus mentioned this pull request Nov 23, 2018

Refactor work with jq #79

Closed

Nastradamus added 2 commits November 23, 2018 18:56

fix results

201bc4a

migrate to _PSQL

a958899

Nastradamus changed the title ~~[WIP] multiple host support~~ multiple host support part1 Nov 23, 2018

Nastradamus self-assigned this Nov 23, 2018

Nastradamus requested review from dmius and NikolayS November 23, 2018 16:15

Merge branch 'master' into 64-multiple-host-support

a3bc726

NikolayS reviewed Nov 24, 2018

View reviewed changes

check Outdated

# Generate json report

# Globals:

# CURRENT_CHECK, SCRIPT_DIR, PROJECT,

# HOST,

Copy link

Collaborator

NikolayS Nov 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra ,

NikolayS reviewed Nov 24, 2018

View reviewed changes

check

local epoch=null

[[ -z ${3+x} ]] && err "function needs 3 arguments"

Copy link

Collaborator

NikolayS Nov 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool way to check it!

NikolayS reviewed Nov 24, 2018

View reviewed changes

NikolayS previously approved these changes Nov 24, 2018

View reviewed changes

Nastradamus commented Nov 26, 2018

View reviewed changes

Nastradamus added 2 commits November 26, 2018 11:26

#64 little fixes

e3667fe

Merge branch '64-multiple-host-support' of github.com:postgres-ai/pos…

02c3c79

…tgres-health-check into 64-multiple-host-support

Nastradamus dismissed NikolayS’s stale review via 02c3c79 November 26, 2018 08:28

dmius approved these changes Nov 26, 2018

View reviewed changes

Nastradamus merged commit 36cc88e into master Nov 26, 2018

ane4ka added the S4 Sprint №4 label Nov 29, 2018

NikolayS deleted the 64-multiple-host-support branch December 7, 2018 15:22


		local epoch=null

		[[ -z ${3+x} ]] && err "function needs 3 arguments"

multiple host support part1 #77

multiple host support part1 #77

Uh oh!

Conversation

Nastradamus commented Nov 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NikolayS left a comment

Choose a reason for hiding this comment

Uh oh!

Nastradamus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!