Sonarr v4 beta

[mreid-tt]

Description

This is a new deployment of Sonarr v4 beta (.NET) for Synology which replaces #5520. As this is a major change, the package internal name has also changed from nzbdrone to sonarr. The upgrade steps from Sonarr v3 would thus be:

1. Installation of Sonarr v4 package
2. Automatic migration of previous configuration data
3. Automatic uninstallation of Sonarr v3
4. (optional) Uninstallation of Mono if no longer required

Fixes #5051

Checklist

• Build rule all-supported completed successfully
• New installation of package completed successfully
• Package upgrade completed successfully (Manually install the package again)
• Package functionality was tested
• Any needed documentation is updated/created

Type of change

• Bug fix
• New Package
• Includes small framework changes
• This change requires a documentation update (e.g. Wiki)

[mreid-tt]

@hgy59, continuing the discussion in this new PR as the old one was closed after I rebased...

@mreid-tt is it required to use a different port for Sonarr v4? I thought that we could use the same port, when a package is replaced by a new one.

I did some more investigation and apparently, a method to deploy and avoid the port conflict check was attempted before in 4d88fa9. I've taken this logic and included it in the latest commit and I was able to confirm that the package installed without flagging a conflicting port with the addition of the checkport field.

Thanks for your additional comments:

• we should not only replace sonarr v3 by v4 for DSM 7 but also for DSM 6

With regard to the DSM6 config, I'll work on it once I have successfully tested a package replace under DSM7. I've done a basic implementation but may need some assistance with this as there seems to be a few other things done with that older OS looking at the other examples.

• for all arch/dsm version combinations that successfully build Sonarr v4 we should add the package replacement.

I thought the SynoCommunity spksrc platform would include the code I provided for all relevant architectures. Is there something more I would need to do?

• Only DSM 5.x will stay on Sonarr v3

Yes, that makes sense. I believe that we can just configure the build not to make anything for DSM5 (if not already configured to do so).

• currently we build DSM 6 packages for DSM 6.1-15047 - package replacement is supported from DSM 6.1-15117 (i.e. 6.1.2). So we have to ajust this for our DSM 6 packages. Both DSM 6.1 versions are fairly old (03/2017 and 09/2017) and all models supporting DSM 6.1 have updates at least for DSM 6.2.4.

Thanks for confirming that the prereplace and postreplace functions are recognised in DSM 6.1. As for DSM 6.0 we can work on that together.

• SRM 1.x is compatible with DSM 6 installer, but we have to approve this, before deploying packages with replacement of Sonarr v3.

I'm not sure what SRM 1.x is exactly but from a bit of searching I see that SRM is some type of O/S like DSM? I came across it in a post on #5424 where you were asking Synology for toolchains for SRM 1.3. From the link you posted there, it seems that Synology eventually provided a response. Let me know what would be needed from my end.

[mreid-tt]

