@jskladan FYI, I've built an image using this openshift/Dockerfile and pushed it to quay.io/factory2.

@csomh Cool. Do you want me to wait for for @dcallagh before merging?

@jskladan, yes, please let's wait for him having a look first. Thanks!

@mikeb @ralph ptal

Sorry for taking so long to look at this.

It looks good to me. :+1:

One thing I just noticed, looking in the openshift/run_app script... We are initializing the database as part of the app startup. We learnt the hard way in https://pagure.io/waiverdb/issue/121 that that is not a good approach, it should be an OpenShift pre-deployment hook instead. At that point I would probably suggest just eliminating the run_app script and filling in the ENTRYPOINT as mod_wsgi-express ... directly.

But that shouldn't hold up this PR.

Thanks @dcallagh for pointing this out! I'll have a look in that thread and work on a PR to fix the test template and remove run_app.

@dcallagh, I choose to update this PR after all, please take another look.

See cc5d45d and e94f59a, which add the pre-deployment hook to the test-template and change the ENTRYPOINT of the container.

I choose to use 'Retry' as a failure policy in the hook (as opposed to the 'wait_for_db' approach seen in WaiverDB), in order to handle the database becoming available a little bit slower as the hook is executed. Tested locally in a minishift cluster and the hook was always successful on the second try.

FYI, a while back when I was researching this stuff, I discovered that this does not actually do anything, for example: https://github.com/openshift/openshift-docs/pull/5846

Nice idea. I assume this doesn't retry forever, if the migrations are actually failing because of a bug it will give up after 10 minutes or something, right?

:+1: aside from a couple of minor things noted inline

Removed all dependencies annotations, see fixup 4191a19 .

failurePolicy: Retry will actually try until it succeeds, so changed it to Abort and modified the command to try resultsdb init_db for 10 minutes, exit with 0 if succeeds or fail if it doesn't.

Let me know if you find this to be a better approach. Thanks!

Sounds fine to me. :+1:

I think the Waiverdb approach where it first (silently) waits for the db and then tries the upgrade exactly once -- is still nicer. It results in less scary-looking mess in the logs. But no big deal, we can improve that later.

Re-based and squashed fixups.

@jskladan I guess, it's good to be merged. Thanks!

Commit 1f600616 fixes this pull-request