Edgewall Software
Modify

Opened 17 years ago

Closed 16 years ago

Last modified 10 years ago

#7082 closed enhancement (wontfix)

[PATCH] Enable environments to attempt to upgrade automatically

Reported by: Dave Gynn <dgynn@…> Owned by: Jonas Borgström
Priority: normal Milestone:
Component: general Version: devel
Severity: normal Keywords: patch
Cc: osimons Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Enabling new plugins that require upgrade_environment to be run blocks the environment from being accessible. The attached patch allows the plugins to attempt the upgrade if a new auto_upgrade config option is true. Assuming all goes well, the environment is accessible without needing to run trac-admin from the command line. If the upgrade fails, the user is shown the normal Trac Error.

The auto_upgrade option is false by default and should only be enabled in setups where admins can ensure that upgrades will work when run from the server (such as file system permissions).

Attachments (1)

trac-env-auto-upgrade.diff (1.3 KB ) - added by Dave Gynn <dgynn@…> 17 years ago.

Download all attachments as: .zip

Change History (22)

by Dave Gynn <dgynn@…>, 17 years ago

Attachment: trac-env-auto-upgrade.diff added

comment:1 by Remy Blank, 16 years ago

Resolution: wontfix
Status: newclosed

The patch will also automatically perform an upgrade after a Trac update, which I find quite dangerous. I don't know if it would be possible to limit automatic upgrades to plugins only.

As is, I'm -1 on this functionality. Please reopen if you have additional suggestions.

comment:2 by Dave Gynn <dgynn@…>, 16 years ago

Resolution: wontfix
Status: closedreopened

Let me explain the use case for this. We have >200 workspaces and TRAC_ADMINs from those workspaces can enable some optional plugins that are installed but disabled for their environment by default. If one of those plugins (such as TracPastePlugin) requires a database change then that workspace is not available for anyone until a sysadmin can run the trac-admin upgrade from the command line.

I agree that this patch is potentially dangerous which is why it would definitely be off by default. I could modify the patch to exclude cases were IEnvironmentSetupParticipant is from trac.*. But I would trust Trac upgrades just as much an other upgrade. And I would only recommend enabling this option in an environment with known, tested plugins available.

in reply to:  2 comment:3 by Remy Blank, 16 years ago

Cc: osimons added
Keywords: patch added

Replying to Dave Gynn <dgynn@…>:

We have >200 workspaces and TRAC_ADMINs from those workspaces can enable some optional plugins that are installed but disabled for their environment by default.

Oh. I tend to forget that some people actually are deploying Trac on such a massive scale. Now I understand your request much better. Unfortunately, I don't have much experience in this field.

Simon, would you mind commenting on this patch? IIRC, you are familiar with such deployments and the associated issues.

comment:4 by osimons, 16 years ago

Yes, I got the same issues at my site. I considered some sort of auto-upgrade, but as mentioned above it really needs to be used with much caution as it may easily cause much problems - and certainly with the plugin-upload form available in admin that may cause and run upgrades way out of sysadmin control.

So, at the time I went a different route with this. Here is the essence of my strategy - with example for the two plugins I use that involve db changes (trachacks:FullBlogPlugin and trachacks:DiscussionPlugin):

  • In a global trac.ini I enable just the part (component) in the plugin that deals with upgrades - meaning the schema og all environments will always be prepared for the plugin regardless of actual use.
    [components]
    tracfullblog.db.* = enabled
    tracdiscussion.init.* = enabled
    
  • I've disabled the general Plugins admin page, and replaced it with a simple 'Features' admin page where project owners can enable/disable as they please (from my pre-selected list of available features of course). Behind the scenes, this just writes the remaining config settings to the environment - the parts of the plugin that actually deal with user-related things. Example of the remaining settings for the blog plugin that is per-project enabled/disabled:
    [components]
    ...
    tracfullblog.admin.* = enabled
    tracfullblog.core.* = enabled
    tracfullblog.macros.* = enabled
    tracfullblog.web_ui.* = enabled
    

