openid-certification / oidctest

THE CERTIFICATION TEST SUITE HAS BEEN MIGRATED TO A NEW SERVICE https://www.certificatinon.openid.net
Other
50 stars 15 forks source link

Docker containers should not rely on git checkout #151

Closed zandbelt closed 5 years ago

zandbelt commented 5 years ago

As suggested by @travisspencer we should: a) modify the Docker containers in such a way that they do no longer rely on git checkouts b) put up Docker images on Dockerhub to speed up local deployment for those who don't need local modifications anyhow

sozkan commented 5 years ago

Can you elaborate on the requirements a bit? What is the end goal? Is it reducing the total start up time or avoiding git calls at all? For example, would it be enough if we could reduce the startup time by making other changes without touching git calls.

By the way, I am assuming that you mean "git clone"s as in https://github.com/openid-certification/oidctest/blob/master/docker/op_test/Dockerfile#L23.

travisspencer commented 5 years ago

Yes, the link is an example of the problem.

The use case is this: we want to run all tests on every commit of the source code of our OP. To that end, we have automated all tests with Selenium. We build and start the OP test tool beforehand. To do this, we added the OP test tool as a submodule in our git repo at a particular hash where we were certified last. The docker image at this point though has a git clone command in it. This causes problems because the tool's dependencies have unit tests that run when the container is built. These tests (at head of the dependency's repo) do not pass when run with the older OP test tool code that we have checked in. The only fix is to forward the hash of the OP test tool in our repo to head. The consequence is that we 1) have to spend time doing an unplanned upgrade to get our tests back to green, and 2) we end up testing something else than we have certified.

sozkan commented 5 years ago

Thanks for the clarification Travis. I think our/your options are as follows: 1- You create a local docker repository/registry: Instead of building and running the oidctest image every time your tests run (please correct me if this is not the case), you can :

2- We publish docker images to docker hub: We can publish oidctest (and other) docker images every time we do an oidctest release. For example when we deploy oidctest v1.2.1 to production, we can build and publish a docker image to docker hub, e.g oidf/oidctest:1.2.1. Then you can simply pull and run this image in your tests.

3- Changing the dockerfiles: Assuming that we know exactly what version of otest is needed for a specific oidctest version, we can add an argument to dockerfile and checkout the otest version.

RUN git clone https://github.com/openid-certification/otest.git
RUN cd otest && git checkout ${OTEST_VERSION} && python3 setup.py install && cd -

Note the git checkout ${OTEST_VERSION} part, where OTEST_VERSION is a otest tag.

In my opinion, option 2 would be the best as others can also benefit from it. Option 1 would also be an interim solution for you, i.e until we start publishing to docker hub.

@zandbelt what do you think?

zandbelt commented 5 years ago

@rohe is the otest package not on pipy? it would be most convenient to get a release from there

rohe commented 5 years ago

I'll make it so.

On 18 Mar 2019, at 11:00, Hans Zandbelt notifications@github.com wrote:

@rohe https://github.com/rohe is the otest package not on pipy? it would be most convenient to get a release from there

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openid-certification/oidctest/issues/151#issuecomment-473844276, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGGPCbF_p3_9bheD1WPREqVsz-49JABks5vX2PXgaJpZM4b0Pqh.

Otium cum dignitate - latin proverb

sozkan commented 5 years ago

@zandbelt are we going to wait for Roland's change or should I investigate how we can publish images to docker hub? I think it will be nice to publish images even if dockerfiles are changed. It will make life slightly easier for people that want to run the test suite locally.

zandbelt commented 5 years ago

Yes we should do both, as the ticket suggests

rohe commented 5 years ago

The otest package is on pipy. Uploaded the latest version just after I wrote the email below.

On 18 Mar 2019, at 14:30, Roland Hedberg roland@catalogix.se wrote:

I'll make it so.

On 18 Mar 2019, at 11:00, Hans Zandbelt <notifications@github.com mailto:notifications@github.com> wrote:

@rohe https://github.com/rohe is the otest package not on pipy? it would be most convenient to get a release from there

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openid-certification/oidctest/issues/151#issuecomment-473844276, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGGPCbF_p3_9bheD1WPREqVsz-49JABks5vX2PXgaJpZM4b0Pqh.

  • Roland

Otium cum dignitate - latin proverb

— Roland

It is curious that physical courage should be so common in the world, and moral courage so rare. -Mark Twain, author and humorist (30 Nov 1835-1910)

zandbelt commented 5 years ago

@rohe thanks!

BjoernAkAManf commented 5 years ago

One additional note: Publishing the image to a public place allows easy Integration / Unit tests in many major languages that utilise something like https://github.com/testcontainers

That allows tests to integrate smoothly in native tooling (e.g. Maven or Gradle) for the tested Software without manual developer work.

zandbelt commented 5 years ago

After considering a few options that @sozkan provided, I'm inclined to go with option 3:

3- Changing the dockerfiles: Assuming that we know exactly what version of otest is needed for a specific oidctest version, we can add an argument to dockerfile and checkout the otest version.

RUN git clone https://github.com/openid-certification/otest.git RUN cd otest && git checkout ${OTEST_VERSION} && python3 setup.py install && cd -

For the following reasons:

I do agree that we should still put up those binary docker images on dockerhub or similar.

@travisspencer @BjoernAkAManf @sozkan please provide your feedback on this line of reasoning

sozkan commented 5 years ago

Makes sense. I don't have any objections.

travisspencer commented 5 years ago

This would meet our needs.

BjoernAkAManf commented 5 years ago

It's totally reasonable to rely on Git's when pinning the version as specified by @sozkan However there is still one major Problem with the current solution, that is not fixed by simply amending the Dockerfile and that is the building of the Docker image itself. While it's now possible to create an entirely reproducible Dockerbuild that relies solely on a fixed set of dependencies, every Developer and Contributor has to clone the this git Repository and run the Build-Scripts themselves.

Assuming one would like to integrate this Test Suite in a local docker-based Test Suite (e.g. using the Tool i linked above), serious manual Work is required. Either by creating a private Docker Repo ("Harder"* for Open Source Projects) or by requiring / integrating the Build Process of this Repository into the Build Process of the Test Suite.

Thus in addition to changing the Dockerfile (Reproducible Builds), this project should host pre-build public Docker Images for each release. This can easily be automated so it shouldn't be too much effort to maintain (See here)

| * Well not really but it requires additional organisational effort

zandbelt commented 5 years ago

Actually, we already tag and checkout a specific otest version... as part of the stable branch (not the master/development branch): https://github.com/openid-certification/oidctest/blob/stable-release-1.1.x/docker/op_test/Dockerfile#L19

So can we agree we're good on this and the comments were really about an old (or development) version?

Then I think we should change this ticket to (only) be about producing and publishing binary docker images, most probably in the automated way that @BjoernAkAManf describes. Agreed?

travisspencer commented 5 years ago

That looks like it would fix our issues, @zandbelt . Haven't tested yet though, so can't confirm.

sozkan commented 5 years ago

@travisspencer, @BjoernAkAManf, we created a docker hub repository (https://hub.docker.com/u/openidcertification) and published rp_test and op_test images. For now, only v1.2.0, which is the latest, is available, let us know if you need a specific older version. Going forward, we will be publishing images for new releases to this repository.

Please note that you should use a specific version tag, instead of "latest" in your scripts/automation. Otherwise you may have compatibility issues, as the version pointed by 'latest' will change overtime.

https://hub.docker.com/r/openidcertification/rp_test/tags https://hub.docker.com/r/openidcertification/op_test/tags

sozkan commented 5 years ago

@zandbelt I think we can close this issue now. Do you agree?