Add postgresql 10.6 rock-on

[holmesb]

No description provided.

[phillxnet]

@holmesb Thanks for the pull request. Can't review this at the moment but hopefully someone will step up when they have time and we can get this one checked and in. I've added the 'needs review' label.

[FroggyFlox]

Hi @holmesb,

First of all, sorry for not having a look at your pull request earlier.

I just had a look at it and have a few questions as we have a couple of options to make this rock-on work:

1. I see you specified the centos' repository from which to pull the image. Does that mean you want to use this repo instead of docker hub?
2. You apparently would like to pull the 10.6 version, based on the image name you used. It doesn't apparently seem to work from centos' repository. Docker hub has a 10.6 but also a newer 10.10 image available, however. As a result, which version would you prefer?

Thanks for letting us know your intention on these two points and we should be able to move forward from there.

[holmesb]

I'm pulling postgres:10.6 from dockerhub.

(digest is ebdd3a33f8829b595103d17312b52836fe162aed579697bb7be3a13dcc2343c4)

I need v10.6. Best to parameterise the version IMO, but I don't think merging should wait for this. This is a working postgresql rock-on and a sound basis for improvement in future. Having browsed a selection of merged rockons, I can't find any examples of with parameterised image version.

[FroggyFlox]

Best to parameterise the version IMO

Funny you say that... It's actually something I had in the back of my head lately so it's good to see there's some interest. Postgresql does represent a very good opportunity to do so, indeed. As you say, though, I don't think this PR should rely on that. We can specify a tag in this version and if and when a way to parametize this is implemented, there's always the option to update this rock-on definition accordingly.
If you'd like, feel free to submit a corresponding issue on the rockstor-core repo describing this feature request and you think we may implement it.

I'm pulling postgres:10.6 from dockerhub.

Thanks for the confirmation. Thanks to a recent merge by @phillxnet, we can use the "tag" parameter to specify that.
I'll try to submit a proper review of this PR as soon as possible...

Thanks for the additional information, and sorry again for the delay in getting to your PR.

[FroggyFlox]

Hi @holmesb,

I just tried to go ahead with a proper review of this but ran into an issue we identified recently (rockstor/rockstor-core#2016 (comment)) that prevents the use of two different versions of the same image. As the postgres image is already in the database (v9.5) for another rock-on (owncloud), Rockstor simply uses that image for your rock-on as well.

This can be fixed by a small patch:
FroggyFlox/rockstor-core@e797f55

@phillxnet, we are here encountering for the first time a problem with the problem we identified recently but that can be overcome if we account for the image tag when parsing rock-on definitions into the database (previously we only considered the image name, thus preventing us to use different tags of the same image in two different rock-ons). I'll try to submit a corresponding PR in rockstor-core as soon as I tested my fix more thoroughly.

@holmesb, in the meantime, I'll submit to you a few suggestions shortly related to formatting as we're currently trying to harmonize new rock-ons definitions (see #31). Even if this rock-on will be blocked by the obstacle described above, the format will be done. Sorry for the inconvenience.

[FroggyFlox]

For reference, the issue blocking this PR was already submitted at the time on rockstor-core:
rockstor/rockstor-core#2017 (comment)

Once this issue is fixed, it will be possible to use a specific version of the postgres image as needed in this PR.

[FroggyFlox]

@holmesb , @phillxnet ,

Following the recent merges in 3.9.2-51, we now can have several versions of the same docker image, which means we can proceed further with this pull request.

I've made some suggestions to use the "tag" object (for using the proper version), as well as style changes to harmonize it with our other rock-ons.

Hope this helps,

[phillxnet]

@FroggyFlox Is this now ready to publish? Given @holmesb now looks to have applied your suggested changes.

[phillxnet]

@FroggyFlox I may have dropped the ball on this one.

It looks like all your suggestions are now in?

[FroggyFlox]

@phillxnet , I meant to give that one a final test but forgot :(.

I'll try again as soon as possible and report bcak. I notice the "website" suggestion has not been included, but my main "concern" about that rock-on was actually related to its dependency on 3.9.2-51. Indeed, this means that all users on the current testing channel who try to install this rock-on will actually end up with postgresql v9 installed rather than 10.6 as specified in this image (for reasons explained above).

How do you think we should best deal with that? Should we include a note in the description that "this rock-on is compatible only with rockstor-3.9.2-51 and up"?

[phillxnet]

@FroggyFlox Re:

How do you think we should best deal with that? Should we include a note in the description that "this rock-on is compatible only with rockstor-3.9.2-51 and up"?

Yes I think that's the way to go for now. I.e.:
"... requires rockstor version 3.9.2-51 or later ..."
or
"... (requires at least rockstor 3.9.2-51) ..."

[FroggyFlox]

@phillxnet , I just added the suggestion.

[phillxnet]

@holmesb now we have 3.9.2-51 out, do you fancy adding @FroggyFlox last suggestion, referencing this requirement, and then we can get this one finally tested and merged. Apologies it's all taken so long but we had that blocker bug which ended up delaying us even more.

@FroggyFlox Thanks for formalising my suggestion, I should have created code change suggestion at the time myself.

[phillxnet]

I've unresolved the outstanding website link re @FroggyFlox's prior suggestion and:

I notice the "website" suggestion has not been included ...

Plus another suggestion that seems not to have been committed as well:

-        "host_default":5433,

I think we are seeing some suggestions across pull requests being resolved without their actual code suggestions being applied. This pull request looks to be another of those affected.

[phillxnet]

@holmesb We've just had a Postgresql 9.5 Rock-on merged based heavily on what we worked out here in your pull request, with a few minor change to accommodate multiple versions of Postgresql. I'm going to go ahead and merge this one as-is and make the last few changes we worked out in the derived pull request in a follow up one of my own. I'll post here again once the result is published. You submission was unfortunately a little ahead of our capability to support it but I think we are there now, at least in the Stable Channel updates.

[phillxnet]

@holmesb Re:

I'll post here again once the result is published.

You Rock-on is now published and available to all Rockstor users. I ended up reformatting and adding in @Hooverdan96's extension to allow install time admin user pass setup, plus a few minor text changes.

So we now have concurrently capable 9.5 and 10.6 variants. We got there in the end.
Apologies for this dragged out for nearly ever, but again, thanks for getting us started on these PostgeSQL Rock-ons.