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

Add InfluxDB as a package #5900

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

brakenium
Copy link

@brakenium brakenium commented Sep 24, 2023

Description

This PR adds InfluxDB as a new package. This is not a critical application for me personally, so I doubt I'll stay on updating to new versions unless I somehow notice a new version is out. Currently this does not work for armv7 since I am using the pre-compiled binaries (amd64 and arm64 only). I have tried compiling from source (for a different reason), but couldn't get it to work.

I tried figuring out the two logging checks when testing package functionality, but couldn't exactly figure it out. So I'll need some pointers for that. Logs do exist at: /var/packages/influxdb/var/influxdb.log, but not at /var/packages/influxdb/target/var/. The other tests were successful.
Not sure if any documentation is necessary since InfluxDB's own documentation should apply, so I'll keep that open for now.

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)

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

@brakenium thanks for this contribution.

IMHO we should compile influxdb and influxdb-client from source, to not only support x64 and aarch64 CPU models.

And I am not sure, whether prebuilt linux binaries will run on DSM 6 and DSM 7.

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

@brakenium we already have #3709 for influxdb 1

And we have the request #5406.
I already commented there...

  • InfluxDB 2 is supported on 64 bit archs only (so you are right to include x64 and aarch64 only)
  • on x64 you can use influxdb with docker
  • since Synology added docker to the newer aarch64 models, you can use influxdb with docker on DSM 7

So IMHO there is no benefit to have influxdb as package from synocommunity.

@brakenium
Copy link
Author

brakenium commented Sep 24, 2023

Regarding your first message:
Thank you for your reply. The pre-compiled linux binaries run on my DS218 (arm64 binary) with DSM7. I would need some help with compiling these binaries as this is my first time working with make and building go programs. I am also not sure how much more time I can put into this. If it's fairly easy with some help I'll probably have some time to do it.

From what I could see the Makefile from InfluxDB itself does a go build ./cmd/influxd with some extra flags, but I couldn't get it to build from the Docker container for spksrc.

Regarding your second message:
Thanks for pointing me to these issues, I did search beforehand but couldn't find anything. From what I can tell my DS218 (rtd1296?) does not support any official Docker implementation, but if I missed something and it does I'll gladly use it! So assuming Docker does not work for some devices, I think there is still a benefit for this package

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

Regarding your second message: Thanks for pointing me to these issues, I did search beforehand but couldn't find anything. From what I can tell my DS218 (rtd1296?) does not support any official Docker implementation, but if I missed something and it does I'll gladly use it! So assuming Docker does not work for some devices, I think there is still a benefit for this package

Yes, as noted in #5667, docker support for aarch64 is limitted to DSM 7.2 and x20 Series and newer.

I have a DS218 too, that I keep currently on DSM 6.2.4. will give it a try.

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

I tried figuring out the two logging checks when testing package functionality, but couldn't exactly figure it out. So I'll need some pointers for that. Logs do exist at: /var/packages/influxdb/var/influxdb.log, but not at /var/packages/influxdb/target/var/. The other tests were successful.

You can ignore the /var/packages/<pkg_name>/target/var/ folder. It exists on DSM < 7 only.
You can always use /var/packages/<pkg_name>/var/ folder on all DSM versions. (for DSM < 7 our generic installer creates a symbolic link for it).

BTW, yesterday I updated the wiki page that documents Makefile variables to add such details to SYNOPKG_PKGVAR in the section of Available Variables in src/service-setup.sh.

@brakenium
Copy link
Author

You can ignore the /var/packages/<pkg_name>/target/var/ folder. It exists on DSM < 7 only. You can always use /var/packages/<pkg_name>/var/ folder on all DSM versions. (for DSM < 7 our generic installer creates a symbolic link for it).

BTW, yesterday I updated the wiki page that documents Makefile variables to add such details to SYNOPKG_PKGVAR in the section of Available Variables in src/service-setup.sh.

That leaves 'Check "View log"' then. How would one do this? I can't find anything relating to 3rd party packages in log center or support center

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

hgy59 commented Sep 24, 2023

@brakenium i have got it running 😄

There are some options that must be configured in a config file that must be installed with the package:

Some paths are located under /var/packages/influxdb/target/.influxdbv2/
I suppose this must be changed to /var/packages/influxdb/var/.influxdbv2/. Otherwise those data are lost on package updates.

@brakenium
Copy link
Author

brakenium commented Sep 24, 2023

That's great! InfluxDB is configured either through the web UI (which is what I did) or through a command. I guess a wizard that runs the command afterwards could work, but it seems counterproductive to me since the web UI already offers this. The wizards did seem fairly daunting to me at first, so that may be out of scope for me to do.

On DSM7.2 when updating the package I haven't lost any data or are you talking about DSM updates? I also don't see the path you are describing on my system. The .influxdbv2 directory is located at /var/packages/influxdb/home/.influxdbv2 instead. I am guessing saving this directory to SYNOPKG_PKGVAR is sufficient?

