cf-ddns 1.2.0: correctness & reliability overhaul, Python CI - #2
Merged
Conversation
- replace the legacy unittest suite with pytest-native files: conftest.py (FakeResponse + shared fixtures), test_config.py, test_validation.py, test_api.py, test_logic.py - port only the meaningful tests; every test now exercises production code (tautological and false-API-shape tests deleted, incl. the HTTP-200+81058 test that encoded the H1 misunderstanding) - characterize current behavior before the Stage 2 API rewrite, including the H1 defect (404 retried like a network error) as an explicitly commented current-behavior test - 62 passed, ruff clean, branch coverage 59% -> 66%
- Outcome enum (OK/ABSENT/GONE/EXISTS/AMBIGUOUS/TRANSIENT/PERMANENT); shared _cf_request helper classifies every Cloudflare exchange - error bodies parsed before status handling; 4xx permanent (no retry), 5xx/network transient with backoff+jitter, 429 honors Retry-After (delta or HTTP-date) with a 60s budget cap - record writes switch PUT -> PATCH sending only owned fields (M8); 81044 -> GONE, 81057/81058 -> EXISTS (old dead 81058 check removed) - create POST is non-idempotent: single attempt, uncertain -> TRANSIENT - response schema validation (L7); multiple A records -> AMBIGUOUS (L2) - two persistent sessions: authenticated CF + plain check-IP; bearer token never reaches check-IP hosts (tested) - check-IP responses streamed and capped at 64 bytes (L9) - handle_dns_update fail-closed adapter: create only after confirmed ABSENT, errors never mutate (C1); full decision table lands in stage 3 - live-verified against the real API: 403/10000 classified PERMANENT with zero retries and zero create attempts - 102 passed, ruff clean, branch coverage 66% -> 76%
- DdnsState/HttpClients dataclasses (frozen contract); loop body extracted into run_iteration(config, state, clients) — no sleeping, no exiting inside; startup priming moved to startup_state() - handle_dns_update: full decision table returning (Outcome, record_id); create-EXISTS race adopts the existing record (never a second create); GONE-after-fresh-read defers to the next iteration - symmetric failure policy (H2): enforce_failure_policy() exits on PERMANENT/AMBIGUOUS immediately and on an exhausted consecutive-failure budget for either failure class; new CF_DDNS_MAX_FAILURES (default 10) - main() reduced to setup + loop + sleep + policy enforcement - live-verified: steady-state run performs zero writes; invalid token exits 1 with CRITICAL in 0.2s (was: infinite retry loop) - 125 passed, ruff clean, branch coverage 76% -> 89%
- SIGTERM/SIGINT handlers set a global shutdown event (PID-1 safe; fixes the always-SIGKILL container stop, M1) - interval sleep replaced with interruptible shutdown_event.wait(); the event is wired into retry backoffs and checked before every new HTTP attempt — post-signal, no request starts and no DNS mutation is issued; shutdown-caused failures do not count toward the budget - main() lifecycle: try/finally stops and closes the metrics server (httpd.shutdown/server_close + bounded thread join) and closes both HTTP sessions; KeyboardInterrupt special case removed - verified: SIGTERM during a deliberately hung in-flight read returns in 5.01s (one read timeout); live container `podman stop` completes in 0.36s with clean "Received SIGTERM" logs and exit code 0 - 137 passed, ruff clean, branch coverage 89% -> 93%
- startup reconciliation (M2): ttl/proxied drift vs config forces one converging rewrite; effective TTL normalized to Auto (1) for proxied records so comparisons can never flap - periodic reconciliation (M3): CF_DDNS_RECONCILE_INTERVAL (default 3600s, 0 disables) re-reads the record on a monotonic schedule; converges content/ttl/proxied, adopts external id changes, recreates external deletions; injectable clock for tests - strict config validation (M6): whitespace-only required vars exit; host normalized+validated (lowercase, IDNA, RFC1123 labels — the stdlib idna codec alone passes ASCII junk through); TTL only 1 or 30-86400 (30-59 warns Enterprise); proxied strictly true/false - strict public-unicast IP validation (M7/L1): ipaddress-based, rejects malformed forms and private/loopback/link-local/CGNAT/ unspecified/reserved/broadcast/multicast (is_global alone passes multicast — excluded explicitly) - flap damping (M9/R2): CF_DDNS_CONFIRM_CYCLES (default 2) consecutive identical readings required before a new IP is written, first-run creation included; reconcile/force writes exempt (already-confirmed IP); discarded readings logged + new counter cf_ddns_unconfirmed_ip_readings_total - live-verified (M2): TTL 120->300->120 drift cycle — each restart detected the previous run's write and converged, then steady state - 205 passed, ruff clean, branch coverage 94%
- create_metrics() factory with injectable CollectorRegistry; all
private ._value pokes removed (M5) — labeled children materialized
via .labels(), unlabeled metrics exist at 0 by registration
- cf_ddns_ip_info bounded to a single series (M4): new IP set to 1,
old series remove()d; startup primes it from the DNS record's
confirmed content, never from an unconfirmed one-off reading
- write/change split (R1): new cf_ddns_ip_changes_total increments
only when a known different previous IP is replaced; restart
re-sync, first-run creation and settings rewrites do not count
- last_ip_update_timestamp stays process-local (0 after restart);
provider state exposed separately as
cf_ddns_record_modified_timestamp_seconds (parsed from modified_on)
- cf_ddns_build_info{version} via prometheus Info + __version__ (R5)
- metrics endpoint hardening (L5): CF_DDNS_METRICS_ADDR (validated
IP), bind failure -> CRITICAL + exit 1 instead of a raw traceback
- Dockerfile: EXPOSE 9101 + HEALTHCHECK (liveness only; podman needs
--format docker to keep it, documented in AGENTS.md)
- live-verified: restart-sync scrape shows single ip_info series,
changes=0, updates=0, process timestamp 0, provider modified_on
timestamp set, build_info present; healthcheck goes healthy;
stop 0.69s
- 221 passed, ruff clean, branch coverage 96%
- README rewritten to match the shipped code: confirmed updates, reconciliation, error classification table, AMBIGUOUS runbook, restart-safe Prometheus alerting rules with anti-patterns, hardened compose example, full env-var table, Testing section, Python >= 3.11 - TEST_DOCUMENTATION.md deleted; still-relevant content (how to run tests, test-file map, expected error-log note, CI example) migrated into README; stale claims not carried over - CHANGELOG.md added with the 1.2.0 release notes - AGENTS.md finalized as the agent entry point: architecture, outcome classification, invariants, alerting contract, testing philosophy, release procedure, deployment constraints - new files use 2026 in copyright headers - version bump to 1.2.0 in .version, Dockerfile LABEL, __version__ - final verification: clean rebuild 221 passed / ruff clean; dummy-env smoke exits 1 cleanly without a traceback; live e2e run: healthcheck healthy, steady-state metrics restart-safe, SIGTERM stop 0.69s with exit 0, managed record resolves publicly
- python-projects-matrix composite action: detects top-level Python projects by the requirements-dev.txt marker, mirroring the go-projects-matrix change-range logic (source-changes mode with fallback to all projects on unknown range or .github/ changes; version-changes mode for future release workflows) - python-test.yml: job matrix over affected projects; per project installs pinned requirements(-dev).txt with pip cache, runs ruff check (when present in dev deps), pytest, and shellchecks tracked project scripts — same gate structure as go-test.yml - currently matches only cf-ddns; new Python utilities opt in by adding a requirements-dev.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cf_ddns_ip_changes_totalonly counts real IP changes, so monitoring no longer pages on every restart (README ships ready-made alert rules).run_tests.shreplaced by a Makefile, and a new dynamic Python test workflow mirrors the Go one (projects discovered viarequirements-dev.txt); both CI matrices now re-test everything only when their own pipeline definition changes.