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

DietPi-Software | Update Roon Extension Manager to v1.0 #4399

Merged
merged 1 commit into from
May 21, 2021

Conversation

JanKoudijs
Copy link
Contributor

Status: Ready (from my side)

  • Tested clean install, uninstall and reinstall from older version

Reference: https://community.roonlabs.com/t/roon-extension-manager-v1-0-beta3/151438

Commit list/description:

  • DietPi-Software | Update Roon Extension Manager to v1.0

Hi DietPi team,

It's almost 3 years ago that the Roon Extension Manager got integrated in DietPi (#1865), I now made a major update requiring also an update in the Dietpi integration. The new version depends on Docker and no longer on git and Node.js, this PR is already updated to the new dependency handling approach.

If you have suggestions for improvements, please let me know.

Best regards,
Jan Koudijs

@MichaIng
Copy link
Owner

Many thanks for the PR, looks perfectly integrated 👍.

But I have a few questions, also since I don't use Roon and have no experience with it 😅:

  • I guess extensions installed with the old implementation cannot be managed with the new one, is it? Is there a way to migrate them? In the forum I read that new extensions are now Docker images as well, so that doesn't seem to be trivial. I'm just thinking how to deal with users who have the old REM implementation installed and extensions for it, whether we should leave the uninstall code for the old one in place, or migrate users on DietPi update, or inform them about the new REM and how the old one can be manually uninstalled. If a migration breaks installed extensions, then the migration should not be forced.
  • As we generally like to run software as native as possible, is there still a way to run it with Node directly? Or do new extensions require REM itself to run as Docker container? Sorry if the question sounds dump 🙂. Generally, maintaining a systemd unit with a service user and an install + config/data path wouldn't be an issue, but of course if you e.g. need a specific Node version or other environment that cannot be guaranteed on different Raspbian/Debian versions e.g.

@JanKoudijs
Copy link
Contributor Author

Hi @MichaIng,

Thanks for your quick reply.

The old REM supports two distribution types for extensions, the first is via git/npm and the second is via Docker. Docker has always been optional as the DietPi user had to take care of the Docker installation himself (it could not be marked as a dependency a few years back). In the new situation the git/npm option is dropped both for the REM install and for extensions, I just had too many maintenance activities/frustrations with the wide range of npm versions that do appear in the wild. So the new approach is Docker only, similar to how Portainer runs in a container and can create other containers.

In case of an upgrade, all the extensions that were already running in a container will survive, including the user settings. Extensions that were installed via npm have to be reinstalled after the upgrade. This breaking change is also the reason why I switched from the 0.x version to the 1.x version. I have taken care that all extensions remain available in the new setup.

So you are right, if a user has installed the Alarm Clock extension via npm and an upgrade is performed automatically over night then there will be no alarm :(

I'm not sure about the update options of DietPi. My experience is that I have to start updates manually and also that e.g. to update Node.js I have to perform a reinstall manually (in the past users had broken installs while it worked for me because I was still on an older npm version). Is it always the case that such a reinstall has to be done via a manual action by the user?

If so, then I can document the upgrade as follows:

  • Upgrade to v1.0: dietpi-software reinstall 86
  • Reinstall and configure the extensions that were running via npm

Some links with background information, in case you are interested:

@MichaIng
Copy link
Owner

MichaIng commented May 21, 2021

Okay, so the Docker method is fixed then, which is fine.

Yes the DietPi update is still a manual step and we try to avoid force/automated reinstalls during DietPi updates, but instead inform users about important upstream changes the require a reinstall to benefit from. But this often means that we need to add code to dietpi-software to gracefully handle old and new instances of software, including the migration of data and configs in case. We just had the case where Bitwarden_RS was renamed by its developer to vaultwarden, and there we force a reinstall on DietPi update (after informing user and allowing to cancel the update) to avoid massive code overhead.

Does the old setup script cleanly uninstall old REM when the new got installed? In case I think it's fine to inform users about the change and method to upgrade, as you suggested, and additionally show a curl-bash command for uninstalling the old instance, when the new one has been successfully installed and setup. I'll run a test for this now as well. EDIT: Okay the new installer removes the old instance automatically 👍.

@MichaIng MichaIng added this to the v7.2 milestone May 21, 2021
@MichaIng
Copy link
Owner

MichaIng commented May 21, 2021

I ran a test reinstall of the new over old and it works fine, the service is replacing the old one. REM Docker contains takes ~65 MiB memory vs ~55 MiB for the Node version. Docker containerd adds another 110 MiB, but this was the case before as well when users installed Docker to install new extensions.

And:

Removing old installation...

Perfect, then we only need to inform users that old Node-based extensions need to be reinstalled 👍.

I was asking Roon for a free developer account to test everything with remote apps etc. Not mandatory, but would be nice mod/long term to test our Roon implementations, of course.


I'll merge the PR and do update notification and changelog in a separate PR: #4403

@MichaIng MichaIng merged commit 641c048 into MichaIng:dev May 21, 2021
MichaIng added a commit that referenced this pull request May 21, 2021
+ DietPi-Patches | Roon Extension Manager: Inform users about available upgrade: #4399
+ DietPi-Patches | Odroid XU4: Assure that all possible hardware random generator daemons are uninstalled
MichaIng added a commit that referenced this pull request May 21, 2021
+ DietPi-Patches | Roon Extension Manager: Inform users about available upgrade: #4399
+ DietPi-Patches | Odroid XU4: Assure that all possible hardware random generator daemons are uninstalled
@JanKoudijs
Copy link
Contributor Author

JanKoudijs commented May 21, 2021

Thanks for accepting the PR and adding the update notification for the user!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants