Skip to content
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

[updater] A component for self-updating openHAB via the UI #2372

Draft
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented May 19, 2021

This is a first attempt at creating a core module which supports self- updating of OpenHAB via the UI.

This PR is made available to allow OpenHAB maintainers to review the code and give feedback.

It is only a proof of concept (or not). It is not yet intended to be merged at this time.

Instructions: build the jar, drop it in /addons, and then go to Developer Tools | Api Explorer | Update

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/addon-to-update-oh-thoughts-anyone/121576/11

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@cweitkamp cweitkamp added the enhancement An enhancement or new feature of the Core label May 20, 2021
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from pacive May 23, 2021 21:16
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from kaikreuzer May 26, 2021 12:43
@andrewfg
Copy link
Contributor Author

Dear All,
The first reviewers focussed on the UI of this module. There is nothing wrong with that, but IMHO the real focus should be on the functionality rather than the UI. In particular I would suggest (indeed request) some feedback on the following items..

  • The BaseUpdater class, in particular the way it builds the update scripts and injects them into the operating system shell.
  • The classes that extend from BaseUpdater for specific OS; the WindowsUpdater and DebianUpdater are tested, but the others (Mac, Yum, PacMan, Portage, etc.) need someone who uses those systems.
  • The update script templates associated with the respective OS updaters DebianUpdater.txt, WindowsXX.txt etc. etc.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

@kaikreuzer I have made some changes based on your recent feedback. However unfortunately by changing the version from 3.1.0-SNAPSHOT to 3.2.0-SNAPSHOT this PR seems to have "acquired" the 370 or so files that changed when 3.1.0 was released. It is now a mess, and I don't know how to fix that.

@andrewfg
Copy link
Contributor Author

unfortunately by changing the version from 3.1.0-SNAPSHOT to 3.2.0-SNAPSHOT this PR seems to have "acquired" the 370 or so files that changed when 3.1.0 was released

@kaikreuzer the above-mentioned issue has now been resolved :)

@andrewfg
Copy link
Contributor Author

#2372 (comment)

@kaikreuzer resolved

@andrewfg andrewfg requested a review from kaikreuzer July 20, 2021 17:01
@andrewfg
Copy link
Contributor Author

@kaikreuzer => ping!

@andrewfg
Copy link
Contributor Author

andrewfg commented Sep 6, 2021

Just to report that this did successfully install 3.2.M2 today (see screenshot below)..

image

@kaikreuzer
Copy link
Member

Cool! Will look at it soon®, promised, @andrewfg!

@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/validation-api-usage/126439/1

@kaikreuzer kaikreuzer changed the title [updater] A component for self- updating OpenHAB via the UI [updater] A component for self-updating openHAB via the UI Oct 24, 2021
@kaikreuzer
Copy link
Member

Hey @andrewfg! Very sorry that it took so long, but finally I now had a closer look at it and also tested it on macOS.
The code of the PR looks pretty clean, thanks for the good job on it!

Unfortunately, I didn't manage to get it working on macOS. I made a couple of fixes (like also setting the executable flag on the script, calling it as the provided user and not as root, etc.), but the execution of the single steps always fails as the commands expect to be run in some other folder. Manually calling it during my tests it behaved differently than when being called from the JVM and I couldn't really figure out how to fix it.

Thinking about it, I also wasn't sure how the script should actually know, how to correctly start the instance. Some people run it through "bin/karaf start", others might use "bin/start", I am running it within screen, others might do nohup or special service wrappers. I somehow doubt that there's a solution that will just work and not be very fragile.
I have the same concerns for Windows users - the current script simply starts the instance, but what if people run it as a Windows service as described here?
On Linux, the code assumes that openHAB is installed through the package manager of the distro. But what if it is manually installed or used as a snap in a docker container?

The more I think about it, the less convinced I am that we can get this a stable feature.
Happy to discuss how to proceed - also inviting the other @openhab/core-maintainers.

@wborn
Copy link
Member

wborn commented Oct 27, 2021

The more I think about it, the less convinced I am that we can get this a stable feature.

We might just only want to provide APIs here in core, so the UI knows if an update mechanism is available, and what parameters are used (password, available versions). Then the actual update script(s) and files that supply the parameters can move to distro projects. It would also allow users to have a way to plug-in their own update mechanism if they use something of their own. The available versions also depends on what installation method is used. E.g. versions aren't always immediately available for all platforms/installation methods (Synology, snap, etc).

@andrewfg
Copy link
Contributor Author

might just only want to provide APIs here in core, so the UI knows if an update mechanism is available, and what parameters are used (password, available versions). Then the actual update script(s) and files that supply the parameters can move to distro projects.

This is essentially what I did already. There is a main API with a REST GET to read the current update state, and a POST to start the update process. And then for each type of installation / distro there are just two distro specific elements — namely i) a small Java class that extends/ overrides some methods in the main updater class, and ii) a text file that contains the update command script.

I have only written these two elements for OpenHabian an Windows so far..

@wborn
Copy link
Member

wborn commented Oct 27, 2021

IMHO the core should not know anything about package specific upgrade commands. It should only know that there is a script (or class) that can do this and what kind of parameters it expects that can then be provided using the UI. I.e. it uses some abstraction where the package adds a file or property to indicate it can do the update using what parameters. Then the actual upgrade scripts move to either openhab-distro, openhab-linuxpkg, openhab-syno-spk, openhab-snap etc where they can be maintained and which already have all the package specific implementation details.

@andrewfg andrewfg closed this Oct 27, 2021
@andrewfg
Copy link
Contributor Author

the core should not know anything about package specific upgrade commands. It should only know that there is a script (or class) that can do this and what kind of parameters it expects that can then be provided using the UI

Indeed. (That is what I am doing). The scripts contain placeholders for parameters like target version, target version type, password etc. and the core replaces the placeholders in the scripts.

@andrewfg andrewfg reopened this Oct 27, 2021
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/upgrading-system-via-gui/138453/13

@J-N-K
Copy link
Member

J-N-K commented May 14, 2023

@andrewfg What is the state here?

@andrewfg
Copy link
Contributor Author

What is the state here?

I think it is 'sleeping' .. @kaikreuzer / @wborn WDYT? .. if you agree we can close it, or make it WIP ..

@kaikreuzer
Copy link
Member

I think the latest state of discussion was @wborn's suggestion above and I agree with this.
In consequence, any specific updater code should be removed from this PR and be moved to other repos instead.
I am just not sure yet, whether there is a reliable mechanism in place already, which can determine, what installation type has been used and hence, which script (if available) should be used for it.

@Skinah
Copy link
Contributor

Skinah commented Jan 13, 2024

Would love to see this merged one day.

@andrewfg andrewfg marked this pull request as draft March 4, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants