This is a generic Hiveconf bug. RPM is pretty good at handling configuration files (see http://rikers.org/rpmbook/node25.html). But, if the installed file was modified, and the new file is different from the old one, RPM cannot do much about it: It saves the file as file.rpmsave. It's pretty common that this happens with, for example, vsmagent.hconf. In this case, the installation program prints a warning about this, and then the sysadmin can manually migrate the configuration. It would be nice, however, if this manual step could be avoided. This can be done using several different approaches: 1) Start tagging all configuration files as %config(noreplace). There is little documentation on this parameter, but http://www-uxsup.csx.cam.ac.uk/~jw35/docs/rpm_config.html has a lot of good info. In short: Instead of getting the .rpmsave file, the edited version is kept and the new one is installed as .rpmnew. With ThinLinc, this should work, since the "hardcoded" default values for the parameters is (should be...) equal to what's in the shipped config file. So, running with an empty config file should be equal to running with the default config file. And, it almost all cases, and old, edited config file should work with new ThinLinc versions. There are drawbacks with this, approach, however. %config(noreplace) is not a part of LSB. Another serious drawback is that new parameters, and comments, won't be added to the config file: A user looking for a certain parameter in the file, OR using tl-config, will not find it. 2) Install the new config files, just as today, but provide a tool that can migrate the old settings to the new file. All parameters should be migrated, even those that are not used in the new version anymore. (Why? Take a look at /vsmagent/default_environment) This is harmless, however. Another drawback is that comments and the structure will be lost[*]. 3) Keep the old config file, but add new parameters to the old file. Drawbacks: The comment and structure of the new file will not be migrated. I think this last approach is the best: The admins hand-made comments and structure are probably more worth that the shipped default one. We can implement solution 3 without using %config(noreplace): RPM can rename the old file to .rpmsave initially, but then our migration tool can rename it back, when the migration is finished. The migration tool should probably be called from the postinstall script. If the migration tool depends on a working Pythin with a working Hiveconf installation, though, this might be a little tricky. If you are upgrading, though, there SHOULD be a working /usr/bin/python-thinlinc.
We need to think of the Debian and Solaris platforms as well. These does not have the fancy RPM config file magic. Our Solaris package are currently always renaming the old files to .pkgsave. I don't remember the Debian mechanism.
I'm not fully convinced which method is the best, but if necessary, I think we can use %config(noreplace), even if it's not currently mentioned in the LSB spec. http://samba.org/ftp/unpacked/samba_3_0/packaging/LSB/lsb-samba.spec uses it, and https://www.freestandards.org/en/Vsw4Test talks about it. So, I don't think that using %config(noreplace) would be a LSB violation.
My current opinion is that we should go for method 2). I'd say that the typical case is to not change many parameters, or make large changes to the structure, or write large comments. In that case, those advanced users can move back the original file just as they do today. Instead, I think it makes more sense to install our shipped files and then just apply the old settings. Method 2 also does not require any config(noreplace) magic.
I think this code is ready for testing now.
The new page has noticeably stretched the tl-setup window, making everything look rather odd. The UI element used to list the files is also a bit off. In other places we us an indented element without any border (see for instance the package installer).
There is no documentation / information about the migration that provides information so that I as administrator can make the correct choice of: - Migrate configuration from old files to new files - Use old configuration - Use new configuration
There is a catch with the migration process that it needs to be performed within a specified time window since installation. I can't understand that avid thing, nor the actual problem that introduced this time window. This timeout needs to be removed in some way.
(In reply to comment #15) > There is no documentation / information about the migration that provides > information so that I as administrator can make the correct choice of: > > - Migrate configuration from old files to new files > - Use old configuration > - Use new configuration The information in tl-setup migrate configuration screen is not understandable, why is changed files showed. Also it is unclear what information is tried to be delivered in the following text; "Some configuration files have changed since previous version of ThinLinc, and your package manager has saved these old configuration files:" First part of sentence lacks scope, is it configuration shipped with new ThinLinc version that has changed or configuration that existed before the upgrade... One solution might be to break down the migrate configuration into two dialogs, the first dialogs provides information what is about to happen and basic information about the choice the admin needs to do. The second page provides the choices... Doing this split up the information dialog could contain more information than the current "shortend" down version...
(In reply to comment #16) > There is a catch with the migration process that it needs to be performed > within a specified time window since installation. I can't understand that avid > thing, nor the actual problem that introduced this time window. This timeout > needs to be removed in some way. This description is incorrect; the code does not care about then tl-setup is executed. The requirement is that the savefile is created at maximum 24 hours before the actual configfile. For example: xsession.rpmsave must be created at maximum 24 hours before the new "xsession" was installed. At least RPM always updates the mtime, even if the file is not changed in this particular upgrade. However, I'd like to change this slightly, and start using thinlinc-release as a time reference instead. This is because I'm not sure not dpkg works and I don't want to rely on something unknown. With this change, the requirement for processing savefiles will be that it is created "at maximum 24 hours before thinlinc-release was installed".
I believe I have fixed most of the reported issues now. The exception is the comment about using a different widget for the list of files; this wish has only been expressed by one so I am not convinced. Code is ready for more testing now - resolving.
> if PACKAGE_FORMAT == "rpm": should be: > if PACKAGE_FORMAT is RPM:
The time stamp directory is created using: > mkdir -p /opt/thinlinc/etc/.upgrade.stamp This will not update the ctime on a directory that already exists, and I cannot see anything else doing that. Once that is fixed, anything problem appears; the time stamp will be for the last package updated. But config files might have been moved around in earlier packages.
> migrate_conf_oldfiles.append(absname) Typo. Should probably be migrade_conf_oldsfiles.
> shutil.copy(base, newname) Maybe shutil.copy2() is better for this code so that mtime is preserved? Ideally though it would be nice if a move was done rather than a copy. That way even things like the inode is preserved. I assume this is how rpm/dpkg does things. Mostly relevant for .rpmsave/.dpkg-old though.
It looks things might misbehave if something fails halfway through and you need to run things again. Given this scenario: a) foo.hconf foo.hconf.rpmnew We will first get: b) foo.hconf => foo.hconf.tlsave Then: c) foo.hconf.rpmnew => foo.hconf If something goes wrong after this then the user might try to run tl-setup again. At that point step b) will overwrite the save user configuration with the default configuration. The error doesn't even have to be for this file, but can be for any subsequent config file.
(In reply to comment #28) > The time stamp directory is created using: > > > mkdir -p /opt/thinlinc/etc/.upgrade.stamp > > This will not update the ctime on a directory that already exists, and I cannot > see anything else doing that. > > Once that is fixed, anything problem appears; the time stamp will be for the > last package updated. But config files might have been moved around in earlier > packages. Works as intended: The purpose is to get a time stamp for the first package installation.
This seems to be working mostly as intended! I have used my workstation (Fedora 23) to test rpm as well as a Ubuntu 14.04 server for testing dpkg. I have used ThinLinc 4.4.0 and ThinLinc 4.5.0 as bases from which to upgrade from. I have upgraded to the nightly builds 5018 to 5022. I have verified that the following is working: * The three choices work as documented, I am unable to find any corner cases that doesn't work as described. * Only files (.rpmsave, .rpmnew, .dpkg-old, .dpkg-dist) which are created between the package upgrade and tl-setup are handled. * When "parameters" is chosen, it correctly migrates when rpmsave, rpmnew, dpkg-old, and dpkg-dist files exist. * When "old" is chosen, it correctly uses the old files when rpmsave, rpmnew, dpkg-old, and dpkg-dist files exist. * When "ignore" is chosen, it correctly uses the new files when rpmsave, rpmnew, dpkg-old, and dpkg-dist files exist. * All three choices work in GUI mode, text-mode and unattended mode of tl-setup. * I feel that the text in tl-setup is clear and understandable. * The code looks good.
However, I have found a few small issues. In the documentation in the TAG I feel that we are a bit inconsistent with how clear we describe the 3 configuration choices. The "parameters" option is described quite thoroughly, but I think we could be a little bit clearer if we make the following change: -All parameters and values from the listed Hiveconf files are copied over. This means that unchanged parameters will use the default values from the earlier release. +All parameters and values from the listed Hiveconf files are copied over. This means that unchanged parameters will use the default values from the earlier release for these files. In the documentation for the "old" choice, we are not clarifying that new defaults will be introduced to files where there were no conflicts. I propose we make the following change: -With this option, all the old files are used. Custom comments and the file structure are preserved, but no new parameters or comments from the new release are introduced. +With this option, all the old files are used. Custom comments and the file structure are preserved, but no new parameters or comments from the new release are introduced. Please note that configuration files which are identical in the old and new release are not listed or processed. This means that new default values in such files are introduced even with this option. This would mean that we are equally clear compared to the description of the "ignore"-choice. Another option would be to say that we are clear enough in the first paragraph of the section and in that case remove the "Please note that.."-part from the description of the "ignore" choice.
I have also identified an issue with tl-setup in GUI mode. When the configuration migration step presents you with 3 radio-button choices, I can not use TAB to change focus to the list of radio buttons. Normally, for example, in the case in tl-setup where you choose Master or Agent, if you use TAB the focus will jump to the default choice (which in this case is Master). I suspect that this bug is caused by the fact that we actually have 4 choices but the first and default choice is hidden. The reason we have a hidden choice is so that we can have a list of radio buttons, but visually no button selected. Hidden buttons are practically removed from all the semantics in GTK, which is why I think we can not change focus to this list using TAB. If we resolve these issues described in this comment and comment #38, I consider the testing done and the bug can be closed.
(In reply to comment #38) > However, I have found a few small issues. In the documentation in the TAG I > feel that we are a bit inconsistent with how clear we describe the 3 > configuration choices. The "parameters" option is described quite thoroughly, > but I think we could be a little bit clearer if we make the following change: Fixed now, with slightly different wording. Also introduced som "air" to the parameters section; hopefully more readable.
Great! I like the way we ended up with the radio buttons, and they are indeed navigable through using the keyboard now. I am also satisfied with the documentation now. Closing
The configuration migration step "crashes" if: 1. there are configuration files to migrate (say vsmagent.hconf and vsmserver.hconf). 2. the administrator suspends tl-setup to view the differences between the files and the rpmsave files. 3. the administrator manually migrates the changes in vsmagent.hconf and removes vsmagent.hconf.rpmsave. 4. the administrator resumes tl-setup and selects 'parameters' to migrate the changes in vsmserver.hconf. > Please wait, migrating configuration...Traceback (most recent call last): > File "/opt/thinlinc/bin/../bin/hivetool", line 253, in <module> > main() > File "/opt/thinlinc/bin/../bin/hivetool", line 212, in main > imp_walk(hive, ih, "/") > File "/opt/thinlinc/bin/../bin/hivetool", line 87, in imp_walk > for paramname in ih.get_parameters(folderpath): > AttributeError: 'NoneType' object has no attribute 'get_parameters' failed. tl-setup will keep running and move on to the next step though. Tested with the text-mode tl-setup, if it makes a difference.
(In reply to comment #44) > 1. there are configuration files to migrate (say vsmagent.hconf and > vsmserver.hconf). > 2. the administrator suspends tl-setup to view the differences between the > files and the rpmsave files. > 3. the administrator manually migrates the changes in vsmagent.hconf and > removes vsmagent.hconf.rpmsave. > 4. the administrator resumes tl-setup and selects 'parameters' to migrate > the changes in vsmserver.hconf. Related: 1. there are configuration files to migrate (say vsmagent.hconf and vsmserver.hconf). 2. the administrator suspends tl-setup to view the differences between the files and the rpmsave files. 3. the administrator manually migrates the changes in vsmagent.hconf and removes vsmagent.hconf.rpmsave. 4. the administrator is happy with the outcome, resumes tl-setup and selects 'ignore' to keep using the current files as-is. > How should old configuration files be handled? [parameters/old/ignore]?ignore > > Please wait, migrating configuration... failed. > > Could not migrate configuration. > Please send a copy of /var/log/tlsetup.log to the ThinLinc support. > Visit https://www.cendio.com/thinlinc/support for information on how > to contact us. > > Press Enter to continue...
(In reply to comment #45) Also a problem if you select 'old' instead of 'ignore'.
(In reply to comment #46) > (In reply to comment #45) > > Also a problem if you select 'old' instead of 'ignore'. A bit worse actually - configuration files can disappear if you use 'old'. > # ls /opt/thinlinc/etc/conf.d/profiles* > /opt/thinlinc/etc/conf.d/profiles.hconf.tlnew
Inspired by discussion around bug 5723, I have another point of concern. First, in my mind, it is no longer easy to miss if some sort of configuration choice or migration has to be done in an upgrade (since we now require a choice to be made before proceeding). But no longer have a natural pause in the upgrade process when an administrator can make manual changes if he wishes. Earlier this pause was between the installer and tl-setup. If that an admin get's presented with the following list of configuration "conflicts": vsmagent.hconf vsmserver.hconf profiles.hconf tlwebaccess.hconf Ponder that the admin wants to manually merge let's say profiles.hconf and tlwebaccess.hconf but wants to use the "auto-migrate" for the other two files. This is not currently possible. I believe that the best time for allowing the administrator to manually look over his configuration files is at the time when he get's this list presented to him. This would require us to firstly change the wording a bit on this page of tl-setup, but also to make a new check for .rpmsave-files and the like once the "Next"-button is pressed.
(In reply to comment #44) (In reply to comment #45) (In reply to comment #46) (In reply to comment #47) All these cases now work fine.
