Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failed patches don't terminate the containers #238

Closed
rcarpa opened this issue Mar 20, 2023 · 2 comments · Fixed by #284
Closed

failed patches don't terminate the containers #238

rcarpa opened this issue Mar 20, 2023 · 2 comments · Fixed by #284
Assignees

Comments

@rcarpa
Copy link
Contributor

rcarpa commented Mar 20, 2023

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
Copy link
Contributor

rdimaio commented Jan 16, 2024

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

for patchfile in /patch/*
do
echo "Apply patch ${patchfile}"
patch -p3 -d "$RUCIO_PYTHON_PATH" < $patchfile
done

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
Copy link
Contributor Author

rcarpa commented Jan 16, 2024

Looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants