Skip to content

portblock: monitor needs to also check state file of inverse action #2108

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

Draft
lge wants to merge 3 commits into
base: main
Choose a base branch
from
+65 −9
Draft

portblock: monitor needs to also check state file of inverse action #2108

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b2cc2c
portblock: call save_tcp_connection during monitor when promoted
lge Dec 3, 2025
f07425d
portblock: add SayConsideredInactive to clarify when debugging
lge Dec 3, 2025
801a02d
portblock: derive state file from parameters instead of instance
lge Dec 3, 2025
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
74 changes: 65 additions & 9 deletions heartbeat/portblock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,17 @@ OCF_RESKEY_portno_default=""
OCF_RESKEY_direction_default="in"
OCF_RESKEY_action_default=""
OCF_RESKEY_method_default="drop"
OCF_RESKEY_status_check_default="rule"
OCF_RESKEY_ip_default="0.0.0.0/0"
OCF_RESKEY_reset_local_on_unblock_stop_default="false"
OCF_RESKEY_tickle_dir_default=""
OCF_RESKEY_sync_script_default=""

if ocf_is_ms; then
Copy link
Contributor

@oalbrigt oalbrigt Dec 4, 2025

Choose a reason for hiding this comment

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

This will make the metadata suggest it's not running the rule check by default, so I think we should move this down to where you define state_file_base and modify OCF_RESKEY_status_check instead of the default.

OCF_RESKEY_status_check_default="rule"
else
OCF_RESKEY_status_check_default="pseudo"
fi

: ${OCF_RESKEY_firewall=${OCF_RESKEY_firewall_default}}
: ${OCF_RESKEY_protocol=${OCF_RESKEY_protocol_default}}
: ${OCF_RESKEY_portno=${OCF_RESKEY_portno_default}}
Expand Down Expand Up @@ -425,6 +430,10 @@ tickle_local()
done
}

# A long time ago, these messages needed to go to stdout,
# "running" / "OK" being the trigger string
# for heartbeat in haresources mode.
# Now they are still useful for debugging.
SayActive()
{
ocf_log debug "$CMD $method rule [$*] is running (OK)"
Expand All @@ -440,6 +449,11 @@ SayInactive()
ocf_log debug "$CMD $method rule [$*] is inactive"
}

SayConsideredInactive()
{
ocf_log debug "$CMD $method rule [$*] considered to be inactive"
}

#PortStatus {udp|tcp} portno,portno ip {in|out|both} {block|unblock}
PortStatus() {
local rc
Expand All @@ -465,8 +479,17 @@ PortStatus() {
fi
;;
*)
SayInactive $*
rc=$OCF_NOT_RUNNING
if [ "$OCF_RESKEY_status_check" != "rule" ] \
&& test -e "$state_file" && test "$inverse_state_file" -nt "$state_file"; then
# rule present, action=unblock, unblock statefile present,
# block state file more recent.
# apparently an unusual setup: unblock first, block later
SayConsideredActive $*
rc=$OCF_SUCCESS
else
SayInactive $*
rc=$OCF_NOT_RUNNING
fi
;;
esac
elif [ "$OCF_RESKEY_status_check" = "rule" ]; then
Expand All @@ -478,6 +501,7 @@ PortStatus() {
*)
SayActive $*
if [ "$__OCF_ACTION" = "monitor" ] && [ "$promotion_score" = "$SCORE_PROMOTED" ]; then
save_tcp_connections
rc=$OCF_RUNNING_MASTER
else
rc=$OCF_SUCCESS
Expand All @@ -487,7 +511,10 @@ PortStatus() {
else
case $5 in
block)
if ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" status; then
if test -e "$state_file" && test "$inverse_state_file" -nt "$state_file"; then
# rule NOT present, action=block, block state file present,
# unblock state file more recent.
# expected setup: block first, unblock later
SayConsideredActive $*
rc=$OCF_SUCCESS
else
Expand All @@ -496,13 +523,15 @@ PortStatus() {
fi
;;
*)
if ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" status; then
if test -e "$state_file" ; then
# rule NOT present, action=unblock, unblock state file present
SayActive $*
#This is only run on real monitor events.
# This is only run on real monitor events (state file present).
save_tcp_connections
rc=$OCF_SUCCESS
else
SayInactive $*
# rule NOT present, action=unblock, unblock state file NOT present
SayConsideredInactive $*
rc=$OCF_NOT_RUNNING
fi
;;
Expand Down Expand Up @@ -635,7 +664,7 @@ PortUNBLOCK()
#PortStart {udp|tcp} portno,portno ip {in|out|both} {block|unblock}
PortStart()
{
ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" start
ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" start "$state_file"

if [ "$FIREWALL" = "nft" ]; then
$NFTABLES add table inet $TABLE || {
Expand Down Expand Up @@ -678,7 +707,7 @@ PortStart()
#PortStop {udp|tcp} portno,portno ip {in|out|both} {block|unblock}
PortStop()
{
ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" stop
ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" stop "$state_file"

case $5 in
block) PortUNBLOCK "$@"
Expand Down Expand Up @@ -929,6 +958,33 @@ fi

PortValidateAll

# State file name for ha_pseudo_resource
#
# The expected usage of this agent is to pair a "block" with an "unblock",
# and order startup and configuration of some service between these.
#
# The established idiom is to have two separate instances with inverse actions.
# To "reliably" report the status of "block" during a monitor action,
# it is not sufficient to check the existence of the blocking rule.
#
# It is also insufficient to rely on the pseudo resource state file
# of this instance only.
#
# To know our actual expectation, we need to check the state file of the
# "inverse" instance as well.
#
# Because we don't know the OCF_RESOURCE_INSTANCE value of the other instance,
# we override the state file name for both instances to something derived from
# our parameters.
#
# This should give use the same "global state" view as the "promotion score"
# does for the promotable clone variant of this agent.
#
[ "$action" = block ] && inverse_action=unblock || inverse_action=block
state_file_base=$(echo "portblock_${protocol}_${portno}_${ip}_${direction}" | tr -c '0-9a-zA-Z._' _)
state_file=${HA_RSCTMP}/${state_file_base}_${action}
inverse_state_file=${HA_RSCTMP}/${state_file_base}_${inverse_action}

case $__OCF_ACTION in
start)
PortStart "$protocol" "$portno" "$ip" "$direction" "$action"
Expand Down