Make launchpad-templates use Dockerfile instead of ConfigMap for nginx

[aditya-konarde]

Add nginx conf file (extracted from the existing ConfigMap) Add a Dockerfile for building the nginx image. Add a CI script to build the Docker Image. Add a nginx runner script.

[quintesse]

Could you explain what this PR is about exactly @aditya-konarde ?

[vpavlin]

Hi Tako, we are trying to unify our approach to using nginx across openshift.io and the agreement was we'd prefer to bake nginx.conf in the image rather than use configMap (configMap is troublesome because there are no triggers in OpenShift for CM change, so config change will not update the running app..)

Aditya picked up the task of the unification.

@aditya-konarde You will also need to fix the OpenShift template and remove the configmap

[quintesse]

Ok @vpavlin , but then how are we supposed to run these things locally on Minishift?

Edit: or is the idea to make this nginx image publicly available as well, just like the launchpad images?

[aditya-konarde]

@quintesse The CI build script will also push the image to the devshift registry, similar to the other launchpad images. Also, discussed with @vpavlin and we need to change the template for launchpad-nginx to use this image instead of the configmap. [WIP].

[vpavlin]

You can build the nginx image locally or you can pull it from registry.devshift.net (it's public since couple weeks back).

[quintesse]

You can build the nginx image locally or you can pull it from registry.devshift.net

Yeah, for a moment I thought you might have to always build it locally. But as long as it's available publicly it's okay.

Marking it as "changes needed" so nobody merges it accidentally.

[aditya-konarde]

@vpavlin @quintesse updated. Please review.

[quintesse]

@aditya-konarde it looks okay, but we can't merge this until this new image has been accepted on the centos.org CI build server and an image has actually been built. Because without that this template will fail (for people who don't build things locally).

[quintesse]

@aditya-konarde actually to expand on what I said before. Perhaps what we should do is split this into 2 separate PRs. One that contains the CICO script and the Dockerfiles etc so the image can be built, which we can merge immediately. And the other that contains the changes to the template which we will merge the moment the image is available in the registry.

Sounds like a plan?

[aditya-konarde]

@aditya-konarde it looks okay, but we can't merge this until this new image has been accepted on the centos.org CI build server and an image has actually been built. Because without that this template will fail (for people who don't build things locally).

I agree.

@aditya-konarde actually to expand on what I said before. Perhaps what we should do is split this into 2 separate PRs. One that contains the CICO script and the Dockerfiles etc so the image can be built, which we can merge immediately. And the other that contains the changes to the template which we will merge the moment the image is available in the registry. Sounds like a plan?

Sounds good. One other way to do this would also be to build and push the image to the devshift registry once, and then merge this PR. @vpavlin : thoughts?

[vpavlin]

1. @aditya-konarde Please remove the volumes block as well: https://github.com/openshiftio/launchpad-templates/pull/26/files#diff-605327b86e0de8a02c3c4f36f50268e6R63
2. @quintesse Would it be a big deal if the repo is broken for couple minutes? (merge -> build -> push -> deploy -> validate -> if failed, fix and go again:) ). If not a bug deal, I'd say let's merge it as is - it's not going to break prod (that depends on specific commit hash), so the only potential issue is broken stage (which is fine) and local experience from master (guess that's not going to kill anyone:) ).

We still need to update almighty-jobs job before we merge this, btw..

[aditya-konarde]

@vpavlin Removed the volumes block. Also, sent out a PR to update almighty-jobs here

[vpavlin]

@quintesse If you are ok with the approach, we can merge https://github.com/almighty/almighty-jobs/pull/283/files, then merge this PR and then observe some awesomeness (with all fingers crossed;))

[quintesse]

@vpavlin ok, we can do it that way. We can always revert if it takes more than a couple of minutes (it always does ;) )

[vpavlin]

So I'd say let's do it tomorrow morning so that @aditya-konarde has time to fix all he broke:-P

[quintesse]

Ok, I manually merged all the stuff that had to do with CICO. Mainly because we had already added a scripts folder of our own in the meantime which was conflicting. So I moved the CICO files to a new cico folder (except for the build script and the Dockerfile) which is also cleaner IMO.

We can now merge the almighty-jobs PR, wait for the image to be built and merge the rest of the changes (which aren't many).

[gastaldi]

So this issue can be closed now?

[quintesse]

No, we're not done yet :)

[quintesse]

WHen do we do this @vpavlin @aditya-konarde ?

[vpavlin]

@quintesse We fixed this PR, so...Merging in your absence:)

[quintesse]

Btw, @vpavlin @aditya-konarde , why is this nginx image pushed to a different registry than the rest of the images? The other launchpad images are all on Docker Hub.