nextcloud / helm

A community maintained helm chart for deploying Nextcloud on Kubernetes.
GNU Affero General Public License v3.0
295 stars 258 forks source link

feat(nextcloud): add notify_push support #581

Open wrenix opened 2 weeks ago

wrenix commented 2 weeks ago

Pull Request

Description of the change

Not yet tested

Benefits

Possible drawbacks

Applicable issues

Additional information

Checklist

TODO

AndreKoepke commented 2 weeks ago

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
provokateurin commented 2 weeks ago

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.

wrenix commented 2 weeks ago

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?

AndreKoepke commented 1 week ago
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
wrenix commented 1 week ago

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.

AndreKoepke commented 1 week ago

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?

wrenix commented 1 week ago

okay, redis password should work:

PS: found that the redis password is not stored by default in a secret ....


oh yes, that is a bootstrap problem. maybe we could remove it on one part.

AndreKoepke commented 1 week ago

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)