test: add unit test suite and CI for S3 backup sidecar - #24
Merged
Merged
Conversation
Introduce the first automated tests in the repo, covering the sidecar sync logic in src/sidecar/main.py (88% line coverage): - upload_file: DRY_RUN gating, S3 argument contract, missing-client no-op, and exception swallowing - run_sync_cycle: manifest+segment upload, cross-cycle dedup, new segment detection, prefix-collision isolation (match1 vs match10), multi-app isolation, non-directory skipping, and dedup-set pruning Adds pytest config, dev requirements, and a GitHub Actions workflow that runs the suite with coverage on pushes and PRs.
SRS emits both DASH and HLS, but the sidecar only backed up .mpd/.m4s, leaving HLS streams with no disaster-recovery coverage. Generalize run_sync_cycle to scan both packaging formats, keeping segment matching type-aware so a .mpd manifest claims only .m4s segments and a .m3u8 manifest only .ts segments (even when both share a stream-key prefix). The cross-cycle dedup prune now also tracks .ts files so HLS segments aren't re-uploaded every cycle. Adds tests for HLS manifest+segment upload, DASH/HLS coexistence without cross-claiming, and HLS dedup across cycles.
Segments were added to the uploaded_files dedup set *before* the upload ran, and upload_file swallowed all exceptions. A transient S3 error therefore marked a segment permanently uploaded, so it was never retried -- silent gaps in the backup. upload_file now returns success/failure, and run_sync_cycle records a segment as uploaded only after its upload actually succeeds; failures are retried on the next cycle. Adds a regression test plus return-value assertions for the DRY_RUN, no-client, and exception paths.
get_playback_urls returned bare URLs, but the nginx edge enforces secure_link and returned 403 for every playback request -- streams never played. Add streaming_console.secure_link (Frappe-free, unit-tested) to mint nginx-compatible tokens and have get_playback_urls append token/expires to the HLS and DASH URLs. The secure_link hash is bound only to the expiry, not $uri or $remote_addr: segmented-media players don't carry the manifest's per-URI signature onto segment requests, so one token must authorise the whole session, and dropping $remote_addr keeps links valid behind a CDN and across client IP changes. - nginx.conf: secure_link_md5 now hashes "$secure_link_expires SECRET" - docker-compose: pass SECRET_KEY to control-plane so it matches the edge - studio/stream.html: re-apply the token to video.js segment requests - tests: secure_link suite with an nginx golden-vector token; wire the control-plane package into pytest with scoped coverage
SRS can fire on_publish more than once per session (reconnects, retries) while on_unpublish fires once, so current_load ratcheted upward and never returned to zero -- eventually get_best_node saw every node at capacity and threw 'No capacity available' on an idle fleet. - on_publish now only transitions a stream into Live (and increments load) on the first call; repeat calls are a no-op. - on_unpublish only decrements on the first transition out of Live. - Both hooks share _adjust_node_load(), a single atomic UPDATE clamped with GREATEST(0, ...), replacing the inconsistent raw-SQL increment vs get_doc().save() decrement.
STOP_EVENT existed but nothing ever set it, so a container stop killed the process (and any in-flight uploads) immediately. Register SIGTERM/ SIGINT handlers that set STOP_EVENT, and drain the executor on exit so in-progress uploads finish before the process leaves. Adds tests for the signal handler and the loop's stop-and-drain path.
- Add healthchecks for redis, timescaledb, control-plane, and nginx. - Gate startup on readiness: control-plane waits for redis/timescaledb to be healthy, and srs waits for the control-plane to be healthy so the first on_publish webhook isn't a connection-refused. - .env.example: replace the shippable-looking default SECRET_KEY with an obvious change_me placeholder and document how to generate one.
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
2 participants
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.
Introduce the first automated tests in the repo, covering the sidecar
sync logic in src/sidecar/main.py (88% line coverage):
no-op, and exception swallowing
segment detection, prefix-collision isolation (match1 vs match10),
multi-app isolation, non-directory skipping, and dedup-set pruning
Adds pytest config, dev requirements, and a GitHub Actions workflow
that runs the suite with coverage on pushes and PRs.