EDIT: on further inspection the wizard does not seem to bad, I'll give it a go

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

EDIT: on further inspection the wizard does not seem to bad, I'll give it a go

So far I do not see a requirement for a wizard, i.e. for the user to provide configuration parameters at installation time.
All can be configured in the influx web UI.

The package home directory is not available on DSM < 7. So using the var folder is a solution that works on all DSM versions.

We definitifly need a config file for the custom paths.

what about this?
config.yml to be installed to var/config.yml

bolt-path: /var/packages/influxdb/var/.influxdbv2/influxd.bolt
engine-path: /var/packages/influxdb/var/.influxdbv2/engine
sqlite-path: /var/packages/influxdb/var/.influxdbv2/influxd.sqlite

and an appropriate service-setup.sh looks like this:

INFLUXD_CONFIG_PATH=$(SYNOPKG_PKGVAR)/config.yml
INFLUXD=${SYNOPKG_PKGDEST}/bin/influxd

export INFLUXD_CONFIG_PATH=${INFLUXD_CONFIG_PATH}

SERVICE_COMMAND="${INFLUXD} run --http-bind-address :${SERVICE_PORT}"
SVC_BACKGROUND=yes
SVC_WRITE_PID=yes
SVC_CWD="${SYNOPKG_PKGVAR}"

BTW I miss the influxd parameters --pid-file and --config-file of influxdb1.

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

oops, my comment regarding the wizard was 5 min. too late...

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

I prefer the web UI for influxdb configuration over an installation wizard.
Or how can you grab the superuser token with a wizard?

@brakenium
Copy link
Author

I thought I had to do a wizard to set the port, but I was mistaken. I just left it in my last commit since I made it anyway. Will probably end up taking it out since that is what I prefer as well

@brakenium
Copy link
Author

The token will be saved locally, but you should be able to create a new one with the credentials you fill in?

@hgy59
Copy link
Contributor

hgy59 commented Sep 24, 2023

I thought I had to do a wizard to set the port, but I was mistaken. I just left it in my last commit since I made it anyway. Will probably end up taking it out since that is what I prefer as well

The port must be hard coded and cannot defined by the user at installation time
8085 is also a bad option (it was used by an xdm package - we have wiki pages to document the used ports by synology and by synocommunity packages.

I propose to use port 18086 or 8186.
The chosen port must be documented on our wiki page to avoid conflicts with other packages.

@brakenium
Copy link
Author

The port should now be corrected and hard coded. I grepped the makefiles for port 8085, but didn't find any that used it

@osmeest
Copy link

osmeest commented Oct 30, 2023

Hi,
I'm happy to see that someone already took care of packaging influxdb. Is there anything I can do to help you (eg. testing) ?
I've a ds218play, experience with c++ programs on linux, but nothing with regards to "go" or DSM cross compilations.

@brakenium
Copy link
Author

You can help testing. However I personally haven't had the time nor desire the last few weeks to work on this. Though it might be ready to be merged? Can't remember

@brakenium
Copy link
Author

@hgy59 Do you have time to look at this package again? After my education sucked me in I finally found some time again. I think I've some final adjustments today.
Do I have to create any documentation or do anything else for this to get merged?

Comment on lines 11 to 21
service_postinst ()
{
cat << EOF > "${INFLUXD_CONFIG_PATH}"
http-bind-address: ":${SERVICE_PORT}"
reporting-disabled: true
bolt-path: "${SYNOPKG_PKGVAR}/.influxdbv2/influxd.bolt"
engine-path: "${SYNOPKG_PKGVAR}/.influxdbv2/engine"
sqlite-path: "${SYNOPKG_PKGVAR}/.influxdbv2/influxd.sqlite"

EOF
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This config is now static.
I highly recommend to add the config file to the package in the var folder.
This way it will automatically be installed.
Plus the config file in the package will not overwrite the config on package updates.

spk/influxdb/src/config.yml Outdated Show resolved Hide resolved
@osmeest
Copy link

osmeest commented Dec 28, 2023

I know it is a bit off topic or too generalistic but... how do you test your packages ?
I tried building a VirtualBox VM to run DSM 7 but I couldn't get it to work (I'm stuck in setting it up with DSM).
Any good guide would help me testing packages that are in development.
Thanks.

@brakenium
Copy link
Author

I'm testing it on my NAS

@brakenium
Copy link
Author

The config should now be static

@wkobiela
Copy link
Contributor

I know it is a bit off topic or too generalistic but... how do you test your packages ? I tried building a VirtualBox VM to run DSM 7 but I couldn't get it to work (I'm stuck in setting it up with DSM). Any good guide would help me testing packages that are in development. Thanks.

You can use https://github.com/fbelavenuto/arpl to setup virtual dsm

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.

4 participants