@hgy59, so I did some deployment tests and the prereplace flag does not appear in the installation log (see: https://pastebin.com/6nRLNBKb). It does however seem to be called but appears to be terminating prematurely. Based on the log at /var/log/synopkg.log (see https://pastebin.com/5e1qEvbJ), we note the following section:

2022/12/15 06:27:19 install   Begin prereplace
2022/12/15 06:27:19 stop nzbdrone: begin to stop version 20210717-19
2022/12/15 06:27:29 stop nzbdrone: stop version 20210717-19 failed, result 0
2022/12/15 06:27:29 install   End prereplace ret=[0]

What this appears to be is the function starting, passing the validate_prereplace successfully and then stopping in the service_prereplace function. Specifically on this line:

spksrc/spk/sonarr/src/service-setup.sh

Line 54 in 5e5a5d4

synopkg stop nzbdrone 2>&1

I believe it attempts to stop the old NzbDrone but fails to complete because the command is not executed with elevated privileges. From checks elsewhere in the code, all of the scripts under DSM7 seem to be executed with the privilege of the package. The documentation mentions:

Package Framework Changes

1. Force lower privilege for package
All packages should provide conf/privilege with package in run-as explicitly. Any privileged operation should be accomplished via resource worker

I am not able to identify how to use this resource worker to give the script the required privilages in the service_prereplace function and have put some comments above it for the executables which may need a resource worker.

I also did some further testing with the commands commented out but the remainder of the prereplace script does not seem to accomplish its intended function. Specifically, the old configuration files are not migrated.

I don't quite know what should go into the functions themselves. For example, having nzbdrone listed in the install_replace_packages section should remove it but it doesn't as the app is still installed. So should the postreplace contain all the commands to manually remove the package? If you have any guidance here it would be appreciated.

[mreid-tt]

I've also done an upgrade test from 20221216-21 (Sonarr v3) to 20221217-1 (Sonarr v4) to compare DSM 6.2 to DSM 7.1 and found some interesting divergences. Most importantly, under DSM 6.2 the upgrade function actually works and removes the previous version.

Video and logs from DSM 6.2:

Screen.Recording.2022-12-16.at.9.06.50.PM.mp4

/var/log/synopkg.log -- https://pastebin.com/hUPaeFQi
/var/log/packages/sonarr.log -- https://pastebin.com/sntH0f2g

Video and logs from DSM 7.1:

Screen.Recording.2022-12-16.at.9.54.58.PM.mp4

/var/log/synopkg.log -- https://pastebin.com/Gv1BBQU8
/var/log/packages/sonarr.log -- https://pastebin.com/60MehRyv

As you can see, in DSM 6.2 the replace actually works (unlike in DSM 7.1) even though the code is the same. This will need some further investigation. Unfortunately in both situations, the copying of the configuration files to the new version didn't happen so that will need to be looked into more.

I found this interesting tidbit in the /var/log/bash_err.log:

2022-12-16T21:07:16-04:00 DSM62 sh: BASH_ERR: builtin_error [ (16250)"/bin/sh -e /proc/self/fd/9" -> (16252)"/usr/syno/sbin/synopkgctl stop nzbdrone" -> (16254)"/bin/sh /var/packages/nzbdrone/scripts/start-stop-status stop" ] /var/packages/nzbdrone/scripts/start-stop-status:123: kill: (13202) - No such process

This suggested to me that the removal of Sonarr v3 perhaps took place before the prereplace function did its job and I'd need to defer its removal maybe? I'll run some more tests with that concept.

[mreid-tt]

@hgy59, with some further testing I was able to confirm that the logic I wanted to use for the prereplace function was not required since the install_replace_packages field should trigger DSM to uninstall the old package anyway. As such, I've removed all framework code for prereplace and postreplace, only retaining the logic to migrate the old configuration data in the postinst script. I have tested upgrades successfully under DSM6 (log file) and DSM7 (log file) retaining all configuration data.

One challenge remains (mentioned above) regarding the uninstall of the old package under DSM7. I've tried a number of variations including adding the use_deprecated_replace_mechanism field but the uninstall of nzbdrone never triggers. As such, under DSM7, the user will have to manually stop 'Sonarr v3' and then start 'Sonarr v4' to get the new deployment.

This I believe may be a bug in DSM7 itself because I've scoured the internet and can find no discussions on this point. If you have any insight into fixing this I believe that should be the last thing required for this PR.

EDIT: I've also reached out to the community with this support post to hopefully get some assistance.

[mreid-tt]

@hgy59, I opened a ticket with Synology support for the install_replace_packages not working in DSM7 and they came back with a fix. Apparently, in the INFO file, the versioning of the previous version to be replaced was not correct. As such once we changed ...

spksrc/spk/sonarr/Makefile

Line 38 in a160c01

INSTALL_REPLACE_PACKAGES = "nzbdrone>=20210329"

... to ...

spksrc/spk/sonarr/Makefile

Line 38 in 0d9c7eb

INSTALL_REPLACE_PACKAGES = "nzbdrone>=20210329-19"

... this resolved the issue. I've tested it and I believe the package is ready. Once you are comfortable with the changes, I'll squash the commits for you to merge.

[mreid-tt]

I think I've finished all the refinements. The last two included the following:

1. Add a check for the older var location (nzbdrone version 20210329-19 on DSM6)
2. Only set an update check if not performing a Sonarr v3 migration

[mreid-tt]

A question I have is regarding the installation, there is a screen titled "Attention! DSM Permissions" which says:

"Permissions for all download-related packages are managed with the group 'sc-download' in DSM."

Given this is no longer true in DSM7 should we change or remove this message? Or is there a way to have the screen display different information if running DSM7 versus DSM6?

EDIT: I looked at the UI components in Radarr and they seemed fairly up to date so I copied these and changed the app name.

[hgy59]

EDIT: I looked at the UI components in Radarr and they seemed fairly up to date so I copied these and changed the app name.

👍

I suppose for the upgrade wizard we should add information that the package replaces the v3 package (and an extra advice to create a backup first).

It might be necessary to have a different upgrade wizard page when updating from v3 vs updating from v4.

To implement this, we need a shell script for the upgrade_uifile to create different wizard pages - you find example scripts in spk/redis/src/upgrade_uifile.sh or in minio, mosquitto and tt-rss (tt-rss has even different pages for DSM 6 and DSM 7).

[mreid-tt]

It might be necessary to have a different upgrade wizard page when updating from v3 vs updating from v4.

To implement this, we need a shell script for the upgrade_uifile to create different wizard pages - you find example scripts in spk/redis/src/upgrade_uifile.sh or in minio, mosquitto and tt-rss (tt-rss has even different pages for DSM 6 and DSM 7).

Thanks for the guidance. Given that this would be a new package it would be an install_uifile that would be required with some additional logic. I've attempted to implement this and it appears to work under DSM7 however under DSM6, the script does not seem to be executed. I attempted to find why this may be and I think it is because the script files in the generated package do not have the execute bits set compared with the other examples you suggested.

Looking at this package we have:

% tar -tvf sonarr_x64-6.1_20230108-1.spk
[---snip---]
drwxr-xr-x  0 root   root        0 Jan  7 20:58 WIZARD_UIFILES/
-rw-r--r--  0 root   root      579 Jan  7 20:58 WIZARD_UIFILES/upgrade_uifile_fre
-rw-r--r--  0 root   root     4455 Jan  7 20:58 WIZARD_UIFILES/install_uifile.sh
-rw-r--r--  0 root   root     4721 Jan  7 20:58 WIZARD_UIFILES/install_uifile_fre.sh
-rw-r--r--  0 root   root      564 Jan  7 20:58 WIZARD_UIFILES/upgrade_uifile

This is very different than two of the other examples which have the execute bits set:

% tar -tvf redis.v13.f15047\[apollolake-avoton-braswell-broadwell-broadwellnk-broadwellntbap-bromolow-cedarview-denverton-dockerx64-geminilake-grantley-purley-kvmx64-v1000-x86-x86_64\].spk
[---snip---]
drwxr-xr-x  0 root   root        0 Dec 18 18:14 WIZARD_UIFILES/
-rwxr-xr-x  0 root   root      958 Dec 18 18:40 WIZARD_UIFILES/upgrade_uifile.sh
-rw-r--r--  0 0      0        1149 Dec 18 18:40 syno_signature.asc

% tar -tvf tt-rss.v15.f15047\[noarch\].spk 
[---snip---]
drwxr-xr-x  0 root   root        0 Nov 15 15:05 WIZARD_UIFILES/
-rw-r--r--  0 root   root      795 Nov 15 15:52 WIZARD_UIFILES/uninstall_uifile
-rwxr-xr-x  0 root   root     3016 Nov 15 15:52 WIZARD_UIFILES/upgrade_uifile.sh
-rw-r--r--  0 root   root     2106 Nov 15 15:52 WIZARD_UIFILES/install_uifile
-rw-r--r--  0 root   root     2290 Nov 15 15:52 WIZARD_UIFILES/install_uifile_fre
-rwxr-xr-x  0 root   root     2874 Nov 15 15:52 WIZARD_UIFILES/upgrade_uifile_fre.sh
-rw-r--r--  0 root   root      859 Nov 15 15:52 WIZARD_UIFILES/uninstall_uifile_fre
-rw-r--r--  0 0      0        1145 Nov 15 15:53 syno_signature.asc

I've done some light digging around and this setting of the execution bits should happen automatically based on this framework code:

spksrc/mk/spksrc.spk.mk

Lines 394 to 397 in 5efc808

	ifneq ($(wildcard $(DSM_WIZARDS_DIR)/*),)
	@find $(DSM_WIZARDS_DIR) -maxdepth 1 -type f -not -name "*.sh" -print -exec chmod 0644 {} \;
	@find $(DSM_WIZARDS_DIR) -maxdepth 1 -type f -name "*.sh" -print -exec chmod 0755 {} \;
	endif

@hgy59 interested to get your thoughts on the above analysis. Is the reason the uifile scripts are not running under DSM6 because of the file permissions? If so, how can I fix it? If not, what are your thoughts on the problem?

[mreid-tt]

@hgy59 so apparently my analysis was correct. The build checks that are currently performed do not seem to set the execution bits. When I look at the run from "redis: update to v7.0.7 #5526" (Atifacts), the package for x64-6.1 does not have the execution bits set either:

% tar -tvf redis_x64-6.1_7.0.7-13.spk 
[---snip---]
drwxr-xr-x  0 root   root        0 Dec 18 18:05 WIZARD_UIFILES/
-rw-r--r--  0 root   root      958 Dec 18 18:05 WIZARD_UIFILES/upgrade_uifile.sh

So I tried setting the bits manually with the following:

$ sudo -i
# mkdir temp
# tar xvf sonarr_x64-6.1_20230108-1.spk -C temp
[---snip---]
# chmod a+x temp/WIZARD_UIFILES/*.sh
# tar cf temp.spk -C temp $(ls temp/)
# exit

This resulted in an archive with the correct bits:

% tar -tvf temp.spk
[---snip---]
drwxr-xr-x  0 root   root        0 Jan  7 20:58 WIZARD_UIFILES/
-rw-r--r--  0 root   root      579 Jan  7 20:58 WIZARD_UIFILES/upgrade_uifile_fre
-rwxr-xr-x  0 root   root     4455 Jan  7 20:58 WIZARD_UIFILES/install_uifile.sh
-rwxr-xr-x  0 root   root     4721 Jan  7 20:58 WIZARD_UIFILES/install_uifile_fre.sh
-rw-r--r--  0 root   root      564 Jan  7 20:58 WIZARD_UIFILES/upgrade_uifile

With this I was able to successfully test the uifile scrips in DSM6. Assuming the actual published package will have the correct execution bits set, everything looks good to go following your final review.

[bonebox]

Thanks for all your work on this @mreid-tt. How do we get this merged in?

[mreid-tt]

Thanks for all your work on this @mreid-tt. How do we get this merged in?

I can have this merged relatively easily but as this is a new package I would prefer the assistance of a senior dev (i.e. @hgy59 or @publicarray) to review and approve the PR. I would then also need their assistance with the publishing of the package to be made available for download.

[publicarray]

Afaik the data migration/package replacement only worked for DSM7. Did you test this on dsm6?

[publicarray]

Otherwise well done for moving the package to the correct name 👍

[mreid-tt]

Afaik the data migration/package replacement only worked for DSM7. Did you test this on dsm6?

Yes, I tested this under DSM 6 and DSM 7 and both were eventually successful. There was an initial issue with package replacement under DSM 7 but this was because of the version specificity in the INFO file which was corrected. There was a second issue with wizard scripts not running under DSM 6 due to issues with file permissions and this was fixed with #5608.

The main difference with DSM 6 is that because it does a clean uninstall, the previous configuration files are removed on migration (no option to downgrade). In DSM 7 the older configuration files are left in the /var/packages/nzbdrone/var folder which allows you to revert to Sonarr v3 if you wanted to. What do you think about this?

[smaarn]

Regarding the i18n versions of the installers wouldn't be worth using a template mechanism with i18n variables being injected ? (To avoid basically having to maintain separate wizard files which essentially do the same thing except for the text which are differing)

[mreid-tt]

Regarding the i18n versions of the installers wouldn't be worth using a template mechanism with i18n variables being injected ? (To avoid basically having to maintain separate wizard files which essentially do the same thing except for the text which are differing)

I've not used i18n before but looking into it this seems to be a DSM 7 feature (doesn't seem to be present in the DSM 6 developer guide) and related to things like help files and config. I don't see any reference to be able to use this with wizard files. Do you have a specific example of where this could be used?

[smaarn]

I've not used i18n before but looking into it this seems to be a DSM 7 feature (doesn't seem to be present in the DSM 6 developer guide) and related to things like help files and config. I don't see any reference to be able to use this with wizard files. Do you have a specific example of where this could be used?

Actually I was more thinking about using a mechanism such as the one done for rutorrent (which could be extracted if deemed easier to maintain) which can be described in a nutshell as building wizard files by merging the following files:

1. A file specifying i18n variables
2. A base shell file which generates wizards using the variable defined in 1.

TBH I didn't know there was something about it in DSM 7 :D

[mreid-tt]

Actually I was more thinking about using a mechanism such as the one done for rutorrent (which could be extracted if deemed easier to maintain) which can be described in a nutshell as building wizard files by merging the following files:

1. A file specifying i18n variables
2. A base shell file which generates wizards using the variable defined in 1.

An interesting proposal but I don't think this will be easier to maintain. Looking at the developer guide it is expecting the wizard UI files of the form “install_uifile_[DSM language].sh”. If this is the case you will still need to have individual files for each language required. Yes, this can be a shell that points to a base file which then imports the specific internationalised variables for the selected language (perhaps as a parameter) but that is a but too much abstraction in my view.

[th0ma7]

Overall looking quite good. A few proposed changes proposed below.

[mreid-tt]

Overall looking quite good. A few proposed changes proposed below.

Hey @th0ma7, any responses to the feedback on your requested changes?

[AdamEldred]

I did compile and install this over a basic Sonarr v3 setup on DSM7 on a Realtek chip - no issues to report. Sonarr seems to self upgrade just fine after install as well. Nice work!

Relevant changes have been addressed

[mreid-tt]

Hey @th0ma7, my review of the suggested changes show that the important ones were addressed (including updating the Wiki) and I've gone ahead an merged the PR. Do let me know if you can also assist with publishing.

[th0ma7]

Overall looked good, nice work.
Publishing in progress: https://github.com/th0ma7/spksrc/actions/runs/4602153928

EDIT: Now published.

[mreid-tt]

Excellent! Thanks much. I manually installed from the repo on my DS916+ and all went smoothly. As expected, I had to change the permissions allowed on the download and video folders with the change of user from sc-nzbdrone to sc-sonarr.

Also because I am a stickler for file ownership I also did this:

find "/volume1/video/TV Series" -user sc-nzbdrone -exec sudo chown sc-sonarr {} \;

[bonebox]

Worked great on a DS920+ running DSM 7.1.1. Thank you!

[HansGr00ber]

Will there be an armada38x (DS216j) version for DSM6? Thanks

[mreid-tt]

Will there be an armada38x (DS216j) version for DSM6? Thanks

Based on the testing of dotnet *arr packages with ARMv7 archs we did (see #5574), the armada38x platform is only supported with DSM 7.1. DSM 6.x does not have the required libraries to run dotnet 6.x.

[tupac55]

Thanks for the update everything went fine updating on my ds220+