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 8 commits into
base: master
Choose a base branch
from
+27 −12
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 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
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
2 changes: 1 addition & 1 deletion net/keepalived/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk

PKG_NAME:=keepalived
PKG_VERSION:=2.3.3
PKG_RELEASE:=2
PKG_RELEASE:=3

PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
PKG_SOURCE_URL:=https://www.keepalived.org/software
Expand Down
15 changes: 11 additions & 4 deletions net/keepalived/files/keepalived.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,12 @@ config globals 'globals'
# list route "route1"

#config vrrp_sync_group
# option name "VI_sync_group_1"
# list group "VI_1"
# list group "VI_2"
# option name "VI_sync_group_1"
# list group "VI_1"
# list group "VI_2"
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.

The changes

#	option name		  "VI_sync_group_1"
#	list group		  "VI_1"
#	list group		  "VI_2"

are not needed 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.

I have added some documentation for the new option here. Is the documentation not needed?

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.

It’s fine that this is documented. But you’ve changed lines that you didn’t need to change.

# Note that priority will not work with vrrp_script
# when in sync group
# list track_script "vrrp_script1"
Comment on lines +79 to +83
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.

The sync-group example now shows list track_script "vrrp_script1", but the config template still includes (elsewhere) the old config track_script sections and vrrp_instance examples referencing track_script1/track_script2. Since the init script now looks up track_script entries via vrrp_script sections, please update the remaining examples to reference vrrp_script names (and remove/adjust the config track_script examples) to avoid misleading users and potential breakage on upgrade.

Copilot uses AI. Check for mistakes.
# option smtp_alert "1"
# option global_tracking 1

Expand Down Expand Up @@ -132,11 +135,16 @@ config globals 'globals'
# option accept "1"

#config vrrp_script
# option name "vrrp_script1"
# option script "<script-file>"
# option interval "5"
# weight will increase +10 if script status is OK, otherwise -10
# option weight "10"
# option fall "2"
# option rise "3"
# valid values for direction reverse|noreverse -- reverse flips weight change
# option direction "reverse"
# option timeout "5"
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.

