Persistence upgrader: Fix unmanaged config not respected by florian-h05 · Pull Request #5213 · openhab/openhab-core

florian-h05 · 2025-12-20T11:41:10Z

Unmanaged configs were not respected because they were not recognized due to an invalid file extension check.

Looks good to me now:

florianh@zephyrus-fh211 /u/s/openhab> ls /etc/openhab/persistence/
inmemory.persist  readme.txt*
florianh@zephyrus-fh211 /u/s/openhab> rm /var/lib/openhab/jsondb/org.openhab.core.persistence.PersistenceServiceConfiguration.json
florianh@zephyrus-fh211 /u/s/openhab> java -Dorg.slf4j.simpleLogger.defaultLogLevel=DEBUG -jar upgradetool-5.1.0-SNAPSHOT-jar-with-dependencies.jar --command persistenceCopyDefaultStrategy --force 
[main] DEBUG org.openhab.core.common.ThreadPoolManager - Created scheduled thread pool 'JsonStorage' of size 5
[main] DEBUG org.openhab.core.storage.json.internal.JsonStorage - Opened Json storage file at '/var/lib/openhab/jsondb/org.openhab.core.tools.UpgradeTool'.
[main] INFO org.openhab.core.tools.UpgradeTool - Executing persistenceCopyDefaultStrategy: Move persistence default strategy configuration to all persistence configuration without strategy defined
[main] DEBUG org.openhab.core.tools.internal.PersistenceUpgrader - found 2 managed persistence configurations: rrd4j,mapdb
[main] DEBUG org.openhab.core.tools.internal.PersistenceUpgrader - found 1 unmanaged persistence configurations: inmemory
[main] DEBUG org.openhab.core.storage.json.internal.JsonStorage - Opened Json storage file at '/var/lib/openhab/jsondb/org.openhab.core.persistence.PersistenceServiceConfiguration.json'.
[main] INFO org.openhab.core.tools.internal.PersistenceUpgrader - rrd4j: added strategy configurations for persistence service without configuration
[main] INFO org.openhab.core.tools.internal.PersistenceUpgrader - mapdb: added strategy configurations for persistence service without configuration

Signed-off-by: Florian Hotze <dev@florianhotze.com>

lolodomo · 2025-12-20T11:53:01Z

+        logger.debug("found {} managed persistence configurations: {}", managedConfigs.size(),
+                String.join(",", managedConfigs));


This is minor but the log introduces confusion about what is managedConfigs, this is not the managed persistence configurations but rather the persistence services for which a managed persistence configuration is required because there is no unmanaged persistence configuration

I've added a missing in brackets. This is debug logging, so users normally won't see that, and when running repeatedly, those are just managed configs, not missing managed configs.

Signed-off-by: Florian Hotze <dev@florianhotze.com>

kaikreuzer · 2025-12-20T12:32:25Z

Thanks for the fix - let me quickly test it locally before merging as well.

Do you think we should/can address this issue as well, i.e. the fact that the newly created file is created with root permissions, which could potentially cause access issues for the runtime?

kaikreuzer · 2025-12-20T12:50:12Z

let me quickly test it locally

Tests successful! 👍

kaikreuzer

Thanks!

florian-h05 · 2025-12-20T13:40:12Z

Do you think we should/can address openhab/openhab-webui#3123 (comment) as well, i.e. the fact that the newly created file is created with root permissions, which could potentially cause access issues for the runtime?

The upgradetool currently created the file with the executor of the tool as owner. It however seems that when it's run by apt, it is run by root and hence the permissions.
I think we should fix this issue as well, though I am not sure whether this should be done in the upgradetool or the script that calls it.

florian-h05 · 2025-12-20T14:01:54Z

I think the update script in distro is the better place, as permissions are very platform specific and I am not confident I will get it right for Windows (I have no Windows system to test): openhab/openhab-distro#1847

I just triggered a new distro build, will test my changes with it and then mark my distro PR ready.

mherwege · 2025-12-20T14:10:01Z

I think the update script in distro is the better place, as permissions are very platform specific and I am not confident I will get it right for Windows (I have no Windows system to test): openhab/openhab-distro#1847

I just triggered a new distro build, will test my changes with it and then mark my distro PR ready.

That should be fine. Windows wouldn't support the POSIX attributes, and therefore it wouldn't be applied to Windows. It's possible to work out a solution as well for windows (updating the ACL's) but I also thought it is not needed. I agree, changing the user is in general the better way. The only challenge with that is that it will not work if the UpgradeTool is run manually and not from the script, as described here: https://www.openhab.org/docs/installation/?#upgrading

