friendica / docker

Docker image for Friendica
https://friendi.ca
GNU Affero General Public License v3.0
45 stars 18 forks source link

console.php dbstructure update fails to launch upon container upgrade from 2021.09 to 2022.02 #195

Closed elrido closed 2 years ago

elrido commented 2 years ago

Hi there,

Not a major issue, since easily worked around, but we probably can make this even smoother.

I'm using the fpm-alpine flavour and just upgraded from 2021.09 to 2022.02. In the logs I see these messages from the startup scripting:

Feb 13 10:11:35 brooks daemon.info 618192f4403a[2013]: Setup MSMTP for 'Friendica Social Network' with '[redacted]' ...
Feb 13 10:11:35 brooks daemon.info 618192f4403a[2013]: Setup finished
Feb 13 10:11:35 brooks daemon.info 618192f4403a[2013]: Initializing Friendica 2022.02 ...
Feb 13 10:11:35 brooks daemon.info 618192f4403a[2013]: Upgrading Friendica from 2021.09 ...
Feb 13 10:11:39 brooks daemon.info 618192f4403a[2013]: Initializing finished
Feb 13 10:11:39 brooks daemon.info 618192f4403a[2013]: Upgrading Friendica ...
Feb 13 10:11:39 brooks daemon.err 618192f4403a[2013]: sh: php: not found
Feb 13 10:11:44 brooks daemon.info 618192f4403a[2013]: Setup MSMTP for 'Friendica Social Network' with '[redacted]' ...
Feb 13 10:11:44 brooks daemon.info 618192f4403a[2013]: usermod: no changes
Feb 13 10:11:44 brooks daemon.info 618192f4403a[2013]: usermod: no changes
Feb 13 10:11:44 brooks daemon.info 618192f4403a[2013]: Setup finished

I assume that the sh: php: not found comes from this call and the run_as function: https://github.com/friendica/docker/blob/8d5f6d63ea3e8d67a036a93458dfd1ceb59fdb60/2022.02/fpm-alpine/entrypoint.sh#L164 https://github.com/friendica/docker/blob/8d5f6d63ea3e8d67a036a93458dfd1ceb59fdb60/2022.02/fpm-alpine/entrypoint.sh#L5-L11

I worked around this by triggering the update manually:

docker exec --user=www-data friendica-application php bin/console.php dbstructure update

For reference (probably not relevant, just in case that my particular settings matter in this scenario), I use the following configuration in my ansible role:

- name: Friendica container
  docker_container:
    name: friendica-application
    image: friendica:2022.02-fpm-alpine
    hostname: "{{ docker_friendica_fqdn }}"
    state: started
    restart_policy: always
    privileged: no
    read_only: no
    init: yes
    links:
      - friendica-mariadb
    env:
      MYSQL_HOST: friendica-mariadb
      MYSQL_DATABASE: friendica
      MYSQL_USER: friendica
      MYSQL_PASSWORD: "{{ docker_friendica_mysql_password }}"
      PHP_TZ: Europe/Zurich
      FRIENDICA_TZ: Europe/Zurich
      FRIENDICA_ADMIN_MAIL: [redacted]
      SITENAME: "dSSr Social Network"
      SMTP: [redacted]
      SMTP_PORT: "25"
      SMTP_DOMAIN: dssr.ch
      SMTP_FROM: webmaster
    etc_hosts:
#      social.dssr.ch: "[redacted IPv6 address of the nginx server this runs behind]"
      social.dssr.ch: [redacted IPv4 address of the nginx server this runs behind]
    volumes:
      - /srv/friendica-application:/var/www/html
  tags: docker-friendica-server

Thank you all for your work on this image, it does save me so much time!

nupplaphil commented 2 years ago

@elrido - please retry and close this issue after https://github.com/docker-library/official-images/pull/11872 is merged

elrido commented 2 years ago

Thanks, I've prepared a test environment using my ansible role and a backup from Sunday morning before the update and will re-create the update once we got a fresh image released.

elrido commented 2 years ago

