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

All apps get disabled on update #18312

Closed
jancborchardt opened this issue Aug 14, 2015 · 23 comments
Closed

All apps get disabled on update #18312

jancborchardt opened this issue Aug 14, 2015 · 23 comments

Comments

@jancborchardt
Copy link
Member

jancborchardt commented Aug 14, 2015

This is one of the comments I hear most nowadays. People are very annoyed by the fact that an upgrade disables all their apps. Then they manually need to go into the apps management, find all the apps again, and enable them again. That’s too much menial work.

Similar how occ has a command to keep the apps enabled, the web interface might need a button to »Enable apps again« on the upgrade finalization step.

@DeepDiver1975 @karlitschek?

@jancborchardt jancborchardt added this to the 8.2-current milestone Aug 14, 2015
@DeepDiver1975 DeepDiver1975 modified the milestones: 9.0-next, 8.2-current Aug 14, 2015
@DeepDiver1975
Copy link
Member

out of scope for 8.2 - sorry

@jancborchardt
Copy link
Member Author

kk, expected that already. But would indeed be good to think about it afterwards. (Ranges in the top 2 complaints I hear most atm, next to the logo … ;)

@DeepDiver1975
Copy link
Member

(Ranges in the top 2 complaints I hear most atm, next to the logo … ;)

pfff ... talking to UX people ... never heard that complains in my area ... 🙊

@PVince81
Copy link
Contributor

Linked to existing discussions: #14754
and also here where something similar was suggested #14026 (comment)

@PVince81
Copy link
Contributor

