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

[enh] Check that required services are up before running app install and upgrade #670

Merged
merged 2 commits into from Apr 1, 2019

Conversation

Projects
None yet
3 participants
@alexAubin
Copy link
Member

commented Mar 4, 2019

The problem

see this kind of situation : https://forum.yunohost.org/t/nextcloud-erreur-502-et-mise-a-jour-impossible-apres-migration-yunohost-3-4-2-4/7204/11 - an app upgrade miserably failed trying to reload php7.0-fpm which was most probably down before the whole action even started and this could be avoided.

Solution

Add a check that some services are up before running app install or app upgrades, according to the info given in the manifest. Unfortunately the manifest does not seem to be available during restore (or it's just not loaded/used ?) so it's not straigtforward to also add it here

PR Status

Tested for app install

How to test

E.g. stop php7.0-fpm and try installing wordpress

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
return service
services = [replace_alias(s) for s in services]

# We only check those, mostly to ignore "custom" services

This comment has been minimized.

Copy link
@Psycojoker

Psycojoker Mar 4, 2019

Member

Shouldn't we only checked already known services only instead? I'm not sure. That will also filtered out not already installed services and, maybe?, could prevent other errors.

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

"Funny", post-installed failed on my vm because slapd wasn't running, maybe that could be a good place to do that check too, no?

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

So meh ... I dunno what to do with this ... the idea was mostly that I saw a few times happening that nginx or php was down before trying to install an app or upgrade one, and it miserably failed ...

We can try to generalize to all services but meh I really mainly care about php-fpm and mysql because those are the ones "likely" to be broken :|

@alexAubin alexAubin added this to the 3.5.x milestone Apr 1, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

Ok guyz, I'm gonna merge this as is because I mainly want to stop getting those damn failed install / upgrade just because php-fpm ain't running ... we can still iterate later to refine this.

@alexAubin alexAubin merged commit e8b850a into stretch-unstable Apr 1, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the check-service-up-before-app-scripts branch Apr 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.