[fix] Retry pending report_status instead of forcing re-download #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) and discard a malformed marker rather than
  retrying forever.
- Only remove the marker after a successful report_status.

Closes #251

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: 2 people

CHANGELOG.rst

@@ -4,7 +4,12 @@ Change log
 1.3.0 [unreleased]
 ------------------
 
-WIP.
+Bugfixes
+~~~~~~~~
+
+- Fixed retrying pending ``report_status`` instead of forcing a
+  re-download after a transient HTTP error `#251
+  <https://github.com/openwisp/openwisp-config/pull/251>`_.
 
 1.2.1 [2026-04-09]
 ------------------

openwisp-config/files/openwisp.agent

@@ -200,6 +200,7 @@ REGISTRATION_PARAMETERS="$WORKING_DIR/registration_parameters"
 TEST_CHECKSUM="$WORKING_DIR/test_checksum"
 UPDATE_INFO="$WORKING_DIR/update_info"
 STATUS_REPORT="$WORKING_DIR/status_report"
+PENDING_REPORT="$WORKING_DIR/pending_report"
 APPLYING_CONF="$WORKING_DIR/applying_conf"
 UPDATE_CONFIG_LOG="$WORKING_DIR/update-config.log"
 BOOTUP="$WORKING_DIR/bootup"
@@ -825,6 +826,7 @@ update_configuration() {
 		result=$?
 	fi
 
+	local status_to_report
 	if [ "$result" -eq "0" ]; then
 		logger "Configuration applied successfully" \
 			-t openwisp \
@@ -833,14 +835,24 @@ update_configuration() {
 		env -i ACTION="config-applied" /sbin/hotplug-call openwisp
 		# store the new checksum as last known checksum
 		cp "$CONFIGURATION_CHECKSUM" "$PERSISTENT_CHECKSUM"
-		retry_with_backoff report_status "applied"
+		status_to_report="applied"
 	else
-		retry_with_backoff report_status "error"
+		status_to_report="error"
 	fi
-	# if reporting of the status fails, let it retry in the next cycle
+
+	# Persist the pending status BEFORE calling report_status so that a
+	# crash or reboot between the retry attempt and success does not lose
+	# the status. The marker is removed only after report_status succeeds.
+	printf '%s\n' "$status_to_report" >"$PENDING_REPORT"
+
+	retry_with_backoff report_status "$status_to_report"
 	# shellcheck disable=SC2181
-	if [ "$?" -ne "0" ]; then
-		rm -f $CONFIGURATION_CHECKSUM $PERSISTENT_CHECKSUM
+	if [ "$?" -eq "0" ]; then
+		rm -f "$PENDING_REPORT"
+	else
+		logger -s "report_status failed, will retry in the next cycle" \
+			-t openwisp \
+			-p daemon.warning
 	fi
 
 	rm $APPLYING_CONF
@@ -977,6 +989,35 @@ else
 	env -i ACTION="restart" /sbin/hotplug-call openwisp
 fi
 
+retry_pending_report() {
+	local pending_status
+	if [ ! -f "$PENDING_REPORT" ]; then
+		return 0
+	fi
+	# Trim whitespace to tolerate an occasional trailing newline.
+	pending_status=$(tr -d '[:space:]' <"$PENDING_REPORT")
+	# Validate against the same known-good set accepted by report_status.
+	# update_configuration() only ever writes "applied" or "error" today.
+	case "$pending_status" in
+		applied | error) ;;
+		*)
+			logger -s "Pending report marker has invalid status '$pending_status', discarding" \
+				-t openwisp \
+				-p daemon.err
+			rm -f "$PENDING_REPORT"
+			return 1
+			;;
+	esac
+	logger "Retrying pending report_status: $pending_status" \
+		-t openwisp \
+		-p daemon.info
+	retry_with_backoff report_status "$pending_status"
+	# shellcheck disable=SC2181
+	if [ "$?" -eq "0" ]; then
+		rm -f "$PENDING_REPORT"
+	fi
+}
+
 while true; do
 	# check management interface at each iteration
 	# (because the address may change due to
@@ -987,6 +1028,11 @@ while true; do
 
 	if [ "$?" -eq "1" ]; then
 		update_configuration
+	elif [ -f "$PENDING_REPORT" ]; then
+		# Retry a previously failed report_status without re-downloading.
+		# This handles the case where the config was applied successfully
+		# but report_status failed due to a transient server error.
+		retry_pending_report
 	fi
 	env -i ACTION="end-of-cycle" /sbin/hotplug-call openwisp