As I see it, this is just as much about plugin design as it is about auto-upgrade - at least for the use-case you describe - and the possibility to keep just the 'upgrade component' always enabled. Trac and plugin handling for large-scale deployment with distributed administration is just something that is not a priority with Trac for the time being - and you basically have to be prepared to make your own management code to accomodate special needs.

I don't mind the ticket staying open as a place to discuss how best to handle this in the long term, but I'm not too keen on the auto-upgrade either. Perhaps the solution is a more flexible 'Plugins' admin page, with config settings to remove upload form and 'freeze' certain component selections that a project administrator cannot change via web? Perhaps that webadmin component can detect request for enabling of plugin implementing IEnvironmentSetupParticipant, and if a new plugin needs to upgrade then then the admin page/component performs it?

comment:5 by Remy Blank, 16 years ago

Milestone: 2.0

Thanks for your comments.

comment:6 by Dave Gynn <dgynn@…>, 16 years ago

Resolution: wontfix
Status: reopenedclosed

That's a great idea, osimons. I hadn't thought about configuring the upgrade components separately that way. I suppose it is better to have all DB schemas in sync in a multi-project setup even if many envs don't require the tables. We have also replaced the plugin admin panel with a more controlled "options" panel so the upgrade could be kicked off there. But I like the idea of having everything in sync much better.

We can keep the ticket closed. Thanks, guys.

comment:7 by osimons, 16 years ago

Milestone: 2.0

comment:8 by jhammel@…, 16 years ago

i've made a fairly awful plugin that does a less than elegant job of working around this:

http://trac-hacks.org/attachment/wiki/AutoUpgradePlugin

I'd love to see some real resolution to this issue. In the case I encounter all the time — a plugin is enabled TTW — I can't imagine any time when you wouldn't want to upgrade. If there is interest, I can work on a patch for this particular case — upgrading after a plugin that requires an upgrade is enabled.

in reply to:  8 comment:9 by anonymous, 16 years ago

Resolution: wontfix
Status: closedreopened

this would be great. while osimons solution looks very nice, it is not really helping an "out of the box trac installation".

Replying to jhammel@…:

i've made a fairly awful plugin that does a less than elegant job of working around this:

http://trac-hacks.org/attachment/wiki/AutoUpgradePlugin

I'd love to see some real resolution to this issue. In the case I encounter all the time — a plugin is enabled TTW — I can't imagine any time when you wouldn't want to upgrade. If there is interest, I can work on a patch for this particular case — upgrading after a plugin that requires an upgrade is enabled.

comment:10 by Dave Gynn <dgynn@…>, 16 years ago

Resolution: wontfix
Status: reopenedclosed

Re-closing ticket.

comment:11 by mixedpuppy, 16 years ago

I tried using the AutoUpgradePlugin, but it caused a bunch of problems, I'll probably dig into it soon to see if I can fix the issues it caused.

Otherwise, I ran into a situation where the upgrade issue is pretty bad for me. Our projects have the custom fields admin page, and a couple plugins that require a custom field (eg Time Estimation plugin). If the trac admin for a project removes the custom fields added by a plugin, the plugin then says it needs to upgrade and the environment becomes unavailable.

I'd agree that the better fix would be some better design in how plugins work (comment:4) but the reality is that would be a larger effort than somehow allowing an autoupgrade.

in reply to:  11 comment:12 by osimons, 16 years ago

Replying to mixedpuppy:

Otherwise, I ran into a situation where the upgrade issue is pretty bad for me. Our projects have the custom fields admin page, and a couple plugins that require a custom field (eg Time Estimation plugin). If the trac admin for a project removes the custom fields added by a plugin, the plugin then says it needs to upgrade and the environment becomes unavailable.

I'd agree that the better fix would be some better design in how plugins work (comment:4) but the reality is that would be a larger effort than somehow allowing an autoupgrade.

