templates: use pg_isready as liveness probe #206

pkubatrh · 2017-11-16T10:07:47Z

Resolves: #195

pkubatrh · 2017-11-21T08:31:38Z

@praiskup Can you take a quick look at this?

@pvalena @bparees Is there anything more we need to do to get the change into the Openshift templates?

pvalena · 2017-11-21T11:09:28Z

AFAIK you primarily need to add the new version 9.6

as a default for deployment; and
in all texts, such as 9.2, 9.4, 9.5 or latest

Thanks!

pkubatrh · 2017-11-21T11:22:53Z

Makes sense, thanks.

pvalena · 2017-11-21T12:33:22Z

examples/postgresql-ephemeral-template.json

@@ -245,8 +245,8 @@
    {
      "name": "POSTGRESQL_VERSION",
      "displayName": "Version of PostgreSQL Image",
-      "description": "Version of PostgreSQL image to be used (9.2, 9.4, 9.5 or latest).",
-      "value": "9.5",
+      "description": "Version of PostgreSQL image to be used (9.4, 9.5, 9.6 or latest).",


Is 9.2 not relevant anymore?

Has been deprecated for a long while already.

Ok. We'll need to keep an eye on that.

pg_isready is not available for 9.2 anyway

pvalena · 2017-11-21T12:36:25Z

examples/postgresql-ephemeral-template.json

@@ -5,7 +5,7 @@
    "name": "postgresql-ephemeral",
    "annotations": {
      "openshift.io/display-name": "PostgreSQL (Ephemeral)",
-      "description": "PostgreSQL database service, without persistent storage. For more information about using this template, including OpenShift considerations, see https://github.com/sclorg/postgresql-container/blob/master/9.5.\n\nWARNING: Any data stored will be lost upon pod destruction. Only use this template for testing",
+      "description": "PostgreSQL database service, without persistent storage. For more information about using this template, including OpenShift considerations, see https://github.com/sclorg/postgresql-container/.\n\nWARNING: Any data stored will be lost upon pod destruction. Only use this template for testing",


AFAIK this was meant to give container-specific instructions (really simple, basic usage).
Therefore pointing to README.md - for OpenShift; regardless of version (IMHO most ppl just use latest).
IOW: I'd rather keep the 9.6.

f.e. https://github.com/sclorg/postgresql-container/tree/master/9.5#environment-variables-and-volumes

I have modified this in preparation for the last stages of image content generation (see #203 ) - eventually there will not be a 9.6 to directly link to and so the current link would result in a 404.

Additionally, it is currently impossible to link to the 9.6 README anyway, without going through latest/ which might change into a different version in the future. So it makes sense to me to just point users to the root.

pvalena · 2017-11-21T12:55:11Z

-- I hope it's not a problem for OpenShift, as they might link to it somewhere AFAIK.

@bparees, is it fine if 9.6 folder is just a symlink? Can we link root, or possibly latest directory?

pvalena · 2017-11-21T13:03:44Z

FTR: Subsequently I'll create image-streams and PR for openshift/origin.

pvalena · 2017-11-21T13:06:11Z

Also: before this PR gets merged I'd like to have the original template tested it by CI in OpenShift, so the changes can be actually validated.

See #204.

praiskup · 2017-11-21T13:36:24Z

examples/postgresql-ephemeral-template.json

@@ -127,11 +127,11 @@
                  }
                },
                "livenessProbe": {
+                  "exec": {
+                    "command": [ "/bin/sh", "-i", "-c", "pg_isready -h 127.0.0.1 -p 5432" ]


I guess the /bin/sh is necessary, but I would rather avoid using the -i option, if possible.

I have left it in only because it was present in other /bin/sh invocations inside the template

praiskup · 2017-11-21T13:43:18Z

@bparees, is it fine if 9.6 folder is just a symlink? Can we link root, or possibly latest directory?

This is long time discussed thing; IMO linking directly to gitrepo - to concrete paths - is a bad practice. There's a card on trello where we could discuss/implement a better place for documentation.

Regarding the link, it is a temporary issue ... since we could move the "latest" to PostgreSQL 10 soon, at least for testing purposes.

praiskup · 2017-11-21T13:43:48Z

This looks good to me, but having this tested is a must :-)

bparees · 2017-11-21T13:49:11Z

examples/replica/postgresql_replica.json

@@ -71,7 +71,7 @@
      "name": "IMAGESTREAMTAG",
      "displayName": "ImageStreamTag",
      "description": "The OpenShift ImageStreamTag to use for PostgreSQL.",
-      "value": "postgresql:9.5"
+      "value": "postgresql:9.6"


there's no postgres 9.6 imagestream tag in openshift yet:
https://github.com/openshift/origin/blob/master/examples/image-streams/image-streams-centos7.json#L724-L795

so you need to deliver that before you start making templates default to looking for it.

We'll update at once - in our repos. The template will be actually tested with freshly build and tagged image into custom namespace.

bparees · 2017-11-21T13:50:31Z

@praiskup can you lay out the plan/order of operations here? is it to:

deliver this here
deliver the 9.6 imagestream to origin/image-streams
deliver this template to origin/db-templates

?

praiskup · 2017-11-21T13:56:25Z

According to the discussion here, I think so :-), per @pvalena comment:

FTR: Subsequently I'll create image-streams and PR for openshift/origin.

pvalena · 2017-11-21T14:03:39Z

@bparees, the order is a bit parallelized, but basically:

openshift tests
updated template (container folder already done)
generated image-streams, although currently using manual comparison
Pull Requests to openshift/origin

Where (1+2) and 3 are being worked on in parallel (I'm working on the image-streams; and together with @hhorak will provide help with the steps above).

pkubatrh · 2017-11-21T14:18:12Z

I will look into getting the tests up and running so we can test this out properly.

Resolves: sclorg#195

pkubatrh · 2017-12-01T14:44:28Z

[test-openshift]

pkubatrh · 2017-12-04T14:15:13Z

[test-openshift]

pkubatrh · 2017-12-04T15:16:08Z

[test-openshift]

pkubatrh · 2017-12-04T15:33:52Z

@praiskup Seems like resalloc does not want to cooperate again.

praiskup · 2017-12-05T07:27:20Z

[test-openshift]

pkubatrh · 2017-12-05T08:59:57Z

Thanks! Tests are green now

pkubatrh · 2017-12-05T13:24:52Z

Fixed one more issue with pure image tests [test-openshift]

pkubatrh · 2017-12-05T14:25:48Z

[test-openshift]

praiskup · 2017-12-07T08:24:29Z

lgtm

pkubatrh · 2017-12-07T09:39:56Z

Thanks for review! I will make one more rebase against container common scripts to pick up sclorg/container-common-scripts#45

It is not needed for postgresql's openshift tests right now but I would like to have it fixed for eventual s2i (or other cases accessing the internet) before the problem is forgotten again.

pkubatrh · 2017-12-07T09:54:14Z

[test-openshift]

pkubatrh · 2017-12-07T11:02:34Z

Tests are green. Merging this in. Thanks everyone!

pkubatrh force-pushed the liveness branch from 3da9b06 to 3b4ea7f Compare November 21, 2017 11:36

pvalena reviewed Nov 21, 2017

View reviewed changes

praiskup reviewed Nov 21, 2017

View reviewed changes

bparees reviewed Nov 21, 2017

View reviewed changes

pkubatrh added 2 commits December 1, 2017 15:43

templates: use pg_isready as liveness probe

c00ff8e

Resolves: sclorg#195

templates: Use 9.6 as the default version

37cff38

pkubatrh force-pushed the liveness branch from 3b4ea7f to 37cff38 Compare December 1, 2017 14:44

pkubatrh mentioned this pull request Dec 4, 2017

test-libs: add the ability to set ImageStream name and tag sclorg/container-common-scripts#44

Merged

tests: use local namespace instead of openshift

ec1a0b4

tests: set the image version for the template

cc5dd19

tests: explicitly state tag during pure image test

979b709

pkubatrh force-pushed the liveness branch from 78292e4 to 979b709 Compare December 5, 2017 14:25

Update to latest commons

2da5f7b

pkubatrh merged commit 8230ed1 into sclorg:master Dec 7, 2017

pkubatrh deleted the liveness branch December 12, 2017 10:29

pvalena mentioned this pull request Jan 16, 2018

Add image-streams and update db-templates openshift/origin#17922

Merged

templates: use pg_isready as liveness probe #206

templates: use pg_isready as liveness probe #206

Conversation

pkubatrh commented Nov 16, 2017

pkubatrh commented Nov 21, 2017

pvalena commented Nov 21, 2017

pkubatrh commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvalena Nov 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvalena Nov 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvalena commented Nov 21, 2017

pvalena commented Nov 21, 2017

pvalena commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

praiskup commented Nov 21, 2017

praiskup commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparees commented Nov 21, 2017

praiskup commented Nov 21, 2017

pvalena commented Nov 21, 2017 • edited Loading

pkubatrh commented Nov 21, 2017

pkubatrh commented Dec 1, 2017

pkubatrh commented Dec 4, 2017

pkubatrh commented Dec 4, 2017

pkubatrh commented Dec 4, 2017

praiskup commented Dec 5, 2017

pkubatrh commented Dec 5, 2017

pkubatrh commented Dec 5, 2017

pkubatrh commented Dec 5, 2017

praiskup commented Dec 7, 2017

pkubatrh commented Dec 7, 2017

pkubatrh commented Dec 7, 2017

pkubatrh commented Dec 7, 2017

pvalena Nov 21, 2017 •

edited

Loading

pvalena Nov 21, 2017 •

edited

Loading

pvalena commented Nov 21, 2017 •

edited

Loading