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

Generic service support for DSM 5 and 6 #2949

Merged

Conversation

ymartin59
Copy link
Contributor

  • Generic installer and start-stop-status scripts
  • User creation and run-as privilege file
  • Share folder creation with group permissions
  • Target both DSM 5 and 6

Based on BenjV proposal #2904

@m4tt075
Copy link
Contributor

m4tt075 commented Oct 14, 2017

@ymartin59 : Thanks for getting this going. I have cherry-picked your commit and started playing around with your demoservice. Stumbled pretty much at the doorstep though. Currently, cross-compiling fails with...

===> Generating service scripts for demoservice
/bin/sh: 1: cannot create /service-setup: Permission denied
../../mk/spksrc.service.mk:44: recipe for target '/service-setup' failed
make[1]: *** [/service-setup] Error 2
make[1]: Leaving directory '/home/walter/builds/spksrc/spk/demoservice'
../../mk/spksrc.spk.mk:423: recipe for target 'arch-evansport-6.1' failed
make: [arch-evansport-6.1] Error 2 (ignored)

But you probably know this...

@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from e970ab4 to 6939463 Compare October 21, 2017 08:20
@ymartin59
Copy link
Contributor Author

@m4tt075 This version is now building properly... but generated scripts still need intensive testing...

@m4tt075
Copy link
Contributor

m4tt075 commented Oct 21, 2017

@ymartin59 Cheers! OK, I will play around with your demoservice and then try to apply your approach to the TVH package. I have got an XPenology 5.2 and an evansport-6.1 environment for testing. Will report back with the results.

@m4tt075
Copy link
Contributor

m4tt075 commented Oct 22, 2017

@ymartin59 Some progress!

  1. Cross-compilation goes through smoothly now
  2. First tests on XPE 5.2 (basically clean, only TVH and nano packages installed):
  • Manual installation correctly opens up the WebUI Interface (spk seems to be stucturally intact!)
  • I just accepted all defaults (/volume1/downloads, despite non-existent, and sc-download, not being existent) and proceeded through the screens
  • After confirming the last screen (leaving "Run after installation" checked) the actual installation fails with something like "Demoservice could not be installed" (no further notice)
  • I manually created the /volume1/downloads directory and repeated the above, including variations of checking / unchecking "Run after installtion", but same result, the installation fails
  1. Diagnostics so far (via ssh to the XPE system):
  • No log file in the /tmp file
  • No package-related files in /usr/local/
  • No new user created in DSM or in /etc/shadow
  • No new user group created in DSM or in /etc/group

This is how far I got this time. Anything (else) you want me to check or look for?

@m4tt075
Copy link
Contributor

m4tt075 commented Oct 23, 2017

@ymartin59 Repeated the above test on my DSM 6.1 system. Same result / failure notice. No log file, no further hints why it fails.

I then unpacked the spk which was generated and realized that there is no conf folder and that the privilege file is missing. Should not be an issue with 5.2, but might well be one with the 6.1 system.

@ymartin59
Copy link
Contributor Author

@m4tt075 Thanks for testing. I have fixed conf/privilege generation and I am debugging installer scripts. It should work soon.

@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from 6939463 to 9a4a416 Compare October 30, 2017 06:12
@ymartin59
Copy link
Contributor Author

@m4tt075 Some progress... still buggy features but install and log activity

@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from 9a4a416 to cb0b280 Compare October 31, 2017 20:33
@ymartin59
Copy link
Contributor Author

For the moment, installer script faced the following bug on DSM 5.2: servicetool --remove-configure-file --package seems to succeed but without effect, as reinstalling again package fails with message claiming port is already reserved for another application !

I have not testing on DSM 6.x yet

@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from cb0b280 to f0d4220 Compare November 4, 2017 09:44
@ymartin59
Copy link
Contributor Author

Tested on DSM 5.2 and works properly except uninstall do not remove service configuration, leaving port marked as used.

@m4tt075
Copy link
Contributor

m4tt075 commented Nov 4, 2017

