-
Notifications
You must be signed in to change notification settings - Fork 269
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
feat(nextcloud): add notify_push support #581
base: main
Are you sure you want to change the base?
Conversation
4c3cee5
to
46eb0e9
Compare
f5f716e
to
6ed673a
Compare
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
|
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I remove the global-references, then I got this error:
andre@server:~/k8s/nextcloud$ helm upgrade -n nextcloud akops-nextcloud -f values.yml ./helm/charts/nextcloud/
Error: UPGRADE FAILED: YAML parse error on nextcloud/templates/notify_push/deployment.yaml: error converting YAML to JSON: yaml: line 41: did not find expected key
Oh sorry, that was a copy-paste error. |
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 EditI 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 ... 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:
|
@wrenix I'm updated your push file to be |
d69d481
to
4e06db2
Compare
43ba133
to
fc97749
Compare
Okay, i fix the startup problem. So i use: occ config:app:set notify_push base_endpoint --value="URL" instatt of |
@wrenix cool changes. I tried your branch and it worked directly without any issue in my environment. :) |
b73428e
to
fa34027
Compare
fa34027
to
4f953e2
Compare
I move the env handling back to the helper (and split it). So it should be review ready (and merge ready). I create an CHANGELOG.md for the Breaking Changes. |
d879976
to
3e0e0ca
Compare
Signed-off-by: Jesse Hitch <[email protected]> Signed-off-by: WrenIX <[email protected]>
Signed-off-by: WrenIX <[email protected]>
3e0e0ca
to
a6d5be4
Compare
d5360ea
to
da98433
Compare
Pull Request
Description of the change
Not yet tested
Benefits
Possible drawbacks
Applicable issues
Additional information
Checklist
Chart.yaml
according to semver.TODO