fabric8io-images / s2i

OpenShift S2I images for Java and Karaf applications
Apache License 2.0
70 stars 84 forks source link

Self test, and CI for PRs #120

Open vorburger opened 6 years ago

vorburger commented 6 years ago

It could be neat to have a small self contained "self test" (and a CI with it, e.g. Travis, for all PRs) like:

docker build -t fabric8/s2i-java:latest
s2i build s2i-java-example fabric8/s2i-java fabric8/s2i-java-example

and then ideallly, not 100% sure how to do this best, something like:

docker run -p 8080:8080 vorburger:s2i-java-example &
# TODO how to do simplest possible HTTP GET http://localhost:8080 and fail/pass?
# TODO how to best stop docker again
kill $!

I'm happy to move the https://github.com/vorburger/s2i-java-example repo under https://github.com/fabric8io-images, or even just directly contribute that into this repo into some place like s2i/java/example, if either could be of interest or help to get this done.

vorburger commented 6 years ago

and, more minor but while we are at it might as well, if we do something like this, then I guess it should fish-pepper right as part of it, to avoid mistake in contributions changing images/ but not templates/ (I initially made that mistake locally).

vorburger commented 6 years ago

and to avoid e.g. "fixing the jboss community image but breaking the RHEL image" (or the other way around..) from happening which @jamesnetherton picked up on e.g. in https://github.com/fabric8io-images/s2i/pull/114, I guess this really should run both the images/jboss as well as the images/rhel ... but that fails for me:

Sending build context to Docker daemon 82.43 kB
Step 1/28 : FROM jboss/openjdk18-rhel7:1.1-7
Trying to pull repository docker.io/jboss/openjdk18-rhel7 ... 
Trying to pull repository registry.fedoraproject.org/jboss/openjdk18-rhel7 ... 
Trying to pull repository registry.access.redhat.com/jboss/openjdk18-rhel7 ... 
Trying to pull repository docker.io/jboss/openjdk18-rhel7 ... 
repository docker.io/jboss/openjdk18-rhel7 not found: does not exist or no pull access
rhuss commented 6 years ago

Yes, an example makes much sense. 'happy to add it directly to this repository, to keep it simple. Fancy doing a PR ?

wrt/ pulling from registry.access.redhat.com you probably need access to the Red Hat registry. Haven't tried this since quite some time as it build by the product team. @jamesnetherton Any idea what the requirements are for building the rhel based images ?

vorburger commented 6 years ago

Fancy doing a PR ?

see https://github.com/fabric8io-images/s2i/pull/127 for a start and lets discuss any feedback on that? You'll figure that the idea is that running the test.sh performs this "self test" initially described above here. (And as of this moment right now, that will actually fail - because of #113 - great, TDD at it's best! :smiling_imp:)

wrt/ pulling from registry.access.redhat.com you probably need access to

unless @jamesnetherton can provide some guidance how to do this right upstream, lets just wrap this up at least for the community image, and forget about self testing the rhel based image here, for now? Just because from what little I understand of such matters, I somewhat doubt that there is an easy way (with like a test credential or something?), for a test script of the java/images/rhel/ here in an upstream project like this. Probably not worth it then - but having automated test coverage at least for the community image (i.e. java/images/jboss/) would be a great step already I guess.

vorburger commented 6 years ago

https://github.com/fabric8io-images/s2i/pull/136 contributes a working test.sh, which we could probably now use from some service like CircleCI, or Travis. I see that this project actually has a circle.yml, but that does not seem to cause any builds to run on PRs ... does anyone know why? Is there any preference for CircleCI, or can I go ahead and see if we can get this working on https://travis-ci.org, which I have a little bit of experience with?

rhuss commented 6 years ago

We are currently quite happy with circle, so not really keen on changing to travis.

The reason why the build is running only on master or on tags is that because the build is only used for pushing images to Docker hub. Obviously, you dont want to push an image for a PR build.

But its easy to add a CI test for PR as well. Let's get test.sh fixed first and then check how to adapt circleci.yml ...

vorburger commented 6 years ago

Let's get test.sh fixed first and then check how to adapt circleci.yml ...

@rhuss with #136 merged, we could do this now? Dunno exactly what is needed - more than #149 ?

vorburger commented 5 years ago

@rhuss thought about this again while working on the Java 11 stuff (#160) ... running the ./test.sh, just improved in #183, on all PRs would be really useful for non-regression ... if you would be able to find the cycles to get that set up on CircleCI, that would be really cool!

vorburger commented 5 years ago

I currently manually run ./test.sh on every change manually locally to make sure I don't break anything, but others of course will not (and I would also prefer if PRs would self-verify, instead you having to trust me).

vorburger commented 5 years ago

This may be as simple as adding ./test.sh to https://github.com/fabric8io-images/s2i/blob/master/.circleci/config.yml ... the problem, as points out in #149 is to have the s2i CLI itself and fish-pepper available in the Circle environment.

vorburger commented 5 years ago

Fixing #223 req for #149 would wrap this up.