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.

Sign up for GitHub

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

nixos/multipath: add module for multipath-tools package #139833

Merged
merged 4 commits into from
Oct 13, 2021
Merged

nixos/multipath: add module for multipath-tools package #139833

merged 4 commits into from
Oct 13, 2021

Conversation

limeytexan
Copy link
Contributor

@limeytexan limeytexan commented Sep 29, 2021 •
edited
Loading

Motivation for this change

The multipath-tools package has existed in Nixpkgs for some time but
without a nixos module to configure/drive it. This patch creates a module
and updates the package to work in stage-1 boot.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages 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/)
  • 21.11 Release Notes (or backporting 21.05 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.

@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 Sep 29, 2021
@limeytexan limeytexan requested a review from grahamc September 29, 2021 04:35
grahamc
grahamc reviewed Oct 4, 2021
View reviewed changes
pkgs/os-specific/linux/multipath-tools/stage1.patch Outdated
@@ -0,0 +1,60 @@
diff --git a/multipathd/main.c b/multipathd/main.c
Copy link
Member

Choose a reason for hiding this comment

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

Attempted to upstream in opensvc/multipath-tools#17

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've confirmed in testing that the sd_notify() calls when running multipathd in stage will silently fail and that multipath works. 👍

grahamc
grahamc reviewed Oct 5, 2021
View reviewed changes
Copy link
Member

@grahamc grahamc left a comment

Choose a reason for hiding this comment

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

I submitted a PR against your branch to add a test and somewhat refactor the configuration a bit:

flox#1

nixos/modules/services/networking/multipath.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/multipath.nix Outdated Show resolved Hide resolved
@grahamc grahamc mentioned this pull request Oct 6, 2021
Merged
12 tasks
@limeytexan limeytexan force-pushed the multipath branch 3 times, most recently from e5f248d to f60bbe7 Compare October 8, 2021 10:09
@github-actions github-actions bot added 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Oct 8, 2021
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 8, 2021
@github-actions github-actions bot removed 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Oct 8, 2021
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 8, 2021
@limeytexan limeytexan marked this pull request as ready for review October 8, 2021 13:40
grahamc and others added 4 commits October 8, 2021 09:47
@grahamc
boot.initrd.extraFiles: allow users to add arbitrary files to the initrd
829bbc7
@grahamc
iscsi root initiator: allow specifying arbitrary iscsi commands too
a85dcd9
@limeytexan
multipath-tools: deprecate systemd-udev-settle.service
c344317 
The systemd service manifest provided with the multipath-tools source
makes reference to the systemd-udev-settle.service, and this in turn
triggers deprecation warnings on startup. This patch removes these
references to silence these warnings.
@limeytexan
nixos/multipath: add multipath module
121cfd1 
The multipath-tools package had existed in Nixpkgs for some time but
without a nixos module to configure/drive it. This module provides
attributes to drive the majority of multipath configuration options
and is being successfully used in stage-1 and stage-2 boot to mount
/nix from a multipath-serviced iSCSI volume.

Credit goes to @grahamc for early contributions to the module and
authoring the NixOS module test.
@github-actions github-actions bot added 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Oct 8, 2021
@github-actions github-actions bot removed 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Oct 9, 2021
@github-actions github-actions bot added 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Oct 9, 2021
cole-h
cole-h approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

LGTM; I ran the test locally, and it passed :)

@grahamc
Copy link
Member

grahamc commented Oct 12, 2021

@ofborg eval

@grahamc grahamc merged commit a997f19 into NixOS:master Oct 13, 2021
@limeytexan limeytexan deleted the multipath branch June 25, 2023 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grahamc grahamc grahamc left review comments

@cole-h cole-h cole-h approved these changes

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@limeytexan @grahamc @edrex @balsoft @cole-h