search

sclorg / mysql-container

MySQL container images based on Red Hat Software Collections and intended for OpenShift and general usage. Users can choose between Red Hat Enterprise Linux, Fedora, and CentOS based images.
http://softwarecollections.org
Apache License 2.0
128 stars 201 forks source link

[DO NOT MERGE] Revert "Do not use VOLUME command in Dockerfiles" #224

Closed hhorak closed 6 years ago

hhorak commented 6 years ago

This reverts commit 987e5c5e4e7cda060246e5d0007f2dca26516ab3, because it breaks things: https://github.com/openshift/origin/pull/19470

hhorak commented 6 years ago

[test-openshift]

hhorak commented 6 years ago

This effectively reverts #223, but @mfojtik said we should wait for @bparees before merging. Ben, let us know :)

bparees commented 6 years ago

https://github.com/sclorg/httpd-container/issues/30 hasn't been resolved, so why would we do this?

bparees commented 6 years ago

since origin was fixed by https://github.com/openshift/origin/pull/19470, i would say nothing further needs to be done.

(other than perhaps introducing a new volume test in origin)

hhorak commented 6 years ago

sclorg/httpd-container#30 hasn't been resolved, so why would we do this?

It actually was fixed already, just github issues was not closed (I've done it now).

pkubatrh commented 6 years ago

I thought we wanted to keep the issue open in order to track it for a future fix when Openshift Online is able to work with VOLUMEs

hhorak commented 6 years ago

@pkubatrh ah, forgot about that, re-opening.

hhorak commented 6 years ago

@bparees, Now I'm puzzled a bit, from what @mfojtik said I had a feeling that VOLUME set for databases images is actually beneficial, because users can be advised to use PV.. is that not a fair assumption? it seems to me like it's a trade-off between this advice and making s2i working in OpenShift online.. and I'm kinda clue-less what is more important use case. :)

bparees commented 6 years ago

@hhorak yes that's pretty much the trade off. there's just no good answer today.

but ultimately i'd say making s2i work in online is more important because there is no workaround for that issue. Users not understanding which volume they need to map is unfortunate, but that's in part why we have templates that help set all that up.

bparees commented 6 years ago

(docs for the images that tell users what the volumes should be can also help)

hhorak commented 6 years ago

@bparees Ok, thanks for you PoV. We did the change deliberately just in one database image at this point, to see what issues we can cause (we hoped for none, but were proofed wrong). I'd wait for further feedback (~month) before doing the same thing again for other images, so we don't break everything at once. @mfojtik, if there are no other issues and we'll decide removing the VOLUME in other database images, can we expect the same thing fail again on your site?

bparees commented 6 years ago

@mfojtik, if there are no other issues and we'll decide removing the VOLUME in other database images, can we expect the same thing fail again on your site?

we'd have to scan our tests to see if we have other tests consuming the images you're changing and that expect to see volumes. I wouldn't expect a bunch, we probably just picked the mysql image to exercise new-app.

hhorak commented 6 years ago

It seems like we're ok to keep the image as it is, without VOLUME. So, closing.