@ymartin59 Great stuff! I can confirm what you have written. Here some more detailed observations from my testing process:

  1. Cross-compilation runs smoothly
  2. First-time installation on XPE 5.2 (basically clean, only TVH and nano packages installed):
  • Manual installation correctly opens up the WebUI Interface
  • I just accepted all defaults (/volume1/downloads, despite non-existent, and sc-download, not being existent) and proceeded through the screens
  • Installation finishes successfully
  • Running the demoservice correctly opens up the package folder and gives me access to the relevant files. Beautiful!
  • The downloads folder is created with root:users ownership.
  • demouser appears as user via DSM (this is still 5.2 after all)
  • sc-downloads appears as group via DSM (as above)
  • Manual checking of /etc/passwd, /etc/shadow and /etc/group: Everything looks as it should be
  • The demoservice.sc file is correctly installed into /usr/local/etc/services.d/
  1. Uninstalling the package
  • Finishes cleanly
  • Demoservice user is correctly removed
  • sc-downloads group is correctly removed
  • The demoservice.sc file incorrectly survives in /usr/local/etc/services.d/, which prevents a re-installation, as you have been describing.
  • Manually removing the sc-file from the above directory works around the problem.
  1. I repeated step 1) with different (non-default) group name and download directory. Result as above. Everything working as intended.

  2. Updating the package (simulated by simply manually re-installing the package without uninstalling it first)

  • WebUI confirms that the package has been updated successfully
  • However, afterwards the package cannot be executed anymore
  • Checking why, I found that the demoservice user and sc-download group are gone. They are removed, but not re-created. Consequently, the permissions of the package folders are screwed up as well.
  • I think this is just because something goes wrong with the sequence of executing commands during the update process.
  1. Just an observation: The shared folders that are created survive uninstalling the packages, but I assume this is intended behavior.

  2. I manually played around with ´servicetool´ command to figure out what goes wrong. I figured out that it fails if you specify the sc-file including it's path. However, it works if you just skip the path. Correct usage is /usr/syno/bin/servicetool --remove-configure-file --package demoservice.sc. This might be the culprit...

Hope this helps. Again, many thanks for taking this on!

@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from f0d4220 to 68a87c9 Compare November 5, 2017 10:24
@ymartin59
Copy link
Contributor Author

@m4tt075 Many thanks for your testing. I fixed upgrade process and service configuration handling (with DSM 6.x in mind)
You're right about observation "4.", it is intended not to delete share folder.
I now have to adapt and test for DSM 6.x

@ymartin59 ymartin59 changed the title WIP DRAFT generic service support for DSM 6 WIP Generic service support for DSM 5.2 and 6.x Nov 5, 2017
@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch 2 times, most recently from 4d0a586 to 7a44c5d Compare November 12, 2017 17:18
@ymartin59 ymartin59 changed the title WIP Generic service support for DSM 5.2 and 6.x Generic service support for DSM 5.2 and 6.x Nov 12, 2017
@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from 8b7dc7b to 0e1aade Compare November 18, 2017 16:41
@ymartin59
Copy link
Contributor Author

@m4tt075 You probably have an old branch version script. I have fixed FWPORTS support, it should work now.

@m4tt075
Copy link
Contributor

m4tt075 commented Nov 22, 2017

@ymartin59 I can't get the propiretary FWPORTS definition to work. Could you please take a look at the spk/Makefile I'm using? Here it is:

SPK_NAME    = tvheadend
SPK_VERS    = 4.2.4
SPK_REV     = 12
SPK_ICON    = src/tvheadend.png
DSM_UI_DIR  = app
BETA        = 1

# DEPENDS = cross/busybox cross/$(SPK_NAME)
DEPENDS = cross/$(SPK_NAME)

MAINTAINER   = m4tt075
DESCRIPTION  = Tvheadend is a TV streaming server and recorder for Linux, FreeBSD and Android supporting DVB-S, DVB-S2, DVB-C, DVB-T, ATSC, ISDB-T, IPTV, SAT IP and HDHomeRun as input sources. Tvheadend offers HTTP, HTSP and SAT IP streaming.
# ADMIN_PORT = 9981
RELOAD_UI    = yes
DISPLAY_NAME = Tvheadend
CHANGELOG    = "Update for full DSM6 compliance"
HOMEPAGE     = https://www.lonelycoder.com/tvheadend/
LICENSE      = GPL v3