It should be possible to set a flag for such apps in the database to be able to re-read them later.

  • store temp flag about disable apps in oc_appconfig
  • provide PHP function to reenable such apps
  • provide ajax/appframework endpoint to trigger that function
  • provide UI to reenable such apps with one click (might need to combine that with Disable app that bricks the server after enabling #17451), triggers the endpoint
  • provide ./occ command to reenable the temp disabled apps

This also reminds me that after reenabling an app, it might require a DB upgrade. This means even more clicking in the UI. So we need to figure out a way for doing the DB upgrade on the fly through the ajax call (with maintenance mode too), similar to the approach from #17451. (maybe a new endpoint "enable + update on the fly")

The OCC command that reenables the apps could automatically call "occ upgrade" afterwards to do it in one go.

Then once these are available, it might be possible to streamline them into the upgrade process, for both web UI update (doing several ajax calls) or the CLI update (having a master occ:update command that calls system('php occ:update-again') to do a system check after the update, etc.

@PVince81
Copy link
Contributor

The ideal update would work as follows, starting with the web UI:

  1. user clicks "upgrade"
  2. JS code does ajax call to trigger the DB upgrade, waits for success
  3. JS code does ajax call to reenable first of the disabled app A
  4. JS code checks if OC still works (the approach from Disable app that bricks the server after enabling #17451)
  5. if system broken, JS code calls ajax endpoint to re-disable the app
  6. if system not broken, perform the database update of the app A
  7. JS code checks again if OC still works
  8. if system broken, JS code calls ajax endpoint to re-disable the app
  9. JS code loops to 3) with the next app to enable
  10. JS redirects to index page

A similar process can be used with the CLI upgrade, but we need to figure out a way to check whether OC still works after enabling an app. I suppose the main ./occ upgrade command could use system() calls to call ./occ app:enable A to make sure it runs as a different process, so it makes it possible to detect breakages.

@karlitschek
Copy link
Contributor

Let's remember why we implemented this behavior in the first place. We had the problem that a 3rd party app might not be compatible with a newer ownCloud version. So it a user tries to upgrade to a new owncloud and the 3rdparty app is still enabled then upgrade breaks in the middle of the upgrade and the ownCloud instance is destroyed. So we decided to be better safe then sorry to disable 3rdparty app, do a core upgrade and then give the user the option to enable the 3rdparty apps again manually. it it then breaks then it is clear where the problem is and the admin can work around it.
The worst problem ownCloud still has is that it kills itself including the data during upgrade. Preventing this has the highest priority. The need to press 'enable' on the apps page after a core upgrade is annoying but 1000x better then killing everything. This is a real world problem that we had a lot in the past and still have.

We can try to do something that @PVince81 proposed but only if this is proven to 100% robust. Protecting the instance and the data is a lot more important then a minor inconvenience during upgrade.

@DeepDiver1975
Copy link
Member

I want to give php-sandbox for loading apps a try - https://github.com/fieryprophet/php-sandbox

But this is a topic for 9.0 since the project is currently not compatible with the libs we use and under heavy refactoring.

The idea is to invoke any app loading call within a sandbox - this gives us the opportunity to catch any fatal php errors and disable the app.

@KenBW2
Copy link

KenBW2 commented Sep 8, 2015

Maybe an answer to @karlitschek's concerns is to have a "Previously Enabled" tab in the Apps page. It doesn't automatically reenable them for the reasons cited above, but does allow you to quickly reenable apps that you were using before.

The experience as I see it goes:

  1. Start Upgrade process, with notice "Your apps will be disabled to avoid incompatibility issues but you will be able to reenable them after the upgrade has completed"
  2. Upgrade process takes place
  3. "Your upgrade has completed successfully. [Reenable your apps]" takes you to the Previously Enabled tab
  4. See a filtered list of apps so you don't have to fish through all available apps, and enable one by one as is current

@DeepDiver1975
Copy link
Member

Maybe we add a step to the new updater tool to reenable previously disabled apps.

@PVince81
Copy link
Contributor

What to do with this in the context of 9.0 ?

@PVince81
Copy link
Contributor

Should the updater attempt to enable the apps one by one remotely (with occ commands) and then check whether they brick the server ? If they do, disable them again ? (how?)

Shorter question: is this something the updater app should do rather than the occ upgrade command ?

@PVince81
Copy link
Contributor

CC @VicDeo

@VicDeo
Copy link
Member

VicDeo commented Feb 18, 2016

@PVince81 as I see it occ upgrade should not be monolithic and try to do everything at once. (this is something for future)

For 9.0 updater predisables all 3rdparty apps and reenables them after the core and shipped apps are ready.

But. In case there is no compatible version of app (e.g. we have a fresh major release) most of apps are potentially incompatible with target version (and core won't allow to reenable them)

@jancborchardt
Copy link
Member Author

For 9.0 updater predisables all 3rdparty apps and reenables them after the core and shipped apps are ready.

Nice, that sounds like a very good improvement!

But. In case there is no compatible version of app (e.g. we have a fresh major release) most of apps are potentially incompatible with target version (and core won't allow to reenable them)

This is fine and we need to make sure that app developers test with the current betas to iron out any issues and then set it to compatible.

@PVince81
Copy link
Contributor

@VicDeo what happens if the updater tries to reenable an app and there is an error ? Does the updater detect it and re-disables that app ?
There are two possible error cases:

  1. core refuses to reenable the app due to version issue, this should result in a clean OC. No need to re-disable
  2. core happily enables the app, but the app's code bricks OC.

Often in the case of 2) it is not possible to redisable the app unless there is a priviledged code path that doesn't try and load all app's code. I'm not sure this works yet ?

@DeepDiver1975
Copy link
Member

We need to add a step in occ upgrade / web upgrade to reenable them step by step .... I'd leave this to 9.2 @cmonteroluque @MTRichards @PVince81

@PVince81 PVince81 modified the milestones: 9.2-next, 9.1-current Jun 3, 2016
@PVince81
Copy link
Contributor

PVince81 commented Jun 3, 2016

Yeah, let's move it...

@PVince81
Copy link
Contributor

PVince81 commented Dec 8, 2016

We still don't have a good way to safely reenable apps within the occ upgrade process.

@DeepDiver1975 @VicDeo do we want to invest time for this in 10.0 ? Else I'd move to backlog.

Note that apps can be reenabled in a scripted manner with occ app:enable, but it's unconvenient still.

@pmaier1

@PVince81 PVince81 modified the milestones: backlog, 10.0 Dec 8, 2016
@pmaier1
Copy link
Contributor

pmaier1 commented Dec 14, 2016

Ok, backlog for now. What about php-sandbox suggested by @DeepDiver1975? Would this now be usable? IMO this is important as it makes the system feel more robust/complete and makes an admin's life easier when having many apps enabled.

@VicDeo
Copy link
Member

VicDeo commented Apr 24, 2017

#27730 (comment)

@PVince81
Copy link
Contributor

In 10.0 third party apps will not be automatically disabled any more.

@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants