Merge pull request #69 from osg-htc/fix/fasterdata-pacing-dryrun

fix(fasterdata): respect --dry-run for packet pacing; audit checks actual qdisc (v1.3.6)

Author: ShawnMcKee

docs/perfsonar/tools_scripts/CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Fixed
 
 - **Critical JSON state save corruption fix**: Fixed invalid JSON generation in `--save-state` that caused `--restore-state` to fail
+- **Fix (1.3.6)**: Respect `--dry-run` in packet pacing application and ensure audit logic checks actual qdisc state; prevent `--mode apply --dry-run` from changing system qdisc or sysctl state.
   - Properly quote non-numeric ring buffer values (e.g., `Mini:`, `push`, `n/a`)
   - Properly quote non-numeric `nm_mtu` values (e.g., `auto`)
   - Sanitize ring buffer values to ensure numeric-only or properly quoted strings

docs/perfsonar/tools_scripts/fasterdata-tuning.sh

@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 # fasterdata-tuning.sh
 # --------------------
-# Version: 1.3.5
+# Version: 1.3.6
 # Author: Shawn McKee, University of Michigan
 # Acknowledgements: Supported by IRIS-HEP and OSG-LHC
 #
@@ -1271,36 +1271,39 @@ iface_apply() {
 iface_packet_pacing_audit() {
   local iface="$1"
   local pacing_rate="$2"
-  
+
   # Only audit packet pacing for DTN nodes
   if [[ "$TARGET_TYPE" != "dtn" ]]; then
     return 0
   fi
-  
+
   # Check current qdisc - fq enables Linux TCP pacing; tbf indicates explicit rate cap
   local current_qdisc
-  current_qdisc=$(tc qdisc show dev "$iface" 2>/dev/null | head -n1)
-  
-  # If we're not applying pacing, just report current state
-  if [[ $APPLY_PACKET_PACING -eq 0 ]]; then
-    # Accept either fq (TCP pacing) or tbf (explicit cap) as "pacing applied"
-    if [[ "$current_qdisc" == *" fq"* || "$current_qdisc" == fq* || "$current_qdisc" == *" tbf"* ]]; then
-      return 0
-    fi
-    return 1
+  current_qdisc=$(tc qdisc show dev "$iface" 2>/dev/null | head -n1 || echo "")
+
+  # Accept either fq (TCP pacing) or tbf (explicit cap) as "pacing applied"
+  if [[ "$current_qdisc" == *" fq"* || "$current_qdisc" == fq* || "$current_qdisc" == *" tbf"* ]]; then
+    return 0
   fi
-  
-  return 0
+
+  # Not found
+  return 1
 }
 
 iface_apply_packet_pacing() {
   local iface="$1"
   local pacing_rate="$2"
-  
+
+  # Respect DRY_RUN and target type / feature flag
+  if [[ $DRY_RUN -eq 1 ]]; then
+    log_info "Dry-run: would apply packet pacing to $iface (skipped)"
+    return
+  fi
+
   if [[ "$TARGET_TYPE" != "dtn" ]] || [[ $APPLY_PACKET_PACING -eq 0 ]]; then
     return
   fi
-  
+
   # Decide on tbf vs fq: use tbf if explicitly requested or a cap rate is provided; otherwise fq
   if [[ $USE_TBF_CAP -eq 1 || -n "$TBF_CAP_RATE" ]]; then
     # Determine effective rate: explicit cap wins; else compute fraction of iface speed

tests/test-fasterdata-dryrun.sh

@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+# Simple regression test: verify --mode apply --dry-run does not actually apply packet pacing
+set -euo pipefail
+SCRIPT="$(pwd)/docs/perfsonar/tools_scripts/fasterdata-tuning.sh"
+if [[ ! -x "$SCRIPT" ]]; then
+  echo "Script not found or not executable: $SCRIPT" >&2
+  exit 1
+fi
+# Run dry-run apply (may require sudo for access to some commands); expect messages indicating dry-run
+OUT=$(sudo "$SCRIPT" --mode apply --target dtn --apply-packet-pacing --dry-run --yes 2>&1 || true)
+# Check for dry-run markers
+if ! echo "$OUT" | grep -q "Dry-run: would apply packet pacing"; then
+  echo "FAIL: dry-run did not report packet pacing skip" >&2
+  echo "Output was:\n$OUT" >&2
+  exit 2
+fi
+# Ensure summary does not claim pacing was enabled
+if echo "$OUT" | grep -q "Packet pacing: ENABLED"; then
+  echo "FAIL: dry-run unexpectedly shows Packet pacing ENABLED" >&2
+  exit 3
+fi
+# Basic success
+echo "PASS: dry-run packet pacing behaved as expected"