florian-h05 · 2025-12-20T14:11:53Z

Probably the easiest way to „fix“ this would be to update the docs and recommend running openhab-cli reset-ownership afterwards.
WDYT?

mherwege · 2025-12-20T14:12:54Z

Probably the easiest way to „fix“ this would be to update the docs and recommend running openhab-cli reset-ownership afterwards. WDYT?

Yes, that would indeed be a good solution.

florian-h05 · 2025-12-20T14:14:44Z

Do you want to create a docs PR?
I can review and merge it as long as it’s now my own PR.

florian-h05 · 2025-12-20T14:16:18Z

Just forgot one thing: openhab-cli isn’t available on Windows, it comes with the linuxpkg and the Homebrew package.
Probably better to give a command to set ownership for the contents of the JSONDB folder.

mherwege · 2025-12-20T14:45:41Z

Do you want to create a docs PR? I can review and merge it as long as it’s now my own PR.

I just created a PR. I have also completed the documentation on the upgrade tool while touching it.

kaikreuzer · 2025-12-20T14:46:59Z

@mherwege I would tend to prefer openhab/openhab-distro#1847 over #5215, wdyt?

mherwege · 2025-12-20T14:50:55Z

@mherwege I would tend to prefer openhab/openhab-distro#1847 over #5215, wdyt?

Yes, I agree. I just wanted to help cleaning up the mess the hurried fixes left.
The one challenge I saw (running the upgrade tool without the script, as explained in the documentation) could be solved with documentation improvements and I provided a documentation PR for that.

lolodomo · 2025-12-20T15:21:46Z

What will happen to users that upgraded to 5.1 RC1 and who have now an unexpected JSON file ?
When upgrading to the final 5.1, this unexpected file will still be there and persistence will still be broken...

mherwege · 2025-12-20T15:35:20Z

What will happen to users that upgraded to 5.1 RC1 and who have now an unexpected JSON file ? When upgrading to the final 5.1, this unexpected file will still be there and persistence will still be broken...

If all services are unmanaged, the easiest fix (and I believe already stated on the forum), will be to stop OH, remove the newly created json file, and starting OH again. You could also remove the persistence definitions from the UI. If there is a mix of managed and unmanaged, one would have to remove the parts for the unmanaged persistence services from the json file (instead of completely removing). Services without any configuration before should be considered as managed.

helmar74 · 2025-12-20T15:39:34Z

just for your information.
I can confirm. After changing permissions with chown openhab:openhab, I was able to delete configuration in webui.
After that the file stays in the system, but is "empty:

cat /var/lib/openhab/jsondb/org.openhab.core.persistence.PersistenceServiceConfiguration.json
{}

lolodomo · 2025-12-21T07:31:44Z

Still reported as not working in RC2 by one user!
Edit: he previously upgraded to RC1.

mherwege · 2025-12-21T08:13:23Z

Still reported as not working in RC2 by one user! Edit: he previously upgraded to RC1.

Most likely the cleanup was not done. RC2 wouldn’t automatically correct the wrong configuration from RC1.

helmar74 · 2025-12-21T08:16:21Z

I can confirm, that permissions are correct for persistence configuration during update from RC1 to RC2. Also filebased configuration for persistence services is used:

2025-12-21 08:50:24.139 [INFO ] [el.core.internal.ModelRepositoryImpl] - Loading DSL model 'mapdb.persist'
2025-12-21 08:50:24.220 [INFO ] [el.core.internal.ModelRepositoryImpl] - Loading DSL model 'rrd4j.persist'

Permissions before upgrade:

/var/lib/openhab/jsondb $ ls -l
insgesamt 2544
-rw-r--r-- 1 openhab openhab    5423 20. Dez 11:53 amazonechocontrol:account:135801ce.json
-rw-r--r-- 1 openhab openhab    1423 14. Jul 09:31 amazonechocontrol:flashbriefingprofile:135801ce:1.json
drwxr-xr-x 2 openhab openhab   49152 20. Dez 12:37 backup
-rw-r--r-- 1 openhab openhab     186 14. Jul 01:46 hue.emulation.config.json
-rw-r--r-- 1 openhab openhab       2 14. Jul 01:46 hue.emulation.lights.json
-rw-r--r-- 1 openhab openhab       2 14. Jul 01:46 hue.emulation.users.json
-rw-r--r-- 1 openhab openhab     455 14. Jul 01:46 mailtext.txt
-rw-r--r-- 1 openhab openhab   10926 14. Jul 01:46 org.eclipse.smarthome.core.thing.Thing.json_crash
-rw-r--r-- 1 openhab openhab    4382 20. Dez 11:53 org.openhab.core.config.discovery.DiscoveryResult.json
-rw-r--r-- 1 openhab openhab  112230 19. Jul 10:45 org.openhab.core.items.Item.json
-rw-r--r-- 1 openhab openhab  835625 21. Jul 19:04 org.openhab.core.items.Metadata.json
-rw-r--r-- 1 openhab openhab       2 20. Dez 11:41 org.openhab.core.persistence.PersistenceServiceConfiguration.json
-rw-r--r-- 1 openhab openhab  361963 14. Jul 01:46 org.openhab.core.thing.link.ItemChannelLink.json
-rw-r--r-- 1 openhab openhab 1150482 20. Dez 11:53 org.openhab.core.thing.Thing.json
-rw-r--r-- 1 openhab openhab    1585 20. Dez 09:05 org.openhab.core.tools.UpgradeTool
-rw-r--r-- 1 openhab openhab       2 14. Jul 01:46 StorageHandler.For.OAuthClientService.json
-rw-r--r-- 1 openhab openhab      85 14. Jul 09:37 thing_status_storage.json
-rw-r--r-- 1 openhab openhab    2390 14. Jul 01:46 uicomponents_ui_page.json
-rw-r--r-- 1 openhab openhab   14720 20. Dez 12:37 users.json

After upgrade:

ls -l
insgesamt 2544
-rw-r–r-- 1 openhab openhab 5423 20. Dez 11:53 amazonechocontrol:account:135801ce.json
-rw-r–r-- 1 openhab openhab 1423 14. Jul 09:31 amazonechocontrol:flashbriefingprofile:135801ce:1.json
drwxr-xr-x 2 openhab openhab 49152 20. Dez 12:37 backup
-rw-r–r-- 1 openhab openhab 186 14. Jul 01:46 hue.emulation.config.json
-rw-r–r-- 1 openhab openhab 2 14. Jul 01:46 hue.emulation.lights.json
-rw-r–r-- 1 openhab openhab 2 14. Jul 01:46 hue.emulation.users.json
-rw-r–r-- 1 openhab openhab 455 14. Jul 01:46 mailtext.txt
-rw-r–r-- 1 openhab openhab 10926 14. Jul 01:46 org.eclipse.smarthome.core.thing.Thing.json_crash
-rw-r–r-- 1 openhab openhab 4382 20. Dez 11:53 org.openhab.core.config.discovery.DiscoveryResult.json
-rw-r–r-- 1 openhab openhab 112230 19. Jul 10:45 org.openhab.core.items.Item.json
-rw-r–r-- 1 openhab openhab 835625 21. Jul 19:04 org.openhab.core.items.Metadata.json
-rw-r–r-- 1 openhab openhab 2 20. Dez 11:41 org.openhab.core.persistence.PersistenceServiceConfiguration.json
-rw-r–r-- 1 openhab openhab 361963 14. Jul 01:46 org.openhab.core.thing.link.ItemChannelLink.json
-rw-r–r-- 1 openhab openhab 1150482 20. Dez 11:53 org.openhab.core.thing.Thing.json
-rw-r–r-- 1 openhab openhab 1585 20. Dez 09:05 org.openhab.core.tools.UpgradeTool
-rw-r–r-- 1 openhab openhab 2 14. Jul 01:46 StorageHandler.For.OAuthClientService.json
-rw-r–r-- 1 openhab openhab 85 14. Jul 09:37 thing_status_storage.json
-rw-r–r-- 1 openhab openhab 2390 14. Jul 01:46 uicomponents_ui_page.json
-rw-r–r-- 1 openhab openhab 14720 20. Dez 12:37 users.json

holgerfriedrich · 2025-12-21T08:17:19Z

FYI: When testing RC2, I had to force the execution of the upgrade tool. The persistence updater had been executed before, so it was skipped. Rpi /deb package.

florian-h05 · 2025-12-21T13:02:48Z

I’ve already throught about whether we could do some automatic clean-up in the upgrade tool for the RC1 issue.
If a unmanaged config has been found, we could remove the managed config from JSONDB.

WDYT? I always consider such things a bit risky, but normally there shouldn’t be both managed and unmanaged config. If a user however creates a unmanaged config for any reason this will remove his managed config.