WIZARDS_DIR  = src/wizard/
CONF_DIR     = src/conf/
FWPORTS      = src/${SPK_NAME}.sc

# 'auto' reserved value grabs SPK_NAME
SERVICE_USER         = auto
SERVICE_WIZARD_GROUP = wizard_group
SERVICE_WIZARD_SHARE = wizard_download_dir
SERVICE_SETUP        = src/service-setup.sh
# Using standard ports 9981 for http and 9982 for htsp. Adapt SERVICE_COMMAND below if needed
SERVICE_COMMAND      = '$$\{SYNOPKG_PKGDEST\}/bin/tvheadend.sh 9981 9982 $$\{PID_FILE\}'

# INSTALLER_SCRIPT = src/installer.sh
# SSS_SCRIPT       = src/dsm-control.sh

# INSTALL_PREFIX = /usr/local/$(SPK_NAME)

PRE_COPY_TARGET = tvheadend_install

# BUSYBOX_CONFIG = usrmng
# ENV += BUSYBOX_CONFIG="$(BUSYBOX_CONFIG)"

include ../../mk/spksrc.spk.mk

.PHONY: tvheadend_install
# Replace standard copy/install targets
tvheadend_install:
	rm -fr $(STAGING_DIR)
	mkdir $(STAGING_DIR)
	mkdir --parents $(STAGING_INSTALL_PREFIX)
	install -m 755 -d $(STAGING_INSTALL_PREFIX)/bin
	install -m 755 src/tvheadend.sh $(STAGING_INSTALL_PREFIX)/bin
	install -m 755 -d $(STAGING_DIR)/app
	install -m 644 src/app/config $(STAGING_DIR)/app/config
	install -m 755 -d $(STAGING_DIR)/var
	install -m 755 -d $(STAGING_DIR)/var/accesscontrol
	install -m 755 -d $(STAGING_DIR)/var/passwd
	# The following filenames stem from an arbitrary (valid) combination created by TVH
	install -m 644 src/accesscontrol.json $(STAGING_DIR)/var/accesscontrol/d80ccc09630261ffdcae1497a690acc8
	install -m 644 src/passwd.json $(STAGING_DIR)/var/passwd/a927e30a755504f9784f23a4efac5109

What am I doing wrong?

@ymartin59
Copy link
Contributor Author

I guess it does not work because of commented ADMIN_PORT. My fault...

@m4tt075
Copy link
Contributor

m4tt075 commented Nov 23, 2017

@ymartin59 Thanks, and no worries. For every mistake you make, I make 20. I still have not been able to get the FWPORTS running properly, but I have done some further testing. Here some more observations:

  1. Before I install anything, I always extract the spks on my Windows laptop, to compare the integrity of the package itself. Whenever I do this, 7-zip asks me whether I would like to overwrite the files in the conf folder with new ones. The "old" ones contain content, the "new" ones don't (0 kb file size), so I negate the question. Not sure what this means, but wanted to share...
  2. XPE 5.2: I tried to upgrade an old standard TVH package, which was installed via the busybox method. The upgrade seemed to work. But uninstalling the package afterwards did not, as the tvheadend user could not be removed properly. I figured out that users, who were installed via the busybox method cannot be removed via the new approach. I had to reinstall an old package to get the commands, before I was able to clean the system again. I believe we have to keep the busybox dependency, at least initially, in order to properly remove legacy users and group.
  3. XPE 5.2: A clean installation seems to work well in terms of user / group creation and permissions.

For transparency, I have not yet been able to run my TVH package, but that's likely related to the 20 mistakes I introduce while you make 1...

@m4tt075
Copy link
Contributor

m4tt075 commented Nov 24, 2017

@ymartin59 I finally managed to get FWPORTS working. The only thing needed was to move the tvheadend.sc file from the src to the conf folder. Now it is picked up correctly. Most files in my package are installed correctly now. Apparently the FWPORTS variable in the spk/Makefile does not matter anymore.