In the vrrp_script example, the newly added timeout line has inconsistent spacing (#\toption timeout "5"). For readability and consistency with the rest of the template, align it to the same option <key>\t\t"<val>" format used by the surrounding options.

Suggested change
# option timeout "5"
# option timeout "5"

Copilot uses AI. Check for mistakes.

#config virtual_server
# option enabled "1"
Expand Down Expand Up @@ -191,4 +199,3 @@ config globals 'globals'
# option name "url2"
# option path "/testurl/test2.jsp"
# option digest "22"

22 changes: 15 additions & 7 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
[ -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 @@ -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?

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 @@ -413,7 +422,7 @@ vrrp_instance() {
[ -z "$optval" ] && continue
printf '%b%s {\n' "${INDENT_1}" "$opt" >> "$KEEPALIVED_CONF"
for t in $optval; do
config_foreach print_track_script_indent track_script "$t" "$INDENT_2"
config_foreach print_track_script_indent vrrp_script "$t" "$INDENT_2"
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 see what you do, but can you please explain why this is needed?

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.

Currently theres no support for track_script section, only for vrrp_script.
So inside print_track_script_indent what config_get does (from my understanding) is tries to fetch track_script section, which doesnt exist. But vrrp_script does.

I am not sure if this is how it was supposed to be done so correct me if I'm wrong but I tested this and it works.
In the future once track_script section is supported then we can add support for both here.

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 the old option is still valid, we need to extend it to include the new option too. What would a correct keepalived configuration look like if both the track_script and the vrrp_script options is set in the uci.

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.

The old option doesn't exist in the code. However its still present in the keepalived.config as of now.

This is the sample configuration

config vrrp_instance 'vrrp_instance_10'
        option name 'vrrp_instance_10'
        option state 'MASTER'
        option interface 'br-i10'
        list virtual_ipaddress 'floatingIPaddr_10'
        option virtual_router_id '100'
        option priority '100'
        option advert_int '1'
        option preempt_delay '0'
        option auth_type 'PASS'
        option auth_pass 'pass'
        #list track_script 'sla_check'

config ipaddress 'floatingIPaddr_10'
        option name 'floatingIPaddr_10'
        option address '192.168.10.1/24'
        option device 'br-i10'
        option scope 'global'

config vrrp_sync_group 'vrrp_sync_group'
        option name 'sync_group'
        list group 'vrrp_instance_10'
        list track_script 'sla_check'

config vrrp_script 'sla_check'
        option name 'sla_check'
        option timeout '1'
        option interval '1'
        option fall '2'
        option rise '2'
        #option weight '50'
        option script '/etc/scripts/vrrp_sla_check.sh'

config track_script 'script1'
        option name     "track_script1"
        option value "/etc/scripts/vrrp_sla_check.sh"
        option weight   "1"

The above config generates this keepalived.conf file

! Configuration file for keepalived (autogenerated via init script)
! Written Tue Mar 31 09:39:16 2026

global_defs {
}

static_ipaddress {
}

static_routes {
}

interface_up_down_delays {
}

vrrp_script sla_check {
        script /etc/scripts/vrrp_sla_check.sh
        interval 1
        fall 2
        rise 2
        timeout 1
}

vrrp_sync_group sync_group {
        group {
                vrrp_instance_10
        }
        track_script {
                sla_check
        }
        notify_backup "/bin/busybox env -i ACTION=NOTIFY_BACKUP TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
        notify_master "/bin/busybox env -i ACTION=NOTIFY_MASTER TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
        notify_fault "/bin/busybox env -i ACTION=NOTIFY_FAULT TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
        notify "/bin/busybox env -i ACTION=NOTIFY TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
}

vrrp_instance vrrp_instance_10 {
        authentication {
                auth_type PASS
                auth_pass pass
        }
        state MASTER
        interface br-i10
        virtual_router_id 100
        priority 100
        advert_int 1
        preempt_delay 0
        notify_backup "/bin/busybox env -i ACTION=NOTIFY_BACKUP TYPE=INSTANCE NAME=vrrp_instance_10 /sbin/hotplug-call keepalived"
        notify_master "/bin/busybox env -i ACTION=NOTIFY_MASTER TYPE=INSTANCE NAME=vrrp_instance_10 /sbin/hotplug-call keepalived"
        notify_fault "/bin/busybox env -i ACTION=NOTIFY_FAULT TYPE=INSTANCE NAME=vrrp_instance_10 /sbin/hotplug-call keepalived"
        notify_stop "/bin/busybox env -i ACTION=NOTIFY_STOP TYPE=INSTANCE NAME=vrrp_instance_10 /sbin/hotplug-call keepalived"
        virtual_ipaddress {
                192.168.10.1/24 dev br-i10 label br-i10:vip scope global
        }
}

As you can see no section for track_script was generated

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.

But then I do not understand the uci why do i have to specifies the path to the script /ec/scripts/vrr_sla_check.sh twice in vrrp_script and track_script section. In the generate keepalived.conf the track_script references only to thevrrp_script section. In your case sla_check.
I’ll have to take a closer look at that when I get the more time.

config vrrp_script 'sla_check'
        option name 'sla_check'
        option timeout '1'
        option interval '1'
        option fall '2'
        option rise '2'
        #option weight '50'
        option script '/etc/scripts/vrrp_sla_check.sh'

config track_script 'script1'
        option name     "track_script1"
        option value "/etc/scripts/vrrp_sla_check.sh"
        option weight   "1"

vrrp_script sla_check {
        script /etc/scripts/vrrp_sla_check.sh
        interval 1
        fall 2
        rise 2
        timeout 1
}

vrrp_sync_group sync_group {
        group {
                vrrp_instance_10
        }
        track_script {
                sla_check
        }
        notify_backup "/bin/busybox env -i ACTION=NOTIFY_BACKUP TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
        notify_master "/bin/busybox env -i ACTION=NOTIFY_MASTER TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
        notify_fault "/bin/busybox env -i ACTION=NOTIFY_FAULT TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
        notify "/bin/busybox env -i ACTION=NOTIFY TYPE=GROUP NAME=sync_group /sbin/hotplug-call keepalived"
}

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.

You are absolutely right, we dont need the section track_script. On reviewing the docs of keepalived, a seprate track_script section doesnt exist. My guess is that it was removed and the config file was not updated, same with the init file.

I think we should update keepalived.config accordingly

done
printf '%b}\n' "${INDENT_1}" >> "$KEEPALIVED_CONF"
done
Expand Down Expand Up @@ -466,7 +475,7 @@ vrrp_script() {

config_section_open "vrrp_script" "$name"

print_elems_indent "$1" "$INDENT_1" script interval weight fall rise
print_elems_indent "$1" "$INDENT_1" script interval weight fall rise timeout

config_section_close
}
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

Loading