Extend Updatescript

[bjoernbrings]

Signed-off-by: Björn Brings bjoernbrings@web.de

Following the discussion of openhab/openhab-core#674 I tried to adapt the update scripts accordingly. As I have no idea on how to test the script properly this script is currently (sorry for this) untested.
If you have any idea on how to test or suggestions please feel free to comment.
The intention of this PR is to push the solution of the mentioned issue to get finally the M2 :)

[kaikreuzer]

Great, many thanks @bjoernbrings for your engagement!
@openhab-5iver Would you like to test it?

[BClark09]

Thanks @bjoernbrings! I won't be able to test until later this week as I am away celebrating my graduation.

Just by looking I think we'll need to use a different sed delimiter than / as paths may confuse the script.

[5iver]

My current test and production environments are using manual installations, so I have nothing to test this on, and the docker containers use manual installations too. Mixing manual and repository installations causes problems because of the environment variables being set in the repo install.

As for the script, I think deleting the backup json is dangerous, unless there was some way to confirm that a backup had been performed.

However, I don't think the update script is the best place for this, since it would not fix the issue in all cases. It doesn't really fit with the intent of the scripts, but setenv and setenv.bat would cover all cases. It would be more appropriate to create new scripts that are called from karaf and karaf.bat.

Steps to test are straightforward. Install the NGRE, create UI rules, disable a rule, note the DTO in /userdata/jsondb/automation_rules.json and the disabled json. Update OH. Confirm DTO has changed, disabled rule is still disabled and can be enabled, and rules function.

[kaikreuzer]

@openhab-5iver Your comment might better fit in openhab/openhab-core#674, but I must admit that I do not really understand it. Are you saying that "manual installations" do not use the update script and thus this script is not the correct place? If so, I do not agree - the update script is THE central part that must be used by all installation methods for doing version updates. Only if this is the case, we can be sure that the same logic is applied everywhere.

[bjoernbrings]

@BClark09 Congrats. As you seem to be the expert here. When do you think you could have a look? Changed the delimiter according to your comment to #.
@openhab-5iver But the trigger will never hold true until a M2 is released. So how to upgrade and from / to which version?

[5iver]

But the trigger will never hold true until a M2 is released. So how to upgrade and from / to which version?

I'm not clear on what you mean. Setup 2.4, 2.5M1, or an older snapshot and replace the update script with the one you have modified, then run it to update. That would be a full test, but you could also just run the script with everything commented out except what is needed for the renaming.

Are you saying that "manual installations" do not use the update script and thus this script is not the correct place? If so, I do not agree - the update script is THE central part that must be used by all installation methods for doing version updates.

@kaikreuzer, I confused things with my response about why it would be difficult for me to test this. What I was trying to say was that, if a script is to be used, we can't know for sure that everyone will use the update script. To be certain that this fix gets to everyone, it would need to be in a script that we know will run. The OOTB backup and update scripts are great templates, but they don't fit everyone's use cases. For example, I use a custom script to backup and update, and I know there are others who do the same... especially the people who have been using their own scripts since before the OOTB scripts existed.

[bjoernbrings]

I did some tests on linux. Sadly my commands are ignored (even when I removed the -M2 section or created a [[post]][2.5.0] section ...

But what I recognized: Disabled rules are just referencing to original rules. No search an replace required.

So we have to wait here for @BClark09

Is the upgradescript really used? I have never used it, just extracted a new version over the old one and right now I'd just replace the docer container (which has the same effect). So I'd agree with @openhab-5iver ...
Nevertheless I have more and more the feeling that this script is not necessary for M2 as the engine itself is already flagged as experimental.

[kaikreuzer]

we can't know for sure that everyone will use the update script.

The script is the only supported way to do an update. So anyone who does not use it is on his own to do the required changes manually, which is not recommended. We specifically introduced the update script to simplify support by having a single way of how updates are performed.

Sadly my commands are ignored

For testing, you will have to tweak the script. Afaik, it is using the update.lst file of the downloaded distro (and not the "old" one which you locally changed).

Nevertheless I have more and more the feeling that this script is not necessary for M2 as the engine itself is already flagged as experimental.

This is correct and that was my position in the past as well. Nonetheless, I learnt that there are quite many people using it already nonetheless and it would be a pain for them, if all the rules break. As the "fix" is pretty simple and we will need that for future namespace changed (for things etc.) anyhow, it feels to be the right moment to provide this feature now already.

[bjoernbrings]

@kaikreuzer thanks for the hint. Indead the update script and the update list are taken from download. Hadn't analysed the scripts that far as I hoped for an easy fix :)

Still the script is behaving strange. Have analyzed it with set +x. The script pretends to execute
sed -i s:org.eclipse.smarthome.automation.dto.RuleDTO:org.openhab.core.automation.dto.RuleDTO:g ./userdata/jsondb/automation_rules.json
but nothing happens. When executing only this line seperatly everything behaves like expected ... weird

[bjoernbrings]

Did some further debugging. Strang thing is when I add some commands doing copies for debugging the script works as expected. I'd guess that the problem occurs because of the docker environment, where openhab is running during the upgrade process
The script itself works (when I insert a copy of the file directly after the 'sed' command I can see that it is correctly modified. However at the end of the upgrade process it is in the old state again. When I do a full copy of the /tmp/openhab directory before the post commands are executed [which should be completly unrelated] the whole script works as expected ...)

Conclusion: I'd be happy if somebody could test it

[kaikreuzer]

For usage with docker, I'd think that @wborn should be the expert to test & comment.

[wborn]

The openHAB Docker containers currently don't use this update script. They use their own update commands in the entrypoint scripts. I see that the update script has come a long way. So perhaps it can nowadays also be used for upgrading openHAB in Docker containers.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/preparation-for-2-5m2/75738/124

[bjoernbrings]

Tests in linux where finally successfull.
For Windows I did tests on the command itself.

So any maintainer arround who could 4-eye-check the commit?

[wborn]

The changes look good to me! Maybe @BClark09 again has some time to also have a look at these changes?

[BClark09]

So sorry for the delay!

The main changes look good to me and I've tested the script on both Windows and Linux and confirm the replacement to be working. For me, it would be okay to merge but I had two comments for consideration first:

1. Users without the file listed in DELETEDIR will get an error message saying the file couldn't be found. I think we should check if the target file exists firsts before attempting any command.

2. Whilst I have never found that I have needed it, are we okay deleting the user's jsondb/backup for a 2.4 -> 2.5 upgrade? If not, we will have to find a way of recursvely seding through files and this will need to be approached with a lot of caution.

[BClark09]

For future reference, here's how I test the upgrade script with update changes:

• With the PR changes in place, edit the line in the upgrade script (Windows | Linux/MacOS) to edit). that downloads the file with a simple copy (from a path to a zip file you create later) to the download location
• Zip up the distribution again so that it's path matches what you put in the previous step.
• You can now repeatedly run ./runtime/bin/update[.bat] 2.5.0 under different conditions to test.

[bjoernbrings]

@BClark09 First of all thanks for testing and your howto. Nevertheless I lack a Windows environment of openhab

Agreeing to what you amended I removed the deletion of the backup. I first did include all steps that were proposed in the initial post. Furthermore I don't think that this fix is important enough to iterate through all backups (I guess sed would accept * for files ...)

So from my point of view it is ready to go.

[bjoernbrings]

@wborn Could you please include the update in your docker script?

[kaikreuzer]

Thank you all!

Users without the file listed in DELETEDIR will get an error message saying the file couldn't be found.

Is this also the case for the REPLACE command? What if a user does not use the NGRE and thus does not have such files?

[bjoernbrings]

Added a further check if the file exists

[kaikreuzer]

Thanks!