sclorg / httpd-container

Apache HTTP 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
47 stars 123 forks source link

VOLUME declaration prevents this image working on OpenShift Online #30

Closed bparees closed 7 months ago

bparees commented 6 years ago

This image declares a VOLUME which means it can't be run as an s2i builder because it is impossible to run images on openshift online if they declare a volume and you do not map that volume to a mount point.

For running an image it's fine because the user can map the volume.

But when the image is run as a builder via s2i, there is no opportunity for the user to map a mount to the VOLUME and openshift online fails the build because the volume can't be written to.

I think the VOLUME declaration should be removed from the image, it's not like users can't still map a volume if they want to anyway.

@hhorak @sspeiche fyi.

omron93 commented 6 years ago

This image declares a VOLUME which means it can't be run as an s2i builder because it is impossible to run images on openshift online if they declare a volume and you do not map that volume to a mount point.

@bparees What is a reason of this? It means that database images (which are meant to be also extendable using s2i) can't be used as builder images too in OpenShift online, right?

I think the VOLUME declaration should be removed from the image, it's not like users can't still map a volume if they want to anyway.

I don't know how OpenShift exactly runs images. But with Docker this causes performance degradation - because if mount point is not specified "volume" is stored on layered file system (which has lower performance).

bparees commented 6 years ago

@bparees What is a reason of this?

the reason is that we can't manage/monitor disk usage that is allocated by docker in this way, so the only way to prevent abuse currently is to not allow these volumes to be created by docker at all.

It means that database images (which are meant to be also extendable using s2i) can't be used as builder images too in OpenShift online, right?

correct. it's a problem for any image that declares a VOLUME, but this is the first case we've had of an image that's likely to be used as a builder image, that also declares a VOLUME. (Technically the Jenkins image also has this problem, but it seems no one has run into it yet, but we should consider fixing that one too).

I don't know how OpenShift exactly runs images. But with Docker this causes performance degradation - because if mount point is not specified "volume" is stored on layered file system (which has lower performance).

I'm not sure what you're saying.... if the image did not declare a VOLUME then there would be two modes for the image to run under: 1) no mounts specified, everything is in the container filesystem. no issue. 2) user explicitly creates an openshift volume and mounts it to the container. no issue.

What we are explicitly trying to avoid is the third mode which is: 3) image declares a volume. user does not mount anything to that volume. docker creates a local volume and mounts it. Which I think is the case you're saying performs badly anyway (and in online, it fails completely).

hhorak commented 6 years ago

@bparees Is there some documentation for the behaviour of OpenShift, when VOLUME is specified? If we removed all VOLUMES (or maybe better commented-out for documentation purposes), we should clearly link to the reasoning.

bparees commented 6 years ago

Is there some documentation for the behaviour of OpenShift, when VOLUME is specified?

no, and it's not openshift behavior, it's docker behavior.

If we removed all VOLUMES

i'm not suggesting you remove all of them. I'm not even necessarily suggesting you permanently remove this one(once online can tolerate running containers with unmapped VOLUMEs, you could reintroduce the declaration), but for now if you want this image to work as an s2i builder in openshift online, you need to remove the VOLUME.

omron93 commented 6 years ago

I'm not sure what you're saying.... if the image did not declare a VOLUME then there would be two modes for the image to run under:

I think I've read it somewhere. But now I can't find it again... so take this note as no such serious :-)

  1. no mounts specified, everything is in the container filesystem. no issue.

This is the default. Lets take and example of database which uses one directory for storing data. This directory is heavily used for writing, other parts of image are mainly read-only. So in this case everything in that directory is stored in container writable layer (so in background using devicemapper, loop-devicemapper, overlayfs,... filesystem).

What we are explicitly trying to avoid is the third mode which is: 3) image declares a volume. user does not mount anything to that volume. docker creates a local volume and mounts it. Which I think is the case you're saying performs badly anyway (and in online, it fails completely).

So as I understand it, this is the good option. Even if mount directory on host is generated by docker somewhere in /var/lib/docker/volumes. Heavily writable directory is stored in host filesystem which is probably more performant because it don't have to union several layers each time it is used.

But this could change with new docker supported filesystems or something like this.

pkubatrh commented 6 years ago

Makes sense to me to remove it temporarily if Openshift is unable to work with them when the images are used as s2i builders. But it should really only be temporary and eventually be reintroduced when Openshift is fixed. It would be nice to have some issue against Openshift to track the state of the implementation there.

bparees commented 6 years ago

It would be nice to have some issue against Openshift to track the state of the implementation there.

there are open bugs for it. @abhgupta can point you to them.

and again to be clear, this is only a limitation in openshift online (unless an openshift administrator intentionally sets up their docker storage in the same we we've configured the online nodes).

luciddreamz commented 6 years ago

Related question - why is this so different than the s2i-php-container which also runs httpd?

hhorak commented 6 years ago

The VOLUMES are commented out now, although I'd keep this issue opened to track that it's not perfect now and once https://bugzilla.redhat.com/show_bug.cgi?id=1481918 is fixed, we can re-enable VOLUMES again.

hhorak commented 6 years ago

This was already fixed via #41, closing.

hhorak commented 6 years ago

The VOLUMES are commented out now, although I'd keep this issue opened to track that it's not perfect now and once https://bugzilla.redhat.com/show_bug.cgi?id=1481918 is fixed, we can re-enable VOLUMES again.

I've forgot about this ^^, so let's keep it opened.

pkubatrh commented 5 years ago

Hi @bparees I see the bz (https://bugzilla.redhat.com/show_bug.cgi?id=1481918) is now closed. Has the underlying issue in Openshift Online been dealt with (and we can enable VOLUMEs in Dockerfile again)? Or was the BZ closed because s2i builds no longer fail due to VOLUME commands being commented in the Dockerfile?

bparees commented 5 years ago

@pkubatrh i think it was fixed by updating online to use an httpd image that doesn't declare a volume:

https://github.com/sclorg/httpd-container/blob/master/2.4/Dockerfile#L69 https://github.com/sclorg/httpd-container/commit/05911349ef58be477f58804cc689ddb4807bff8e

so i guess this can be closed

pkubatrh commented 5 years ago

@bparees Thanks. That means the underlying issue of online not being able to run s2i builds with images that declare volumes in their Dockerfile is still there, so lets keep this open.

@abhgupta From the older comments I see that you should know where the issue in online is tracked. Can we have some links to the bugs so that we do not have to continue polling once in a while?

abhgupta commented 5 years ago

IIRC, fixing this required Online to transition to using buildah as S2I builds would also have had to deal with optionally provision emptyDir instead of docker volumes. That transition was not done and currently we are focused on 4.x and not adding new features to Online. So, I don't have a tracker card for getting this addressed in Online.

phracek commented 7 months ago

@abhgupta @bparees Is this still valid issue even in OpenShift 4? if not please close this issue. Thanks.

bparees commented 7 months ago

i think we can close this. i don't believe it's an issue anymore and if it is a new bug can be opened.