juanluisbaptiste / docker-otrs

The unofficial Znuny/OTRS Ticketing System docker image
https://www.juanbaptiste.tech/category/otrs
GNU Lesser General Public License v3.0
175 stars 104 forks source link

Perl modules not being copied on minor update #95

Closed bschmalhofer closed 4 years ago

bschmalhofer commented 4 years ago

Hi, I am working on improving the Docker support for OTOBO, specifically https://github.com/RotherOSS/otobo/issues/203 . Of course I'm looking at docker-otrs for ideas. Thank you for your inspiring work!

My understanding is that for a minor version, or patch level, upgrade at least two steps must be executed.

  1. copy the config and Perl modules from /Kernel to /opt/otrs/Kernel
  2. execute scripts/DBUpdate-to-6.pl --non-interactive

Step 1. seems to be handled in check_host_mount_dir. I understand that this function is called when firsttime is still present. But how can it be that the copying is actually done when there is the condition is:

if ([ "$(ls -A ${OTRS_CONFIG_MOUNT_DIR})" ] && [ ! "$(ls -A ${OTRS_CONFIG_DIR})" ]) || [ "${OTRS_UPGRADE}" == "yes" ];

$(ls -A ${OTRS_CONFIG_DIR}) is, as far as I understand it, true because the previous installation still exists in the mounted volume ./volumes/config .Setting$OTRS_UPGRADE="yes"`seems to be no option as this would trigger a major version upgrade. Did I miss anything or is this a bug in functions.sh ?

juanluisbaptiste commented 4 years ago

Hi,

That condition is in place to avoid overwriting custom modifications done inside /opt/otrs/Kernel (${OTRS_CONFIG_DIR}), so copying should be done when the container is started for the first time and the config volume is empty, or when doing a major version upgrade. But indeed that condition avoids copying updated modules on a minor update as the check sees if ${OTRS_CONFIG_DIR} is empty, but for a minor version upgrade it will not be empty so you are right, the modules will not be copied on that case.

I'm working on a patch right now to fix this behavior.

juanluisbaptiste commented 4 years ago

@bschmalhofer please test fix on branch issue_95_fix.

bschmalhofer commented 4 years ago

Hello Juan,

I have taken a look at issue_95_fixand tested it with the following steps:

I did the following sanity checks:

bschmalhofer commented 4 years ago

The above steps are not exactly the real steps, as I doubled up a couple of times. The result look good and from my side the ticket can be closed. Thanks for the fix!

juanluisbaptiste commented 4 years ago

It would be easier if you have used docker-compose-prod.yml to launch the OTRS 6.0.28 image first to create a new install, then stop it and use docker-compose.yml, update the Dockerfile to the latest versoin 6.0.29 (which I will update after I merge this fix), build it and launch it using the same volumes.

Anyway I'm glad it worked, I will merge the fix into master as soon as I can.

bschmalhofer commented 4 years ago

Yes, you are right. I realized only after the second try that the code checked against the DOCKER_VERSION that is defined in the Dockerfile.

juanluisbaptiste commented 4 years ago

Fix merged, thanks for reporting.