mherwege · 2025-12-21T13:09:08Z

FYI: When testing RC2, I had to force the execution of the upgrade tool. The persistence updater had been executed before, so it was skipped. Rpi /deb package.

I am not sure you had to. Was anything broken with your persistence configuration? Did running he upgrade tool change anything? If not, it was not required. If so, you needed to do manual fixes anyway.
The corrections in RC2 do not fix issues created by M4 or RC1. They only make sure it should behave as expected with anything before these.

mherwege · 2025-12-21T13:12:35Z

WDYT? I always consider such things a bit risky, but normally there shouldn’t be both managed and unmanaged config. If a user however creates a unmanaged config for any reason this will remove his managed config.

I am not sure it is worth it. You would have to force run the upgrade tool to pick it up anyway. In that case you can just as well do the changes manually as already documented.

What could make more sense is to flag it in the health check. That would then also flag a user mistake of having 2 conflicting configurations, independent of the upgrade tool. But that could be an enhancement after release.

florian-h05 · 2025-12-21T14:19:27Z

What could make more sense is to flag it in the health check. That would then also flag a user mistake of having 2 conflicting configurations, independent of the upgrade tool. But that could be an enhancement after release.

That would indeed be a nice feature 👍

mherwege · 2025-12-21T16:20:33Z

That would indeed be a nice feature 👍

First draft PR created to extend the REST endpoint: #5217

Persistence upgrader: Fix unmanaged config not respected

29ad1cf

Signed-off-by: Florian Hotze <dev@florianhotze.com>

florian-h05 requested a review from a team as a code owner December 20, 2025 11:41

This was referenced Dec 20, 2025

Persistence strategies not automatically applied openhab/openhab-webui#3123

Merged

Persistence upgrader: Create default persistence settings if no configuration so far #5212

Merged

lolodomo reviewed Dec 20, 2025

View reviewed changes

Address review

8ebc5cf

Signed-off-by: Florian Hotze <dev@florianhotze.com>

kaikreuzer added bug An unexpected problem or unintended behavior of the Core critical labels Dec 20, 2025

kaikreuzer added this to the 5.1 milestone Dec 20, 2025

kaikreuzer approved these changes Dec 20, 2025

View reviewed changes

kaikreuzer merged commit 05ba25e into openhab:main Dec 20, 2025
6 checks passed

florian-h05 deleted the persistence-upgrader branch December 20, 2025 13:39

mherwege mentioned this pull request Dec 20, 2025

Extend documentation for upgrade tool openhab/openhab-docs#2617

Merged

		logger.debug("found {} managed persistence configurations: {}", managedConfigs.size(),
		String.join(",", managedConfigs));

Uh oh!

Uh oh!

Conversation

florian-h05 commented Dec 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lolodomo Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

florian-h05 Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

kaikreuzer commented Dec 20, 2025

Uh oh!

kaikreuzer commented Dec 20, 2025

Uh oh!

kaikreuzer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

florian-h05 commented Dec 20, 2025

Uh oh!

florian-h05 commented Dec 20, 2025

Uh oh!

mherwege commented Dec 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

florian-h05 commented Dec 20, 2025

Uh oh!

mherwege commented Dec 20, 2025

Uh oh!

florian-h05 commented Dec 20, 2025

Uh oh!

florian-h05 commented Dec 20, 2025

Uh oh!

mherwege commented Dec 20, 2025

Uh oh!

kaikreuzer commented Dec 20, 2025

Uh oh!

mherwege commented Dec 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lolodomo commented Dec 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mherwege commented Dec 20, 2025

Uh oh!

helmar74 commented Dec 20, 2025

Uh oh!

lolodomo commented Dec 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mherwege commented Dec 21, 2025

Uh oh!

helmar74 commented Dec 21, 2025

Uh oh!

holgerfriedrich commented Dec 21, 2025

Uh oh!

florian-h05 commented Dec 21, 2025

Uh oh!

mherwege commented Dec 21, 2025

Uh oh!

mherwege commented Dec 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

florian-h05 commented Dec 21, 2025

Uh oh!

mherwege commented Dec 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

florian-h05 commented Dec 20, 2025 •

edited

Loading

mherwege commented Dec 20, 2025 •

edited

Loading

mherwege commented Dec 20, 2025 •

edited

Loading

lolodomo commented Dec 20, 2025 •

edited

Loading

lolodomo commented Dec 21, 2025 •

edited

Loading

mherwege commented Dec 21, 2025 •

edited

Loading