Skip to content

keepalived: Added track_script option to sync group and fixed print_track_script_indent #29027

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

Open
rishabhshah2005 wants to merge 10 commits into
base: master
Choose a base branch
from
+27 −11
Open

keepalived: Added track_script option to sync group and fixed print_track_script_indent #29027

Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6f43925
keepalived: Fixed print_track_script_indent function
rishabhshah2005 Mar 30, 2026
6d48739
keepalived: Added track_script option for sync group
rishabhshah2005 Mar 30, 2026
169fcb7
keepalived: Updated keepalived.config
rishabhshah2005 Mar 30, 2026
b956c7f
keepalived: Added option timeout in vrrp_script.
rishabhshah2005 Mar 30, 2026
2983ba4
keepalived: changed curr_track_elem in vrrp_instance
rishabhshah2005 Mar 30, 2026
e527de8
keepalived: Bumped keepalived PKG_RELEASE to 3
rishabhshah2005 Mar 30, 2026
4285957
keepalived: Removed keyword 'if'
rishabhshah2005 Mar 31, 2026
8848d0b
keepalived: added back the trailing lines
rishabhshah2005 Mar 31, 2026
70f77b5
keepalived: added new line ending
rishabhshah2005 Apr 5, 2026
c2efbec
Merge branch 'openwrt:master' into master
rishabhshah2005 Apr 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions net/keepalived/files/keepalived.init
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,16 +215,14 @@ print_track_script_indent() {

local name value weight direction
config_get name "$section" name
[ "$name" != "$curr_track_elem" ] && return 0
if [ -z "$name" ] && return 0

config_get value "$section" value
config_get weight "$section" weight
Comment on lines 216 to 220
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

print_track_script_indent() currently has a shell syntax error (if [ -z "$name" ] && return 0) and it no longer filters by curr_track_elem. As a result, every config_foreach ... vrrp_script invocation will print all vrrp_script sections (and, in the current loops, likely print them repeatedly), instead of only the script referenced by the list entry. Replace the invalid if with a normal test, and restore a name != curr_track_elem guard (while still printing the referenced script name).

Copilot uses AI. Check for mistakes.
config_get direction "$section" direction

[ -z "$value" ] && return 0
[ "$direction" != "reverse" ] && [ "$direction" != "noreverse" ] && unset direction

printf '%b%s' "$indent" "$value" >> "$KEEPALIVED_CONF"
printf '%b%s' "$indent" "$name" >> "$KEEPALIVED_CONF"
[ -n "$weight" ] && printf ' weight %s' "$weight ${direction:+${direction}}" >> "$KEEPALIVED_CONF"
printf '\n' >> "$KEEPALIVED_CONF"
}
Expand Down Expand Up @@ -333,7 +331,7 @@ vrrp_sync_group() {
config_get group "$1" group
[ -z "$group" ] && return 0

# Check if we have 'vrrp_instance's defined for
# Check if we have 'vrrp_instance's defined for
Copy link
Copy Markdown
Member

@feckert feckert Mar 31, 2026

Choose a reason for hiding this comment

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

Does not belongs to this change.
Either new commit or leave it as it is.

# each member and remove names with not vrrp_instance defined
for m in $group; do
vrrp_instance_check "$m" && valid_group="$valid_group $m"
Expand All @@ -347,6 +345,17 @@ vrrp_sync_group() {

print_elems_indent "$1" "$INDENT_1" no_val_smtp_alert no_val_global_tracking

# Handle track_script list for sync group
local track_script_val
config_get track_script_val "$1" track_script
if [ -n "$track_script_val" ]; then
printf '%btrack_script {\n' "${INDENT_1}" >> "$KEEPALIVED_CONF"
Copy link
Copy Markdown
Member

@feckert feckert Mar 31, 2026

Choose a reason for hiding this comment

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

Can we not use config_section_open there?

Copy link
Copy Markdown
Author

@rishabhshah2005 rishabhshah2005 Mar 31, 2026

Choose a reason for hiding this comment

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

We could use config_section_open here but it would mess up the indentation in keepalived.conf. Besides for this is referenced referenced the track_script logic in vrrp_instance

Copy link
Copy Markdown
Member

@feckert feckert Mar 31, 2026

Choose a reason for hiding this comment

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

I think we should extend the config_section_open and config_section_close function, to add a new indent function argument. If argument is not set, proceed as use the default indent as before, but if the argument indent is set (INDENT_1|INDENT_2 ...), then add this indent.

Copy link
Copy Markdown
Author

@rishabhshah2005 rishabhshah2005 Mar 31, 2026

Choose a reason for hiding this comment

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

yeah we can do that. But after this should all the functions that hardcoded this must be changed?

Copy link
Copy Markdown
Author

@rishabhshah2005 rishabhshah2005 Apr 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Hey man you there? @feckert

for t in $track_script_val; do
config_foreach print_track_script_indent vrrp_script "$t" "$INDENT_2"
done
printf '%b}\n' "${INDENT_1}" >> "$KEEPALIVED_CONF"
Copy link
Copy Markdown
Member

@feckert feckert Mar 31, 2026

Choose a reason for hiding this comment

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

Can we not use config_section_close there?

fi

print_notify "GROUP" "$name" "$INDENT_1" notify_backup notify_master \
notify_fault notify

Expand Down Expand Up @@ -670,4 +679,3 @@ start_service() {
procd_set_param respawn
procd_close_instance
}

Copy link
Copy Markdown
Member

@feckert feckert Mar 31, 2026

Choose a reason for hiding this comment

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

Does not belongs to this change.
Either new commit or leave it as it is.

Copy link
Copy Markdown
Author

@rishabhshah2005 rishabhshah2005 Mar 31, 2026

Choose a reason for hiding this comment

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

Cant this be accepted? Or should make changes in another commit to leave it as it is?

Copy link
Copy Markdown
Member

@feckert feckert Mar 31, 2026

Choose a reason for hiding this comment

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

As I said. If this is not related to the change move it to a new commit.
I this case for example keepalived: remove empty line or in the other case keepalived: remove trailing whitespace