Well, comment:4 was targeted at db updates and not modifications to trac.ini. Plugins needing config settings - even ticket-custom fields, should really define Option properties on their plugin class and not use the db-upgrade interface. That way the option will always exist as long as the component is enabled, and can only be deleted if the plugin gets disabled.

I suppose my customfield admin plugin could also quite easily check if the custom field was provided as an Option and disable modification/deleting of such fields if the "delete-then-reappear" gets confusing for admins.

comment:13 by sebastian@…, 15 years ago

Could someone please explain why (or in which case) this is potentially dangerous? I fail to see the point here. For me, as a Trac user, everytime Trac informs me that an upgrade is necessary I just do it. Why shouldn't I?

comment:14 by Sebastian Krysmanski <sebastian@…>, 15 years ago

Cc: sebastian@… added

comment:15 by osimons, 15 years ago

Just some notes from real-life experience today:

Most of the time upgrades are straight forward. Sometimes they are not. See bitten:ticket:214 for a prime example of a new fix that resolves duplicates that have been accumulating over time but where resolution cannot be automated as the user needs to decide what builds stay and which to delete.

Most upgrades I've seen also expects console output by printing information using regular print statements. This debug and potentially important information may well just be 'swallowed' by the Trac process without reaching the user/administrator.

comment:16 by Sebastian Krysmanski <sebastian@…>, 15 years ago

Just wanted to post some insight resulting from a chat with osimons:

  • Trac upgrades may fail. This is not a problem for automatic upgrades as it simply fails and then does nothing more (provided two consecutive upgrades don't damage the database).
  • Trac upgrades may seem to have succeeded but the output on the console states otherwise. This is the big issue for automatic upgrades as the console output is usually discarded (at least as it's currently implemented by the patches attached to this ticket). However, such a behaviour is clearly an error in the plugins upgrade script.
  • A Trac upgrade script may even be interactive. I don't know how such a script would react when it's called from within the webserver (i.e. when there is no stdin).

in reply to:  4 comment:17 by Steffen Hoffmann, 12 years ago

Replying to osimons: […]

Perhaps the solution is a more flexible 'Plugins' admin page, with config settings to remove upload form and 'freeze' certain component selections that a project administrator cannot change via web?

Probably useful to note, that such a component is available with th:wiki:SecurePluginPanelPlugin, written by Sebastian Krysmanski. It hasn't been updated for recent trac versions, but works very well even with trac-0.13dev, respect.

in reply to:  15 ; comment:18 by Peter Suter, 12 years ago

Replying to osimons:

Most of the time upgrades are straight forward. Sometimes they are not.

Would it make sense to allow upgrades to be marked as "safe for auto-upgrade"?

in reply to:  18 comment:19 by osimons, 12 years ago

Replying to psuter:

Replying to osimons:

Most of the time upgrades are straight forward. Sometimes they are not.

Would it make sense to allow upgrades to be marked as "safe for auto-upgrade"?

The problem with upgrades is really that it 'all depends', and it can fail for any number of reasons - related or unrelated to the code. What if the request disconnects during processing? What if the DB is full, or locks or timeouts abort the update? Or the classic issue of insufficient file system permissions for whatever we want to do?

Do we provide a possible upgrade check on every single request? Do we lock out access from other requests while upgrading happens? And, if an upgrade fails will it be attempted again and again for each incoming request until someone takes down the webserver and either fixes or disables the plugin / problem?

I don't think upgrades can be considered 'safe', and somehow supporting this notion is wrong IMHO. Running upgrades in web server context will just make them even more un-safe as I see it.

comment:20 by Peter Suter, 12 years ago

You make a quite convincing case against the idea. Thanks for the insights!

comment:21 by Sebastian Krysmanski <sebastian@…>, 10 years ago

Cc: sebastian@… removed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jonas Borgström.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jonas Borgström to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.