diff --git a/DESIGN_DOCUMENT.md b/DESIGN_DOCUMENT.md index 74c6229..76d31f6 100644 --- a/DESIGN_DOCUMENT.md +++ b/DESIGN_DOCUMENT.md @@ -145,11 +145,37 @@ spec: name: namespace: default # optional, defaults to PipelineInstance namespace - # sourceRef references the PipelineSource for input configuration + # SOURCE BINDING (PLAT-1071): exactly one of `sourceRef` or `sources` + # must be set. Enforced by an admission-time XValidation rule on the + # spec; setting both, or neither, is rejected by the kube API server. + + # sourceRef (legacy, single-source): references the PipelineSource for + # input configuration. The source URL is broadcast to every filter + # container as RTSP_URL (streaming) or as a single VIDEO_INPUT_PATH + # download target (batch). Existing single-source CRs continue to apply + # unchanged. sourceRef: name: namespace: default # optional, defaults to PipelineInstance namespace + # sources (multi-source): each entry binds a PipelineSource to a + # specific filter container by name. The controller injects the + # source-derived env vars into ONLY the matching container, enabling + # multi-camera pipelines where each VideoIn filter consumes a distinct + # source. Supported in both streaming (RTSP_URL per container) and + # batch (per-binding direct-mode init claimer + VIDEO_INPUT_PATH per + # container). See section 5.4 for the per-container env injection + # contract; section 5.5 covers the batch-specific constraints + # (every binding's PipelineSource Bucket.Prefix must be a FULL object + # key so the direct-mode claimer has a deterministic download). + sources: + - filterName: front-cam + sourceRef: + name: pipelinesource-front + - filterName: back-cam + sourceRef: + name: pipelinesource-back + # execution defines how the pipeline should be executed execution: parallelism: 10 # max concurrent pods (default: 10) @@ -370,6 +396,104 @@ The claimer is a standalone Go binary (`cmd/claimer/main.go`) that runs as an in - On success: all filters exit 0 → Pod Succeeded - Filters should be idempotent (at-least-once delivery semantics) +### 5.4 Multi-source contract (PLAT-1071) + +A PipelineInstance with `Spec.Sources` bound to N filter containers runs +all of them in the **same Pod** (streaming mode: one Deployment, one +replica, N+M containers; batch mode: one Job with a single Pod, one init +claimer per binding — see 5.5). Per-container source plumbing: + +- The reconciler normalizes `Spec.SourceRef` (legacy) and `Spec.Sources` + into a uniform `[]ResolvedSourceBinding` via + `PipelineInstanceSpec.EffectiveSources()`. Legacy `SourceRef` becomes + one entry with `FilterName == ""` — the broadcast sentinel — and the + source env vars apply to every container in the pod (today's behavior). +- For each entry where `FilterName != ""`, the controller injects + `RTSP_URL` (and, when credentials are present, `_RTSP_USERNAME` / + `_RTSP_PASSWORD` from the referenced Secret) into ONLY the container + whose `pipeline.spec.filters[].name` matches `FilterName`. Downstream + filters (no matching binding) receive no source env vars and wire to + upstream siblings through their own `sources: tcp://localhost:...` + filter config. +- Every non-empty `sources[].filterName` must match a + `pipeline.spec.filters[].name` exactly. An unmatched binding would be a + silent no-op (streaming: the source env never lands on any container; + batch: the claimer downloads a file no filter reads), so the reconciler + rejects it with a `Degraded` condition (reason `SourceBindingUnmatched`) + whose message lists the unmatched and available filter names. The + condition clears automatically once the bindings validate. +- **Idle timeout anchoring (streaming, V1)**: multi-source instances apply + the idle-timeout policy from `sourceBindings[0]` — the first `sources[]` + entry — ONLY. This is deliberate: the semantic of "ANY source idle vs + ALL sources idle" is not yet defined for multi-camera (see the Step 3 + comment in `reconcileStreaming`). An `idleTimeout` set on any other + binding's source is silently ignored; operators who need per-source idle + budgets should author one PipelineInstance per source today. +- Pipeline authoring constraints the user must satisfy when emitting a + multi-VideoIn Pipeline: + - **Stagger output ports by 2.** OpenFilter binds the configured port + AND `port + 1` per `outputs` (the +1 carries ZMQ control-plane + traffic). For N VideoIn filters use 5550, 5552, 5554, … — never + 5550, 5551. + - **Tag every VideoIn output topic explicitly** via the `;topic` + suffix in its `sources` config. Without it every VideoIn publishes + to the default topic `main` and any downstream that fans-in two + upstreams crashes with `RuntimeError: duplicate topic 'main'`. Use + the filter name as the topic for determinism. + + Neither constraint is enforced by the controller — they live in the + user-authored `filter.config` strings. See the + `pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml` sample + for a complete worked example. + + Note on ownership (deviation from the original PLAT-1141 draft): the + ticket sketched the controller injecting `FILTER_SOURCES` and a + deterministic port scheme (5550 + index). The implementation instead + treats ports, topics, and downstream `sources` wiring as + **authoring-time** concerns — baked into the Pipeline CRD by the + plainsight-api export (or by hand, per the samples) — and the + controller only injects the per-binding env (`RTSP_URL` / + `VIDEO_INPUT_PATH`) into the matching container. The pivot was + validated end-to-end with multi-camera runs (see PR #74). + +### 5.5 Multi-source batch + +Batch mode supports both single-source (the legacy queue-based +parallel-files-per-bucket flow, unchanged) and multi-source bindings +(one Pod, N init claimers, N VideoIns). + +The reconciler branches on `len(Sources)` at the top of +`reconcileBatch`: + +- **Single-source**: existing flow. List bucket, enqueue files in + Valkey, parallel Pods each pop one file via the init claimer, run + the filter chain. + +- **Multi-source** (`reconcileBatchMultiSource`): no Valkey work + queue, no bucket listing. One init claimer per binding in **direct + mode** (S3_OBJECT_KEY env tells the claimer to skip Valkey and + download exactly that object), each writing to + `/ws/.`. Filter containers run as a chain in the + same Pod with VIDEO_INPUT_PATH injected on each matching VideoIn + container. Job is `parallelism=1, completions=1` and completion is + the Job's own Succeeded/Failed status (no per-file accounting). + +Constraints on multi-source batch: + +- Every binding's PipelineSource must be a Bucket source — RTSP makes + no sense for batch. +- Every binding's `Bucket.Prefix` must name a specific S3 object key + (the platform's media model carries single-file keys in prefix). The + default queue-based flow (list a prefix and enqueue every file under + it) doesn't apply here: the user has named exactly which media goes + to which VideoIn, so each claimer needs a deterministic key. + +Both constraints surface as Degraded conditions +(`MultiSourceBatchInvalidSource` / `MultiSourceBatchMissingObject`) +with operator-actionable messages. A controller watch on PipelineSource +re-triggers reconcile when a referenced source changes, and the +Degraded condition clears automatically once validation passes. + ## 6. Controller Responsibilities The PipelineInstance controller (`internal/controller/pipelineinstance_controller.go`) is the orchestrator for all pipeline execution logic. diff --git a/api/v1alpha1/pipeline_types.go b/api/v1alpha1/pipeline_types.go index b8ea150..b49876b 100644 --- a/api/v1alpha1/pipeline_types.go +++ b/api/v1alpha1/pipeline_types.go @@ -47,6 +47,19 @@ type BucketSource struct { // +optional Prefix string `json:"prefix,omitempty"` + // singleObject declares that prefix is a FULL object key naming exactly + // one file, not a prefix to scan. Batch instances whose every binding is + // a single-object bucket run in direct-download mode (one init claimer + // per binding, no Valkey work queue) — the same path multi-source + // instances always use — preserving the object's file extension in the + // per-filter input path (queue mode downloads to a fixed input.mp4, + // which breaks extension-sensitive filters like image-in). The + // deployment agent sets this for single-file media kinds + // (external_video, external_image, uploads); hand-authored CRs without + // it keep the queue-based prefix-scan flow. + // +optional + SingleObject bool `json:"singleObject,omitempty"` + // endpoint is the S3-compatible endpoint URL (required for non-AWS S3) // Examples: // - MinIO: "http://minio.example.com:9000" diff --git a/api/v1alpha1/pipelineinstance_types.go b/api/v1alpha1/pipelineinstance_types.go index a741330..2b8cf31 100644 --- a/api/v1alpha1/pipelineinstance_types.go +++ b/api/v1alpha1/pipelineinstance_types.go @@ -42,15 +42,54 @@ type ExecutionConfig struct { PendingTimeout *metav1.Duration `json:"pendingTimeout,omitempty"` } -// PipelineInstanceSpec defines the desired state of PipelineInstance +// PipelineInstanceSpec defines the desired state of PipelineInstance. +// +// Source binding (PLAT-1071): a PipelineInstance binds one or more +// PipelineSources to specific filter containers in the referenced Pipeline. +// Exactly one of the following must be set (enforced by the XValidation +// rule on this type): +// +// - sourceRef (single-source convenience form): the bound source URL is +// broadcast to every filter container as RTSP_URL (streaming) or as a +// single VIDEO_INPUT_PATH download target (batch). The natural shape +// for one-camera pipelines — no filter names required. +// +// - sources (multi-source): each entry binds a PipelineSource to the +// filter container whose name matches `filterName`. The controller +// injects the source-derived env vars (RTSP_URL / VIDEO_INPUT_PATH) +// into ONLY that container. Multiple bindings produce multiple +// per-container source URLs, enabling multi-camera pipelines where +// each VideoIn filter consumes a distinct source. +// +// Setting both fields, or neither, is a validation error rejected at +// admission time. +// +kubebuilder:validation:XValidation:rule="(has(self.sourceRef) && !has(self.sources)) || (!has(self.sourceRef) && has(self.sources) && size(self.sources) > 0)",message="exactly one of `sourceRef` or `sources` must be set (sources must be non-empty)" type PipelineInstanceSpec struct { // pipelineRef references the Pipeline resource to execute // +required PipelineRef PipelineReference `json:"pipelineRef"` - // sourceRef references the PipelineSource resource for input configuration - // +required - SourceRef SourceReference `json:"sourceRef"` + // sourceRef references the PipelineSource resource for input + // configuration. This is the convenience form for single-source + // pipelines — the source URL is broadcast to every filter container, + // so the author doesn't need to know filter names. For multi-source + // pipelines, use `sources` instead. Both forms are first-class and + // supported indefinitely. + // +optional + SourceRef *SourceReference `json:"sourceRef,omitempty"` + + // sources binds one or more PipelineSources to specific filter + // containers by name. Each entry targets the container whose + // `pipeline.spec.filters[].name` matches `filterName`; the controller + // injects the source-derived env vars into only that container. Use + // for multi-camera pipelines where each VideoIn filter consumes a + // distinct source. + // + // Exactly one of `sourceRef` or `sources` must be set. + // +optional + // +listType=map + // +listMapKey=filterName + Sources []NamedSourceRef `json:"sources,omitempty"` // execution defines how the pipeline should be executed // +optional @@ -99,6 +138,50 @@ type SourceReference struct { Namespace *string `json:"namespace,omitempty"` } +// NamedSourceRef binds a PipelineSource to a specific filter container by +// name. The controller injects the source-derived env vars (RTSP_URL for +// streaming, VIDEO_INPUT_PATH for batch) into only the container whose +// `pipeline.spec.filters[].name` matches `filterName`. This lets a single +// PipelineInstance run a multi-camera pipeline where each VideoIn filter +// pulls a distinct source. +type NamedSourceRef struct { + // filterName names the filter container in the referenced Pipeline + // that consumes this source. Must match a `pipeline.spec.filters[].name` + // value exactly. Constrained to DNS-1123 label rules (lowercase + // alphanum + hyphens, max 63 chars) so it can safely flow into + // generated container / volume / env names. + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$` + FilterName string `json:"filterName"` + + // sourceRef references the PipelineSource resource that provides the + // input for the filter named by `filterName`. + // +required + SourceRef SourceReference `json:"sourceRef"` +} + +// EffectiveSources returns the canonical list of (filterName, source) bindings +// for this PipelineInstance, normalizing the singular `SourceRef` field into +// the plural shape. When `SourceRef` is set, the returned list has a single entry +// with an empty FilterName — a sentinel that means "broadcast to every +// container" in the reconciler's per-binding loop. When `Sources` is set, the +// returned list is the user's bindings verbatim. Validation at admission time +// guarantees exactly one of the two is set. +func (s *PipelineInstanceSpec) EffectiveSources() []NamedSourceRef { + if len(s.Sources) > 0 { + return s.Sources + } + if s.SourceRef != nil { + return []NamedSourceRef{{ + FilterName: "", + SourceRef: *s.SourceRef, + }} + } + return nil +} + // PipelineInstanceStatus defines the observed state of PipelineInstance. type PipelineInstanceStatus struct { // counts tracks the number of files in each state (Batch mode only) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2b63de2..e64b8f3 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -148,6 +148,22 @@ func (in *Filter) DeepCopy() *Filter { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NamedSourceRef) DeepCopyInto(out *NamedSourceRef) { + *out = *in + in.SourceRef.DeepCopyInto(&out.SourceRef) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamedSourceRef. +func (in *NamedSourceRef) DeepCopy() *NamedSourceRef { + if in == nil { + return nil + } + out := new(NamedSourceRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Pipeline) DeepCopyInto(out *Pipeline) { *out = *in @@ -238,7 +254,18 @@ func (in *PipelineInstanceList) DeepCopyObject() runtime.Object { func (in *PipelineInstanceSpec) DeepCopyInto(out *PipelineInstanceSpec) { *out = *in in.PipelineRef.DeepCopyInto(&out.PipelineRef) - in.SourceRef.DeepCopyInto(&out.SourceRef) + if in.SourceRef != nil { + in, out := &in.SourceRef, &out.SourceRef + *out = new(SourceReference) + (*in).DeepCopyInto(*out) + } + if in.Sources != nil { + in, out := &in.Sources, &out.Sources + *out = make([]NamedSourceRef, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Execution != nil { in, out := &in.Execution, &out.Execution *out = new(ExecutionConfig) diff --git a/cmd/claimer/main.go b/cmd/claimer/main.go index 16252b5..821512f 100644 --- a/cmd/claimer/main.go +++ b/cmd/claimer/main.go @@ -56,6 +56,12 @@ const ( EnvS3PathStyle = "S3_USE_PATH_STYLE" EnvS3SkipTLS = "S3_INSECURE_SKIP_TLS_VERIFY" EnvVideoInput = "VIDEO_INPUT_PATH" + // EnvS3ObjectKey selects "direct download" mode for the multi-source + // batch path (PLAT-1071). When set, the claimer skips Valkey and + // downloads exactly this S3 object key to VIDEO_INPUT_PATH, then + // exits. Used by the per-VideoIn init claimer the batch reconciler + // emits for multi-source bindings. + EnvS3ObjectKey = "S3_OBJECT_KEY" // Volume mount paths defaultInputPath = "/ws/input.mp4" @@ -91,6 +97,24 @@ func run() error { return fmt.Errorf("failed to load config: %w", err) } + // Direct-download mode (PLAT-1071): when S3_OBJECT_KEY is set, the + // caller has named a specific object the claimer must download. This + // is the multi-source batch path — one init claimer per VideoIn + // binding, each pointing at its bound media's object. No Valkey + // work-queue is involved. + if cfg.S3ObjectKey != "" { + log.Printf("Direct-download mode: bucket=%s, key=%s, dest=%s", cfg.S3Bucket, cfg.S3ObjectKey, cfg.VideoInputPath) + minioClient, err := createMinIOClient(cfg) + if err != nil { + return fmt.Errorf("failed to create MinIO client: %w", err) + } + if err := downloadFile(ctx, minioClient, cfg.S3Bucket, cfg.S3ObjectKey, cfg.VideoInputPath); err != nil { + return fmt.Errorf("failed to download file: %w", err) + } + log.Printf("Downloaded %s to %s", cfg.S3ObjectKey, cfg.VideoInputPath) + return nil + } + // Create Valkey client (with retry for outages during startup) var valkeyClient valkey.Client clientBackoff := newExponentialBackoff(30 * time.Second) @@ -163,6 +187,9 @@ type Config struct { S3UsePathStyle bool S3SkipTLS bool VideoInputPath string + // S3ObjectKey selects direct-download mode (PLAT-1071). Empty in the + // legacy queue-based path. + S3ObjectKey string } func loadConfig() (*Config, error) { @@ -186,18 +213,22 @@ func loadConfig() (*Config, error) { } return defaultInputPath }(), + S3ObjectKey: os.Getenv(EnvS3ObjectKey), } // Parse boolean flags cfg.S3UsePathStyle = getEnvOrDefault(EnvS3PathStyle, "false") == "true" cfg.S3SkipTLS = getEnvOrDefault(EnvS3SkipTLS, "false") == "true" - // Validate required fields - if cfg.Stream == "" { - return nil, fmt.Errorf("STREAM is required") - } - if cfg.Group == "" { - return nil, fmt.Errorf("GROUP is required") + // STREAM / GROUP are required only in queue-based mode. In direct + // mode (S3_OBJECT_KEY set) the claimer skips Valkey entirely. + if cfg.S3ObjectKey == "" { + if cfg.Stream == "" { + return nil, fmt.Errorf("STREAM is required") + } + if cfg.Group == "" { + return nil, fmt.Errorf("GROUP is required") + } } // POD_NAME and POD_NAMESPACE are optional; retained for diagnostics only. if cfg.S3Bucket == "" { diff --git a/cmd/claimer/main_test.go b/cmd/claimer/main_test.go index bfbd00e..35644ee 100644 --- a/cmd/claimer/main_test.go +++ b/cmd/claimer/main_test.go @@ -55,6 +55,70 @@ func TestExponentialBackoff(t *testing.T) { } } +// TestLoadConfig_DirectMode covers the PLAT-1071 direct-download mode +// shape on loadConfig: when S3_OBJECT_KEY is set the Stream/Group env +// requirement is lifted (because the claimer skips Valkey), and the +// resulting Config carries the object key for run() to dispatch on. +func TestLoadConfig_DirectMode(t *testing.T) { + // Required for every mode. + t.Setenv("S3_BUCKET", "test-bucket") + + t.Run("direct_mode_no_stream_or_group_required", func(t *testing.T) { + t.Setenv("S3_OBJECT_KEY", "media/front.mp4") + // Deliberately do NOT set STREAM or GROUP. + t.Setenv("STREAM", "") + t.Setenv("GROUP", "") + cfg, err := loadConfig() + if err != nil { + t.Fatalf("direct mode must not require STREAM/GROUP, got error: %v", err) + } + if cfg.S3ObjectKey != "media/front.mp4" { + t.Errorf("S3ObjectKey = %q, want %q", cfg.S3ObjectKey, "media/front.mp4") + } + }) + + t.Run("queue_mode_still_requires_stream", func(t *testing.T) { + // Leaving S3_OBJECT_KEY empty puts the claimer back in queue mode, + // where STREAM/GROUP remain mandatory — the existing contract. + t.Setenv("S3_OBJECT_KEY", "") + t.Setenv("STREAM", "") + t.Setenv("GROUP", "g") + if _, err := loadConfig(); err == nil { + t.Error("queue mode must reject missing STREAM, got nil error") + } + }) + + t.Run("queue_mode_still_requires_group", func(t *testing.T) { + t.Setenv("S3_OBJECT_KEY", "") + t.Setenv("STREAM", "s") + t.Setenv("GROUP", "") + if _, err := loadConfig(); err == nil { + t.Error("queue mode must reject missing GROUP, got nil error") + } + }) + + t.Run("s3_bucket_required_in_either_mode", func(t *testing.T) { + t.Setenv("S3_BUCKET", "") + t.Setenv("S3_OBJECT_KEY", "media/front.mp4") + if _, err := loadConfig(); err == nil { + t.Error("S3_BUCKET must be required even in direct mode, got nil error") + } + }) + + t.Run("video_input_path_default_applies_in_direct_mode", func(t *testing.T) { + t.Setenv("S3_BUCKET", "test-bucket") + t.Setenv("S3_OBJECT_KEY", "media/front.mp4") + t.Setenv("VIDEO_INPUT_PATH", "") + cfg, err := loadConfig() + if err != nil { + t.Fatalf("expected default path to satisfy validation, got: %v", err) + } + if cfg.VideoInputPath == "" { + t.Error("expected defaultInputPath to be applied, got empty") + } + }) +} + func TestLoadConfig_ValkeyPassword(t *testing.T) { t.Setenv("STREAM", "test-stream") t.Setenv("GROUP", "test-group") diff --git a/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml index 4f9f0c8..66e055d 100644 --- a/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml +++ b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml @@ -1003,8 +1003,13 @@ spec: - name type: object sourceRef: - description: sourceRef references the PipelineSource resource for - input configuration + description: |- + sourceRef references the PipelineSource resource for input + configuration. This is the convenience form for single-source + pipelines — the source URL is broadcast to every filter container, + so the author doesn't need to know filter names. For multi-source + pipelines, use `sources` instead. Both forms are first-class and + supported indefinitely. properties: name: description: name is the name of the PipelineSource resource @@ -1017,6 +1022,60 @@ spec: required: - name type: object + sources: + description: |- + sources binds one or more PipelineSources to specific filter + containers by name. Each entry targets the container whose + `pipeline.spec.filters[].name` matches `filterName`; the controller + injects the source-derived env vars into only that container. Use + for multi-camera pipelines where each VideoIn filter consumes a + distinct source. + + Exactly one of `sourceRef` or `sources` must be set. + items: + description: |- + NamedSourceRef binds a PipelineSource to a specific filter container by + name. The controller injects the source-derived env vars (RTSP_URL for + streaming, VIDEO_INPUT_PATH for batch) into only the container whose + `pipeline.spec.filters[].name` matches `filterName`. This lets a single + PipelineInstance run a multi-camera pipeline where each VideoIn filter + pulls a distinct source. + properties: + filterName: + description: |- + filterName names the filter container in the referenced Pipeline + that consumes this source. Must match a `pipeline.spec.filters[].name` + value exactly. Constrained to DNS-1123 label rules (lowercase + alphanum + hyphens, max 63 chars) so it can safely flow into + generated container / volume / env names. + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + sourceRef: + description: |- + sourceRef references the PipelineSource resource that provides the + input for the filter named by `filterName`. + properties: + name: + description: name is the name of the PipelineSource resource + type: string + namespace: + description: |- + namespace is the namespace of the PipelineSource resource + If not specified, the PipelineInstance's namespace is used + type: string + required: + - name + type: object + required: + - filterName + - sourceRef + type: object + type: array + x-kubernetes-list-map-keys: + - filterName + x-kubernetes-list-type: map tolerations: description: |- tolerations are appended to the pod spec, alongside any controller-injected @@ -1061,8 +1120,12 @@ spec: type: array required: - pipelineRef - - sourceRef type: object + x-kubernetes-validations: + - message: exactly one of `sourceRef` or `sources` must be set (sources + must be non-empty) + rule: (has(self.sourceRef) && !has(self.sources)) || (!has(self.sourceRef) + && has(self.sources) && size(self.sources) > 0) status: description: status defines the observed state of PipelineInstance properties: diff --git a/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml b/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml index 00299a8..a66c28b 100644 --- a/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml +++ b/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml @@ -94,6 +94,19 @@ spec: region is the bucket region (e.g., "us-east-1") Required for AWS S3, optional for other providers type: string + singleObject: + description: |- + singleObject declares that prefix is a FULL object key naming exactly + one file, not a prefix to scan. Batch instances whose every binding is + a single-object bucket run in direct-download mode (one init claimer + per binding, no Valkey work queue) — the same path multi-source + instances always use — preserving the object's file extension in the + per-filter input path (queue mode downloads to a fixed input.mp4, + which breaks extension-sensitive filters like image-in). The + deployment agent sets this for single-file media kinds + (external_video, external_image, uploads); hand-authored CRs without + it keep the queue-based prefix-scan flow. + type: boolean usePathStyle: default: false description: |- diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index 06bbba8..476957c 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -5,4 +5,6 @@ resources: - pipelines_v1alpha1_pipelinesource_bucket.yaml - pipelines_v1alpha1_pipelinesource_rtsp.yaml - pipelines_v1alpha1_pipelineinstance.yaml +- pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml +- pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml # +kubebuilder:scaffold:manifestskustomizesamples diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml new file mode 100644 index 0000000..910aaf8 --- /dev/null +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml @@ -0,0 +1,69 @@ +# Multi-source batch PipelineInstance (PLAT-1071) — e.g. a multi-camera +# benchmark run comparing two recorded clips against per-camera ground truth. +# +# When `spec.sources` has more than one entry the controller switches the +# batch path to DIRECT mode: a single-Pod Job (parallelism=1, completions=1) +# with one init claimer per binding. Each claimer downloads its bound +# bucket object to `/ws/` (extension preserved from the +# object key, defaulting to .mp4) and skips the Valkey work queue entirely +# (S3_OBJECT_KEY env tells it which single object to fetch). The +# single-source path — `sourceRef`, or a one-element `sources` — keeps the +# existing Valkey queue model for multi-file buckets. +# +# Constraints enforced by the controller (surfaced as Degraded conditions): +# - every binding's PipelineSource must be a Bucket source (no RTSP), and +# - every Bucket source's `prefix` must be a FULL object key (how +# single-file medias ship from the platform — no prefix scans in +# multi-source mode). +# +# The controller injects `VIDEO_INPUT_PATH = /ws/` into +# ONLY the filter container whose name matches the binding's filterName, so +# the referenced Pipeline authors each VideoIn source as +# `file://$(VIDEO_INPUT_PATH)` — the value resolves per container. +# +# The same two openfilter authoring constraints as the streaming +# multi-source sample apply: stagger output ports by 2 (5550, 5552, …) and +# disambiguate fan-in topics (publisher tags shown below; the +# plainsight-api export emits the equivalent subscriber-side remap form — +# see the streaming multisource sample). Example `pipeline-batch-multi`: +# +# filters: +# - name: front-cam +# config: +# - name: sources +# value: "file://$(VIDEO_INPUT_PATH);front-cam" +# - name: outputs +# value: tcp://*:5550 +# - name: back-cam +# config: +# - name: sources +# value: "file://$(VIDEO_INPUT_PATH);back-cam" +# - name: outputs +# value: tcp://*:5552 # +2 not +1 +# - name: tracker +# config: +# - name: sources +# value: "tcp://localhost:5550;front-cam, tcp://localhost:5552;back-cam" +# +# This sample assumes two PipelineSources, each a Bucket source whose +# prefix is a full object key, e.g.: +# +# spec: +# bucket: +# endpoint: https://storage.googleapis.com +# name: my-benchmark-clips +# prefix: clips/front_lobby_001.mp4 +apiVersion: filter.plainsight.ai/v1alpha1 +kind: PipelineInstance +metadata: + name: pipelineinstance-batch-multisource +spec: + pipelineRef: + name: pipeline-batch-multi + sources: + - filterName: front-cam + sourceRef: + name: pipelinesource-front-clip + - filterName: back-cam + sourceRef: + name: pipelinesource-back-clip diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml new file mode 100644 index 0000000..bb5dc2c --- /dev/null +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml @@ -0,0 +1,73 @@ +# Multi-source streaming PipelineInstance (PLAT-1071). +# +# Each entry under `spec.sources` binds a PipelineSource to a specific filter +# container in the referenced Pipeline by name. The controller injects +# RTSP_URL into ONLY the named container — so a Pipeline with two `video-in` +# filters (front-cam + back-cam) and a downstream `webvis` can serve two +# distinct RTSP streams, one per camera, in a single Pod. +# +# This sample assumes: +# - A Pipeline named `pipeline-rtsp-multi` whose filters[] contains containers +# named `front-cam`, `back-cam`, and `webvis`. +# - Two PipelineSources named `pipelinesource-front` and `pipelinesource-back`. +# +# IMPORTANT — two openfilter authoring constraints the referenced Pipeline +# must satisfy (both validated empirically while writing this sample): +# +# 1. **Stagger output ports by 2.** OpenFilter binds the configured port +# AND port+1 per `outputs` entry (the +1 carries ZMQ control-plane +# traffic). Use 5550, 5552, 5554, … — never 5550, 5551 (would crash +# the second VideoIn with `Address already in use`). See +# https://docs.plainsight.ai/docs/openfilter/your-first-filter/. +# +# 2. **Disambiguate fan-in topics.** Every VideoIn publishes on the default +# topic `main`; any downstream that fans-in two upstreams (webvis, +# recorder, …) crashes with `RuntimeError: duplicate topic 'main'` +# unless the topics are made distinct. Two equivalent ways: +# a. Publisher tags (shown below): `;topic` suffix on each VideoIn's +# `sources`, downstream subscribes the distinct topics. Convention +# for hand-authoring: topic = the filter `name`. +# b. Subscriber-side remap (what the plainsight-api export emits, +# validated end-to-end): VideoIns stay on `main`; the fan-in +# filter's sources use `;main>_main` per upstream, e.g. +# `tcp://localhost:5550;main>front_cam_main`. +# Note filter names (and therefore `sources[].filterName` below) must be +# DNS-1123 labels — lowercase alphanumerics and hyphens, no underscores — +# because they flow into container names. +# +# Example pipeline `pipeline-rtsp-multi` config that satisfies both: +# +# filters: +# - name: front-cam +# config: +# - name: sources +# value: "$(RTSP_URL);front-cam" # ;front-cam tags the topic +# - name: outputs +# value: tcp://*:5550 +# - name: back-cam +# config: +# - name: sources +# value: "$(RTSP_URL);back-cam" +# - name: outputs +# value: tcp://*:5552 # +2 not +1 +# - name: webvis +# config: +# - name: sources +# value: "tcp://localhost:5550;front-cam, tcp://localhost:5552;back-cam" +# +# Use `sourceRef` for single-source pipelines instead — `sources` is for the +# multi-camera case. +apiVersion: filter.plainsight.ai/v1alpha1 +kind: PipelineInstance +metadata: + name: pipelineinstance-rtsp-multisource +spec: + pipelineRef: + name: pipeline-rtsp-multi + sources: + - filterName: front-cam + sourceRef: + name: pipelinesource-front + - filterName: back-cam + sourceRef: + name: pipelinesource-back diff --git a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml index 4f9f0c8..66e055d 100644 --- a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml +++ b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml @@ -1003,8 +1003,13 @@ spec: - name type: object sourceRef: - description: sourceRef references the PipelineSource resource for - input configuration + description: |- + sourceRef references the PipelineSource resource for input + configuration. This is the convenience form for single-source + pipelines — the source URL is broadcast to every filter container, + so the author doesn't need to know filter names. For multi-source + pipelines, use `sources` instead. Both forms are first-class and + supported indefinitely. properties: name: description: name is the name of the PipelineSource resource @@ -1017,6 +1022,60 @@ spec: required: - name type: object + sources: + description: |- + sources binds one or more PipelineSources to specific filter + containers by name. Each entry targets the container whose + `pipeline.spec.filters[].name` matches `filterName`; the controller + injects the source-derived env vars into only that container. Use + for multi-camera pipelines where each VideoIn filter consumes a + distinct source. + + Exactly one of `sourceRef` or `sources` must be set. + items: + description: |- + NamedSourceRef binds a PipelineSource to a specific filter container by + name. The controller injects the source-derived env vars (RTSP_URL for + streaming, VIDEO_INPUT_PATH for batch) into only the container whose + `pipeline.spec.filters[].name` matches `filterName`. This lets a single + PipelineInstance run a multi-camera pipeline where each VideoIn filter + pulls a distinct source. + properties: + filterName: + description: |- + filterName names the filter container in the referenced Pipeline + that consumes this source. Must match a `pipeline.spec.filters[].name` + value exactly. Constrained to DNS-1123 label rules (lowercase + alphanum + hyphens, max 63 chars) so it can safely flow into + generated container / volume / env names. + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + sourceRef: + description: |- + sourceRef references the PipelineSource resource that provides the + input for the filter named by `filterName`. + properties: + name: + description: name is the name of the PipelineSource resource + type: string + namespace: + description: |- + namespace is the namespace of the PipelineSource resource + If not specified, the PipelineInstance's namespace is used + type: string + required: + - name + type: object + required: + - filterName + - sourceRef + type: object + type: array + x-kubernetes-list-map-keys: + - filterName + x-kubernetes-list-type: map tolerations: description: |- tolerations are appended to the pod spec, alongside any controller-injected @@ -1061,8 +1120,12 @@ spec: type: array required: - pipelineRef - - sourceRef type: object + x-kubernetes-validations: + - message: exactly one of `sourceRef` or `sources` must be set (sources + must be non-empty) + rule: (has(self.sourceRef) && !has(self.sources)) || (!has(self.sourceRef) + && has(self.sources) && size(self.sources) > 0) status: description: status defines the observed state of PipelineInstance properties: diff --git a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml index 00299a8..a66c28b 100644 --- a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml +++ b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml @@ -94,6 +94,19 @@ spec: region is the bucket region (e.g., "us-east-1") Required for AWS S3, optional for other providers type: string + singleObject: + description: |- + singleObject declares that prefix is a FULL object key naming exactly + one file, not a prefix to scan. Batch instances whose every binding is + a single-object bucket run in direct-download mode (one init claimer + per binding, no Valkey work queue) — the same path multi-source + instances always use — preserving the object's file extension in the + per-filter input path (queue mode downloads to a fixed input.mp4, + which breaks extension-sensitive filters like image-in). The + deployment agent sets this for single-file media kinds + (external_video, external_image, uploads); hand-authored CRs without + it keep the queue-based prefix-scan flow. + type: boolean usePathStyle: default: false description: |- diff --git a/internal/controller/pipelineinstance_admission_test.go b/internal/controller/pipelineinstance_admission_test.go new file mode 100644 index 0000000..15e3689 --- /dev/null +++ b/internal/controller/pipelineinstance_admission_test.go @@ -0,0 +1,157 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "fmt" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + pipelinesv1alpha1 "github.com/PlainsightAI/openfilter-pipelines-controller/api/v1alpha1" +) + +// These specs pin the PipelineInstance CRD's admission-time validation +// contract (PLAT-1071) against a real API server: the CEL xor rule between +// `sourceRef` and `sources`, the listMapKey uniqueness of +// `sources[].filterName`, and the DNS-1123 filterName pattern. Both the +// deployment agent (plainsight-deployment-agent#65) and the portal +// (client-portal#429) build on these invariants holding at the cluster +// boundary, so a `make manifests` regeneration that drops or weakens any +// of them must fail here. +var _ = Describe("PipelineInstance CRD admission validation", func() { + var nameSeq int + + newPI := func(mutate func(*pipelinesv1alpha1.PipelineInstanceSpec)) *pipelinesv1alpha1.PipelineInstance { + nameSeq++ + pi := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("admission-test-%d", nameSeq), + Namespace: "default", + }, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "some-pipeline"}, + }, + } + mutate(&pi.Spec) + return pi + } + + mustReject := func(pi *pipelinesv1alpha1.PipelineInstance, fragment string) { + GinkgoHelper() + err := k8sClient.Create(ctx, pi) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(fragment)) + } + + mustAccept := func(pi *pipelinesv1alpha1.PipelineInstance) { + GinkgoHelper() + Expect(k8sClient.Create(ctx, pi)).To(Succeed()) + Expect(k8sClient.Delete(ctx, pi)).To(Succeed()) + } + + namedSource := func(filterName, sourceName string) pipelinesv1alpha1.NamedSourceRef { + return pipelinesv1alpha1.NamedSourceRef{ + FilterName: filterName, + SourceRef: pipelinesv1alpha1.SourceReference{Name: sourceName}, + } + } + + Context("xor rule between sourceRef and sources", func() { + It("accepts the singular sourceRef form", func() { + mustAccept(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.SourceRef = &pipelinesv1alpha1.SourceReference{Name: "single-source"} + })) + }) + + It("accepts the plural sources form", func() { + mustAccept(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.Sources = []pipelinesv1alpha1.NamedSourceRef{ + namedSource("front-cam", "src-front"), + namedSource("back-cam", "src-back"), + } + })) + }) + + It("rejects setting both sourceRef and sources", func() { + mustReject(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.SourceRef = &pipelinesv1alpha1.SourceReference{Name: "single-source"} + s.Sources = []pipelinesv1alpha1.NamedSourceRef{namedSource("front-cam", "src-front")} + }), "exactly one of") + }) + + It("rejects setting neither sourceRef nor sources", func() { + mustReject(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) {}), "exactly one of") + }) + + It("rejects an empty sources list", func() { + mustReject(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.Sources = []pipelinesv1alpha1.NamedSourceRef{} + }), "exactly one of") + }) + }) + + Context("sources listMapKey uniqueness on filterName", func() { + It("rejects two entries binding the same filterName", func() { + mustReject(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.Sources = []pipelinesv1alpha1.NamedSourceRef{ + namedSource("front-cam", "src-a"), + namedSource("front-cam", "src-b"), + } + }), "Duplicate value") + }) + + It("accepts distinct filterNames binding the same source", func() { + // The same media bound to two VideoIns is allowed (useful for + // N-camera topologies driven by one test source). + mustAccept(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.Sources = []pipelinesv1alpha1.NamedSourceRef{ + namedSource("front-cam", "shared-src"), + namedSource("back-cam", "shared-src"), + } + })) + }) + }) + + Context("filterName DNS-1123 pattern", func() { + reject := func(name string) { + GinkgoHelper() + mustReject(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.Sources = []pipelinesv1alpha1.NamedSourceRef{namedSource(name, "src")} + }), "filterName") + } + accept := func(name string) { + GinkgoHelper() + mustAccept(newPI(func(s *pipelinesv1alpha1.PipelineInstanceSpec) { + s.Sources = []pipelinesv1alpha1.NamedSourceRef{namedSource(name, "src")} + })) + } + + It("rejects underscores", func() { reject("front_cam") }) + It("rejects uppercase", func() { reject("FrontCam") }) + It("rejects a leading hyphen", func() { reject("-front") }) + It("rejects a trailing hyphen", func() { reject("front-") }) + It("rejects the empty string", func() { reject("") }) + It("rejects 64 characters", func() { reject(strings.Repeat("a", 64)) }) + + It("accepts a single character", func() { accept("a") }) + It("accepts 63 characters", func() { accept(strings.Repeat("a", 63)) }) + It("accepts interior hyphens and digits", func() { accept("cam-2-front") }) + }) +}) diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index c4bb0d0..d04a6c7 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -23,6 +23,7 @@ import ( "encoding/base64" "fmt" "net/http" + "strings" "time" "github.com/minio/minio-go/v7" @@ -65,6 +66,21 @@ const ( ReasonPipelineNotFound = "PipelineNotFound" ReasonPipelineSourceNotFound = "PipelineSourceNotFound" + // ReasonSourceBindingUnmatched marks a Degraded condition raised when a + // `sources[].filterName` matches no `pipeline.spec.filters[].name`. Without + // this validation the binding is a silent no-op: in streaming the + // per-filter env never lands on any container, and in multi-source batch + // the claimer downloads a file no filter container reads. + ReasonSourceBindingUnmatched = "SourceBindingUnmatched" + + // ReasonMultiSourceBatchInvalidSource / ReasonMultiSourceBatchMissingObject + // mark Degraded conditions raised by multi-source batch binding validation + // (see reconcileBatchMultiSource). Centralized here so the + // clear-on-recovery path and the set path can never drift apart on the + // reason string. + ReasonMultiSourceBatchInvalidSource = "MultiSourceBatchInvalidSource" + ReasonMultiSourceBatchMissingObject = "MultiSourceBatchMissingObject" + // Reconciliation intervals StatusUpdateInterval = 30 * time.Second @@ -487,10 +503,12 @@ func (r *PipelineInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, err } - // Get the PipelineSource resource - pipelineSource, err := r.getPipelineSource(ctx, pipelineInstance) + // Resolve all source bindings (legacy single SourceRef → one ResolvedSourceBinding + // with empty FilterName for broadcast; new Sources → one binding per entry, + // each scoped to a specific filter container). + resolvedSources, err := r.resolveSourceBindings(ctx, pipelineInstance) if err != nil { - log.Error(err, "Failed to get PipelineSource") + log.Error(err, "Failed to resolve source bindings") r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, ReasonPipelineSourceNotFound, err.Error()) if err := r.Status().Update(ctx, pipelineInstance); err != nil { log.Error(err, "Failed to update status") @@ -498,6 +516,62 @@ func (r *PipelineInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, err } + // Validate that every named binding targets a filter container that + // actually exists in the Pipeline. An unmatched `sources[].filterName` + // would otherwise be a silent no-op: streaming never injects the source + // env into any container, and multi-source batch downloads a file no + // filter reads. Surface it as Degraded with an operator-actionable + // message and return nil (consistent with the other validation failures + // — retrying won't help until the CR or Pipeline is fixed; the Pipeline + // watch and CR-spec generation bump re-trigger reconcile on a fix). + if unmatched := unmatchedBindingFilterNames(resolvedSources, pipeline); len(unmatched) > 0 { + available := make([]string, 0, len(pipeline.Spec.Filters)) + for _, f := range pipeline.Spec.Filters { + available = append(available, f.Name) + } + msg := fmt.Sprintf( + "sources[].filterName value(s) [%s] match no pipeline.spec.filters[].name; available filter names: [%s]", + strings.Join(unmatched, ", "), strings.Join(available, ", ")) + log.Info("Source binding validation failed", "unmatched", unmatched, "available", available) + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, ReasonSourceBindingUnmatched, msg) + // A validation-blocked instance is not making progress — clear any + // stale Progressing=True from a prior pass so the condition set + // reads consistently for automation. (Succeeded is left untouched: + // it records a past terminal outcome, not current activity.) + r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionFalse, ReasonSourceBindingUnmatched, msg) + if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { + // Propagate so controller-runtime retries: returning nil here + // would leave the server-side status stale with no requeue + // (validation failures intentionally don't requeue). + if apierrors.IsConflict(statusErr) { + return ctrl.Result{Requeue: true}, nil + } + log.Error(statusErr, "Failed to update status after unmatched source binding validation") + return ctrl.Result{}, statusErr + } + return ctrl.Result{}, nil + } + + // Bindings resolve and validate — clear any stale Degraded left by a + // previous failing pass so external watchers see the recovery. Both + // reasons matter: SourceBindingUnmatched from a since-fixed filterName, + // and PipelineSourceNotFound from the creation race where the + // PipelineInstance is applied before its PipelineSources (the + // deployment-agent's intentional order) — the PipelineSource watch + // re-reconciles us here once the sources land, and without this clear + // the stale condition lingers and the agent's provisioning monitor + // tears the instance down as degraded. + // Same conflict semantics as the PipelineNotFound clear above. + if err := r.clearDegradedReason(ctx, pipelineInstance, ReasonSourceBindingUnmatched, ReasonPipelineSourceNotFound); err != nil { + if apierrors.IsConflict(err) { + log.V(1).Info("Status update conflict while clearing stale Degraded; requeueing", + "reason", ReasonSourceBindingUnmatched) + return ctrl.Result{Requeue: true}, nil + } + log.Error(err, "Failed to clear stale Degraded condition") + return ctrl.Result{}, err + } + // Branch by pipeline mode mode := pipeline.Spec.Mode if mode == "" { @@ -516,11 +590,11 @@ func (r *PipelineInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Req ) if mode == pipelinesv1alpha1.PipelineModeStream { - return r.reconcileStreaming(ctx, pipelineInstance, pipeline, pipelineSource) + return r.reconcileStreaming(ctx, pipelineInstance, pipeline, resolvedSources) } // Default: Batch mode - return r.reconcileBatch(ctx, pipelineInstance, pipeline, pipelineSource) + return r.reconcileBatch(ctx, pipelineInstance, pipeline, resolvedSources) } // Helper functions shared between batch and streaming modes are defined below. @@ -528,22 +602,72 @@ func (r *PipelineInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Req // - pipelineinstance_controller_batch.go // - pipelineinstance_controller_streaming.go -// getPipelineSource retrieves the PipelineSource resource referenced by the PipelineInstance -func (r *PipelineInstanceReconciler) getPipelineSource(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance) (*pipelinesv1alpha1.PipelineSource, error) { - namespace := pipelineInstance.Namespace - if pipelineInstance.Spec.SourceRef.Namespace != nil { - namespace = *pipelineInstance.Spec.SourceRef.Namespace +// ResolvedSourceBinding pairs a target filter name with the PipelineSource that +// feeds it. FilterName=="" is the legacy "broadcast to every filter container" +// sentinel produced when the PipelineInstance uses the singular +// `sourceRef` field. Non-empty FilterName targets the container whose +// `pipeline.spec.filters[].name` matches exactly. +type ResolvedSourceBinding struct { + FilterName string + Source *pipelinesv1alpha1.PipelineSource +} + +// resolveSourceBindings normalizes `Spec.SourceRef` (legacy, broadcast) and +// `Spec.Sources` (multi-source) into a uniform list of (filterName, source) +// pairs. The first error from any Get is returned with the binding context so +// the caller surfaces an operator-actionable message. +func (r *PipelineInstanceReconciler) resolveSourceBindings(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance) ([]ResolvedSourceBinding, error) { + effective := pipelineInstance.Spec.EffectiveSources() + if len(effective) == 0 { + // Validation should catch this at admission, but a defensive error + // here avoids a downstream nil-deref in the reconciler. + return nil, fmt.Errorf("PipelineInstance has neither sourceRef nor sources set") } - pipelineSource := &pipelinesv1alpha1.PipelineSource{} - if err := r.Get(ctx, types.NamespacedName{ - Name: pipelineInstance.Spec.SourceRef.Name, - Namespace: namespace, - }, pipelineSource); err != nil { - return nil, fmt.Errorf("failed to get pipeline source: %w", err) + bindings := make([]ResolvedSourceBinding, 0, len(effective)) + for _, ref := range effective { + namespace := pipelineInstance.Namespace + if ref.SourceRef.Namespace != nil { + namespace = *ref.SourceRef.Namespace + } + src := &pipelinesv1alpha1.PipelineSource{} + if err := r.Get(ctx, types.NamespacedName{ + Name: ref.SourceRef.Name, + Namespace: namespace, + }, src); err != nil { + if ref.FilterName == "" { + return nil, fmt.Errorf("failed to get pipeline source %q: %w", ref.SourceRef.Name, err) + } + return nil, fmt.Errorf("failed to get pipeline source %q (bound to filter %q): %w", ref.SourceRef.Name, ref.FilterName, err) + } + bindings = append(bindings, ResolvedSourceBinding{ + FilterName: ref.FilterName, + Source: src, + }) } + return bindings, nil +} - return pipelineSource, nil +// unmatchedBindingFilterNames returns, in binding order, every non-empty +// binding FilterName that matches no `pipeline.spec.filters[].name`. The +// broadcast sentinel (FilterName=="") is skipped — it targets every container +// by design. Names are unique per the CRD's listType=map key, so no +// deduplication is needed. +func unmatchedBindingFilterNames(bindings []ResolvedSourceBinding, pipeline *pipelinesv1alpha1.Pipeline) []string { + known := make(map[string]struct{}, len(pipeline.Spec.Filters)) + for _, f := range pipeline.Spec.Filters { + known[f.Name] = struct{}{} + } + var unmatched []string + for _, b := range bindings { + if b.FilterName == "" { + continue + } + if _, ok := known[b.FilterName]; !ok { + unmatched = append(unmatched, b.FilterName) + } + } + return unmatched } // getCredentials retrieves S3 credentials for the pipelineSource bucket secret, if configured. @@ -630,12 +754,24 @@ func (r *PipelineInstanceReconciler) listBucketFiles(ctx context.Context, pipeli if object.Err != nil { return nil, fmt.Errorf("error listing objects: %w", object.Err) } + if isDirectoryPlaceholder(object.Key, object.Size) { + continue + } files = append(files, object.Key) } return files, nil } +// isDirectoryPlaceholder reports whether a listed object is a zero-byte +// directory-placeholder key (ending in "/", created by the GCS/S3 console +// for "folders"). They are not processable files — enqueued, one would fail +// its download/decode attempts, land in the DLQ, and fail the whole run +// (observed empirically against a console-created GCS folder). +func isDirectoryPlaceholder(key string, size int64) bool { + return strings.HasSuffix(key, "/") && size == 0 +} + // getPipeline retrieves the Pipeline resource referenced by the PipelineInstance func (r *PipelineInstanceReconciler) getPipeline(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance) (*pipelinesv1alpha1.Pipeline, error) { namespace := pipelineInstance.Namespace @@ -833,15 +969,25 @@ func (r *PipelineInstanceReconciler) setCondition(pipelineInstance *pipelinesv1a meta.SetStatusCondition(&pipelineInstance.Status.Conditions, condition) } -// clearDegradedReason removes a Degraded condition whose Reason matches the -// given value and persists the status update. It's a no-op when no matching -// condition exists. Used to clear a stale Degraded set during a transient -// dependency miss once the dependency reappears, so external watchers (e.g. -// plainsight-deployment-agent) see the recovery instead of treating the -// previous condition as terminal. -func (r *PipelineInstanceReconciler) clearDegradedReason(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, reason string) error { +// clearDegradedReason removes a Degraded condition whose Reason matches any +// of the given values and persists the status update. It's a no-op when no +// matching condition exists. Used to clear a stale Degraded set during a +// transient dependency miss or a since-fixed validation failure, so external +// watchers (e.g. plainsight-deployment-agent) see the recovery instead of +// treating the previous condition as terminal. +func (r *PipelineInstanceReconciler) clearDegradedReason(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, reasons ...string) error { cond := meta.FindStatusCondition(pipelineInstance.Status.Conditions, ConditionTypeDegraded) - if cond == nil || cond.Reason != reason { + if cond == nil { + return nil + } + matched := false + for _, reason := range reasons { + if cond.Reason == reason { + matched = true + break + } + } + if !matched { return nil } meta.RemoveStatusCondition(&pipelineInstance.Status.Conditions, ConditionTypeDegraded) @@ -863,6 +1009,15 @@ func (r *PipelineInstanceReconciler) SetupWithManager(mgr ctrl.Manager) error { &pipelinesv1alpha1.Pipeline{}, handler.EnqueueRequestsFromMapFunc(r.pipelineInstancesForPipeline), ). + // Wake any PipelineInstance bound to a PipelineSource when that source + // changes. Multi-source batch validation Degrades on a misconfigured + // source (e.g. empty Bucket.Prefix) and returns without requeue — + // without this watch, fixing the PipelineSource would never re-trigger + // reconcile and the instance would stay Degraded forever. + Watches( + &pipelinesv1alpha1.PipelineSource{}, + handler.EnqueueRequestsFromMapFunc(r.pipelineInstancesForPipelineSource), + ). Named("pipelineinstance"). Complete(r) } @@ -915,3 +1070,52 @@ func (r *PipelineInstanceReconciler) pipelineInstancesForPipeline(ctx context.Co } return requests } + +// pipelineInstancesForPipelineSource returns reconcile requests for every +// PipelineInstance that references the PipelineSource — +// either via `spec.sources[].sourceRef.name` or the legacy +// `spec.sourceRef.name` (both shapes are normalized by EffectiveSources). +// Wired in SetupWithManager so a PipelineSource create/update event wakes any +// PipelineInstance whose last reconcile Degraded on that source (e.g. +// multi-source batch validation failures, which return without a requeue). +// +// Scope: cluster-wide list (the informer cache already holds every +// PipelineInstance), with per-ref namespace resolution below — so a +// cross-namespace sourceRef (ref.namespace set) wakes its instance exactly +// like a same-namespace one. +func (r *PipelineInstanceReconciler) pipelineInstancesForPipelineSource(ctx context.Context, obj client.Object) []reconcile.Request { + source, ok := obj.(*pipelinesv1alpha1.PipelineSource) + if !ok { + return nil + } + var list pipelinesv1alpha1.PipelineInstanceList + if err := r.List(ctx, &list); err != nil { + logf.FromContext(ctx).Error(err, "Failed to list PipelineInstances for PipelineSource watch", + "pipelineSource", source.Name, "namespace", source.Namespace) + return nil + } + requests := make([]reconcile.Request, 0, len(list.Items)) + for i := range list.Items { + pi := &list.Items[i] + for _, ref := range pi.Spec.EffectiveSources() { + if ref.SourceRef.Name != source.Name { + continue + } + // SourceRef.Namespace may override the PipelineInstance's own + // namespace; honor that so a same-namespace name match doesn't + // fire when the ref actually points elsewhere. + refNS := pi.Namespace + if ref.SourceRef.Namespace != nil { + refNS = *ref.SourceRef.Namespace + } + if refNS != source.Namespace { + continue + } + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: pi.Namespace, Name: pi.Name}, + }) + break + } + } + return requests +} diff --git a/internal/controller/pipelineinstance_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index 115a784..1dd67f5 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -38,8 +38,40 @@ import ( "github.com/PlainsightAI/openfilter-pipelines-controller/internal/tracing" ) -// reconcileBatch handles the batch (Job-based) reconciliation path -func (r *PipelineInstanceReconciler) reconcileBatch(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, pipelineSource *pipelinesv1alpha1.PipelineSource) (ctrl.Result, error) { +// reconcileBatch handles the batch (Job-based) reconciliation path. +// +// Two sub-paths (PLAT-1071): +// +// - Single-source: one PipelineSource bound (either via legacy +// `Spec.SourceRef` or a one-entry `Spec.Sources`). Existing +// queue-based flow — list bucket → enqueue files → parallel Pods +// each pop one file via Valkey, claim via the init claimer, run +// the filter chain. Unchanged from before PR2. Note: on this path +// the binding's filterName is NOT used for routing — env injection +// is broadcast-equivalent, exactly as single-source always behaved +// (the unmatched-filterName validation still runs upstream). +// +// - Multi-source: ≥2 PipelineSources bound, each pinned to a +// specific filter container by name. Per-binding the source's +// `Bucket.Prefix` must be a full S3 object key so each VideoIn's +// init claimer can download a deterministic file. Job runs as +// parallelism=1, completions=1 — one Pod, N init claimers each +// writing to `/ws/.`, M filter containers running +// the chain. Completion is the Job's normal status (no per-file +// queue accounting). Delegates to reconcileBatchMultiSource. +// +// A SINGLE binding also takes the direct path when its bucket declares +// `singleObject: true` (prefix is a full object key) AND the binding names +// a filter (plural `sources` form, not the legacy broadcast sentinel): the +// object is known up front, so the queue adds nothing — and queue mode's +// fixed input.mp4 download destination destroys the file extension, which +// extension-sensitive entry filters (image-in) need. The deployment agent +// sets singleObject for single-file media kinds. +func (r *PipelineInstanceReconciler) reconcileBatch(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding) (ctrl.Result, error) { + if len(sourceBindings) > 1 || isDirectModeSingleBinding(sourceBindings) { + return r.reconcileBatchMultiSource(ctx, pipelineInstance, pipeline, sourceBindings) + } + pipelineSource := sourceBindings[0].Source log := logf.FromContext(ctx) // Ensure counts object exists before initialization @@ -523,9 +555,24 @@ func (r *PipelineInstanceReconciler) buildJob(ctx context.Context, pipelineInsta }) } - // Start with config env vars, then add filter-specific env vars - // Filter-specific env vars can override config if they have the same name - containerEnv := make([]corev1.EnvVar, 0, len(configEnvVars)+len(filter.Env)) + // Start with the claimer's download destination, THEN config env, + // then filter-specific env vars (later entries override earlier + // duplicates on the running container). + // + // VIDEO_INPUT_PATH MUST precede the FILTER_* config env in the + // list: Kubernetes dependent-env expansion only resolves $(VAR) + // references to variables defined EARLIER — with the old + // config-first ordering, `sources: file://$(VIDEO_INPUT_PATH)` + // arrived in the container as a literal unexpanded string. + // Exposed to every filter container (single-source has no + // per-filter binding to key on — legacy broadcast); only VideoIn + // entries reference it. Same contract the multi-source path + // provides per-binding (see buildBatchFilterContainersForMultiSource). + containerEnv := make([]corev1.EnvVar, 0, len(configEnvVars)+len(filter.Env)+1) + containerEnv = append(containerEnv, corev1.EnvVar{ + Name: "VIDEO_INPUT_PATH", + Value: videoInputPath, + }) containerEnv = append(containerEnv, configEnvVars...) // Inject GPU env vars before user env vars so users can override if needed. diff --git a/internal/controller/pipelineinstance_controller_batch_multisource.go b/internal/controller/pipelineinstance_controller_batch_multisource.go new file mode 100644 index 0000000..c65ebff --- /dev/null +++ b/internal/controller/pipelineinstance_controller_batch_multisource.go @@ -0,0 +1,461 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "path/filepath" + "strings" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + pipelinesv1alpha1 "github.com/PlainsightAI/openfilter-pipelines-controller/api/v1alpha1" + "github.com/PlainsightAI/openfilter-pipelines-controller/internal/tracing" +) + +// reconcileBatchMultiSource handles the multi-source batch path (PLAT-1071). +// +// Multi-source batch runs as a single Pod (parallelism=1, completions=1) +// with one init claimer per binding — each downloads its specific S3 +// object to `/ws/.` — followed by the filter chain. The +// Valkey work-queue (designed for parallel-Pods-per-bucket fan-out) is +// skipped entirely: the bindings already name exactly which files to +// process. Completion is detected via the Job's own status (Succeeded / +// Failed) the same way Kubernetes would for any one-shot Job. +// +// Constraints enforced here (V1): +// - Every binding's PipelineSource must have a Bucket source set +// (RTSP makes no sense for batch). +// - Every binding must resolve a deterministic object key for its +// direct-mode claimer: `Bucket.Prefix` used as the full key (the +// platform's media model carries single-file keys in prefix — see +// bindingObjectKey). +// +// Either constraint failing surfaces as a Degraded condition with an +// operator-actionable message; the reconciler returns nil so the kube +// API server doesn't pile up retry events on a known-bad CR. +func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding) (ctrl.Result, error) { + // Stamp the canonical pipeline-instance pivot so the multi-source + // batch reconcile shares the same Cloud Trace grouping as + // reconcileBatch / reconcileStreaming. Satisfies the spancov gate + // and gives operators one query (`pipeline_instance.id = ""`) + // that returns every reconcile pass regardless of path. + tracing.Stamp(ctx, tracing.PipelineInstanceID(pipelineInstance)) + log := logf.FromContext(ctx) + + // Validate every binding has the shape multi-source batch needs. + for _, b := range sourceBindings { + if b.Source == nil || b.Source.Spec.Bucket == nil { + msg := fmt.Sprintf("multi-source batch requires every binding's PipelineSource to be a Bucket source (filter %q is not)", b.FilterName) + return r.degradeBatchValidation(ctx, pipelineInstance, ReasonMultiSourceBatchInvalidSource, msg) + } + if bindingObjectKey(b) == "" { + msg := fmt.Sprintf("multi-source batch requires every binding's PipelineSource Bucket.Prefix to name a full object key; %q has an empty prefix", b.FilterName) + return r.degradeBatchValidation(ctx, pipelineInstance, ReasonMultiSourceBatchMissingObject, msg) + } + } + + // Validation passed — clear any stale Degraded condition a previous + // failing pass left behind. The validation failures above return without + // a requeue, so recovery rides on the PipelineSource watch mapping in + // SetupWithManager re-triggering this reconcile once the source is fixed; + // without this clear the instance would stay Degraded forever even after + // the fix. + if err := r.clearDegradedReason(ctx, pipelineInstance, + ReasonMultiSourceBatchInvalidSource, ReasonMultiSourceBatchMissingObject); err != nil { + log.Error(err, "Failed to clear stale multi-source batch validation Degraded condition") + return ctrl.Result{}, err + } + + // Stamp StartTime once for trace correlation and to anchor the + // Job-name derivation below to a stable value across reconciles. + if pipelineInstance.Status.StartTime == nil { + now := metav1.Now() + pipelineInstance.Status.StartTime = &now + if err := r.Status().Update(ctx, pipelineInstance); err != nil { + log.Error(err, "Failed to set StartTime") + return ctrl.Result{}, err + } + } + + jobName := pipelineInstance.Name + "-job" + + // Idempotent Job ensure: if the Job already exists we just observe + // its status; if not, build and create. + job := &batchv1.Job{} + getErr := r.Get(ctx, types.NamespacedName{Name: jobName, Namespace: pipelineInstance.Namespace}, job) + switch { + case apierrors.IsNotFound(getErr): + desired := r.buildMultiSourceBatchJob(ctx, pipelineInstance, pipeline, sourceBindings, jobName) + if err := controllerutil.SetControllerReference(pipelineInstance, desired, r.Scheme); err != nil { + return ctrl.Result{}, fmt.Errorf("set owner ref on job: %w", err) + } + if err := r.Create(ctx, desired); err != nil { + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "JobCreationFailed", err.Error()) + if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { + log.Error(statusErr, "Failed to update status after job create failure") + } + return ctrl.Result{}, fmt.Errorf("create job: %w", err) + } + pipelineInstance.Status.JobName = jobName + log.Info("Created multi-source batch Job", "job", jobName, "bindings", len(sourceBindings)) + case getErr != nil: + return ctrl.Result{}, fmt.Errorf("get job: %w", getErr) + } + + // Observe Job status. We translate the Job's terminal state into + // our standard Succeeded / Degraded conditions; while running we + // stay in Progressing. + if pipelineInstance.Status.JobName == "" { + pipelineInstance.Status.JobName = jobName + } + for _, c := range job.Status.Conditions { + if c.Status != corev1.ConditionTrue { + continue + } + switch c.Type { + case batchv1.JobComplete, batchv1.JobSuccessCriteriaMet: + r.setCondition(pipelineInstance, ConditionTypeSucceeded, metav1.ConditionTrue, "Completed", "Job completed successfully") + r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionFalse, "Completed", "Job completed successfully") + if pipelineInstance.Status.CompletionTime == nil { + now := metav1.Now() + pipelineInstance.Status.CompletionTime = &now + } + if err := r.Status().Update(ctx, pipelineInstance); err != nil { + log.Error(err, "Failed to update status after Job success") + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + case batchv1.JobFailed, batchv1.JobFailureTarget: + reason := c.Reason + if reason == "" { + reason = "JobFailed" + } + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, reason, c.Message) + r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionFalse, reason, c.Message) + r.setCondition(pipelineInstance, ConditionTypeSucceeded, metav1.ConditionFalse, reason, c.Message) + if pipelineInstance.Status.CompletionTime == nil { + now := metav1.Now() + pipelineInstance.Status.CompletionTime = &now + } + if err := r.Status().Update(ctx, pipelineInstance); err != nil { + log.Error(err, "Failed to update status after Job failure") + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + } + + // Still progressing. This branch re-runs every StatusUpdateInterval — + // skip the status write when the condition is already in this exact + // state (meta.SetStatusCondition reports whether anything changed), so + // the steady-state loop doesn't issue a no-op API write every 30s. + changed := meta.SetStatusCondition(&pipelineInstance.Status.Conditions, metav1.Condition{ + Type: ConditionTypeProgressing, + Status: metav1.ConditionTrue, + ObservedGeneration: pipelineInstance.Generation, + LastTransitionTime: metav1.Now(), + Reason: "Processing", + Message: "Multi-source batch Job is running", + }) + if changed { + if err := r.Status().Update(ctx, pipelineInstance); err != nil { + log.Error(err, "Failed to update status") + return ctrl.Result{}, err + } + } + return ctrl.Result{RequeueAfter: StatusUpdateInterval}, nil +} + +// buildMultiSourceBatchJob constructs the Job for the multi-source batch +// path. One init claimer per binding (direct mode), then the user's +// filter containers with a per-binding VIDEO_INPUT_PATH env var injected +// into the matching VideoIn container only. +func (r *PipelineInstanceReconciler) buildMultiSourceBatchJob(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding, jobName string) *batchv1.Job { + log := logf.FromContext(ctx) + instanceID := pipelineInstance.GetInstanceID() + + // Build the per-binding download path map up front; reused by both + // the init containers (where to write) and the filter containers + // (where their VideoIn reads). + downloadPath := make(map[string]string, len(sourceBindings)) + for _, b := range sourceBindings { + downloadPath[b.FilterName] = perFilterInputPath(b.FilterName, bindingObjectKey(b)) + } + + // One init claimer per binding (direct mode). + initContainers := make([]corev1.Container, 0, len(sourceBindings)) + for i, b := range sourceBindings { + initContainers = append(initContainers, corev1.Container{ + Name: claimerContainerName(b.FilterName), + Image: r.ClaimerImage, + Env: r.buildDirectClaimerEnv(b, downloadPath[b.FilterName]), + VolumeMounts: []corev1.VolumeMount{ + {Name: "workspace", MountPath: "/ws"}, + }, + }) + log.V(1).Info("Added direct-mode claimer", "index", i, "filter", b.FilterName, "object", bindingObjectKey(b), "destination", downloadPath[b.FilterName]) + } + + // Build filter containers with per-VideoIn VIDEO_INPUT_PATH env. + filterContainers := r.buildBatchFilterContainersForMultiSource(pipeline, pipelineInstance, downloadPath) + + // GPU sharing reuse (same shape as single-source batch). + if requiresGPU(filterContainers) { + applyGPUContainerSharing(filterContainers, instanceGPUCount(pipelineInstance.Spec)) + } + + pipelineRequiresGPU := requiresGPU(filterContainers) + nodeSelector := mergeNodeSelector(r.GPUNodeSelectorLabels, pipelineInstance.Spec.NodeSelector, pipelineRequiresGPU) + + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: jobName, + Namespace: pipelineInstance.Namespace, + Labels: buildPodLabels(instanceID, pipelineInstance), + }, + Spec: batchv1.JobSpec{ + CompletionMode: ptr.To(batchv1.NonIndexedCompletion), + Completions: ptr.To(int32(1)), + Parallelism: ptr.To(int32(1)), + BackoffLimit: ptr.To(int32(2)), + TTLSecondsAfterFinished: ptr.To(int32(86400)), + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: buildPodLabels(instanceID, pipelineInstance), + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + NodeSelector: nodeSelector, + ImagePullSecrets: pipeline.Spec.ImagePullSecrets, + Volumes: []corev1.Volume{ + { + Name: "workspace", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + }, + InitContainers: initContainers, + Containers: filterContainers, + }, + }, + }, + } + + applyInstanceScheduling(&job.Spec.Template.Spec, pipelineInstance.Spec) + return job +} + +// buildDirectClaimerEnv assembles the env for a direct-mode claimer init +// container. The presence of S3_OBJECT_KEY tells the claimer to skip +// Valkey and download exactly that key (see cmd/claimer/main.go). +func (r *PipelineInstanceReconciler) buildDirectClaimerEnv(b ResolvedSourceBinding, destPath string) []corev1.EnvVar { + bucket := b.Source.Spec.Bucket + env := []corev1.EnvVar{ + {Name: "S3_BUCKET", Value: bucket.Name}, + {Name: "S3_OBJECT_KEY", Value: bindingObjectKey(b)}, + {Name: "VIDEO_INPUT_PATH", Value: destPath}, + {Name: "S3_ENDPOINT", Value: bucket.Endpoint}, + {Name: "S3_REGION", Value: bucket.Region}, + {Name: "S3_USE_PATH_STYLE", Value: fmt.Sprintf("%t", bucket.UsePathStyle)}, + {Name: "S3_INSECURE_SKIP_TLS_VERIFY", Value: fmt.Sprintf("%t", bucket.InsecureSkipTLSVerify)}, + } + if bucket.CredentialsSecret != nil { + env = append(env, + corev1.EnvVar{ + Name: "S3_ACCESS_KEY_ID", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: bucket.CredentialsSecret.Name}, + Key: "accessKeyId", + }, + }, + }, + corev1.EnvVar{ + Name: "S3_SECRET_ACCESS_KEY", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: bucket.CredentialsSecret.Name}, + Key: "secretAccessKey", + }, + }, + }, + ) + } + return env +} + +// buildBatchFilterContainersForMultiSource walks pipeline.Spec.Filters +// and builds the container slice. For each filter whose name matches a +// binding's FilterName, injects VIDEO_INPUT_PATH = the destination the +// matching claimer wrote to. Non-matching filters are built unchanged +// — they're downstream consumers and their `sources` config wires them +// to siblings via tcp://localhost, exactly like single-source today. +func (r *PipelineInstanceReconciler) buildBatchFilterContainersForMultiSource(pipeline *pipelinesv1alpha1.Pipeline, pipelineInstance *pipelinesv1alpha1.PipelineInstance, downloadPath map[string]string) []corev1.Container { + containers := make([]corev1.Container, 0, len(pipeline.Spec.Filters)) + for _, filter := range pipeline.Spec.Filters { + configEnv := make([]corev1.EnvVar, 0, len(filter.Config)) + for _, cfg := range filter.Config { + configEnv = append(configEnv, corev1.EnvVar{ + Name: "FILTER_" + strings.ToUpper(cfg.Name), + Value: cfg.Value, + }) + } + + // Per-binding env: the VideoIn container whose name matches a + // binding picks up VIDEO_INPUT_PATH = its claimer's download + // destination, so authors write `sources: file://$(VIDEO_INPUT_PATH)`. + // + // VIDEO_INPUT_PATH MUST precede the FILTER_* config env in the + // list: Kubernetes dependent-env expansion only resolves $(VAR) + // references to variables defined EARLIER — a forward reference + // stays a literal string and the VideoIn fails to open its input. + // (Mirrors the streaming builder's RTSP-env-first ordering.) + env := make([]corev1.EnvVar, 0, len(configEnv)+1) + if path, ok := downloadPath[filter.Name]; ok { + env = append(env, + corev1.EnvVar{Name: "VIDEO_INPUT_PATH", Value: path}, + ) + } + env = append(env, configEnv...) + + // GPU + tracing env (mirrors single-source batch). + if filter.Resources != nil && containerResourcesRequireGPU(*filter.Resources) { + if r.GPULibraryPath != "" { + env = append(env, corev1.EnvVar{Name: ldLibraryPathEnvName, Value: r.GPULibraryPath}) + } + if r.GPUBinPath != "" { + env = append(env, corev1.EnvVar{Name: appendPathEnvName, Value: r.GPUBinPath}) + } + } + env = append(env, r.tracingEnvVars(pipelineInstance)...) + env = append(env, filter.Env...) + + c := corev1.Container{ + Name: filter.Name, + Image: filter.Image, + Command: filter.Command, + Args: filter.Args, + Env: env, + ImagePullPolicy: filter.ImagePullPolicy, + VolumeMounts: []corev1.VolumeMount{ + {Name: "workspace", MountPath: "/ws"}, + }, + } + if filter.Resources != nil { + c.Resources = *filter.Resources + } + containers = append(containers, c) + } + return containers +} + +// bindingObjectKey resolves the exact S3 object a binding's claimer must +// download: Bucket.Prefix used as the full key. The platform's media model +// has no separate object concept — single-file medias ship their full +// object key in prefix (the agent parses the media URL path into it), and +// the legacy queue path has always relied on prefix-as-key listing for +// single-file sources. Direct mode uses the same value as the exact GET +// key; a folder-style prefix fails at download with a per-claimer error +// naming the key. +func bindingObjectKey(b ResolvedSourceBinding) string { + if b.Source == nil || b.Source.Spec.Bucket == nil { + return "" + } + return b.Source.Spec.Bucket.Prefix +} + +// claimerContainerName derives the init-claimer container name for a +// binding's filter. Kubernetes container names are DNS-1123 labels capped at +// 63 characters, and the CRD allows filterName itself to be 63 characters — +// "claimer-" (8 chars) plus a 63-char filterName would be 71. So the +// filterName is truncated to 55 characters (63 - len("claimer-")) before +// prefixing. Truncation is a deterministic prefix cut so repeated reconciles +// render byte-identical Job specs; trailing hyphens left by the cut are +// trimmed because DNS-1123 labels must end alphanumeric. Collisions between +// two 63-char filterNames sharing a 55-char prefix are theoretically possible +// but rejected by the apiserver at Job create (duplicate container name) — +// an operator-visible failure rather than a silent one. +func claimerContainerName(filterName string) string { + const maxFilterNameLen = 63 - len("claimer-") // 55 + if len(filterName) > maxFilterNameLen { + filterName = strings.TrimRight(filterName[:maxFilterNameLen], "-") + } + return "claimer-" + filterName +} + +// perFilterInputPath computes the workspace path the claimer writes its +// downloaded object to. The extension is preserved from the bucket key +// when present (so openfilter's VideoIn can sniff the format from the +// file path); otherwise we default to ".mp4". +func perFilterInputPath(filterName, objectKey string) string { + ext := filepath.Ext(objectKey) + if ext == "" { + ext = ".mp4" + } + return fmt.Sprintf("/ws/%s%s", filterName, ext) +} + +// isDirectModeSingleBinding reports whether a single-binding batch instance +// should run in direct-download mode: the binding must name a filter +// container (plural `sources` form — the legacy broadcast sentinel has no +// container to scope the per-filter input path to) and its bucket must +// declare `singleObject: true` (prefix is a full object key, nothing to +// scan). Multi-binding instances always run direct; bindings without the +// flag keep the queue-based prefix-scan flow. +func isDirectModeSingleBinding(bindings []ResolvedSourceBinding) bool { + if len(bindings) != 1 { + return false + } + b := bindings[0] + return b.FilterName != "" && + b.Source != nil && + b.Source.Spec.Bucket != nil && + b.Source.Spec.Bucket.SingleObject +} + +// degradeBatchValidation marks a multi-source batch validation failure: +// Degraded=True with the given reason, Progressing=False (a +// validation-blocked instance is not making progress; Succeeded is left +// untouched — it records a past terminal outcome). The status write error +// is propagated so controller-runtime retries — these validation paths +// intentionally don't requeue, so a swallowed write failure would leave the +// server-side status stale forever. +func (r *PipelineInstanceReconciler) degradeBatchValidation(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, reason, msg string) (ctrl.Result, error) { + log := logf.FromContext(ctx) + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, reason, msg) + r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionFalse, reason, msg) + if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { + if apierrors.IsConflict(statusErr) { + return ctrl.Result{Requeue: true}, nil + } + log.Error(statusErr, "Failed to update status after multi-source batch validation") + return ctrl.Result{}, statusErr + } + return ctrl.Result{}, nil +} diff --git a/internal/controller/pipelineinstance_controller_batch_multisource_test.go b/internal/controller/pipelineinstance_controller_batch_multisource_test.go new file mode 100644 index 0000000..4ec5e94 --- /dev/null +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -0,0 +1,639 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package controller + +import ( + "context" + "strings" + "testing" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + pipelinesv1alpha1 "github.com/PlainsightAI/openfilter-pipelines-controller/api/v1alpha1" +) + +// These tests close the coverage gap on reconcileBatchMultiSource — the +// top-level multi-source batch reconciler. Each test drives the function +// with a fake K8s client and verifies one of its six branches: +// +// 1. Bucket-source validation (binding whose source isn't a Bucket) +// 2. Object-key validation (binding whose Bucket.Prefix — the full object key — is empty) +// 3. Happy-path Job create + StartTime stamp +// 4. Job-complete observation → PipelineInstance Succeeded +// 5. Job-failed observation → PipelineInstance Degraded +// 6. Job-progressing observation → PipelineInstance Progressing +// +// The builders themselves (buildMultiSourceBatchJob etc.) are covered by +// pipelineinstance_multisource_test.go; these tests focus on the +// state-machine side. + +// makeMultiSourcePI returns the PipelineInstance / Pipeline / 2-binding +// fixture the reconciler tests reuse. The bindings reference fake +// in-memory PipelineSource objects (also returned) so callers can mutate +// them per-test to drive the validation branches. +func makeMultiSourcePI(t *testing.T) (*pipelinesv1alpha1.PipelineInstance, *pipelinesv1alpha1.Pipeline, []ResolvedSourceBinding) { + t.Helper() + pi := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ms-batch-pi", + Namespace: "default", + UID: types.UID("ms-batch-pi-uid"), + }, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "ms-batch-pipeline"}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-front"}}, + {FilterName: "back-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-back"}}, + }, + }, + } + pipeline := &pipelinesv1alpha1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "ms-batch-pipeline", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineSpec{ + Mode: pipelinesv1alpha1.PipelineModeBatch, + Filters: []pipelinesv1alpha1.Filter{ + {Name: "front-cam", Image: "videoin:latest"}, + {Name: "back-cam", Image: "videoin:latest"}, + {Name: "image-out", Image: "imageout:latest"}, + }, + }, + } + bindings := []ResolvedSourceBinding{ + {FilterName: "front-cam", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "front.mp4"}, + }, + }}, + {FilterName: "back-cam", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "back.mp4"}, + }, + }}, + } + return pi, pipeline, bindings +} + +// newMSReconciler returns a reconciler wired to a fake client with the +// given seed objects, including status-subresource support so the +// reconciler's r.Status().Update calls succeed against the fake. +func newMSReconciler(t *testing.T, objects ...interface{}) *PipelineInstanceReconciler { + t.Helper() + sch := reconcileSpanScheme(t) + + // Convert to client.Object via type assertion guarded by the schema + // registration above. Fake builder is happy with the interface. + clientObjects := make([]any, 0, len(objects)) + clientObjects = append(clientObjects, objects...) + + builder := fake.NewClientBuilder(). + WithScheme(sch). + WithStatusSubresource(&pipelinesv1alpha1.PipelineInstance{}) + for _, o := range clientObjects { + switch obj := o.(type) { + case *pipelinesv1alpha1.PipelineInstance: + builder = builder.WithObjects(obj) + case *pipelinesv1alpha1.Pipeline: + builder = builder.WithObjects(obj) + case *batchv1.Job: + builder = builder.WithObjects(obj) + default: + t.Fatalf("unsupported seed object type %T", obj) + } + } + return &PipelineInstanceReconciler{ + Client: builder.Build(), + Scheme: sch, + ClaimerImage: "claimer:test", + } +} + +// findCondition returns the condition with the given type, or zero value. +func findCondition(t *testing.T, conds []metav1.Condition, ctype string) metav1.Condition { + t.Helper() + for _, c := range conds { + if c.Type == ctype { + return c + } + } + return metav1.Condition{} +} + +// TestReconcileBatchMultiSource_RejectsNonBucketSource pins branch 1: +// any binding whose PipelineSource isn't a Bucket source surfaces a +// `MultiSourceBatchInvalidSource` Degraded condition and no Job is +// created. +func TestReconcileBatchMultiSource_RejectsNonBucketSource(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + // Corrupt the back-cam binding to look like an RTSP source. + bindings[1].Source = &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "cam2"}, + }, + } + r := newMSReconciler(t, pi) + + if _, err := r.reconcileBatchMultiSource(context.Background(), pi, pipeline, bindings); err != nil { + t.Fatalf("expected nil error (validation failure surfaces via Condition), got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded) + if cond.Status != metav1.ConditionTrue || cond.Reason != "MultiSourceBatchInvalidSource" { + t.Errorf("expected Degraded=True (MultiSourceBatchInvalidSource), got %+v", cond) + } + if prog := findCondition(t, updated.Status.Conditions, ConditionTypeProgressing); prog.Status == metav1.ConditionTrue { + t.Errorf("expected Progressing!=True alongside the validation Degraded, got %+v", prog) + } + + // No Job must have been created when validation rejects. + job := &batchv1.Job{} + err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name + "-job", Namespace: pi.Namespace}, job) + if err == nil { + t.Errorf("expected no Job to be created after validation failure") + } +} + +// TestReconcileBatchMultiSource_RejectsMissingObject pins branch 2: a +// binding with a Bucket source whose Prefix is empty (no object key to +// resolve) surfaces a `MultiSourceBatchMissingObject` Degraded condition. +func TestReconcileBatchMultiSource_RejectsMissingObject(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + // Strip the object key on back-cam. + bindings[1].Source.Spec.Bucket.Prefix = "" + r := newMSReconciler(t, pi) + + if _, err := r.reconcileBatchMultiSource(context.Background(), pi, pipeline, bindings); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded) + if cond.Status != metav1.ConditionTrue || cond.Reason != "MultiSourceBatchMissingObject" { + t.Errorf("expected Degraded=True (MultiSourceBatchMissingObject), got %+v", cond) + } + if prog := findCondition(t, updated.Status.Conditions, ConditionTypeProgressing); prog.Status == metav1.ConditionTrue { + t.Errorf("expected Progressing!=True alongside the validation Degraded, got %+v", prog) + } +} + +// TestReconcileBatchMultiSource_ClearsValidationDegradedOnRecovery pins the +// recovery half of the validation contract: a reconcile that Degrades on a +// missing object key must NOT leave the instance Degraded forever — once the +// PipelineSource is fixed (the event the PipelineSource watch mapping in +// SetupWithManager re-triggers reconcile for), the next pass clears the +// Degraded condition and proceeds to create the Job. +func TestReconcileBatchMultiSource_ClearsValidationDegradedOnRecovery(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + // Break the back-cam binding: empty prefix → no object key. + bindings[1].Source.Spec.Bucket.Prefix = "" + r := newMSReconciler(t, pi) + + // Pass 1: validation fails, Degraded=True is persisted. + if _, err := r.reconcileBatchMultiSource(context.Background(), pi, pipeline, bindings); err != nil { + t.Fatalf("first reconcile: expected nil error, got %v", err) + } + degraded := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, degraded); err != nil { + t.Fatalf("re-fetch PI after first reconcile: %v", err) + } + cond := findCondition(t, degraded.Status.Conditions, ConditionTypeDegraded) + if cond.Status != metav1.ConditionTrue || cond.Reason != ReasonMultiSourceBatchMissingObject { + t.Fatalf("expected Degraded=True (%s) after first reconcile, got %+v", ReasonMultiSourceBatchMissingObject, cond) + } + + // Fix the source, then re-reconcile against the freshly fetched PI — + // the shape a watch-triggered reconcile would see. + bindings[1].Source.Spec.Bucket.Prefix = "back.mp4" + if _, err := r.reconcileBatchMultiSource(context.Background(), degraded, pipeline, bindings); err != nil { + t.Fatalf("second reconcile: expected nil error, got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI after second reconcile: %v", err) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded); cond.Type != "" { + t.Errorf("expected Degraded condition to be cleared after the source was fixed, got %+v", cond) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeProgressing); cond.Status != metav1.ConditionTrue { + t.Errorf("expected Progressing=True after recovery, got %+v", cond) + } + job := &batchv1.Job{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name + "-job", Namespace: pi.Namespace}, job); err != nil { + t.Errorf("expected Job to be created after recovery: %v", err) + } +} + +// TestClaimerContainerName pins the 63-char container-name budget: the CRD +// allows filterName up to 63 chars, and "claimer-" (8 chars) on top of that +// would exceed Kubernetes' DNS-1123 container-name limit. The helper must +// truncate the filterName to 55 chars (63 - 8), deterministically, and never +// leave a trailing hyphen at the cut point. +func TestClaimerContainerName(t *testing.T) { + if got := claimerContainerName("front-cam"); got != "claimer-front-cam" { + t.Errorf("short name must pass through untruncated, got %q", got) + } + + long := strings.Repeat("a", 63) // max the CRD pattern allows + got := claimerContainerName(long) + if len(got) > 63 { + t.Errorf("claimer name for 63-char filterName exceeds 63 chars: %q (len %d)", got, len(got)) + } + if want := "claimer-" + strings.Repeat("a", 55); got != want { + t.Errorf("claimer name = %q, want %q", got, want) + } + if again := claimerContainerName(long); again != got { + t.Errorf("truncation must be deterministic: %q != %q", again, got) + } + + // A hyphen landing exactly at the cut point must be trimmed — DNS-1123 + // labels can't end with '-'. + hyphenAtCut := strings.Repeat("a", 54) + "-" + strings.Repeat("b", 8) // 63 chars; [:55] ends with '-' + got = claimerContainerName(hyphenAtCut) + if strings.HasSuffix(got, "-") { + t.Errorf("claimer name must not end with a hyphen, got %q", got) + } + if want := "claimer-" + strings.Repeat("a", 54); got != want { + t.Errorf("claimer name = %q, want %q", got, want) + } +} + +// TestPerFilterInputPath pins the claimer↔filter path contract: the claimer +// downloads to this exact path and the filter container's +// `file://$(VIDEO_INPUT_PATH)` must resolve to the same bytes — a drift here +// breaks batch runs with a file-not-found deep inside the pod. +func TestPerFilterInputPath(t *testing.T) { + cases := []struct { + name string + filterName string + objectKey string + want string + }{ + {"extension preserved", "front-cam", "clips/front_lobby_001.mp4", "/ws/front-cam.mp4"}, + {"non-mp4 extension preserved", "back-cam", "clips/back.mkv", "/ws/back-cam.mkv"}, + {"no extension defaults to mp4", "front-cam", "clips/raw-dump", "/ws/front-cam.mp4"}, + {"empty key defaults to mp4", "front-cam", "", "/ws/front-cam.mp4"}, + {"double extension keeps last", "cam", "clips/video.tar.gz", "/ws/cam.gz"}, + {"dot in directory not an extension", "cam", "v1.2/clip", "/ws/cam.mp4"}, + {"trailing dot yields bare dot ext", "cam", "clips/video.", "/ws/cam."}, + {"uppercase extension preserved verbatim", "cam", "clips/VIDEO.MP4", "/ws/cam.MP4"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := perFilterInputPath(tc.filterName, tc.objectKey); got != tc.want { + t.Errorf("perFilterInputPath(%q, %q) = %q, want %q", tc.filterName, tc.objectKey, got, tc.want) + } + }) + } +} + +// TestReconcileBatchMultiSource_CreatesJobAndStampsStartTime pins branch +// 3: with valid bindings, the reconciler creates the Job (named +// "-job"), stamps StartTime, sets Status.JobName, and leaves a +// Progressing=True condition. +func TestReconcileBatchMultiSource_CreatesJobAndStampsStartTime(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + r := newMSReconciler(t, pi) + + if _, err := r.reconcileBatchMultiSource(context.Background(), pi, pipeline, bindings); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + job := &batchv1.Job{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name + "-job", Namespace: pi.Namespace}, job); err != nil { + t.Fatalf("expected Job %s-job to exist: %v", pi.Name, err) + } + if len(job.Spec.Template.Spec.InitContainers) != 2 { + t.Errorf("expected 2 init claimers, got %d", len(job.Spec.Template.Spec.InitContainers)) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + if updated.Status.StartTime == nil { + t.Errorf("expected StartTime to be stamped on first reconcile") + } + if updated.Status.JobName != pi.Name+"-job" { + t.Errorf("expected Status.JobName = %q, got %q", pi.Name+"-job", updated.Status.JobName) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeProgressing); cond.Status != metav1.ConditionTrue { + t.Errorf("expected Progressing=True after Job create, got %+v", cond) + } +} + +// TestReconcileBatchMultiSource_JobCompleteMarksSucceeded pins branch 4: +// when the observed Job has a JobComplete=True condition the reconciler +// surfaces Succeeded=True on the PipelineInstance and stamps +// CompletionTime. +func TestReconcileBatchMultiSource_JobCompleteMarksSucceeded(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + // Pre-seed the PI with StartTime so reconciler skips the stamp. + now := metav1.Now() + pi.Status.StartTime = &now + // Pre-seed the Job in Complete state. + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: pi.Name + "-job", + Namespace: pi.Namespace, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + {Type: batchv1.JobComplete, Status: corev1.ConditionTrue, Reason: "Completed", Message: "ok"}, + }, + }, + } + r := newMSReconciler(t, pi, job) + + if _, err := r.reconcileBatchMultiSource(context.Background(), pi, pipeline, bindings); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeSucceeded); cond.Status != metav1.ConditionTrue { + t.Errorf("expected Succeeded=True, got %+v", cond) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeProgressing); cond.Status != metav1.ConditionFalse { + t.Errorf("expected Progressing=False once Succeeded, got %+v", cond) + } + if updated.Status.CompletionTime == nil { + t.Errorf("expected CompletionTime to be stamped on success") + } +} + +// TestReconcileBatchMultiSource_JobFailedMarksDegraded pins branch 5: +// JobFailed=True translates to Degraded=True with the Job condition's +// Reason/Message propagated for operator visibility. +func TestReconcileBatchMultiSource_JobFailedMarksDegraded(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + now := metav1.Now() + pi.Status.StartTime = &now + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: pi.Name + "-job", + Namespace: pi.Namespace, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + {Type: batchv1.JobFailed, Status: corev1.ConditionTrue, Reason: "BackoffLimitExceeded", Message: "Job has reached the specified backoff limit"}, + }, + }, + } + r := newMSReconciler(t, pi, job) + + if _, err := r.reconcileBatchMultiSource(context.Background(), pi, pipeline, bindings); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded) + if cond.Status != metav1.ConditionTrue { + t.Fatalf("expected Degraded=True, got %+v", cond) + } + if cond.Reason != "BackoffLimitExceeded" { + t.Errorf("expected Job condition Reason to propagate (got %q)", cond.Reason) + } + if cond.Message == "" || cond.Message != "Job has reached the specified backoff limit" { + t.Errorf("expected Job condition Message to propagate (got %q)", cond.Message) + } + if updated.Status.CompletionTime == nil { + t.Errorf("expected CompletionTime to be stamped on failure") + } +} + +// TestReconcileBatchMultiSource_JobProgressingStaysProgressing pins +// branch 6: a Job without any terminal condition leaves the PI in +// Progressing=True. This is the common steady-state pass. +func TestReconcileBatchMultiSource_JobProgressingStaysProgressing(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + now := metav1.Now() + pi.Status.StartTime = &now + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: pi.Name + "-job", + Namespace: pi.Namespace, + }, + Status: batchv1.JobStatus{Active: 1}, + } + r := newMSReconciler(t, pi, job) + + res, err := r.reconcileBatchMultiSource(context.Background(), pi, pipeline, bindings) + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if res.RequeueAfter == 0 { + t.Errorf("expected RequeueAfter set for progressing reconcile") + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeProgressing); cond.Status != metav1.ConditionTrue { + t.Errorf("expected Progressing=True for active Job, got %+v", cond) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeSucceeded); cond.Status == metav1.ConditionTrue { + t.Errorf("Succeeded must not be True while Job is still active, got %+v", cond) + } +} + +// TestIsDirectModeSingleBinding pins the routing predicate for the +// single-binding direct path: only a NAMED binding (plural sources form) +// whose bucket declares singleObject takes direct mode; everything else +// keeps the queue flow. +func TestIsDirectModeSingleBinding(t *testing.T) { + singleObjBucket := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "clip.png", SingleObject: true}, + }, + } + prefixBucket := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "clips/"}, + }, + } + rtsp := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "cam.example"}, + }, + } + + cases := []struct { + name string + bindings []ResolvedSourceBinding + want bool + }{ + {"named binding with singleObject bucket", []ResolvedSourceBinding{{FilterName: "image-in", Source: singleObjBucket}}, true}, + {"named binding without singleObject", []ResolvedSourceBinding{{FilterName: "image-in", Source: prefixBucket}}, false}, + {"broadcast sentinel (legacy sourceRef) never direct", []ResolvedSourceBinding{{FilterName: "", Source: singleObjBucket}}, false}, + {"rtsp source never direct", []ResolvedSourceBinding{{FilterName: "cam", Source: rtsp}}, false}, + {"nil source", []ResolvedSourceBinding{{FilterName: "cam", Source: nil}}, false}, + {"two bindings handled by the multi-source branch, not this predicate", []ResolvedSourceBinding{ + {FilterName: "a", Source: singleObjBucket}, {FilterName: "b", Source: singleObjBucket}, + }, false}, + {"no bindings", nil, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isDirectModeSingleBinding(tc.bindings); got != tc.want { + t.Errorf("isDirectModeSingleBinding() = %v, want %v", got, tc.want) + } + }) + } +} + +// TestReconcileBatch_SingleBindingSingleObject_UsesDirectMode pins the fix +// for single-source batch image pipelines: a single named binding whose +// bucket declares singleObject routes through reconcileBatch into the +// direct-download path — one init claimer carrying S3_OBJECT_KEY (no +// Valkey env), VIDEO_INPUT_PATH preserving the object's extension +// (/ws/image-in.png, not the queue path's fixed input.mp4), and a +// parallelism=1/completions=1 Job. ValkeyClient is intentionally nil: +// touching the queue would panic. +func TestReconcileBatch_SingleBindingSingleObject_UsesDirectMode(t *testing.T) { + pi := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "single-direct-pi", + Namespace: "default", + UID: types.UID("single-direct-pi-uid"), + }, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "single-direct-pipeline"}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "image-in", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-img"}}, + }, + }, + } + pipeline := &pipelinesv1alpha1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "single-direct-pipeline", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineSpec{ + Mode: pipelinesv1alpha1.PipelineModeBatch, + Filters: []pipelinesv1alpha1.Filter{ + {Name: "image-in", Image: "imagein:latest"}, + {Name: "image-out", Image: "imageout:latest"}, + }, + }, + } + bindings := []ResolvedSourceBinding{ + {FilterName: "image-in", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "images/triangle.png", SingleObject: true}, + }, + }}, + } + r := newMSReconciler(t, pi) + + if _, err := r.reconcileBatch(context.Background(), pi, pipeline, bindings); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + job := &batchv1.Job{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name + "-job", Namespace: pi.Namespace}, job); err != nil { + t.Fatalf("expected direct-mode Job to exist: %v", err) + } + if len(job.Spec.Template.Spec.InitContainers) != 1 { + t.Fatalf("expected 1 init claimer, got %d", len(job.Spec.Template.Spec.InitContainers)) + } + claimer := job.Spec.Template.Spec.InitContainers[0] + if claimer.Name != "claimer-image-in" { + t.Errorf("claimer name = %q, want claimer-image-in", claimer.Name) + } + var objectKey, inputPath string + hasValkey := false + for _, e := range claimer.Env { + switch e.Name { + case "S3_OBJECT_KEY": + objectKey = e.Value + case "VIDEO_INPUT_PATH": + inputPath = e.Value + case "VALKEY_URL", "STREAM", "GROUP": + hasValkey = true + } + } + if objectKey != "images/triangle.png" { + t.Errorf("S3_OBJECT_KEY = %q, want images/triangle.png", objectKey) + } + if inputPath != "/ws/image-in.png" { + t.Errorf("VIDEO_INPUT_PATH = %q, want /ws/image-in.png (extension preserved)", inputPath) + } + if hasValkey { + t.Errorf("direct-mode claimer must not carry Valkey env, got %v", claimer.Env) + } +} + +// TestReconcileBatch_SingleBindingWithoutFlag_KeepsQueueMode pins backward +// compatibility: a single binding whose bucket does NOT declare +// singleObject must keep the queue flow — reconcileBatch must not route +// it into the direct path (asserted by the absence of the direct-mode +// Job; the queue path errors immediately on this fixture's nil +// ValkeyClient, which is fine — reaching the queue IS the assertion). +func TestReconcileBatch_SingleBindingWithoutFlag_KeepsQueueMode(t *testing.T) { + pi := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "single-queue-pi", + Namespace: "default", + UID: types.UID("single-queue-pi-uid"), + }, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "single-queue-pipeline"}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "video-in", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-vid"}}, + }, + }, + } + pipeline := &pipelinesv1alpha1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "single-queue-pipeline", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineSpec{ + Mode: pipelinesv1alpha1.PipelineModeBatch, + Filters: []pipelinesv1alpha1.Filter{{Name: "video-in", Image: "videoin:latest"}}, + }, + } + bindings := []ResolvedSourceBinding{ + {FilterName: "video-in", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "clips/"}, + }, + }}, + } + r := newMSReconciler(t, pi) + + // The queue path will fail fast on the nil Valkey client — that is the + // expected (and asserted) routing outcome; what must NOT happen is the + // direct-mode Job appearing. + _, _ = r.reconcileBatch(context.Background(), pi, pipeline, bindings) + + job := &batchv1.Job{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name + "-job", Namespace: pi.Namespace}, job); err == nil { + if len(job.Spec.Template.Spec.InitContainers) == 1 { + for _, e := range job.Spec.Template.Spec.InitContainers[0].Env { + if e.Name == "S3_OBJECT_KEY" { + t.Fatalf("single binding without singleObject must not run in direct mode") + } + } + } + } +} diff --git a/internal/controller/pipelineinstance_controller_reconcile_span_test.go b/internal/controller/pipelineinstance_controller_reconcile_span_test.go index 1651345..a983e19 100644 --- a/internal/controller/pipelineinstance_controller_reconcile_span_test.go +++ b/internal/controller/pipelineinstance_controller_reconcile_span_test.go @@ -168,7 +168,7 @@ func TestReconcileBatch_BuildAndApplyAreSiblingsOfReconcile(t *testing.T) { }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: pipeline.Name}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: source.Name}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: source.Name}, }, Status: pipelinesv1alpha1.PipelineInstanceStatus{ StartTime: &startTime, @@ -193,7 +193,7 @@ func TestReconcileBatch_BuildAndApplyAreSiblingsOfReconcile(t *testing.T) { } ctx, endReconcile := withReconcileSpan(context.Background()) - if _, err := r.reconcileBatch(ctx, pi, pipeline, source); err != nil { + if _, err := r.reconcileBatch(ctx, pi, pipeline, []ResolvedSourceBinding{{Source: source}}); err != nil { t.Fatalf("reconcileBatch returned error: %v", err) } endReconcile() @@ -231,7 +231,7 @@ func makeStreamingFixtures(name string) (*pipelinesv1alpha1.Pipeline, *pipelines }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: pipeline.Name}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: source.Name}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: source.Name}, }, Status: pipelinesv1alpha1.PipelineInstanceStatus{ StartTime: &startTime, @@ -264,7 +264,7 @@ func TestReconcileStreaming_Create_BuildAndApplyAreSiblingsOfReconcile(t *testin } ctx, endReconcile := withReconcileSpan(context.Background()) - if _, err := r.reconcileStreaming(ctx, pi, pipeline, source); err != nil { + if _, err := r.reconcileStreaming(ctx, pi, pipeline, []ResolvedSourceBinding{{Source: source}}); err != nil { t.Fatalf("reconcileStreaming (create) returned error: %v", err) } endReconcile() @@ -379,7 +379,7 @@ func TestReconcileStreaming_Update_BuildAndApplyAreSiblingsOfReconcile(t *testin } ctx, endReconcile := withReconcileSpan(context.Background()) - if _, err := r.reconcileStreaming(ctx, pi, pipeline, source); err != nil { + if _, err := r.reconcileStreaming(ctx, pi, pipeline, []ResolvedSourceBinding{{Source: source}}); err != nil { t.Fatalf("reconcileStreaming (update) returned error: %v", err) } endReconcile() diff --git a/internal/controller/pipelineinstance_controller_streaming.go b/internal/controller/pipelineinstance_controller_streaming.go index a3e9333..b3e0183 100644 --- a/internal/controller/pipelineinstance_controller_streaming.go +++ b/internal/controller/pipelineinstance_controller_streaming.go @@ -37,8 +37,13 @@ import ( "github.com/PlainsightAI/openfilter-pipelines-controller/internal/tracing" ) -// reconcileStreaming handles the streaming (Deployment-based) reconciliation path -func (r *PipelineInstanceReconciler) reconcileStreaming(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, pipelineSource *pipelinesv1alpha1.PipelineSource) (ctrl.Result, error) { +// reconcileStreaming handles the streaming (Deployment-based) reconciliation +// path. `sourceBindings` is the resolved list of (filterName, source) pairs; +// for legacy single-source `Spec.SourceRef` PipelineInstances the list has +// one entry with FilterName="" (broadcast). For multi-source `Spec.Sources` +// PipelineInstances the list has one entry per binding and the controller +// targets the matching filter container by name. +func (r *PipelineInstanceReconciler) reconcileStreaming(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding) (ctrl.Result, error) { log := logf.FromContext(ctx) // Handle deletion (finalizer cleanup) before any status initialization @@ -106,7 +111,7 @@ func (r *PipelineInstanceReconciler) reconcileStreaming(ctx context.Context, pip } // Step 1: Ensure Deployment exists - if err := r.ensureStreamingDeployment(ctx, pipelineInstance, pipeline, pipelineSource); err != nil { + if err := r.ensureStreamingDeployment(ctx, pipelineInstance, pipeline, sourceBindings); err != nil { log.Error(err, "Failed to ensure streaming deployment") r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "DeploymentCreationFailed", err.Error()) if err := r.Status().Update(ctx, pipelineInstance); err != nil { @@ -131,10 +136,20 @@ func (r *PipelineInstanceReconciler) reconcileStreaming(ctx context.Context, pip // Don't fail reconciliation, just log and continue } - // Step 3: Check for idle timeout - if pipelineSource != nil && pipelineSource.Spec.RTSP != nil && pipelineSource.Spec.RTSP.IdleTimeout != nil { - if shouldComplete, reason := r.checkIdleTimeout(pipelineInstance, pipelineSource); shouldComplete { - log.Info("Streaming run idle timeout reached, marking as complete", "reason", reason) + // Step 3: Check for idle timeout. Multi-source pipelines anchor the + // idle-timeout policy to ONE binding's source for V1 — the semantic of + // "ANY source idle vs ALL sources idle" is not yet well-defined for + // multi-camera. Operators who need per-source idle budgets can author + // multiple PipelineInstances today. The anchor is the binding with the + // lexicographically-smallest filterName (NOT slice position: sources is + // a listType=map field, so element order is not semantically stable + // across patches), making the applied policy deterministic. + idleAnchor := idleAnchorBinding(sourceBindings) + idleSource := idleAnchor.Source + idleAnchorName := idleAnchor.FilterName + if idleSource != nil && idleSource.Spec.RTSP != nil && idleSource.Spec.RTSP.IdleTimeout != nil { + if shouldComplete, reason := r.checkIdleTimeout(pipelineInstance, idleSource); shouldComplete { + log.Info("Streaming run idle timeout reached, marking as complete", "reason", reason, "anchorBinding", idleAnchorName) r.setCondition(pipelineInstance, ConditionTypeSucceeded, metav1.ConditionTrue, "IdleTimeout", reason) r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionFalse, "IdleTimeout", reason) @@ -176,8 +191,10 @@ func (r *PipelineInstanceReconciler) reconcileStreaming(ctx context.Context, pip return ctrl.Result{RequeueAfter: StatusUpdateInterval}, nil } -// ensureStreamingDeployment creates or updates the Deployment for streaming mode -func (r *PipelineInstanceReconciler) ensureStreamingDeployment(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, pipelineSource *pipelinesv1alpha1.PipelineSource) error { +// ensureStreamingDeployment creates or updates the Deployment for streaming +// mode. `sourceBindings` carries the per-filter source bindings; for legacy +// single-source CRs there is one entry with FilterName="" (broadcast). +func (r *PipelineInstanceReconciler) ensureStreamingDeployment(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding) error { log := logf.FromContext(ctx) // Lazy-init Status.Streaming so callers (and tests) don't have to @@ -204,7 +221,7 @@ func (r *PipelineInstanceReconciler) ensureStreamingDeployment(ctx context.Conte // batch path (buildJob) so a SetControllerReference failure flows // into the build span rather than escaping the trace. buildCtx, endBuild := tracing.StartSpan(ctx, spanBuild) - desiredDeployment := r.buildStreamingDeployment(buildCtx, pipelineInstance, pipeline, pipelineSource, deploymentName) + desiredDeployment := r.buildStreamingDeployment(buildCtx, pipelineInstance, pipeline, sourceBindings, deploymentName) desiredContainers := desiredDeployment.Spec.Template.Spec.Containers desiredReplicas := int32(0) if desiredDeployment.Spec.Replicas != nil { @@ -292,15 +309,36 @@ func (r *PipelineInstanceReconciler) ensureStreamingDeployment(ctx context.Conte return nil } -// buildStreamingDeployment constructs a Deployment for streaming mode -func (r *PipelineInstanceReconciler) buildStreamingDeployment(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, pipelineSource *pipelinesv1alpha1.PipelineSource, deploymentName string) *appsv1.Deployment { +// buildStreamingDeployment constructs a Deployment for streaming mode. +// +// `sourceBindings` carries the per-filter source bindings (PLAT-1071). For +// each filter container we look up the matching binding by name and inject +// the source-derived env vars (RTSP_URL plus, when credentials are present, +// _RTSP_USERNAME / _RTSP_PASSWORD) into only that container. Legacy CRs that +// use the singular `Spec.SourceRef` convenience form resolve to one binding with +// FilterName="" which broadcasts the env vars to every filter container — +// preserving today's behavior. +func (r *PipelineInstanceReconciler) buildStreamingDeployment(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding, deploymentName string) *appsv1.Deployment { log := logf.FromContext(ctx) - log.V(1).Info("Building streaming deployment", "deployment", deploymentName, "filters", len(pipeline.Spec.Filters)) + log.V(1).Info("Building streaming deployment", "deployment", deploymentName, "filters", len(pipeline.Spec.Filters), "bindings", len(sourceBindings)) replicas := int32(1) maxUnavailable := intstr.FromInt32(0) maxSurge := intstr.FromInt32(1) + // Split bindings into per-filter (FilterName!="") and broadcast (FilterName==""). + // The per-filter map gives O(1) lookup inside the container loop; the + // broadcast slice is applied to every container after the per-filter env. + bindingsByFilter := make(map[string]*pipelinesv1alpha1.PipelineSource, len(sourceBindings)) + broadcastSources := make([]*pipelinesv1alpha1.PipelineSource, 0, 1) + for i := range sourceBindings { + if sourceBindings[i].FilterName == "" { + broadcastSources = append(broadcastSources, sourceBindings[i].Source) + } else { + bindingsByFilter[sourceBindings[i].FilterName] = sourceBindings[i].Source + } + } + // Build filter containers containers := make([]corev1.Container, 0, len(pipeline.Spec.Filters)) for _, filter := range pipeline.Spec.Filters { @@ -317,14 +355,35 @@ func (r *PipelineInstanceReconciler) buildStreamingDeployment(ctx context.Contex container.Args = filter.Args } - // Inject RTSP environment variables first so they can be referenced in filter config + // Resolve the source(s) feeding THIS container. Order is: + // 1. The per-filter binding whose `filterName` matches this + // container's name (multi-source path). At most one because + // `Sources` is listType=map keyed on filterName. + // 2. Any broadcast bindings (legacy `Spec.SourceRef` path, exactly + // one entry). These are injected into every container. + // In the multi-source path, containers without a matching binding + // receive NO source env vars — they're downstream filters whose + // `sources: tcp://localhost:...` config wires them to siblings. + var perContainerSources []*pipelinesv1alpha1.PipelineSource + if src, ok := bindingsByFilter[filter.Name]; ok { + perContainerSources = append(perContainerSources, src) + } + perContainerSources = append(perContainerSources, broadcastSources...) + + // Inject RTSP environment variables first so they can be referenced + // in filter config (filter authors write `sources: "$(RTSP_URL)"`). + // If multiple sources end up applied to one container — only happens + // when a singular SourceRef coexists with a Sources entry that targets + // this container, which validation rejects — the last write wins. var envVars []corev1.EnvVar - if pipelineSource != nil && pipelineSource.Spec.RTSP != nil { + for _, src := range perContainerSources { + if src == nil || src.Spec.RTSP == nil { + continue + } // If credentials are provided, inject internal env vars for username/password // and build URL with embedded credentials - if pipelineSource.Spec.RTSP.CredentialsSecret != nil { - secretName := pipelineSource.Spec.RTSP.CredentialsSecret.Name - // Internal env vars for credential substitution + if src.Spec.RTSP.CredentialsSecret != nil { + secretName := src.Spec.RTSP.CredentialsSecret.Name envVars = append(envVars, corev1.EnvVar{ Name: "_RTSP_USERNAME", @@ -345,15 +404,13 @@ func (r *PipelineInstanceReconciler) buildStreamingDeployment(ctx context.Contex }, }, ) - // Build URL with credential placeholders - rtspURL := buildRTSPURLWithCredentials(pipelineSource.Spec.RTSP) + rtspURL := buildRTSPURLWithCredentials(src.Spec.RTSP) envVars = append(envVars, corev1.EnvVar{ Name: "RTSP_URL", Value: rtspURL, }) } else { - // No credentials, build simple URL - rtspURL := buildRTSPURL(pipelineSource.Spec.RTSP) + rtspURL := buildRTSPURL(src.Spec.RTSP) envVars = append(envVars, corev1.EnvVar{ Name: "RTSP_URL", Value: rtspURL, @@ -708,3 +765,19 @@ func (r *PipelineInstanceReconciler) deleteFilterServices(ctx context.Context, p return nil } + +// idleAnchorBinding picks the binding whose source anchors the V1 +// idle-timeout policy: the lexicographically-smallest FilterName. NOT slice +// position — `spec.sources` is a listType=map field, so element order is +// not semantically stable across patches; sorting makes the applied policy +// deterministic. The legacy broadcast sentinel (single binding, +// FilterName=="") trivially anchors itself. +func idleAnchorBinding(bindings []ResolvedSourceBinding) ResolvedSourceBinding { + anchor := bindings[0] + for _, b := range bindings[1:] { + if b.FilterName < anchor.FilterName { + anchor = b + } + } + return anchor +} diff --git a/internal/controller/pipelineinstance_controller_test.go b/internal/controller/pipelineinstance_controller_test.go index be279ad..6982b54 100644 --- a/internal/controller/pipelineinstance_controller_test.go +++ b/internal/controller/pipelineinstance_controller_test.go @@ -404,7 +404,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -449,7 +449,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, Execution: &pipelinesv1alpha1.ExecutionConfig{ @@ -504,6 +504,15 @@ var _ = Describe("PipelineInstance Controller", func() { // Only user-defined filters should be present. Expect(job.Spec.Template.Spec.Containers).To(HaveLen(1)) Expect(job.Spec.Template.Spec.Containers[0].Name).To(Equal("test-filter")) + // Filter containers receive the claimer's download destination so + // VideoIn sources can be authored as `file://$(VIDEO_INPUT_PATH)` + // — the same contract the multi-source path provides per-binding. + // It must be the FIRST entry, before any FILTER_* config env: + // Kubernetes dependent-env expansion only resolves $(VAR) + // references to variables defined earlier in the list. + filterEnv := job.Spec.Template.Spec.Containers[0].Env + Expect(filterEnv).To(ContainElement(corev1.EnvVar{Name: "VIDEO_INPUT_PATH", Value: "/ws/custom-input.mp4"})) + Expect(filterEnv[0].Name).To(Equal("VIDEO_INPUT_PATH")) }) It("should inject per-namespace Valkey credentials from namespace secret in claimer", func() { @@ -516,7 +525,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -603,7 +612,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -661,7 +670,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: "nonexistent-pipeline", }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -705,7 +714,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: "nonexistent-pipeline", }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -746,7 +755,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, // points at a Pipeline we'll create in step 2 }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -832,14 +841,14 @@ var _ = Describe("PipelineInstance Controller", func() { ObjectMeta: metav1.ObjectMeta{Name: matchName, Namespace: namespace}, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: pipelineName}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, }, } missPI := &pipelinesv1alpha1.PipelineInstance{ ObjectMeta: metav1.ObjectMeta{Name: missName, Namespace: namespace}, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "some-other-pipeline"}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, }, } crossPI := &pipelinesv1alpha1.PipelineInstance{ @@ -849,7 +858,7 @@ var _ = Describe("PipelineInstance Controller", func() { Name: pipelineName, Namespace: &otherNS, }, - SourceRef: pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, }, } Expect(k8sClient.Create(ctx, matchPI)).To(Succeed()) @@ -887,7 +896,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: "nonexistent-source", }, }, @@ -969,7 +978,7 @@ var _ = Describe("PipelineInstance Controller", func() { }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: streamPipelineName}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, }, } Expect(k8sClient.Create(ctx, streamInstance)).To(Succeed()) @@ -1012,7 +1021,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1128,7 +1137,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1219,7 +1228,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1322,7 +1331,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1378,7 +1387,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1456,7 +1465,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1512,7 +1521,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1577,7 +1586,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1616,7 +1625,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1660,7 +1669,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1738,7 +1747,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1805,7 +1814,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1873,7 +1882,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1942,7 +1951,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2021,7 +2030,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2144,7 +2153,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2207,7 +2216,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2299,7 +2308,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2376,7 +2385,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2462,7 +2471,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, Execution: &pipelinesv1alpha1.ExecutionConfig{ @@ -2553,7 +2562,7 @@ var _ = Describe("PipelineInstance Controller", func() { }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: streamPipelineName}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, }, } Expect(k8sClient.Create(ctx, streamInstance)).To(Succeed()) @@ -2668,7 +2677,7 @@ var _ = Describe("PipelineInstance Controller", func() { }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: streamPipelineName}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, }, } Expect(k8sClient.Create(ctx, streamInstance)).To(Succeed()) @@ -2737,7 +2746,7 @@ var _ = Describe("PipelineInstance Controller", func() { }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: pipelineName}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: pipelineSourceName}, }, } Expect(k8sClient.Create(ctx, batchInstance)).To(Succeed()) @@ -2820,7 +2829,7 @@ var _ = Describe("PipelineInstance Controller", func() { }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: streamPipelineName}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: rtspSourceName}, }, } Expect(k8sClient.Create(ctx, streamInstance)).To(Succeed()) diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go new file mode 100644 index 0000000..923bdcc --- /dev/null +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -0,0 +1,1026 @@ +package controller + +import ( + "context" + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + pipelinesv1alpha1 "github.com/PlainsightAI/openfilter-pipelines-controller/api/v1alpha1" +) + +// frontCredSecretName is the secret-name fixture used by the +// credentials-injection test below. Constant to satisfy goconst. +const frontCredSecretName = "front-rtsp-creds" + +// findContainerByName is a local helper used by the multi-source tests to +// look up a container by name. The pre-existing `findContainer` in +// pipelineinstance_scheduling_test.go has a different signature, so we +// keep this scoped to avoid collision. +func findContainerByName(t *testing.T, containers []corev1.Container, name string) corev1.Container { + t.Helper() + for _, c := range containers { + if c.Name == name { + return c + } + } + names := make([]string, 0, len(containers)) + for _, c := range containers { + names = append(names, c.Name) + } + t.Fatalf("container %q not found; have: %v", name, names) + return corev1.Container{} // unreachable +} + +// rtspURLEnv returns the value of the RTSP_URL env var on a container, or +// empty string if not set. Kept narrow on purpose — the multi-source tests +// only inspect RTSP_URL today. +func rtspURLEnv(envs []corev1.EnvVar) string { + for _, e := range envs { + if e.Name == "RTSP_URL" { + return e.Value + } + } + return "" +} + +// hasEnv reports whether `envs` contains any entry with the given name. +func hasEnv(envs []corev1.EnvVar, name string) bool { + for _, e := range envs { + if e.Name == name { + return true + } + } + return false +} + +// TestBuildStreamingDeployment_MultiSource_PerContainerRTSP exercises the +// PLAT-1071 multi-source path: a Pipeline with two VideoIn-style filters +// (front-cam, back-cam) plus one downstream filter (webvis). The +// PipelineInstance binds each VideoIn to a distinct PipelineSource via +// NamedSourceRef. The build step must: +// +// - Inject RTSP_URL into the `front-cam` container, valued at the +// `front` source's URL, and NOT the `back` source's URL. +// - Inject RTSP_URL into the `back-cam` container, valued at the +// `back` source's URL. +// - Leave the `webvis` container with NO RTSP_URL env var — it's a +// downstream filter that consumes from siblings via its own +// `sources: tcp://localhost:...` filter config. +func TestBuildStreamingDeployment_MultiSource_PerContainerRTSP(t *testing.T) { + r := &PipelineInstanceReconciler{} + pi := makeMinimalStreamingPipelineInstance() + pi.Spec.Sources = []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "front-source"}}, + {FilterName: "back-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "back-source"}}, + } + + pipeline := &pipelinesv1alpha1.Pipeline{ + Spec: pipelinesv1alpha1.PipelineSpec{ + Filters: []pipelinesv1alpha1.Filter{ + {Name: "front-cam", Image: "videoin:latest"}, + {Name: "back-cam", Image: "videoin:latest"}, + {Name: "webvis", Image: "webvis:latest"}, + }, + }, + } + + frontSource := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "front.example", Port: 554, Path: "/stream"}, + }, + } + backSource := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "back.example", Port: 554, Path: "/stream"}, + }, + } + bindings := []ResolvedSourceBinding{ + {FilterName: "front-cam", Source: frontSource}, + {FilterName: "back-cam", Source: backSource}, + } + + deployment := r.buildStreamingDeployment(context.Background(), pi, pipeline, bindings, "ms-deployment") + containers := deployment.Spec.Template.Spec.Containers + + frontContainer := findContainerByName(t, containers, "front-cam") + backContainer := findContainerByName(t, containers, "back-cam") + webvisContainer := findContainerByName(t, containers, "webvis") + + wantFront := buildRTSPURL(frontSource.Spec.RTSP) + wantBack := buildRTSPURL(backSource.Spec.RTSP) + + if got := rtspURLEnv(frontContainer.Env); got != wantFront { + t.Errorf("front-cam RTSP_URL = %q, want %q", got, wantFront) + } + if got := rtspURLEnv(backContainer.Env); got != wantBack { + t.Errorf("back-cam RTSP_URL = %q, want %q", got, wantBack) + } + if hasEnv(webvisContainer.Env, "RTSP_URL") { + t.Errorf("webvis must not receive RTSP_URL — it's a downstream filter; got env: %v", webvisContainer.Env) + } + + // Defense-in-depth: the front binding must not leak into the back + // container or vice versa (would surface as a swap bug in the + // bindingsByFilter lookup). + if got := rtspURLEnv(frontContainer.Env); got == wantBack { + t.Errorf("front-cam ended up with back source URL — bindings swapped") + } + if got := rtspURLEnv(backContainer.Env); got == wantFront { + t.Errorf("back-cam ended up with front source URL — bindings swapped") + } +} + +// TestBuildStreamingDeployment_LegacyBroadcast confirms the singular +// `Spec.SourceRef` form still broadcasts RTSP_URL to every container, +// matching pre-PLAT-1071 behavior. The resolver upstream produces a +// single ResolvedSourceBinding with FilterName=="" — the sentinel for +// broadcast. +func TestBuildStreamingDeployment_LegacyBroadcast(t *testing.T) { + r := &PipelineInstanceReconciler{} + pi := makeMinimalStreamingPipelineInstance() + pi.Spec.SourceRef = &pipelinesv1alpha1.SourceReference{Name: "legacy-source"} + + pipeline := &pipelinesv1alpha1.Pipeline{ + Spec: pipelinesv1alpha1.PipelineSpec{ + Filters: []pipelinesv1alpha1.Filter{ + {Name: "video-in", Image: "videoin:latest"}, + {Name: "webvis", Image: "webvis:latest"}, + }, + }, + } + + source := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "legacy.example", Port: 554, Path: "/stream"}, + }, + } + // FilterName="" — the legacy-broadcast sentinel the resolver produces + // when only Spec.SourceRef is set. + bindings := []ResolvedSourceBinding{{FilterName: "", Source: source}} + + deployment := r.buildStreamingDeployment(context.Background(), pi, pipeline, bindings, "legacy-deployment") + containers := deployment.Spec.Template.Spec.Containers + + wantURL := buildRTSPURL(source.Spec.RTSP) + for _, c := range containers { + got := rtspURLEnv(c.Env) + if got != wantURL { + t.Errorf("container %q RTSP_URL = %q, want %q (legacy broadcast must hit every container)", c.Name, got, wantURL) + } + } +} + +// TestBuildMultiSourceBatchJob_PerBindingInitClaimersAndEnv pins the +// multi-source batch path (PLAT-1071): one init claimer per binding in +// direct mode (S3_OBJECT_KEY set + queue env absent), and each VideoIn +// filter container gets `VIDEO_INPUT_PATH` pointing at its own bound +// download path. Downstream filters get no VIDEO_INPUT_PATH. +func TestBuildMultiSourceBatchJob_PerBindingInitClaimersAndEnv(t *testing.T) { + r := &PipelineInstanceReconciler{ClaimerImage: "claimer:test"} + pi := makeMinimalStreamingPipelineInstance() // shape-only; values reused + pi.Spec.Sources = []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "front-source"}}, + {FilterName: "back-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "back-source"}}, + } + + pipeline := &pipelinesv1alpha1.Pipeline{ + Spec: pipelinesv1alpha1.PipelineSpec{ + Mode: pipelinesv1alpha1.PipelineModeBatch, + Filters: []pipelinesv1alpha1.Filter{ + {Name: "front-cam", Image: "videoin:latest"}, + {Name: "back-cam", Image: "videoin:latest"}, + {Name: "image-out", Image: "imageout:latest"}, + }, + }, + } + + frontSource := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{ + Name: "media", + Prefix: "front.mp4", + CredentialsSecret: &pipelinesv1alpha1.SecretReference{ + Name: "s3-creds", + }, + }, + }, + } + backSource := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{ + Name: "media", + Prefix: "back.mp4", + CredentialsSecret: &pipelinesv1alpha1.SecretReference{ + Name: "s3-creds", + }, + }, + }, + } + bindings := []ResolvedSourceBinding{ + {FilterName: "front-cam", Source: frontSource}, + {FilterName: "back-cam", Source: backSource}, + } + + job := r.buildMultiSourceBatchJob(context.Background(), pi, pipeline, bindings, "ms-job") + + initContainers := job.Spec.Template.Spec.InitContainers + if len(initContainers) != 2 { + t.Fatalf("expected 2 init claimers (one per binding), got %d", len(initContainers)) + } + + // Each init claimer must be in direct mode and target the correct object/path. + for _, c := range initContainers { + objKey := envValue(c.Env, "S3_OBJECT_KEY") + dest := envValue(c.Env, "VIDEO_INPUT_PATH") + switch c.Name { + case "claimer-front-cam": + if objKey != "front.mp4" { + t.Errorf("claimer-front-cam S3_OBJECT_KEY = %q, want %q", objKey, "front.mp4") + } + if dest != "/ws/front-cam.mp4" { + t.Errorf("claimer-front-cam VIDEO_INPUT_PATH = %q, want %q", dest, "/ws/front-cam.mp4") + } + case "claimer-back-cam": + if objKey != "back.mp4" { + t.Errorf("claimer-back-cam S3_OBJECT_KEY = %q, want %q", objKey, "back.mp4") + } + if dest != "/ws/back-cam.mp4" { + t.Errorf("claimer-back-cam VIDEO_INPUT_PATH = %q, want %q", dest, "/ws/back-cam.mp4") + } + default: + t.Errorf("unexpected init container %q", c.Name) + } + // Direct mode = no Valkey env. + if envValue(c.Env, "STREAM") != "" || envValue(c.Env, "GROUP") != "" { + t.Errorf("claimer %q has Valkey env set, expected direct-mode only", c.Name) + } + } + + // VideoIn filter containers must each carry VIDEO_INPUT_PATH for their + // own download path; the downstream image-out filter must not. + containers := job.Spec.Template.Spec.Containers + front := findContainerByName(t, containers, "front-cam") + back := findContainerByName(t, containers, "back-cam") + imageOut := findContainerByName(t, containers, "image-out") + if got := envValue(front.Env, "VIDEO_INPUT_PATH"); got != "/ws/front-cam.mp4" { + t.Errorf("front-cam VIDEO_INPUT_PATH = %q, want %q", got, "/ws/front-cam.mp4") + } + if got := envValue(back.Env, "VIDEO_INPUT_PATH"); got != "/ws/back-cam.mp4" { + t.Errorf("back-cam VIDEO_INPUT_PATH = %q, want %q", got, "/ws/back-cam.mp4") + } + // ORDERING contract: VIDEO_INPUT_PATH must precede every FILTER_* env + // entry. Kubernetes dependent-env expansion only resolves $(VAR) + // references to variables defined earlier in the list, so a + // config-first ordering turns `sources: file://$(VIDEO_INPUT_PATH)` + // into a literal unexpanded string inside the container. + for _, c := range []corev1.Container{front, back} { + assertEnvPrecedesFilterConfig(t, c, "VIDEO_INPUT_PATH") + } + + if hasEnv(imageOut.Env, "VIDEO_INPUT_PATH") { + t.Errorf("image-out must not receive VIDEO_INPUT_PATH; it's a downstream consumer") + } + + // Job shape: single Pod (parallelism=1, completions=1). + if job.Spec.Parallelism == nil || *job.Spec.Parallelism != 1 { + t.Errorf("expected parallelism=1, got %v", job.Spec.Parallelism) + } + if job.Spec.Completions == nil || *job.Spec.Completions != 1 { + t.Errorf("expected completions=1, got %v", job.Spec.Completions) + } +} + +// envValue returns the inline value of the env var named `name`, or "". +func envValue(envs []corev1.EnvVar, name string) string { + for _, e := range envs { + if e.Name == name { + return e.Value + } + } + return "" +} + +// TestEffectiveSources covers the spec-shape normalization: SourceRef +// becomes a one-entry broadcast list; Sources is returned verbatim; both +// unset returns nil (the reconciler treats this as an error). +func TestEffectiveSources(t *testing.T) { + t.Run("legacy_source_ref_becomes_broadcast_sentinel", func(t *testing.T) { + spec := pipelinesv1alpha1.PipelineInstanceSpec{ + SourceRef: &pipelinesv1alpha1.SourceReference{Name: "src1"}, + } + got := spec.EffectiveSources() + if len(got) != 1 || got[0].FilterName != "" || got[0].SourceRef.Name != "src1" { + t.Errorf("legacy SourceRef did not normalize to broadcast sentinel: %+v", got) + } + }) + t.Run("sources_returned_verbatim", func(t *testing.T) { + spec := pipelinesv1alpha1.PipelineInstanceSpec{ + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "a", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-a"}}, + {FilterName: "b", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-b"}}, + }, + } + got := spec.EffectiveSources() + if len(got) != 2 || got[0].FilterName != "a" || got[1].FilterName != "b" { + t.Errorf("Sources not returned verbatim: %+v", got) + } + }) + t.Run("nothing_set_returns_nil", func(t *testing.T) { + spec := pipelinesv1alpha1.PipelineInstanceSpec{} + got := spec.EffectiveSources() + if got != nil { + t.Errorf("empty spec must yield nil, got: %+v", got) + } + }) +} + +// findSecretEnvRef returns the SecretKeySelector for the env var with the +// given name, or nil. Used by the credentials-injection test below to +// assert on ValueFrom-style env vars without dragging full struct +// comparisons into the call sites. +func findSecretEnvRef(envs []corev1.EnvVar, name string) *corev1.SecretKeySelector { + for _, e := range envs { + if e.Name == name && e.ValueFrom != nil { + return e.ValueFrom.SecretKeyRef + } + } + return nil +} + +// TestBuildStreamingDeployment_MultiSource_WithCredentials covers the +// secretRef path: when a per-binding RTSP source has CredentialsSecret +// set the controller must inject _RTSP_USERNAME / _RTSP_PASSWORD env +// vars sourced from that secret into the matching VideoIn container +// alongside RTSP_URL. The pre-existing PerContainerRTSP test covers the +// no-credentials case; this fills the credentials code path the +// multi-source change touches. +func TestBuildStreamingDeployment_MultiSource_WithCredentials(t *testing.T) { + r := &PipelineInstanceReconciler{} + pi := makeMinimalStreamingPipelineInstance() + pi.Spec.Sources = []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "front-source"}}, + {FilterName: "back-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "back-source"}}, + } + + pipeline := &pipelinesv1alpha1.Pipeline{ + Spec: pipelinesv1alpha1.PipelineSpec{ + Filters: []pipelinesv1alpha1.Filter{ + {Name: "front-cam", Image: "videoin:latest"}, + {Name: "back-cam", Image: "videoin:latest"}, + }, + }, + } + + bindings := []ResolvedSourceBinding{ + {FilterName: "front-cam", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{ + Host: "front.example", + Port: 554, + Path: "/stream", + CredentialsSecret: &pipelinesv1alpha1.SecretReference{Name: frontCredSecretName}, + }, + }, + }}, + {FilterName: "back-cam", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{ + Host: "back.example", + Port: 554, + Path: "/stream", + // No credentials — verifies one-with, one-without works. + }, + }, + }}, + } + + deployment := r.buildStreamingDeployment(context.Background(), pi, pipeline, bindings, "ms-deployment") + containers := deployment.Spec.Template.Spec.Containers + front := findContainerByName(t, containers, "front-cam") + back := findContainerByName(t, containers, "back-cam") + + usernameRef := findSecretEnvRef(front.Env, "_RTSP_USERNAME") + passwordRef := findSecretEnvRef(front.Env, "_RTSP_PASSWORD") + if usernameRef == nil || usernameRef.Name != frontCredSecretName || usernameRef.Key != "username" { + t.Errorf("front-cam _RTSP_USERNAME secretKeyRef = %+v, want %s/username", usernameRef, frontCredSecretName) + } + if passwordRef == nil || passwordRef.Name != frontCredSecretName || passwordRef.Key != "password" { + t.Errorf("front-cam _RTSP_PASSWORD secretKeyRef = %+v, want %s/password", passwordRef, frontCredSecretName) + } + if got := rtspURLEnv(front.Env); got == "" { + t.Errorf("front-cam RTSP_URL must still be injected alongside credentials, got empty") + } + + if findSecretEnvRef(back.Env, "_RTSP_USERNAME") != nil { + t.Errorf("back-cam must not have _RTSP_USERNAME (no credentials configured)") + } + if findSecretEnvRef(back.Env, "_RTSP_PASSWORD") != nil { + t.Errorf("back-cam must not have _RTSP_PASSWORD (no credentials configured)") + } + for _, e := range back.Env { + if e.ValueFrom != nil && e.ValueFrom.SecretKeyRef != nil && e.ValueFrom.SecretKeyRef.Name == frontCredSecretName { + t.Errorf("front-cam's secret leaked into back-cam container env: %+v", e) + } + } +} + +// TestResolveSourceBindings_SourceMissingSurfacesActionableError covers +// the error path of resolveSourceBindings: when the referenced +// PipelineSource doesn't exist in the cluster the function returns a +// wrapped error naming the binding's FilterName so operators can locate +// the misconfiguration in the CR. +func TestResolveSourceBindings_SourceMissingSurfacesActionableError(t *testing.T) { + sch := reconcileSpanScheme(t) + pi := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pi", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-front"}}, + {FilterName: "back-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-back-missing"}}, + }, + }, + } + // Only front exists; back is missing on purpose. + frontSrc := &pipelinesv1alpha1.PipelineSource{ + ObjectMeta: metav1.ObjectMeta{Name: "src-front", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "front"}, + }, + } + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch).WithObjects(pi, frontSrc).Build(), + Scheme: sch, + } + + _, err := r.resolveSourceBindings(context.Background(), pi) + if err == nil { + t.Fatalf("expected error when a referenced source is missing, got nil") + } + if !strings.Contains(err.Error(), "src-back-missing") { + t.Errorf("error must name the missing source, got: %v", err) + } + if !strings.Contains(err.Error(), "back-cam") { + t.Errorf("error must name the filter binding, got: %v", err) + } +} + +// TestResolveSourceBindings_LegacySourceRefMissingSurfacesError covers +// the same error path for the legacy SourceRef shape (FilterName==""). In +// the legacy path the error wording omits the filter-name suffix since +// the binding isn't filter-scoped. +func TestResolveSourceBindings_LegacySourceRefMissingSurfacesError(t *testing.T) { + sch := reconcileSpanScheme(t) + pi := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pi", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + SourceRef: &pipelinesv1alpha1.SourceReference{Name: "src-missing"}, + }, + } + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch).WithObjects(pi).Build(), + Scheme: sch, + } + + _, err := r.resolveSourceBindings(context.Background(), pi) + if err == nil { + t.Fatalf("expected error, got nil") + } + if !strings.Contains(err.Error(), "src-missing") { + t.Errorf("error must name the missing source, got: %v", err) + } + if strings.Contains(err.Error(), "bound to filter") { + t.Errorf("legacy SourceRef error should not mention filter binding (FilterName is empty), got: %v", err) + } +} + +// assertEnvPrecedesFilterConfig fails the test unless env var `name` appears +// in c.Env BEFORE every FILTER_*-prefixed entry — the position Kubernetes +// dependent-env expansion requires for $(name) references inside filter +// config values to resolve. +func assertEnvPrecedesFilterConfig(t *testing.T, c corev1.Container, name string) { + t.Helper() + nameIdx := -1 + firstFilterIdx := -1 + for i, e := range c.Env { + if e.Name == name && nameIdx == -1 { + nameIdx = i + } + if strings.HasPrefix(e.Name, "FILTER_") && firstFilterIdx == -1 { + firstFilterIdx = i + } + } + if nameIdx == -1 { + t.Fatalf("container %q: env %s not found", c.Name, name) + } + if firstFilterIdx != -1 && nameIdx > firstFilterIdx { + t.Errorf("container %q: %s at index %d must precede first FILTER_* env at index %d (K8s $(VAR) expansion only resolves earlier-defined vars)", c.Name, name, nameIdx, firstFilterIdx) + } +} + +// TestPipelineInstancesForPipelineSource_MapsReferencingInstances pins the +// PipelineSource→PipelineInstance watch mapping wired in SetupWithManager: a +// PipelineSource event must enqueue every PipelineInstance in the source's +// namespace that references it — via spec.sources[].sourceRef.name OR the +// legacy spec.sourceRef.name — and nothing else. Without this mapping, an +// instance Degraded by multi-source batch validation (which returns without +// requeue) would never reconcile again after the source is fixed. +func TestPipelineInstancesForPipelineSource_MapsReferencingInstances(t *testing.T) { + sch := reconcileSpanScheme(t) + otherNS := "elsewhere" + + src := &pipelinesv1alpha1.PipelineSource{ + ObjectMeta: metav1.ObjectMeta{Name: "shared-src", Namespace: "default"}, + } + + piMulti := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-multi", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "shared-src"}}, + {FilterName: "back-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "some-other-src"}}, + }, + }, + } + piLegacy := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-legacy", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: "shared-src"}, + }, + } + piUnrelated := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-unrelated", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: "different-src"}, + }, + } + // Same name but the ref explicitly points at another namespace — the + // event for default/shared-src must NOT wake this instance. + piCrossNSAway := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-cross-ns-away", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "shared-src", Namespace: &otherNS}}, + }, + }, + } + // The inverse: an instance in ANOTHER namespace whose ref explicitly + // points at default/shared-src MUST wake on the event — the list is + // cluster-wide and the match resolves the ref's namespace. + defaultNS := "default" + piCrossNSToward := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-cross-ns-toward", Namespace: otherNS}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "shared-src", Namespace: &defaultNS}}, + }, + }, + } + // Same-name source in the other namespace must not be confused with + // default/shared-src: an instance referencing it locally stays asleep. + piOtherNSLocal := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-other-ns-local", Namespace: otherNS}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: "shared-src"}, + }, + } + + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch). + WithObjects(src, piMulti, piLegacy, piUnrelated, piCrossNSAway, piCrossNSToward, piOtherNSLocal).Build(), + Scheme: sch, + } + + requests := r.pipelineInstancesForPipelineSource(context.Background(), src) + got := make(map[string]bool, len(requests)) + for _, req := range requests { + got[req.Name] = true + } + if len(requests) != 3 || !got["pi-multi"] || !got["pi-legacy"] || !got["pi-cross-ns-toward"] { + t.Errorf("expected exactly [pi-multi pi-legacy pi-cross-ns-toward], got %v", requests) + } +} + +// makeUnmatchedBindingFixtures builds the Pipeline / PipelineSource / +// PipelineInstance triple the SourceBindingUnmatched Reconcile tests share. +// The instance pre-seeds the Valkey-credentials finalizer so the full +// Reconcile entry point flows straight through to source-binding validation +// instead of returning early to persist the finalizer. +func makeUnmatchedBindingFixtures(t *testing.T, mode pipelinesv1alpha1.PipelineMode, filterName string) (*pipelinesv1alpha1.Pipeline, *pipelinesv1alpha1.PipelineSource, *pipelinesv1alpha1.PipelineInstance) { + t.Helper() + pipeline := &pipelinesv1alpha1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "unmatched-pipeline", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineSpec{ + Mode: mode, + Filters: []pipelinesv1alpha1.Filter{ + {Name: "front-cam", Image: "videoin:latest"}, + {Name: "webvis", Image: "webvis:latest"}, + }, + }, + } + src := &pipelinesv1alpha1.PipelineSource{ + ObjectMeta: metav1.ObjectMeta{Name: "unmatched-src", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "cam.example"}, + Bucket: nil, + }, + } + if mode == pipelinesv1alpha1.PipelineModeBatch { + src.Spec.RTSP = nil + src.Spec.Bucket = &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "clip.mp4"} + } + pi := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unmatched-pi", + Namespace: "default", + UID: types.UID("unmatched-pi-uid"), + Finalizers: []string{FinalizerValkeyCredentials}, + }, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: pipeline.Name}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: filterName, SourceRef: pipelinesv1alpha1.SourceReference{Name: src.Name}}, + }, + }, + } + return pipeline, src, pi +} + +// TestReconcile_UnmatchedSourceBinding_DegradesStreaming pins the +// streaming-mode half of the unmatched-binding validation: a +// sources[].filterName that matches no pipeline.spec.filters[].name must +// surface Degraded=True (SourceBindingUnmatched) with a message naming both +// the unmatched and the available filter names, and no Deployment may be +// created (the binding would otherwise be a silent no-op — the RTSP env +// would land on no container). +func TestReconcile_UnmatchedSourceBinding_DegradesStreaming(t *testing.T) { + sch := reconcileSpanScheme(t) + pipeline, src, pi := makeUnmatchedBindingFixtures(t, pipelinesv1alpha1.PipelineModeStream, "ghost-cam") + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch). + WithStatusSubresource(&pipelinesv1alpha1.PipelineInstance{}). + WithObjects(pipeline, src, pi).Build(), + Scheme: sch, + } + + if _, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, + }); err != nil { + t.Fatalf("expected nil error (validation failure surfaces via Condition), got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded) + if cond.Status != metav1.ConditionTrue || cond.Reason != ReasonSourceBindingUnmatched { + t.Fatalf("expected Degraded=True (%s), got %+v", ReasonSourceBindingUnmatched, cond) + } + if prog := findCondition(t, updated.Status.Conditions, ConditionTypeProgressing); prog.Status == metav1.ConditionTrue { + t.Errorf("expected Progressing!=True alongside the validation Degraded, got %+v", prog) + } + if !strings.Contains(cond.Message, "ghost-cam") { + t.Errorf("message must name the unmatched binding, got %q", cond.Message) + } + if !strings.Contains(cond.Message, "front-cam") || !strings.Contains(cond.Message, "webvis") { + t.Errorf("message must list the available filter names, got %q", cond.Message) + } + + deploy := &appsv1.Deployment{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name + "-deploy", Namespace: pi.Namespace}, deploy); err == nil { + t.Errorf("expected no Deployment to be created after validation failure") + } +} + +// TestReconcile_UnmatchedSourceBinding_DegradesBatch pins the batch-mode +// half: the same validation fires before reconcileBatch ever runs (shared +// path), so no Job is created and no Valkey access happens. +func TestReconcile_UnmatchedSourceBinding_DegradesBatch(t *testing.T) { + sch := reconcileSpanScheme(t) + pipeline, src, pi := makeUnmatchedBindingFixtures(t, pipelinesv1alpha1.PipelineModeBatch, "ghost-cam") + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch). + WithStatusSubresource(&pipelinesv1alpha1.PipelineInstance{}). + WithObjects(pipeline, src, pi).Build(), + Scheme: sch, + // ValkeyClient intentionally nil: validation must reject before any + // batch-mode queue work is attempted. + } + + if _, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, + }); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded) + if cond.Status != metav1.ConditionTrue || cond.Reason != ReasonSourceBindingUnmatched { + t.Fatalf("expected Degraded=True (%s), got %+v", ReasonSourceBindingUnmatched, cond) + } + + job := &batchv1.Job{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name + "-job", Namespace: pi.Namespace}, job); err == nil { + t.Errorf("expected no Job to be created after validation failure") + } +} + +// TestReconcile_UnmatchedSourceBinding_ClearsOnFix pins the recovery half of +// the shared-path validation: an instance previously Degraded with +// SourceBindingUnmatched must have the condition cleared on the next +// reconcile once its bindings validate (e.g. after the Pipeline gained the +// missing filter or the CR's filterName was corrected — both events +// re-trigger reconcile via the Pipeline watch / spec generation bump). +func TestReconcile_UnmatchedSourceBinding_ClearsOnFix(t *testing.T) { + sch := reconcileSpanScheme(t) + // Binding targets front-cam, which the Pipeline HAS — validation passes. + pipeline, src, pi := makeUnmatchedBindingFixtures(t, pipelinesv1alpha1.PipelineModeStream, "front-cam") + // Pre-seed the stale Degraded condition a previous failing pass left. + pi.Status.Conditions = []metav1.Condition{{ + Type: ConditionTypeDegraded, + Status: metav1.ConditionTrue, + Reason: ReasonSourceBindingUnmatched, + Message: "stale", + LastTransitionTime: metav1.Now(), + }} + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch). + WithStatusSubresource(&pipelinesv1alpha1.PipelineInstance{}). + WithObjects(pipeline, src, pi).Build(), + Scheme: sch, + } + + if _, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, + }); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded); cond.Type != "" { + t.Errorf("expected stale Degraded/%s condition to be cleared once bindings validate, got %+v", ReasonSourceBindingUnmatched, cond) + } +} + +// TestReconcile_PipelineSourceNotFound_ClearsOnceSourceExists pins the +// creation-race recovery the deployment agent depends on: the agent applies +// the PipelineInstance BEFORE its PipelineSources (intentional — cleanup +// treats a source as orphaned when no PI references it), so the first +// reconcile finds no sources and sets Degraded/PipelineSourceNotFound. Once +// the sources land, the PipelineSource watch re-reconciles and the stale +// condition MUST clear — the agent's provisioning monitor tears the whole +// instance down on any lingering Degraded (observed live in the cross-PR +// smoke test before this clear existed). +func TestReconcile_PipelineSourceNotFound_ClearsOnceSourceExists(t *testing.T) { + sch := reconcileSpanScheme(t) + pipeline, src, pi := makeUnmatchedBindingFixtures(t, pipelinesv1alpha1.PipelineModeStream, "front-cam") + // Phase 1: PI exists, its PipelineSource does NOT (agent apply order). + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch). + WithStatusSubresource(&pipelinesv1alpha1.PipelineInstance{}). + WithObjects(pipeline, pi).Build(), + Scheme: sch, + } + + if _, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, + }); err == nil { + t.Fatalf("expected error while the PipelineSource is missing, got nil") + } + + updated := &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded) + if cond.Status != metav1.ConditionTrue || cond.Reason != ReasonPipelineSourceNotFound { + t.Fatalf("expected Degraded=True (%s) while source is missing, got %+v", ReasonPipelineSourceNotFound, cond) + } + + // Phase 2: the source lands (what the agent does milliseconds later). + if err := r.Create(context.Background(), src); err != nil { + t.Fatalf("create PipelineSource: %v", err) + } + if _, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, + }); err != nil { + t.Fatalf("expected nil error once the source exists, got %v", err) + } + + updated = &pipelinesv1alpha1.PipelineInstance{} + if err := r.Get(context.Background(), types.NamespacedName{Name: pi.Name, Namespace: pi.Namespace}, updated); err != nil { + t.Fatalf("re-fetch PI: %v", err) + } + if cond := findCondition(t, updated.Status.Conditions, ConditionTypeDegraded); cond.Type != "" { + t.Errorf("expected stale Degraded/%s to clear once the PipelineSource exists, got %+v", ReasonPipelineSourceNotFound, cond) + } +} + +// TestBindingObjectKey pins the object-key resolution the direct-mode +// claimers depend on: Bucket.Prefix is the full object key (the platform's +// media model carries single-file object keys in prefix — there is no +// separate object concept anywhere in the train); nil/empty resolves empty +// and the reconciler Degrades. +func TestBindingObjectKey(t *testing.T) { + mk := func(prefix string) ResolvedSourceBinding { + return ResolvedSourceBinding{ + FilterName: "front-cam", + Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{ + Name: "b", + Prefix: prefix, + }, + }, + }, + } + } + if got := bindingObjectKey(mk("clips/front_001.mp4")); got != "clips/front_001.mp4" { + t.Errorf("Prefix must serve as the full key, got %q", got) + } + if got := bindingObjectKey(mk("")); got != "" { + t.Errorf("empty prefix must resolve empty, got %q", got) + } + if got := bindingObjectKey(ResolvedSourceBinding{FilterName: "x"}); got != "" { + t.Errorf("nil source must resolve empty, got %q", got) + } +} + +// TestIsDirectoryPlaceholder pins the bucket-listing filter: zero-byte +// keys ending in "/" (GCS/S3 console "folders") are skipped; real files — +// including zero-byte regular files and oddly-named keys — are kept. +func TestIsDirectoryPlaceholder(t *testing.T) { + cases := []struct { + key string + size int64 + want bool + }{ + {"videos/", 0, true}, + {"videos/nested/", 0, true}, + {"videos/train.mp4", 10821688, false}, + {"videos/empty.mp4", 0, false}, + {"videos/", 5, false}, // non-empty trailing-slash key: keep, let processing decide + {"", 0, false}, + } + for _, tc := range cases { + if got := isDirectoryPlaceholder(tc.key, tc.size); got != tc.want { + t.Errorf("isDirectoryPlaceholder(%q, %d) = %v, want %v", tc.key, tc.size, got, tc.want) + } + } +} + +// TestIdleAnchorBinding pins the deterministic idle-timeout anchor: the +// lexicographically-smallest filterName wins regardless of slice order +// (sources is listType=map — element order is not semantically stable). +func TestIdleAnchorBinding(t *testing.T) { + a := ResolvedSourceBinding{FilterName: "a-cam"} + m := ResolvedSourceBinding{FilterName: "m-cam"} + z := ResolvedSourceBinding{FilterName: "z-cam"} + + for name, order := range map[string][]ResolvedSourceBinding{ + "sorted": {a, m, z}, + "reversed": {z, m, a}, + "shuffled": {m, z, a}, + } { + t.Run(name, func(t *testing.T) { + if got := idleAnchorBinding(order); got.FilterName != "a-cam" { + t.Errorf("anchor = %q, want a-cam", got.FilterName) + } + }) + } + + // Legacy broadcast sentinel: single binding anchors itself. + if got := idleAnchorBinding([]ResolvedSourceBinding{{FilterName: ""}}); got.FilterName != "" { + t.Errorf("broadcast sentinel must anchor itself") + } +} + +// TestBuildStreamingDeployment_MultiSource_GPUCoexistence pins the +// 3-cams + 1-GPU-filter topology (PLAT-1141 + PR #66 coexistence): on a +// Spec.Sources instance, applyGPUContainerSharing must put the +// nvidia.com/gpu limit on exactly one container (the GPU filter), inject +// NVIDIA_VISIBLE_DEVICES there, and leave the camera containers untouched +// apart from their per-binding RTSP_URL. +func TestBuildStreamingDeployment_MultiSource_GPUCoexistence(t *testing.T) { + r := &PipelineInstanceReconciler{} + pi := makeMinimalStreamingPipelineInstance() + pi.Spec.Sources = []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "cam-a", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-a"}}, + {FilterName: "cam-b", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-b"}}, + {FilterName: "cam-c", SourceRef: pipelinesv1alpha1.SourceReference{Name: "src-c"}}, + } + + pipeline := &pipelinesv1alpha1.Pipeline{ + Spec: pipelinesv1alpha1.PipelineSpec{ + Filters: []pipelinesv1alpha1.Filter{ + {Name: "cam-a", Image: "videoin:latest"}, + {Name: "cam-b", Image: "videoin:latest"}, + {Name: "cam-c", Image: "videoin:latest"}, + {Name: "detector", Image: "detector:latest", Resources: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{"nvidia.com/gpu": resource.MustParse("1")}, + }}, + }, + }, + } + mkSrc := func(host string) *pipelinesv1alpha1.PipelineSource { + return &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: host, Port: 554, Path: "/s"}, + }, + } + } + bindings := []ResolvedSourceBinding{ + {FilterName: "cam-a", Source: mkSrc("a.example")}, + {FilterName: "cam-b", Source: mkSrc("b.example")}, + {FilterName: "cam-c", Source: mkSrc("c.example")}, + } + + deployment := r.buildStreamingDeployment(context.Background(), pi, pipeline, bindings, "gpu-coexist") + containers := deployment.Spec.Template.Spec.Containers + + detector := findContainerByName(t, containers, "detector") + if got := detector.Resources.Limits["nvidia.com/gpu"]; got.Value() != 1 { + t.Errorf("detector nvidia.com/gpu limit = %v, want 1", got) + } + if !hasEnv(detector.Env, "NVIDIA_VISIBLE_DEVICES") { + t.Errorf("detector must carry NVIDIA_VISIBLE_DEVICES, env: %v", detector.Env) + } + if hasEnv(detector.Env, "RTSP_URL") { + t.Errorf("detector must not receive RTSP_URL") + } + + gpuLimited := 0 + for _, c := range containers { + if _, ok := c.Resources.Limits["nvidia.com/gpu"]; ok { + gpuLimited++ + } + } + if gpuLimited != 1 { + t.Errorf("exactly one container must hold the nvidia.com/gpu limit, got %d", gpuLimited) + } + for _, name := range []string{"cam-a", "cam-b", "cam-c"} { + c := findContainerByName(t, containers, name) + if rtspURLEnv(c.Env) == "" { + t.Errorf("%s must carry its per-binding RTSP_URL", name) + } + if hasEnv(c.Env, "NVIDIA_VISIBLE_DEVICES") { + t.Errorf("%s is CPU-only and must not carry NVIDIA_VISIBLE_DEVICES", name) + } + } +} + +// TestBuildStreamingDeployment_RTSPURLPrecedesFilterConfig pins the +// streaming-side env ordering: RTSP_URL must appear before any FILTER_* +// entry in the bound container's env so Kubernetes dependent-env expansion +// resolves $(RTSP_URL) inside filter config values (the batch counterpart +// is pinned via the same assertEnvPrecedesFilterConfig helper). +func TestBuildStreamingDeployment_RTSPURLPrecedesFilterConfig(t *testing.T) { + r := &PipelineInstanceReconciler{} + pi := makeMinimalStreamingPipelineInstance() + pi.Spec.Sources = []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "front-source"}}, + } + + pipeline := &pipelinesv1alpha1.Pipeline{ + Spec: pipelinesv1alpha1.PipelineSpec{ + Filters: []pipelinesv1alpha1.Filter{ + {Name: "front-cam", Image: "videoin:latest", Config: []pipelinesv1alpha1.ConfigVar{ + {Name: "sources", Value: "$(RTSP_URL);front-cam"}, + {Name: "outputs", Value: "tcp://*:5550"}, + }}, + }, + }, + } + bindings := []ResolvedSourceBinding{ + {FilterName: "front-cam", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + RTSP: &pipelinesv1alpha1.RTSPSource{Host: "front.example", Port: 554, Path: "/s"}, + }, + }}, + } + + deployment := r.buildStreamingDeployment(context.Background(), pi, pipeline, bindings, "ordering") + cam := findContainerByName(t, deployment.Spec.Template.Spec.Containers, "front-cam") + assertEnvPrecedesFilterConfig(t, cam, "RTSP_URL") +} diff --git a/internal/controller/pipelineinstance_streaming_update_test.go b/internal/controller/pipelineinstance_streaming_update_test.go index 7b1bcf8..d92c4b7 100644 --- a/internal/controller/pipelineinstance_streaming_update_test.go +++ b/internal/controller/pipelineinstance_streaming_update_test.go @@ -70,7 +70,7 @@ var _ = Describe("PipelineInstance streaming update path", func() { }, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: pipelineName}, - SourceRef: pipelinesv1alpha1.SourceReference{Name: sourceName}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: sourceName}, }, } Expect(k8sClient.Create(ctx, pi)).To(Succeed()) @@ -81,7 +81,7 @@ var _ = Describe("PipelineInstance streaming update path", func() { Scheme: k8sClient.Scheme(), } - Expect(r.ensureStreamingDeployment(ctx, pi, pipeline, source)).To(Succeed()) + Expect(r.ensureStreamingDeployment(ctx, pi, pipeline, []ResolvedSourceBinding{{Source: source}})).To(Succeed()) depName := pi.Name + "-deploy" depKey := types.NamespacedName{Name: depName, Namespace: ns} @@ -98,7 +98,7 @@ var _ = Describe("PipelineInstance streaming update path", func() { pi.Labels["plainsight.ai/project-id"] = "proj-NEW" Expect(k8sClient.Update(ctx, pi)).To(Succeed()) - Expect(r.ensureStreamingDeployment(ctx, pi, pipeline, source)).To(Succeed()) + Expect(r.ensureStreamingDeployment(ctx, pi, pipeline, []ResolvedSourceBinding{{Source: source}})).To(Succeed()) Expect(k8sClient.Get(ctx, depKey, dep)).To(Succeed()) diff --git a/internal/controller/pod_labels_test.go b/internal/controller/pod_labels_test.go index f01698e..d22702d 100644 --- a/internal/controller/pod_labels_test.go +++ b/internal/controller/pod_labels_test.go @@ -214,7 +214,7 @@ func TestBuildStreamingDeployment_PropagatesPlainsightLabels(t *testing.T) { }, } - dep := r.buildStreamingDeployment(context.Background(), pi, pipeline, ps, "test-deploy") + dep := r.buildStreamingDeployment(context.Background(), pi, pipeline, []ResolvedSourceBinding{{Source: ps}}, "test-deploy") for _, tc := range []struct { name string