search

rucio / containers

Containers for Rucio
Apache License 2.0
8 stars 54 forks source link

failed patches don't terminate the containers #238

Closed rcarpa closed 6 months ago

rcarpa commented 1 year ago

We have a logic to apply patches inside the container before running rucio. However, if those patches fail (for example, because the new version of rucio is not compatible with the patch), rucio is still run.

In most cases, this is an non-issue, because this results in a non-valid python file; so rucio fails to start.

Still, it will much better to explicitly fail the container startup when the patch command fails.

rdimaio commented 6 months ago

From the patch man page (https://man7.org/linux/man-pages/man1/patch.1.html):

   patch's exit status is 0 if all hunks are applied successfully, 1
   if some hunks cannot be applied or there were merge conflicts,
   and 2 if there is more serious trouble.  When applying a set of
   patches in a loop it behooves you to check this exit status so
   you don't apply a later patch to a partially patched file.

Applying patch in a loop is exactly what we're doing here

https://github.com/rucio/containers/blob/d33dbff33271a0f82cb8f7223e6b731df2a24861/server/docker-entrypoint.sh#L49-L53

so I'd say the logic here would be:

  1. attempt to apply patch
  2. if exit status is anything other than 0, break out of the loop and fail startup
rcarpa commented 6 months ago

Looks good to me