feat(nextcloud): add notify_push support
Pull Request
Description of the change
Not yet tested
Benefits
- support notify_push
- improve ServiceMonitor (to collect data from nextcloud-exporter and notify_push)
Possible drawbacks
Applicable issues
- fixes #109
- closes #551
Additional information
Checklist
- [x] DCO has been signed off on the commit.
- [x] Chart version bumped in
Chart.yamlaccording to semver. - [x] (optional) Variables are documented in the README.md
TODO
- [x] redis password from existing secret (or URL)
- [x] put redis password from env to secrets
- [x] solve bootstrap problem (nextcloud and notify_push needs to be online)
- [x] option to set urlEnv on notifyPush (for external redis)
Todo after Review:
- [x] redis-env https://github.com/nextcloud/helm/pull/581#discussion_r1903146252
- [ ] selftest as unittest: https://github.com/nextcloud/helm/pull/581#discussion_r1903146391
Can we ensure that the notify_push-plugin is installed? Maybe something like this?
lifecycle:
postStart:
exec:
command: ["occ", "app:install notify_push"]
And we have to active that plugin by running sudo -u www-data ./occ notify_push:setup https://NEXTCLOUD_HOST/push. Maybe we should add a little script like this (pseudocode):
if (!notify_push installed)
install notify_push
/occ notify_push:setup https://NEXTCLOUD_HOST/push
I think it makes sense to give this option so the installation and setup is completely automatic, but I'd rather put it behind a second config flag:
notify_push:
enabled: true
automatic_setup: true
Maybe some people don't want to have this done automatically, so it's nice to give them the option.
Therefore we has that hook of the container script (see #525), i write a ConfigMap for it.
PS: the same way, then in #480 (@provokateurin you wanted to take a look there ...)
PSS: Does somebody test this/my code?
andre@server:~/k8s/nextcloud$ helm upgrade -n nextcloud akops-nextcloud -f values.yml ./helm/charts/nextcloud/
Error: UPGRADE FAILED: template: nextcloud/templates/notify_push/deployment.yaml:41:31: executing "nextcloud/templates/notify_push/deployment.yaml" at <$.Values.global.image.registry>: nil pointer evaluating interface {}.image
Oh sorry, that was a copy-paste error.
normally i has on my helm-charts a global.image part to overwrite for registry and so on.
I was able to try an install. The result was this:
Configuring Redis as session handler
=> Searching for scripts (*.sh) to run, located in the folder: /docker-entrypoint-hooks.d/before-starting
==> Running the script (cwd: /var/www/html): "/docker-entrypoint-hooks.d/before-starting/notify_push.sh"
notify_push already installed
✓ redis is configured
🗴 push server is not receiving redis messages (received 272721789, got 0)
==> Failed at executing "/docker-entrypoint-hooks.d/before-starting/notify_push.sh". Exit code: 1
Edit
I have a password for redis and it was not set. Can add this like this?
- name: REDIS_URL
value: "redis://:<PASSWORD>@{{ template "nextcloud.redis.fullname" . }}-master:{{ .Values.redis.master.service.ports.redis }}"
When I did this locally, then we have the chicken-egg-problem ... The notify_push application needs a running nextcloud-instance to fully start. And the nextcloud-instance need a running notify_push.
Maybe there is a better hook after nextcloud has started?
With the fixed port, I still unable to run it.
Logs from notify_push pod:
[2024-06-19 06:44:36.746890 +00:00] ERROR [notify_push] src/main.rs:78: Self test failed: Error while communicating with nextcloud instance: error sending request for url (http://akops-nextcloud.nextcloud.svc.cluster.local:8080/index.php/apps/notify_push/test/version): error trying to connect: tcp connect error: Connection refused (os error 111)
@wrenix I'm updated your push file to be .tpl instead of .gotmpl to match the rest of the tpl files. I may also pop in here and write a quick test so this gets tested in CI?
Okay, i fix the startup problem. It is inside the nextcloud "broken", blocking.
So i use:
occ config:app:set notify_push base_endpoint --value="URL"
instatt of occ notify_push:setup URL (which runs a self-test and needs nextcloud running (but we make the hook, before nextcloud starts)
@wrenix cool changes. I tried your branch and it worked directly without any issue in my environment. :)
I move the env handling back to the helper (and split it). So we have nearly the same logic for database(_url) and redis_url for nextcloud and notify-push.
So it should be review ready (and merge ready).
I create an CHANGELOG.md for the Breaking Changes.
move the test developing to #697