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
nextcloud module: init #47159
nextcloud module: init #47159
Conversation
FWIW I'm using this on my own server and it seems OK: https://github.com/eqyiel/dotfiles/blob/66dc34c3071fb5678afaca4229530aac9190bdf5/nix/.config/nixpkgs/nixops/realms/maher.fyi/config/nextcloud.nix#L6-L46 |
@GrahamcOfBorg test nextcloud |
No attempt on x86_64-linux (full log) The following builds were skipped because they don't evaluate on x86_64-linux: tests.nextcloud Partial log (click to expand)
|
No attempt on aarch64-linux (full log) The following builds were skipped because they don't evaluate on aarch64-linux: tests.nextcloud Partial log (click to expand)
|
The test does not work yet:
|
@Mic92 sorry about that. Can I do this too? @GrahamcOfBorg test nextcloud |
@GrahamcOfBorg test nextcloud |
@eqyiel You need to be known by ofborg for this. This can be done by opening a PR similar to NixOS/ofborg#239 to the ofborg repository :) That said, you can also test locally, with eg. |
@eqyiel I was giving this another run, together with #47220 I got nextcloud 14 to be installed, backed by a MySQL server :-) I didn't yet test redis and memcached, but can we use We could then also stop setting |
Failure on aarch64-linux (full log) Attempted: tests.nextcloud Partial log (click to expand)
|
Timed out, unknown build status on x86_64-linux (full log) Attempted: tests.nextcloud Partial log (click to expand)
|
default = { | ||
"short_open_tag" = "Off"; | ||
"expose_php" = "Off"; | ||
"disable_functions" = "pcntl_alarm,pcntl_fork,pcntl_waitpid,pcntl_wait,pcntl_wifexited,pcntl_wifstopped,pcntl_wifsignaled,pcntl_wexitstatus,pcntl_wtermsig,pcntl_wstopsig,pcntl_signal,pcntl_signal_dispatch,pcntl_get_last_error,pcntl_strerror,pcntl_sigprocmask,pcntl_sigwaitinfo,pcntl_sigtimedwait,pcntl_exec,pcntl_getpriority,pcntl_setpriority"; |
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.
This will cause nextcloud-occ
to complain
The process control (PCNTL) extensions are required in case you want to interrupt long running commands - see http://php.net/manual/en/book.pcntl.php
on each run, as that's baked intonextcloud-occ
s config as well.
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.
I've removed this line, but I'd like to know why too!
[ -e ${cfg.home}/config/config.php ] && exit 0 | ||
|
||
ln -sf ${autoConfig} ${cfg.home}/config/autoconfig.php | ||
chown -R nextcloud:nginx ${cfg.home}/config ${cfg.home}/data ${cfg.home}/store-apps |
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.
The initial request currently does the whole nextcloud installation. This ran into timeouts:
nextcloud# [ 140.665158] nginx[690]: 2018/09/23 13:55:18 [error] 691#691: *1 upstream timed out (110: Connection timed out) while reading response header from upstream, client: 192.168.1.1, server: nextcloud, request: "GET /login HTTP/1.1", upstream: "fastcgi://unix:/run/phpfpm/nextcloud", host: "nextcloud"
It might help to run the nextcloud installation via nextcloud-occ inside this nextcloud-setup
unit - On the first HTTP request, everything will be setup already.
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.
That's a good idea. The problem I'm having now is that the tests pass interactively (run with nix-build nixos/tests/nextcloud.nix -A driver --show-trace -I nixpkgs=.
and running testScript
on the CLI) but not with nix-build nixos/tests/nextcloud.nix --show-trace -I nixpkgs=.
. I'm not sure what that's about.
nextcloud# [ 21.213558] Nextcloud[575]: {PHP} mkdir(): Read-only file system at /nix/store/6bzrcj4ryhilnz3xsh1l94r5aykybm3w-nextcloud-14.0.0/lib/private/Setup.php#300
nextcloud# [ 21.257576] 1jgyw0ca6flrapn5bh55nrrsjwdixk2a-unit-script-nextcloud-setup-start[520]: Set an admin password.
nextcloud# [ 21.269205] 1jgyw0ca6flrapn5bh55nrrsjwdixk2a-unit-script-nextcloud-setup-start[520]: Can't create or write into the data directory /nix/store/6bzrcj4ryhilnz3xsh1l94r5aykybm3w-nextcloud-14.0.0/data
nextcloud# [ 21.289701] sudo[557]: pam_unix(sudo:session): session closed for user nextcloud
nextcloud# [ 21.549481] systemd[1]: nextcloud-setup.service: Main process exited, code=exited, status=1/FAILURE
nextcloud# [ 21.554644] systemd[1]: nextcloud-setup.service: Failed with result 'exit-code'.
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.
One cannot use the autoconfig.php with that AFAICS
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.
Uff, I need to investigate.
Other thing: The nextcloud-setup
script is idempotent, and seems to be executed on each boot - so we should check with occ if nextcloud is already installed, and skip the installation in that case.
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.
Seems like this is true: nextcloud/server#8524
I don't know how to resolve this except to increase the gateway timeout, so I'll do that.
@flokli thanks for the thorough review. I should be able to do this tonight (UTC+9:30) |
@eqyiel thanks! |
ln -sf ${autoConfig} ${cfg.home}/config/autoconfig.php | ||
chown -R nextcloud:nginx ${cfg.home}/config ${cfg.home}/data ${cfg.home}/store-apps | ||
|
||
${occ}/bin/nextcloud-occ maintenance:install |
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.
cool. This means migration is done automatically? Profit!
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.
As mentioned below, this does not work, which is why it was done during the first request with the help of the autoconfig.php
.
The
|
Increasing the timeout seems to do it for now. |
@eqyiel I found the issue! For some reasons, before nextcloud is installed, It's possible to set these options manually, see https://docs.nextcloud.com/server/14/admin_manual/configuration_server/occ_command.html#command-line-installation-label . I did a POC in my nextcloud branch. I didn't test anything other than the vm test, especially the |
I think this is good now, should I squash it into two commits (module init and package version bump)? |
I have been following the discussion, and wonder: do we know how well this module will handle upgrades? If I understand #47159 (comment) correctly, upgrading currently requires manual intervention, which would be unfortunate for NixOS. Would it be possible to automate it, or at least enforce the package version through |
@Ekleog nah, that was most likely a messed up state from the time we used autoconfig.
I was able to upgrade from 14.0.0 to 14.0.1 (in case that introduced DB migrations) using this module. What's currently not tracked is upgrades of additionally installed "apps", but I'm not sure about the tooling for those anyhow. @globin, @fpletz, what are your thoughts on that? You seem to be using the saml app… Should we add support for installing apps through this nixos module in a followup PR? @eqyiel 👍 for squahing this into three commits (module init, package bump, sendmail path), and then this should be good to go - and TBH, I'd love to see this backported for 18.09 as well :-) |
Squashed! |
@eqyiel you can actually use the |
Co-authored-by: Franz Pletz <fpletz@fnordicwalking.de> Co-authored-by: Robin Gloster <mail@glob.in> Co-authored-by: Janne Heß <janne@hess.ooo> Co-authored-by: Florian Klink <flokli@flokli.de>
Co-authored-by: Robin Gloster <mail@glob.in>
@flokli done 👌 |
@eqyiel seems like you are missing the two empty lines before the co-author options : https://help.github.com/articles/creating-a-commit-with-multiple-authors/#creating-co-authored-commits-on-the-command-line |
At least github seem to have detected it. |
OK sorry, seems to be the phone version not showing correctly |
@nyanloutre my bad, I'll fix it, but I won't get a chance to until tomorrow. |
@eqyiel it's fine, the mobile interface simply doesn't display it correctly. |
@globin any reservations on the third-party apps matter, or can we merge this one into here and to 18.09?
|
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.
Thanks for the improvements! LGTM 👍
I'm getting mixed messages here 😅 |
Works perfectly just migrated an old installation. |
Regarding backport please open a pull request, so we can also run tests. I am not sure about backporting major upgrades though. |
@Mic92 thanks! |
extraTrustedDomains = mkOption { | ||
type = types.listOf types.str; | ||
default = []; | ||
description = '' |
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.
Would it not make sense to also put extraTrustedDomains
into services.nginx.virtualHosts.<hostname>.serverAliases
?
At least this is what I would expected this option do.
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.
I think there should be a clear distinction there. It might be the desired behavior in 99.9% of all cases, but especially with HTTP servers, some people have pretty wild configs, so this reduces flexibility.
What do you think? Wait until someone complains or leave it flexible?
Of course, there is always the option to configure nginx
manually...
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.
The least we can do is to add it to the description.
Motivation for this change
This is #44994 with the requested changes from reviews.
I don't think I have permission to push to that branch so I'm doing it this way.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)