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

Jump to bottom

nextcloud19: init at 19.0.0 #89427

Merged
merged 2 commits into from Jun 20, 2020
Merged

nextcloud19: init at 19.0.0 #89427

merged 2 commits into from Jun 20, 2020

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jun 3, 2020 •
edited

Motivation for this change

https://nextcloud.com/blog/nextcloud-hub-brings-productivity-to-home-office/

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Ma27 Ma27 added the 9.needs: port to stable A PR needs a backport to the stable release. label Jun 3, 2020
@Ma27 Ma27 requested review from fpletz, samueldr and aanderse June 3, 2020 17:51
@ofborg ofborg bot requested review from bachp, schneefux and globin June 3, 2020 17:59
@samueldr
Copy link
Member

samueldr commented Jun 3, 2020

The wording of the warnings looks alright. Though I will not test as I am not using Nextcloud in the end.

@bachp
Copy link
Member

bachp commented Jun 3, 2020 •
edited

@Ma27 When I to access a file after the upgrade a get an error:

\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\ServiceUnavailable\",\"Message\":\"filesystem not setup\"

Running systemctl restart phpfpm-nextcloud.service seems to fix the issue.

@Ma27
Copy link
Member Author

Ma27 commented Jun 4, 2020

That's interesting. I experienced a similar behavior, but (wrongly) assumed that I screwed up my initial upgrade attempt.

In my case, Nextcloud reported that The files of the app [all apps in /var/lib/nextcloud/apps] were not replaced correctly and I also managed to fix this by restarting phpfpm.

Before merging this, we should at least find the cause for issue.

@ajs124
Copy link
Member

ajs124 commented Jun 4, 2020

We've also been observing weird issues with nextcloud where restarting phpfpm-nextcloud helps. For now, I've added the configured nextcloud package and some php stuff as a restartTrigger to it, as a workaround.

@bachp
Copy link
Member

bachp commented Jun 4, 2020

@ajs124 I was also thinking if adding package to the restart trigger would be the safest option.

@Ma27
Copy link
Member Author

Ma27 commented Jun 4, 2020

Sounds reasonable. I'd like to investigate the why before that though :)

@Ma27
Copy link
Member Author

Ma27 commented Jun 11, 2020

I managed to reproduce the issue in a minimal NixOS container using the configs from nixos/tests/nextcloud/basic.nix. Unfortunately I didn't find the culprit (neither in the code nor in the issue-tracker), but @ajs124's suggestion seems reasonable and fixed the issue in my local test environment.

I assume that most of the people hosting Nextcloud somewhere else are probably restarting these services anyways and didn't notice this.

@Ma27
Copy link
Member Author

Ma27 commented Jun 16, 2020

Rebased the patches onto latest master to resolve the merge-conflict in the release-notes.

@Ma27
Copy link
Member Author

Ma27 commented Jun 16, 2020

@ajs124 @bachp can you have another look? I'd merge this in a few days otherwise :)

bachp
bachp approved these changes Jun 17, 2020
View reviewed changes
Copy link
Member

@bachp bachp left a comment

Choose a reason for hiding this comment

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

Still works with the updated MR.

@Ma27
Copy link
Member Author

Ma27 commented Jun 19, 2020

Rebased onto master to resolve the merge-conflict. Anything else tbd or is this mergeable now?

@ofborg ofborg bot requested a review from bachp June 19, 2020 20:33
@Ma27 Ma27 requested a review from nlewo June 20, 2020 08:07
@Ma27
nixos/nextcloud: update upgrade-path for nextcloud19
f326e5a 
Enhance the heuristics to make sure that a user doesn't accidentally
upgrade across two major versions of Nextcloud (e.g. from v17 to v19).

The original idea/discussion has been documented in the nixpkgs manual[1].

This includes the following changes:

* `nextcloud19` will be selected automatically when having a stateVersion
  greater or equal than 20.09. For existing setups, the package has to
  be selected manually to avoid accidental upgrades.

* When using `nextcloud18` or older, a warning will be thrown which recommends
  upgrading to `nextcloud19`.

* Added a brief paragraph about `nextcloud19` in the NixOS 19.09 release
  notes.

* Restart `phpfpm` if the Nextcloud-package (`cfg.package`) changes[2].

[1] https://nixos.org/nixos/manual/index.html#module-services-nextcloud-maintainer-info
[2] NixOS#89427 (comment)
@Ma27 Ma27 requested a review from nlewo June 20, 2020 09:31
@nlewo
Copy link
Member

nlewo commented Jun 20, 2020

Thanks!

@nlewo nlewo merged commit 2fd146f into NixOS:master Jun 20, 2020
@Ma27 Ma27 deleted the nextcloud19 branch June 20, 2020 11:59
@Ma27
Copy link
Member Author

Ma27 commented Jun 20, 2020 •
edited

Ported the addition of nextcloud19 as cdd1ece to stable as documented in https://nixos.org/nixos/manual/index.html#module-services-nextcloud-maintainer-info

The module changes are targeted for 20.09 only.

@Ma27 Ma27 added 8.has: port to stable A PR already has a backport to the stable release. and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Jun 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlewo nlewo nlewo approved these changes

@fpletz fpletz Awaiting requested review from fpletz

@samueldr samueldr Awaiting requested review from samueldr

@aanderse aanderse Awaiting requested review from aanderse

@schneefux schneefux Awaiting requested review from schneefux

@globin globin Awaiting requested review from globin

@bachp bachp Awaiting requested review from bachp

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 8.has: package (new) 8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Ma27 @samueldr @bachp @ajs124 @nlewo