Two more questions though:

  • On a 5.2 system, is the launch command specified in the SERVICE_COMMAND executed as root or as [username] as defied in the package?
  • Should the actual command running the service (as demon for TVH) always be encapsulated in a [username].sh script (also to generate the pid-file) or should it rather be inserted as SERVICE_COMMAND itself?

Background: I have tried the encapsulated version following your demoservice example, but ran into issues with lacking access rights to certain configuration folders. This is all still on a XPE 5.2 system.

@ymartin59
Copy link
Contributor Author

@m4tt075 On DSM 5.2 installer script chowns only var folder. If your code requires write access to other location, you have to do chown in postinstall script (ideally only on DSM 5)
Yes, SERVICE_COMMAND process runs as SERVICE_USER because of su command on DSM 5. https://github.com/SynoCommunity/spksrc/pull/2949/files#diff-9fe71615984368bf44c02ed52c3b1fceR26
DSM 5 does not understand conf/privilege file, which is only for DSM 6.
If SERVICE_COMMAND process is able to produce PID_FILE, then there is no need for a shell script like demoservice.sh I have only written for demonstration purpose.

@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from 0e1aade to 1f9010b Compare November 25, 2017 17:15
@ymartin59
Copy link
Contributor Author

@m4tt075 I just fixed FWPORTS with your specific situation: CONF_DIR is set and ADMIN_PORT is not. I tested with src/demo.sc and it gets copied as conf/demoservice.sc as expected: https://github.com/SynoCommunity/spksrc/pull/2949/files#diff-c265147645e98b75c07672b914840285R141

@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from 1f9010b to 50b92ed Compare December 2, 2017 13:48
@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from 50b92ed to b02877f Compare December 2, 2017 13:54
@Safihre
Copy link
Contributor

Safihre commented Dec 3, 2017

So this is ready and @BenjV also likes it?
I can try and adapt the SABnzbd package for it?

@BenjV
Copy link

BenjV commented Dec 3, 2017

@ymartin59 and @m4tt075
As far I can see at the moment you guys did a great job.

@Safihre
If you create a SABnzbd package I will happly review and test it because it is a noarch package and can be installed on any Synology Nas.

@Safihre
Copy link
Contributor

Safihre commented Dec 3, 2017

@BenjV Not really noarch due to the compiled components unrar, par2cmdline and Python C-extension sabyenc. But that's off-topic, I'll get started on it 👍

@ymartin59
Copy link
Contributor Author

For information, I have plan to disjoin ADMIN_PORT and an additional SERVICE_PORT to generate service configuration (.sc) for firewall. (found it relevant when processing mosquitto)

- Generic installer and start-stop-status scripts with logging
- Service configuration generation for tcp port
- User creation and run-as privilege file
- Share folder creation with group permissions
- Support for non service application package (non startable)
- Target both DSM 5 and 6
- demoservice package as usage example

Based on BenjV proposal SynoCommunity#2904
@ymartin59 ymartin59 force-pushed the dsm6-2904-spk-generic-service branch from b02877f to abc88bf Compare December 3, 2017 14:28
When service protocol is not browsable, this switch allow
to disable UI shortcut icon generation for service.
@ymartin59
Copy link
Contributor Author

Thanks for this good team work... I think we are on the right path to get easier job with all these packages maintenance.
I have provided reference documentation on wiki with a (draft) migration guide:

As a first example, I have published two PRs for Mosquitto package migration: #3025 (master) and #3028 (dsm6)

@Safihre
Copy link
Contributor

Safihre commented Dec 3, 2017

@ymartin59 Thanks!
The Service user transition part seems a bit of a problem to me. What if a user doesn't get this intermediate package but goes straight from an old package to the then published newer package?
There is no way to do this in 1 go? From @BenjV comment here I understand that just using the syno commands to remove the old busybox user/group should work, even if it errors?

@ymartin59
Copy link
Contributor Author

No the right place for such a question. See #2904 (comment)

@ymartin59 ymartin59 merged commit 05d8159 into SynoCommunity:dsm6 Dec 4, 2017
@ymartin59 ymartin59 deleted the dsm6-2904-spk-generic-service branch February 4, 2018 11:00
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.

5 participants