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

nixos/kubo: fix potential panic on startup #272041

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

Luflosi
Copy link
Contributor

@Luflosi Luflosi commented Dec 4, 2023

Description of changes

This fixes a panic of the kubo daemon which could occur under certain conditions when the daemon was starting.
It was caused by the ipfs.service unit not depending on the ipfs-api.socket and ipfs-gateway.socket units with Wants=. This allows the ipfs.service to be started manually or by nixos-rebuild without the sockets being set up before that. When that happens, the daemon won't know about these sockets and will only use what is set in services.kubo.settings.Addresses.Gateway and services.kubo.settings.Addresses.API. By default the API is an empty list in NixOS though. The daemon doesn't like this at all and panics on startup, see ipfs/kubo#10056.
With this commit, starting ipfs.service will first set up the two sockets before starting the actual service.
Adding the Sockets= option implicitly adds a Wants= for the sockets and this is exactly what we need. See https://www.freedesktop.org/software/systemd/man/latest/systemd.service.html#Implicit%20Dependencies . This can be checked with systemctl show ipfs.service.

This should probably be upstreamed to the unit file in the Kubo repo.

The problem can be reproduced in the following way:

  • Add services.kubo.enable = true to /etc/nixos/configuration.nix
  • sudo nixos-rebuild switch (this may already fail, not sure why it's not deterministic for me)
  • sudo systemctl stop ipfs-api.socket
  • sudo systemctl stop ipfs-gateway.socket
  • sudo systemctl stop ipfs.service
  • sudo systemctl start ipfs.service

Fixes #248447.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Dec 4, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 4, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3065

This fixes a panic of the kubo daemon which could occur under certain conditions when the daemon was starting.
It was caused by the `ipfs.service` unit not depending on the `ipfs-api.socket` and `ipfs-gateway.socket` units with `Wants=`. This allows the `ipfs.service` to be started manually or by `nixos-rebuild` without the sockets being set up before that. When that happens, the daemon won't know about these sockets and will only use what is set in `services.kubo.settings.Addresses.Gateway` and `services.kubo.settings.Addresses.API`. By default the `API` is an empty list in NixOS though. The daemon doesn't like this at all and panics on startup, see ipfs/kubo#10056.
With this commit, starting `ipfs.service` will first set up the two sockets before starting the actual service.
Adding the `Sockets=` option implicitly adds a `Wants=` for the sockets and this is exactly what we need. See https://www.freedesktop.org/software/systemd/man/latest/systemd.service.html#Implicit%20Dependencies . This can be checked with `systemctl show ipfs.service`.

This should probably be upstreamed to the unit file in the Kubo repo.

The problem can be reproduced in the following way:
- Add `services.kubo.enable = true` to `/etc/nixos/configuration.nix`
- `sudo nixos-rebuild switch` (this may already fail, not sure why it's not deterministic for me)
- `sudo systemctl stop ipfs-api.socket`
- `sudo systemctl stop ipfs-gateway.socket`
- `sudo systemctl stop ipfs.service`
- `sudo systemctl start ipfs.service`

Fixes NixOS#248447.
@Luflosi Luflosi force-pushed the kubo-fix-potential-panic-on-start branch from a368b4a to d4fcb44 Compare December 15, 2023 22:40
@Luflosi
Copy link
Contributor Author

Luflosi commented Dec 15, 2023

Rebased.

@fpletz fpletz merged commit d6b1e62 into NixOS:master Dec 15, 2023
16 of 18 checks passed
Copy link
Contributor

Successfully created backport PR for release-23.11:

@Luflosi Luflosi deleted the kubo-fix-potential-panic-on-start branch December 15, 2023 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kubo: default installation gives null API field instead of some unix domain socket
3 participants