feat: Add rotating capture support (--file-count) for long-running packet captures - #2527
Conversation
cf0c605 to
2664592
Compare
Retina Code Coverage ReportTotal coverage increased from
|
| Impacted Files | Coverage | |
|---|---|---|
| pkg/controllers/operator/retinaendpoint/retinaendpoint_controller.go | 82.25% ... 83.28% (1.03%) |
⬆️ |
| pkg/capture/provider/network_capture_unix.go | 33.97% ... 42.18% (8.21%) |
⬆️ |
| cli/cmd/capture/create.go | 71.76% ... 73.65% (1.89%) |
⬆️ |
| pkg/capture/crd_to_job.go | 86.15% ... 86.96% (0.81%) |
⬆️ |
| pkg/capture/capture_manager.go | 48.73% ... 67.24% (18.51%) |
⬆️ |
…crosoft/retina into carlota/rollover-file-capture
There was a problem hiding this comment.
Pull request overview
Adds first-class support for long-running rotating packet captures by introducing a fileCount/--file-count option that uses tcpdump’s native -C (rotate by size) + -W (rotate count) behavior to maintain a bounded, circular buffer of recent traffic. This extends Retina’s capture system across CRD, CLI, capture manager, and provider layers, while keeping existing behavior unchanged when the new option is unset.
Changes:
- Add
fileCountto the Capture CRD + Helm CRD manifest, wire it through job env (CAPTURE_FILE_COUNT), and validate it requiresmaxCaptureSize. - Extend capture provider interface to accept
fileCount; implement rotation behavior on Linux (tcpdump) and accept-but-ignore on Windows (netsh limitation). - Improve capture workload shutdown behavior by separating capture cancellation (SIGTERM) from output/upload context so uploads can finish after capture stops.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/capture/node-rotating-capture.yaml | New sample manifest demonstrating rotating capture configuration. |
| pkg/capture/provider/network_capture_win.go | Updates provider signature to accept fileCount (ignored on Windows). |
| pkg/capture/provider/network_capture_unix.go | Adds tcpdump -C/-W rotation support and skips manual size polling when rotating. |
| pkg/capture/provider/network_capture_test.go | Updates tests for new provider signature and adds tcpdump arg construction tests. |
| pkg/capture/provider/mock_network_capture.go | Regenerates/updates mock for the provider interface signature change. |
| pkg/capture/provider/interface.go | Extends provider interface to include fileCount. |
| pkg/capture/crd_to_job.go | Adds CRD validation (fileCount requires maxCaptureSize) and passes env var to workload. |
| pkg/capture/crd_to_job_test.go | Adds/updates tests for env propagation and validation of fileCount behavior. |
| pkg/capture/constants/job_env.go | Introduces CAPTURE_FILE_COUNT env key constant. |
| pkg/capture/capture_manager.go | Parses fileCount/duration env updates and passes fileCount through to provider. |
| pkg/capture/capture_manager_test.go | Adds tests for duration/fileCount env parsing and rotated file archiving. |
| docs/05-Concepts/CRDs/Capture.md | Documents rotating capture usage for CRD users. |
| docs/04-Captures/03-crd.md | Adds CRD-based rotating capture example and stop instructions. |
| docs/04-Captures/02-cli.md | Documents --file-count and adds rotating capture CLI examples. |
| docs/04-Captures/01-overview.md | Mentions rotating capture support and links to detailed docs. |
| deploy/standard/manifests/controller/helm/retina/crds/retina.sh_captures.yaml | Updates rendered CRD schema with fileCount + description/validation. |
| crd/api/v1alpha1/zz_generated.deepcopy.go | Updates deepcopy generation for new CaptureOption.FileCount field. |
| crd/api/v1alpha1/capture_types.go | Adds FileCount to CaptureOption with kubebuilder validation and docs. |
| cli/cmd/capture/create.go | Adds --file-count flag and CLI-side validation; populates CRD field. |
| cli/cmd/capture/capture.go | Adds fileCount to CLI options struct. |
| captureworkload/main.go | Splits contexts so SIGTERM stops capture without canceling output/upload operations. |
Files not reviewed (2)
- crd/api/v1alpha1/zz_generated.deepcopy.go: Generated file
- pkg/capture/provider/mock_network_capture.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…crosoft/retina into carlota/rollover-file-capture
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- crd/api/v1alpha1/zz_generated.deepcopy.go: Generated file
- pkg/capture/provider/mock_network_capture.go: Generated file
…cket captures (microsoft#2527) # Description Adds rotating capture support to Retina's packet capture system using tcpdump's `-W` (file count) and `-C` (file size) flags. This enables long-running captures that maintain a circular buffer of recent network traffic without unbounded disk growth. When `--file-count` is specified alongside `--max-size`, tcpdump rotates through a fixed number of capture files, overwriting the oldest when the limit is reached. For example, `--file-count 3 --max-size 100` keeps approximately the last 300MB of network traffic across 3 files. ### Key changes: - **CRD**: Added `fileCount` field to `CaptureOption` with kubebuilder validation (minimum 1) - **CLI**: Added `--file-count` flag with client-side validation (requires `--max-size`) - **Provider (Linux)**: When `fileCount > 0`, appends `-C` and `-W` to tcpdump and skips manual file-size polling - **Provider (Windows)**: Signature updated; rotating capture is unsupported on Windows (netsh limitation) - **Capture Manager**: Added `captureFileCount()` env parsing; fixed `captureDuration()` to handle empty string (for captures without duration) - **Capture Workload**: Split context into `captureCtx` (cancelled on SIGTERM) and `outputCtx` (fresh, for blob/S3 uploads after capture stops) - **Validation**: `fileCount` requires `maxCaptureSize`; captures with `fileCount` don't require `duration` (can be stopped manually) ## Related Issue N/A — feature request for long-running capture support with bounded disk usage. ## Checklist - [x] I have read the [contributing documentation](https://retina.sh/docs/Contributing/overview). - [x] I signed and signed-off the commits (`git commit -S -s ...`). - [x] I have correctly attributed the author(s) of the code. - [x] I have tested the changes locally. - [x] I have followed the project's style guidelines. - [x] I have updated the documentation, if necessary. - [x] I have added tests, if applicable. ## Screenshots (if applicable) or Testing Completed ### Unit Tests All new logic is covered: | Test | What it verifies | |------|-----------------| | `TestTcpdumpRotatingCaptureArgs` | `-C`/`-W` flag construction (5 subtests: with/without fileCount and maxSize) | | `TestCaptureFileCount` | `captureFileCount()` env parsing (empty, valid, invalid) | | `TestCaptureDuration` | `captureDuration()` returns 0 for empty string | | `TestCaptureNetworkWithRotatingCapture` | CaptureManager passes fileCount to provider | | `TestCaptureNetworkWithNoDuration` | CaptureManager handles missing duration env | | `TestCompressFolderToTarGzIncludesRotatedFiles` | Tar archive includes all `.pcap0`, `.pcap1`, etc. files | | `TestCompressFolderToTarGzPreservesContent` | File content preserved through compression | | `Test_CaptureToPodTranslator_ValidateCapture` | fileCount validation (3 new subtests) | | `Test_CaptureToPodTranslator_ObtainCaptureJobPodEnv` | CAPTURE_FILE_COUNT env var passed to job | ### Live Cluster E2E (manual) Tested on AKS cluster with custom image: - **Rotating capture (5 min, 3 files x 1MB)**: tcpdump ran with `-C 1 -W 3`. After 5 minutes, downloaded tar.gz contained 3 pcap files (`.pcap0`, `.pcap1`, `.pcap2`). Timestamps confirmed rotation — the 5-minute capture only retained the last ~15 seconds of traffic, proving the circular buffer overwrote earlier data correctly. - **Standard capture (backward compat)**: Capture without `--file-count` works identically to before. - **CLI validation**: `--file-count=-1` rejected; `--file-count` without `--max-size` rejected. ## Additional Notes - **Backward compatible**: `fileCount` defaults to 0/nil, preserving existing behavior exactly. - **Windows**: The `fileCount` parameter is accepted in the interface but has no effect on Windows (netsh doesn't support file rotation). This matches the existing pattern where `maxCaptureSize` is also Linux-only. - **Context fix in captureworkload/main.go**: This fixes a pre-existing latent bug where SIGTERM cancellation would also cancel blob/S3 uploads. For rotating captures this becomes critical since SIGTERM is the expected stop mechanism.
…cket captures (microsoft#2527) # Description Adds rotating capture support to Retina's packet capture system using tcpdump's `-W` (file count) and `-C` (file size) flags. This enables long-running captures that maintain a circular buffer of recent network traffic without unbounded disk growth. When `--file-count` is specified alongside `--max-size`, tcpdump rotates through a fixed number of capture files, overwriting the oldest when the limit is reached. For example, `--file-count 3 --max-size 100` keeps approximately the last 300MB of network traffic across 3 files. ### Key changes: - **CRD**: Added `fileCount` field to `CaptureOption` with kubebuilder validation (minimum 1) - **CLI**: Added `--file-count` flag with client-side validation (requires `--max-size`) - **Provider (Linux)**: When `fileCount > 0`, appends `-C` and `-W` to tcpdump and skips manual file-size polling - **Provider (Windows)**: Signature updated; rotating capture is unsupported on Windows (netsh limitation) - **Capture Manager**: Added `captureFileCount()` env parsing; fixed `captureDuration()` to handle empty string (for captures without duration) - **Capture Workload**: Split context into `captureCtx` (cancelled on SIGTERM) and `outputCtx` (fresh, for blob/S3 uploads after capture stops) - **Validation**: `fileCount` requires `maxCaptureSize`; captures with `fileCount` don't require `duration` (can be stopped manually) ## Related Issue N/A — feature request for long-running capture support with bounded disk usage. ## Checklist - [x] I have read the [contributing documentation](https://retina.sh/docs/Contributing/overview). - [x] I signed and signed-off the commits (`git commit -S -s ...`). - [x] I have correctly attributed the author(s) of the code. - [x] I have tested the changes locally. - [x] I have followed the project's style guidelines. - [x] I have updated the documentation, if necessary. - [x] I have added tests, if applicable. ## Screenshots (if applicable) or Testing Completed ### Unit Tests All new logic is covered: | Test | What it verifies | |------|-----------------| | `TestTcpdumpRotatingCaptureArgs` | `-C`/`-W` flag construction (5 subtests: with/without fileCount and maxSize) | | `TestCaptureFileCount` | `captureFileCount()` env parsing (empty, valid, invalid) | | `TestCaptureDuration` | `captureDuration()` returns 0 for empty string | | `TestCaptureNetworkWithRotatingCapture` | CaptureManager passes fileCount to provider | | `TestCaptureNetworkWithNoDuration` | CaptureManager handles missing duration env | | `TestCompressFolderToTarGzIncludesRotatedFiles` | Tar archive includes all `.pcap0`, `.pcap1`, etc. files | | `TestCompressFolderToTarGzPreservesContent` | File content preserved through compression | | `Test_CaptureToPodTranslator_ValidateCapture` | fileCount validation (3 new subtests) | | `Test_CaptureToPodTranslator_ObtainCaptureJobPodEnv` | CAPTURE_FILE_COUNT env var passed to job | ### Live Cluster E2E (manual) Tested on AKS cluster with custom image: - **Rotating capture (5 min, 3 files x 1MB)**: tcpdump ran with `-C 1 -W 3`. After 5 minutes, downloaded tar.gz contained 3 pcap files (`.pcap0`, `.pcap1`, `.pcap2`). Timestamps confirmed rotation — the 5-minute capture only retained the last ~15 seconds of traffic, proving the circular buffer overwrote earlier data correctly. - **Standard capture (backward compat)**: Capture without `--file-count` works identically to before. - **CLI validation**: `--file-count=-1` rejected; `--file-count` without `--max-size` rejected. ## Additional Notes - **Backward compatible**: `fileCount` defaults to 0/nil, preserving existing behavior exactly. - **Windows**: The `fileCount` parameter is accepted in the interface but has no effect on Windows (netsh doesn't support file rotation). This matches the existing pattern where `maxCaptureSize` is also Linux-only. - **Context fix in captureworkload/main.go**: This fixes a pre-existing latent bug where SIGTERM cancellation would also cancel blob/S3 uploads. For rotating captures this becomes critical since SIGTERM is the expected stop mechanism.
Description
Adds rotating capture support to Retina's packet capture system using tcpdump's
-W(file count) and-C(file size) flags. This enables long-running captures that maintain a circular buffer of recent network traffic without unbounded disk growth.When
--file-countis specified alongside--max-size, tcpdump rotates through a fixed number of capture files, overwriting the oldest when the limit is reached. For example,--file-count 3 --max-size 100keeps approximately the last 300MB of network traffic across 3 files.Key changes:
fileCountfield toCaptureOptionwith kubebuilder validation (minimum 1)--file-countflag with client-side validation (requires--max-size)fileCount > 0, appends-Cand-Wto tcpdump and skips manual file-size pollingcaptureFileCount()env parsing; fixedcaptureDuration()to handle empty string (for captures without duration)captureCtx(cancelled on SIGTERM) andoutputCtx(fresh, for blob/S3 uploads after capture stops)fileCountrequiresmaxCaptureSize; captures withfileCountdon't requireduration(can be stopped manually)Related Issue
N/A — feature request for long-running capture support with bounded disk usage.
Checklist
git commit -S -s ...).Screenshots (if applicable) or Testing Completed
Unit Tests
All new logic is covered:
TestTcpdumpRotatingCaptureArgs-C/-Wflag construction (5 subtests: with/without fileCount and maxSize)TestCaptureFileCountcaptureFileCount()env parsing (empty, valid, invalid)TestCaptureDurationcaptureDuration()returns 0 for empty stringTestCaptureNetworkWithRotatingCaptureTestCaptureNetworkWithNoDurationTestCompressFolderToTarGzIncludesRotatedFiles.pcap0,.pcap1, etc. filesTestCompressFolderToTarGzPreservesContentTest_CaptureToPodTranslator_ValidateCaptureTest_CaptureToPodTranslator_ObtainCaptureJobPodEnvLive Cluster E2E (manual)
Tested on AKS cluster with custom image:
-C 1 -W 3. After 5 minutes, downloaded tar.gz contained 3 pcap files (.pcap0,.pcap1,.pcap2). Timestamps confirmed rotation — the 5-minute capture only retained the last ~15 seconds of traffic, proving the circular buffer overwrote earlier data correctly.--file-countworks identically to before.--file-count=-1rejected;--file-countwithout--max-sizerejected.Additional Notes
fileCountdefaults to 0/nil, preserving existing behavior exactly.fileCountparameter is accepted in the interface but has no effect on Windows (netsh doesn't support file rotation). This matches the existing pattern wheremaxCaptureSizeis also Linux-only.