keepalived: Added track_script option to sync group and fixed print_track_script_indent#29027
keepalived: Added track_script option to sync group and fixed print_track_script_indent#29027rishabhshah2005 wants to merge 0 commit intoopenwrt:masterfrom
Conversation
Just have a look at the other commits in this repository – that’s a good place to start. |
|
You want me to change commit messages in this PR itself? |
No. I mean in the commits. The commits get merged into the repository not the PR description. |
|
I meant the commit messages. How can I can I go back and change the messages. Sorry i am new to this and i have no idea how you guys would approach to do this Edit: Never mind I did it. Are the changes acceptable now? |
feckert
left a comment
feckert
left a comment
There was a problem hiding this comment.
It’s always the first time 👍
There are my comments
| procd_set_param respawn | ||
| procd_close_instance | ||
| } | ||
|
|
There was a problem hiding this comment.
Does not belongs to this change.
Either new commit or leave it as it is.
There was a problem hiding this comment.
Cant this be accepted? Or should make changes in another commit to leave it as it is?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenWrt keepalived package integration to support generating track_script { ... } inside vrrp_sync_group blocks (using vrrp_script references), adjusts how track_script entries are rendered, and refreshes the example UCI config accordingly.
Changes:
- Add
track_scriptblock generation tovrrp_sync_group. - Switch
track_scriptlookups in instances/groups to usevrrp_scriptsections and extendvrrp_scriptemission withtimeout. - Update example config and bump
PKG_RELEASE.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
net/keepalived/Makefile |
Bumps package release to publish the integration changes. |
net/keepalived/files/keepalived.init |
Adds sync-group track_script generation and changes track-script rendering / lookup logic. |
net/keepalived/files/keepalived.config |
Updates example configuration/comments for the new track_script/vrrp_script behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Very good. The changes look good now. I’d would suggest to start with the API change by adding the indent. |
|
I moved PGK_RELEASE bump at the end. Not quite sure what you mean by the other thing. If you mean the order, yes the adding of indent in config_section_open-close is first, followed by adding peer to its own section (this part was already there i just added the docs). |
feckert
left a comment
feckert
left a comment
There was a problem hiding this comment.
Sorry, I'm not happy with the way the patch is structured. I won't merge it like this. It's a right mess! Please close this PR and create a new one with your changes correct structed. Do only one thing in each commit, that is mention in the commits message.
| local name value weight direction | ||
| config_get name "$section" name | ||
| [ "$name" != "$curr_track_elem" ] && return 0 | ||
| if [ -z "$name" ] && return 0 |
There was a problem hiding this comment.
The line if [ -n "$name" ] && return 0 is wrong and does not work.
It have to be [ -n "$name" ] && return 0.
| config_get group "$1" group | ||
| [ -z "$group" ] && return 0 | ||
|
|
||
| # Check if we have 'vrrp_instance's defined for |
There was a problem hiding this comment.
As I said, this change doesn’t belong in this commit; it should be in a separate one. As it has nothing to do with the change.
📦 Package Details
Maintainer: @feckert
keepalived: Added track_script section to sync group and fixed print_track_script_indent
Description:
Build Test
Build tested successfully on a x86_64 device
✅ Formalities