Actually, I tested your change in the run_as in a alpine container (using it's busybox sh) and found that the -c in the set line replaces $1, which breaks the subsequent calls. Here is another version of the function with my understanding of each lines purpose in comments, that gives me the expected result when running commands as root or non-root user:

# runs given snippet as a shell script in the www-data users context
run_as() {
  # prepends a directory change, so the scriptlet gets executed from a known location
  # and also concatenates all arguments into a single $1 argument, in case we pass more by accident
  set -- "cd /var/www/html; $*"
  # if we are running as root user, we want to su into the www-data user instead
  if [ "$(id -u)" -eq 0 ]; then
    # due to the su we loose the PATH as well as other environment variables, so we need to forward them
    su - www-data -s /bin/sh -c "export PHP_MEMORY_LIMIT=${PHP_MEMORY_LIMIT}; export PHP_UPLOAD_LIMIT=${PHP_UPLOAD_LIMIT}; $1"
  else
    # we already are a non-root user and calling sh will inherit the exported environment variables
    sh -c "$1"
  fi
}

I tested the above in an alpine:3.15 container and its busybox sh, having it call run_as 'printenv;' echo foo bar to ensure the PHP_MEMORY_LIMIT was preserved in both cases and it would glue together extraneous arguments (AFAIK there are no cases you pass such extra args, it's more of a safeguard against future mistakes).

One alternative to consider would be the use of the -p flag instead of - so that instead of a login-shell, you preserve the environment. I have tested this as well, and the below gives me the same results, with the expected environment variables passed down, but I let you be the judge if that is worth the risk of the extra change:

run_as() {
  set -- "cd /var/www/html; $*"
  if [ "$(id -u)" -eq 0 ]; then
    su -p www-data -s /bin/sh -c "$1"
  else
    sh -c "$1"
  fi
}

Finally, here is a refactored version, using the set mechanism to prepend the su arguments and that avoids the extra else branch:

run_as() {
  set -- /bin/sh -c "cd /var/www/html; $*"
  if [ "$(id -u)" -eq 0 ]; then
    set -- su -p www-data -s "$@"
  fi
  "$@"
}

That last one is a pretty big change, so I'd probably use the first version that only fixes the -c duplication.

nupplaphil commented 2 years ago

Thanks for your detailed explanation!

I was at exactly the same path as you, but the problem is that su -p doesn't preserve the $PATH variable, so that was the reason why php couldn't be found anymore. On the other hand, su -p -l works for alpine, but not for debian, so I had to choose a direction.

I thought, using the user-specific environment and passing the two missing environment variables (PHP_MEMORY_LIMIT and PHP_UPLOAD_LIMIT) was the easier part. But as a docker-library maintainer already said here https://github.com/docker-library/official-images/pull/11872#issuecomment-1040983025 , even this solution doesn't fully fix the root-cause.

So I use his suggested binary gosu instead, having this function:

run_as() {
  set -- sh -c "cd /var/www/html; $*"
  if [ "$(id -u)" -eq 0 ]; then
    set -- gosu www-data "$@"
  fi
  "$@"
}

This is pretty similar to your last example, but as far as I read about gosu, it's exactly build for this use case inside a Docker image. And I'm already using gosu at other places, so it fits well.

See https://github.com/friendica/docker/pull/198 for the solution

elrido commented 2 years ago

Good catch. I concur that using a single dedicated tool will make this more robust. I see the image got published and I'll report back from testing the upgrade on the backup ASAP.

elrido commented 2 years ago

Confirmed! Now the upgrade gets kicked off automatically:

$ docker logs friendica-application
Setup MSMTP for 'Friendica Social Network' with 'smtp.dmz.dssr.ch' ...
Setup finished
Initializing Friendica 2022.02 ...
Upgrading Friendica from 2021.09 ...
Initializing finished
Upgrading Friendica ...
ALTER IGNORE TABLE `contact` MODIFY `contact-type` tinyint NOT NULL DEFAULT 0 COMMENT 'Person, organisation, news, community, relay', MODIFY `manually-approve` boolean COMMENT 'Contact requests have to be approved manually', MODIFY `rating` tinyint NOT NULL DEFAULT 0 COMMENT 'Automatically detected feed poll frequency', MODIFY `priority` tinyint unsigned NOT NULL DEFAULT 0 COMMENT 'Feed poll priority', MODIFY `pending` boolean NOT NULL DEFAULT '1' COMMENT 'Contact request is pending', MODIFY `forum` boolean NOT NULL DEFAULT '0' COMMENT 'contact is a forum. Deprecated, use \'contact-type\' = \'community\' and \'manually-approve\' = false instead', MODIFY `prv` boolean NOT NULL DEFAULT '0' COMMENT 'contact is a private group. Deprecated, use \'contact-type\' = \'community\' and \'manually-approve\' = true instead', MODIFY `usehub` boolean NOT NULL DEFAULT '0' COMMENT 'Deprecated', MODIFY `closeness` tinyint unsigned NOT NULL DEFAULT 99 COMMENT 'Deprecated'; 
ALTER IGNORE TABLE `fcontact` ADD `interacting_count` int unsigned DEFAULT 0 COMMENT 'Number of contacts this contact interactes with', ADD `interacted_count` int unsigned DEFAULT 0 COMMENT 'Number of contacts that interacted with this contact', ADD `post_count` int unsigned DEFAULT 0 COMMENT 'Number of posts and comments';
[...]

Thank you!