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

Sonarr: (dotNET) v4 build > Moved to #5524 #5520

Closed
wants to merge 0 commits into from

Conversation

mreid-tt
Copy link
Contributor

@mreid-tt mreid-tt commented Dec 12, 2022

Description

This is an attempt to deploy a .NET build (v4) of Sonarr for Synology. As this is a major change, the package internal name has also changed from nzbdrone to sonarr. Unfortunately, this means that an upgrade from the previous deployment is not possible as the package name is no longer the same. If you try to deploy it, a conflict with the port used from the old package will be flagged. As such to test this deployment you should:

  1. Uninstall the old Sonarr (but keep existing files)
  2. Uninstall the SynoCommunity Mono
  3. Install this new Sonarr
  4. The old config files (if detected) will be migrated to the new deployment

Fixes #5450

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
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

@mreid-tt mreid-tt changed the title [WIPSonarr: (dotNET) v4 build [WIP] Sonarr: (dotNET) v4 build Dec 12, 2022
@mreid-tt mreid-tt force-pushed the sonarr-v4-widowmaker branch 4 times, most recently from ff4df00 to 0e8c107 Compare December 13, 2022 05:13
@mreid-tt
Copy link
Contributor Author

@bakerboy448, this early test build was inspired by your comment:

Sonarr on .Net also already exists for internal testing. https://github.com/Sonarr/Sonarr/tree/widowmaker

Additionally, based on this recommendation:

For this SSL issue it seems that SynoCommunity's mono package has a bug and fails to sync certs upon install. This should be corrected as soon as it can given all the support issues it's causing.

There has been no apparent success based on #5070

As such, this rough test build was created to determine the feasibility of using the early release of Sonarr on dotNET to resolve these issues.

@bakerboy448
Copy link

Widowmaker is already not supported

The sonarr develop branch is v4 beta.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Dec 13, 2022

The sonarr develop branch is v4 beta.

Thanks for the heads up. The PR has been updated with this branch. Interested to hear your thoughts on the PR.

spk/sonarr/Makefile Outdated Show resolved Hide resolved
cross/sonarr/Makefile Outdated Show resolved Hide resolved
@hgy59
Copy link
Contributor

hgy59 commented Dec 13, 2022

To continue supporting Sonarr v3 with mono dependency (for archs that do not support dotnet core), we need to keep (and rename) existing cross/sonarr/* and spk/sonarr/* to cross/nzbdrone/* and spk/nzbdrone/* (or use sonarr3 as name for former package, but internally it is called nzbdrone).

BTW:
We must not forget to adjust the specific nzbdrone/sonarr handling in .github/actions/prepare.sh.

👍 👍 👍 for the implementation of package replacement ❗

spk/sonarr/Makefile Outdated Show resolved Hide resolved
spk/sonarr/Makefile Outdated Show resolved Hide resolved
@mreid-tt mreid-tt force-pushed the sonarr-v4-widowmaker branch 3 times, most recently from d50e725 to c1ea603 Compare December 13, 2022 13:31
@mreid-tt mreid-tt force-pushed the sonarr-v4-widowmaker branch from c1ea603 to cadb0e0 Compare December 13, 2022 13:42
spk/sonarr/Makefile Outdated Show resolved Hide resolved
@mreid-tt mreid-tt force-pushed the sonarr-v4-widowmaker branch from cadb0e0 to b481a05 Compare December 13, 2022 13:49
@bakerboy448
Copy link

To continue supporting Sonarr v3 with mono dependency (for archs that do not support dotnet core), we need to keep (and rename) existing cross/sonarr/* and spk/sonarr/* to cross/nzbdrone/* and spk/nzbdrone/* (or use sonarr3 as name for former package, but internally it is called nzbdrone).

BTW:

We must not forget to adjust the specific nzbdrone/sonarr handling in .github/actions/prepare.sh.

👍 👍 👍 for the implementation of package replacement ❗

Given nzbdrone is the legacy no longer supported v2. I'd suggest having v3 as Sonarr3. And the most recent Sonarr version (beta or not) as Sonarr.

Also Sonarr v4 should very clearly indicate it is BETA and not stable.

@hgy59
Copy link
Contributor

hgy59 commented Dec 13, 2022

@mreid-tt just a small suggestion while you are working...
I would like to keep the former icon for the sonarr3 package. This gives (a small) visual difference between the two packages.

Further suggestion:
Maybe we first need to publish an update of the sonarr3 package

  • everybody seeds the new display name for the current package before the package replacement
  • The new display name is used on the installed package (and not only in the package repo)
  • we can use such a release for package replacement

@bakerboy448
Copy link

@mreid-tt just a small suggestion while you are working...

I would like to keep the former icon for the sonarr3 package. This gives (a small) visual difference between the two packages.

Further suggestion:

Maybe we first need to publish an update of the sonarr3 package

  • everybody seeds the new display name for the current package before the package replacement

  • The new display name is used on the installed package (and not only in the package repo)

  • we can use such a release for package replacement

Sonarr v3 is the official stable version. Sonarr v4 is still in beta and not released as stable. No package replacement should be occurring until v4 is stable.

Either users can run the v3 stable Sonarr package. Or test the v4 beta. But the v4 beta should not be forced as a replacement currently.

@hgy59
Copy link
Contributor

hgy59 commented Dec 13, 2022

Also Sonarr v4 should very clearly indicate it is BETA and not stable.

@bakerboy448 is there any schedule when Sonarr v4 will get stable?

  • we should not publish v4 before it is stable
  • we should not replace a stable v3 by a prerelease v4

@mreid-tt
Copy link
Contributor Author

@mreid-tt just a small suggestion while you are working... I would like to keep the former icon for the sonarr3 package. This gives (a small) visual difference between the two packages.

Sure, that's not an issue, but I will use the larger 512x512px size to be consistent with the other packages.

Further suggestion: Maybe we first need to publish an update of the sonarr3 package

  • everybody seeds the new display name for the current package before the package replacement
  • The new display name is used on the installed package (and not only in the package repo)
  • we can use such a release for package replacement

Yes, this would be a good idea. Would you want me to implement the change of the external package name from sonarr to sonarr3 into #5515 and you can move that to be published? Then I would re-base this PR.

@bakerboy448
Copy link

bakerboy448 commented Dec 13, 2022

As always with the *arrs there's never any ETA for a new release.

As a reminder/context - sonarr v3 was in "beta" (and working better than v2) for several years before it was finally released as stable and replaced nzbdrone (sonarr v2)

@mreid-tt mreid-tt force-pushed the sonarr-v4-widowmaker branch 3 times, most recently from 98b3779 to 5036dd0 Compare December 14, 2022 03:00
@mreid-tt
Copy link
Contributor Author

hey @hgy59, I was experimenting with adjusting the port numbers of existing deployments as commented on here: https://github.com/SynoCommunity/spksrc/pull/3803/files#r596284050. Unfortunately, when the package builds, I don't get the correct service-setup file being generated in the package. It seems to be pulling from the old spk/sonarr/src/service-setup.sh code before it was moved to spk/sonarr3/src/service-setup.sh.

I don't know if this is the way GitHub is setup or that the changes in #5515 need to be merged into the master first before I can get a new package generated under the old name? Can't seem to continue development testing until this is resolved.

@hgy59
Copy link
Contributor

hgy59 commented Dec 14, 2022

@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.

Further Opinions

  • we should not only replace sonarr v3 by v4 for DSM 7 but also for DSM 6
  • for all arch/dsm version combinations that successfully build Sonarr v4 we should add the package replacement.
  • Only DSM 5.x will stay on Sonarr v3
  • 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.
  • SRM 1.x is compatible with DSM 6 installer, but we have to approve this, before deploying packages with replacement of Sonarr v3.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Dec 14, 2022

@hgy59, from my testing, each time I try to install a package where the port number is not changed, the DSM reports the conflict and the installation cannot proceed. Now from what I read in the documentation about prereplace, this is a new feature that is described as:

It can be used to do data migration when install_replace_packages is defined in INFO for
package replacement. Package replacement will be aborted for non-zero returned value.

Unfortunately, when I look at the installation log, I don't see any evidence that the prereplace function is called. Strangely enough on my last test there is a log entry for postreplace. When I decompressed the package for DSM7, this is where I noted that the prereplace function is not present in the included service-setup file. As such, I don't really know if I need to force a port change or if the prereplace function will permit a package removal before it detects a conflict. Thus, I am at a loss of how to proceed.

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 add it once I have successfully tested a package replace under DSM7 (I don't have an active DSM6 deployment so I'll have to spend time setting one up). Given that in the DSM 6.0 documentation, the prereplace and postreplace functions don't exist I don't know if these will be called at any point during the deployment. As such some workaround (like the alternate port install and port swap I've been tinkering with) would likely be needed there. Otherwise, users would have to manually remove their Sonarr v3 and then have the deployment pick up the config files left behind. This is what I've been trying to avoid but we'll see how the testing goes once I am able to resume.

  • 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. The only adjustment I thought would be needed would be to add DSM6 support.

  • 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.

Ah, if the prereplace and postreplace functions are recognised in DSM 6.1 then this should be straightforward to test. 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. Would this be the reason for the packager not including the correct service-setup.sh content?

EDIT: 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. I'm not exactly sure what toolchains are but they seem to be related to which architectures we can build packages for.

@mreid-tt
Copy link
Contributor Author

@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 we'll see how far that gets us.

@mreid-tt
Copy link
Contributor Author

@hgy59 So the package build worked correctly this time and the contents of the package file service-setup was consistent with the content from the revised service-setup.sh. I was also able to confirm that the package installed without flagging a conflicting port with the addition of the checkport code however the prereplace code still did not fire (see installation log: https://pastebin.com/6nRLNBKb). I need to get this resolved somehow.

Also, 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 mreid-tt force-pushed the sonarr-v4-widowmaker branch 7 times, most recently from 3b9f0c5 to e13d4d6 Compare December 15, 2022 07:36
@mreid-tt
Copy link
Contributor Author

hey @hgy59, so I did some further checks and found that the prereplace was in fact being run but appeared 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:

synopkg stop nzbdrone >/dev/null 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 as a reminder that this needs resolution.

Hopefully you can shed some light on how to resolve this.

@mreid-tt mreid-tt closed this Dec 16, 2022
@mreid-tt mreid-tt force-pushed the sonarr-v4-widowmaker branch from 6e8f59c to 9addc79 Compare December 16, 2022 19:10
@mreid-tt mreid-tt changed the title [WIP] Sonarr: (dotNET) v4 build Sonarr: (dotNET) v4 build > Moved to #5524 Dec 16, 2022
@mreid-tt mreid-tt mentioned this pull request Dec 16, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sonarr uninstall not removing 'appdata' when erasing all data files
3 participants