-
-
Notifications
You must be signed in to change notification settings - Fork 394
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
Use list of system files in userdata folder #647
Conversation
FYI: @openhab/docker-maintainers @openhab/synology-maintainers @openhab/linuxpkg-maintainers @rkoshak @wborn @bdleedy @tmrobert8 @kaikreuzer and @ThomDietrich: This single file can hopefully be used across repos for the relevent upgrade/backup process. Please let me know if you have any suggestions, tips or requests or PRs to help ;). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the system files list! I verified that the list includes all the files that are being updated during the Docker container upgrade process (even some more).
Is this PR a first step towards
#501 ?
fi | ||
done < "$TempDir/filelist.list" | ||
|
||
mv "$WorkingDir/runtime" "$TempDir/runtime/" | ||
|
||
## We need to keep a backup in case the user modified that file | ||
cp "$WorkingDir/userdata/etc/org.ops4j.pax.logging.cfg" "$WorkingDir/userdata/etc/org.ops4j.pax.logging.cfg.bak" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps org.ops4j.pax.logging.cfg
should also be added to userdata_sysfiles.list
?
Why does the upgrade also backup org.ops4j.pax.logging.cfg
? Isn't that already done in the backup script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what to do with it at the moment. org.ops4j.pax.logging.cfg
should be configurable (at the moment) because there's no way of customising the login. The saving a temporary file was necessary when log4j2 was introduced and users would have found a broken logging system by keeping their current configuration.
- If we move it to the system file list, then users will have to edit their logging configuration each time they update.
- If we keep it how it is, they'll need to swap them back if they've already moved over.
- If we remove the line, users upgrading from 2.1.x will have an invalid logging configuration.
- We can add a simple check here to see if the current version is 2.1.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be added to the system file list when the update script renames it to org.ops4j.pax.logging.cfg.bak
before it starts overwriting the old files.
It looks like org.ops4j.pax.logging.cfg
is currently not updated during Docker container updates like in this update script. So logging may be broken after an upgrade for Docker container users. For users reporting such issues see this community thread.
Hopefully one day user customizations can be retained e.g. by splitting the log4j2 configuration file (#516).
@bdleedy, I have edited the Windows scripts. Would you be able to review what I have done? |
I haven't tested it but it looks good. I need to finish/check my last PR so I can verify then. |
Might we not run the risk that the two contradict each other? I'd so far rather thought that those system files are a part of the CSV, i.e. saying that those need to be copied/moved from the extracted download to the respective path in userdata. If we really want to support versioning in CSV (i.e. upgrades over multiple versions at once), we probably will be in trouble with a "system file" list that is unversioned, don't you think? |
The reasoning behind using different files is that the list is used in different ways for each repo. For the .DEB and .RPM packages, you have to define a list of all the files that the user is expected to configure. The package automatically persists these files over different versions and replaces every other file. In otherwords, we have to mark certain files as configurable before the upgrade takes place, otherwise the packaging system will get rid of the file(s).
In my head, because 3 and 4 are dealt with differently across repos, then they'd need to be stored as separate files. With two files it should be possible to complete the general upgrade procedure which is (please correct me if I'm wrong :-) ):
(Anything past this point would be considered post installation by DEB and RPM packaging.)
With the above is there any scenario you can think of where this set of instructions wouldn't work? |
It would be nice if there is a backup / update script that would work for all packages (linuxpkg, windows, docker, synology, rpm, apt, etc.) |
@BClark09 Hey Ben, I am very late to the party here, and that is because I live in a apt-driven world. Now, I have not followed all the discussions on this topic, but I have a question: wrt the upgrades, how will it work for files that are manually modified after installation? For example, on every upgrade I have to change org.apache.karaf.shell.cfg in /userdata so that "sshHost = 0.0.0.0". Shouldn't we somewhere include a diff in the upgrade process, present the changes to the user, and let the user decide between "their" or "mine" changes when a conflict is found? At the very beginning of #299 @kaikreuzer put forward that all this should also work for SNAPSHOT upgrades. Any news on that? ( I guess it depends on the the script being made compatible with apt in the first place?) |
Hi @kgoderis, at the moment the PR in #662 gave the upgrade process an option to backup and default important configuration files. But the list used in this PR makes sure that system files are replaced regardless. The idea was that all the files in userdata wouldn't need manual intervention, but this has proven to be difficult to implement. I think the long term goal is to have any configurable text file sit in the conf folder. @kaikreuzer, is this correct so far? As far as I have been told (I think), all of the WRT to your problem of having to change the ssh host each upgrade, these sorts of settings can also be set in
Hopefully, this will mean that your setting is persisted on upgrades. |
Signed-off-by: Ben Clark <ben@benjyc.uk>
I've re-based the PR to follow on from #662. |
Not really - it all sounds plausible and I am ok to go that way. What is left for you to do in order to remove the WIP marker?
When upgrading my system that used the KAR file, I actually noticed that |
The only minor issue I can see at the moment is that the new
Yeah that should be the best place for it, although I believe the DELETE command will need to be modified to handle directories as well as files (i.e. |
Signed-off-by: Ben Clark <ben@benjyc.uk>
I think I figured it out, PR should be ready now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks!
Could you add such a feature? Not too urgent, I would say, the kar folder does no harm, it just consumes space... |
Use a list inside a file that can be used for the update processes and the different repos, so that everything is finally in sync.
todo:
Signed-off-by: Ben Clark ben@benjyc.uk