nixos/keepalived: Implemented vrrp-instance track scripts and track interfaces #39671
+138
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@mbrgm care to review? |
mbrgm
reviewed
May 4, 2018
| type = types.bool; | ||
| default = false; | ||
| description = '' | ||
| Don't run scripts configured to be run as root if any part of the path. |
There was a problem hiding this comment.
This should be "Don't run scripts configured to be run as root if any part of the path is writable by a non-root user."
| weight = mkOption { | ||
| type = int; | ||
| default = 0; | ||
| description = "Following a failure, adjust the priority with this weight."; |
| description = "Required number of failures for KO transition."; | ||
| }; | ||
|
|
||
| execUser = mkOption { |
There was a problem hiding this comment.
I too think that execUser would be a better name for this parameter, but the keepalived.conf file uses user. I wonder if it'd be better to stick as close to the original parameters as possible... would be happy to have your opinion on that.
| description = "Name of user to run the script under."; | ||
| }; | ||
|
|
||
| execGroup = mkOption { |
There was a problem hiding this comment.
If we change execUser -> user, then this one should be called group.
…rfaces. Tracking scripts in particular, cannot be included in extraOpts, because script declaration has to be above script usage in keepalived.conf. Changes are fully backward compatible.
johanot
force-pushed
the
keepalived-vrrpInstanceTracking
branch
from
d0f4ef8 to
41d4bd2
Compare
|
@mbrgm All of your suggestions have been incorporated in latest squash. :-) |
GrahamcOfBorg
added
6.topic: nixos
|
@GrahamcOfBorg eval |
GrahamcOfBorg
added
10.rebuild-darwin: 0
Janik-Haag
added
the
12. first-time contribution
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
It would be nice to be able to explicitly declare keepalived virtual IP healthcheck scripts and interfaces, in order for failover to make sense.
Furthermore; tracking scripts in particular, cannot be included with
keepalived.extraConfig, because VRRP scripts must be declared above script usage in keepalived.conf , otherwise keepalived will complain with something like:See: http://www.askmrisp.com/2017/04/17/solved-keepalived-not-running-track_script/ for details.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)