Skip to content

Commit 46e3c54

Browse files
Codexclaude
andcommitted
[fix] Retry pending report_status instead of forcing re-download openwisp#251
When report_status fails after a configuration apply (e.g. due to a transient HTTP 502 from the controller), the agent previously deleted the local checksum files to trigger a full re-download on the next cycle. That worked but wasted bandwidth and caused the controller to stay stuck on "modified" until the next real config change. Instead persist the unreported status in a pending_report marker file and retry just report_status on the next polling cycle. Key details addressed from the review: - Define retry_pending_report as a shell function so "local" is valid on BusyBox ash, where "local" outside a function is a parse error that silently crashes the script mid-loop. - Persist the marker BEFORE calling retry_with_backoff report_status so a crash or reboot between the retry attempt and the PENDING_REPORT write does not lose the status. - Validate the marker contents against the known-good status set (applied/error/running) and discard a malformed marker rather than retrying forever. - Only remove the marker after a successful report_status. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d3b6ae1 commit 46e3c54

1 file changed

Lines changed: 50 additions & 5 deletions

File tree

openwisp-config/files/openwisp.agent

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ REGISTRATION_PARAMETERS="$WORKING_DIR/registration_parameters"
200200
TEST_CHECKSUM="$WORKING_DIR/test_checksum"
201201
UPDATE_INFO="$WORKING_DIR/update_info"
202202
STATUS_REPORT="$WORKING_DIR/status_report"
203+
PENDING_REPORT="$WORKING_DIR/pending_report"
203204
APPLYING_CONF="$WORKING_DIR/applying_conf"
204205
UPDATE_CONFIG_LOG="$WORKING_DIR/update-config.log"
205206
BOOTUP="$WORKING_DIR/bootup"
@@ -825,6 +826,7 @@ update_configuration() {
825826
result=$?
826827
fi
827828

829+
local status_to_report
828830
if [ "$result" -eq "0" ]; then
829831
logger "Configuration applied successfully" \
830832
-t openwisp \
@@ -833,14 +835,24 @@ update_configuration() {
833835
env -i ACTION="config-applied" /sbin/hotplug-call openwisp
834836
# store the new checksum as last known checksum
835837
cp "$CONFIGURATION_CHECKSUM" "$PERSISTENT_CHECKSUM"
836-
retry_with_backoff report_status "applied"
838+
status_to_report="applied"
837839
else
838-
retry_with_backoff report_status "error"
840+
status_to_report="error"
839841
fi
840-
# if reporting of the status fails, let it retry in the next cycle
842+
843+
# Persist the pending status BEFORE calling report_status so that a
844+
# crash or reboot between the retry attempt and success does not lose
845+
# the status. The marker is removed only after report_status succeeds.
846+
printf '%s\n' "$status_to_report" >"$PENDING_REPORT"
847+
848+
retry_with_backoff report_status "$status_to_report"
841849
# shellcheck disable=SC2181
842-
if [ "$?" -ne "0" ]; then
843-
rm -f $CONFIGURATION_CHECKSUM $PERSISTENT_CHECKSUM
850+
if [ "$?" -eq "0" ]; then
851+
rm -f "$PENDING_REPORT"
852+
else
853+
logger -s "report_status failed, will retry in the next cycle" \
854+
-t openwisp \
855+
-p daemon.warning
844856
fi
845857

846858
rm $APPLYING_CONF
@@ -977,6 +989,34 @@ else
977989
env -i ACTION="restart" /sbin/hotplug-call openwisp
978990
fi
979991

992+
retry_pending_report() {
993+
local pending_status
994+
if [ ! -f "$PENDING_REPORT" ]; then
995+
return 0
996+
fi
997+
# Trim whitespace to tolerate an occasional trailing newline.
998+
pending_status=$(tr -d '[:space:]' <"$PENDING_REPORT")
999+
# Validate against the same known-good set accepted by report_status.
1000+
case "$pending_status" in
1001+
applied|error|running) ;;
1002+
*)
1003+
logger -s "Pending report marker has invalid status '$pending_status', discarding" \
1004+
-t openwisp \
1005+
-p daemon.err
1006+
rm -f "$PENDING_REPORT"
1007+
return 1
1008+
;;
1009+
esac
1010+
logger "Retrying pending report_status: $pending_status" \
1011+
-t openwisp \
1012+
-p daemon.info
1013+
retry_with_backoff report_status "$pending_status"
1014+
# shellcheck disable=SC2181
1015+
if [ "$?" -eq "0" ]; then
1016+
rm -f "$PENDING_REPORT"
1017+
fi
1018+
}
1019+
9801020
while true; do
9811021
# check management interface at each iteration
9821022
# (because the address may change due to
@@ -987,6 +1027,11 @@ while true; do
9871027

9881028
if [ "$?" -eq "1" ]; then
9891029
update_configuration
1030+
elif [ -f "$PENDING_REPORT" ]; then
1031+
# Retry a previously failed report_status without re-downloading.
1032+
# This handles the case where the config was applied successfully
1033+
# but report_status failed due to a transient server error.
1034+
retry_pending_report
9901035
fi
9911036
env -i ACTION="end-of-cycle" /sbin/hotplug-call openwisp
9921037

0 commit comments

Comments
 (0)