From 535aeeac2c2d68855870ec4fd4609502387852d6 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Tue, 9 Jun 2026 20:44:27 -0300 Subject: [PATCH 01/19] feat(crd): multi-source pipeline instances (PLAT-1071 / PLAT-1141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the per-filter source-binding shape so a single PipelineInstance can run a multi-camera pipeline where each VideoIn filter consumes a distinct PipelineSource. Streaming mode is fully supported; batch mode keeps single-source (multi-source batch is tracked as a focused follow-up that needs the N-init-claimer rework). CRD changes (api/v1alpha1/pipelineinstance_types.go) - Spec.SourceRef switches from value to *SourceReference and is marked deprecated. Existing single-source CRs continue to apply unchanged. - New Spec.Sources []NamedSourceRef binds each PipelineSource to a specific filter container in the referenced Pipeline by name. NamedSourceRef.FilterName is constrained to DNS-1123 label rules so the value can flow safely into generated container/volume/env names. - listType=map + listMapKey=filterName makes Sources stable under server-side apply. - New EffectiveSources() method on PipelineInstanceSpec normalizes both shapes into a uniform []NamedSourceRef. Legacy SourceRef becomes a one-entry list with FilterName="" — the broadcast sentinel the reconciler uses to preserve today's all-containers behavior. Reconciler changes - New ResolvedSourceBinding pairs the bound filter name with the fetched PipelineSource. resolveSourceBindings (replaces getPipelineSource) builds the list once at the top of Reconcile and threads it through to reconcileStreaming / reconcileBatch. - Streaming (pipelineinstance_controller_streaming.go): buildStreamingDeployment now splits bindings into a per-filter map and a broadcast slice, then injects RTSP_URL / _RTSP_USERNAME / _RTSP_PASSWORD into only the matching container plus any broadcast targets. The downstream container (webvis, etc.) gets no source env vars and keeps using its filter.config `sources: tcp://localhost:...` wiring exactly as before. - Idle-timeout policy anchors to bindings[0].Source for V1; an explicit comment flags that "ANY vs ALL idle" semantics for multi-source streams is a follow-up decision. - Batch (pipelineinstance_controller_batch.go): rejects multi-source with a clear MultiSourceBatchUnsupported condition; single-source batch flows through unchanged via bindings[0].Source. Multi-source batch needs the N-init-claimer reshape; tracked separately. Tests - pipelineinstance_multisource_test.go: three new test cases pin the contract — per-container RTSP_URL on the multi-source path, broadcast preserved on the legacy SourceRef path, and EffectiveSources normalization. - 39 existing test sites updated for *SourceReference (was struct, now pointer) and the new reconcileBatch/Streaming / ensureStreamingDeployment signatures. - All controller tests pass; controller package coverage 60.7% → 60.9%. Generated artifacts (regenerated, not hand-edited) - api/v1alpha1/zz_generated.deepcopy.go - config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml - deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml Sample - config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml demonstrates a two-camera binding referencing the front_cam / back_cam filter names. Wired into the kustomization.yaml index. --- api/v1alpha1/pipelineinstance_types.go | 89 +++++++- api/v1alpha1/zz_generated.deepcopy.go | 29 ++- ...ilter.plainsight.ai_pipelineinstances.yaml | 65 +++++- config/samples/kustomization.yaml | 1 + ...1_pipelineinstance_stream_multisource.yaml | 29 +++ ...ilter.plainsight.ai_pipelineinstances.yaml | 65 +++++- .../controller/pipelineinstance_controller.go | 68 ++++-- .../pipelineinstance_controller_batch.go | 21 +- ...instance_controller_reconcile_span_test.go | 10 +- .../pipelineinstance_controller_streaming.go | 94 ++++++-- .../pipelineinstance_controller_test.go | 72 +++--- .../pipelineinstance_multisource_test.go | 207 ++++++++++++++++++ .../pipelineinstance_streaming_update_test.go | 6 +- internal/controller/pod_labels_test.go | 2 +- 14 files changed, 660 insertions(+), 98 deletions(-) create mode 100644 config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml create mode 100644 internal/controller/pipelineinstance_multisource_test.go diff --git a/api/v1alpha1/pipelineinstance_types.go b/api/v1alpha1/pipelineinstance_types.go index a741330..fafc461 100644 --- a/api/v1alpha1/pipelineinstance_types.go +++ b/api/v1alpha1/pipelineinstance_types.go @@ -42,15 +42,52 @@ 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: +// +// - sourceRef (legacy, single-source): the bound 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 +// and demos continue to work unchanged. +// +// - 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. 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. Use this for single-source pipelines; the source URL + // is broadcast to every filter container. For multi-source pipelines, + // use `sources` instead. + // + // Deprecated: prefer `sources` for new pipelines. Retained for legacy + // single-source CRs and existing demos. + // +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 +136,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 legacy `SourceRef` field into the +// new 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/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml index 4f9f0c8..544f1db 100644 --- a/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml +++ b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml @@ -1003,8 +1003,14 @@ spec: - name type: object sourceRef: - description: sourceRef references the PipelineSource resource for - input configuration + description: |- + sourceRef references the PipelineSource resource for input + configuration. Use this for single-source pipelines; the source URL + is broadcast to every filter container. For multi-source pipelines, + use `sources` instead. + + Deprecated: prefer `sources` for new pipelines. Retained for legacy + single-source CRs and existing demos. properties: name: description: name is the name of the PipelineSource resource @@ -1017,6 +1023,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,7 +1121,6 @@ spec: type: array required: - pipelineRef - - sourceRef type: object status: description: status defines the observed state of PipelineInstance diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index 06bbba8..a17c144 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -5,4 +5,5 @@ resources: - pipelines_v1alpha1_pipelinesource_bucket.yaml - pipelines_v1alpha1_pipelinesource_rtsp.yaml - pipelines_v1alpha1_pipelineinstance.yaml +- pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml # +kubebuilder:scaffold:manifestskustomizesamples 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..90d44da --- /dev/null +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml @@ -0,0 +1,29 @@ +# 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`. +# +# 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..544f1db 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,14 @@ spec: - name type: object sourceRef: - description: sourceRef references the PipelineSource resource for - input configuration + description: |- + sourceRef references the PipelineSource resource for input + configuration. Use this for single-source pipelines; the source URL + is broadcast to every filter container. For multi-source pipelines, + use `sources` instead. + + Deprecated: prefer `sources` for new pipelines. Retained for legacy + single-source CRs and existing demos. properties: name: description: name is the name of the PipelineSource resource @@ -1017,6 +1023,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,7 +1121,6 @@ spec: type: array required: - pipelineRef - - sourceRef type: object status: description: status defines the observed state of PipelineInstance diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index c4bb0d0..afef55d 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -487,10 +487,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") @@ -516,11 +518,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 +530,50 @@ 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 deprecated singular +// `sourceRef` field. Non-empty FilterName targets the container whose +// `pipeline.spec.filters[].name` matches exactly. +type ResolvedSourceBinding struct { + FilterName string + Source *pipelinesv1alpha1.PipelineSource +} - 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) +// 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") + } + + 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 pipelineSource, nil + return bindings, nil } // getCredentials retrieves S3 credentials for the pipelineSource bucket secret, if configured. diff --git a/internal/controller/pipelineinstance_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index 115a784..4f6173d 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -38,8 +38,25 @@ 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. +// +// V1 scope (PLAT-1071): batch mode currently supports a single source +// binding. Multi-source batch — used for benchmarking multi-camera +// pipelines against per-media GT — needs N parallel init claimers and a +// reshaped work-queue model. That work is tracked as a focused follow-up; +// today multi-source batch is rejected at the top of this function with a +// clear operator-actionable error. Single-source batch (legacy SourceRef +// or a one-entry Sources array) keeps working exactly as before. +func (r *PipelineInstanceReconciler) reconcileBatch(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding) (ctrl.Result, error) { + if len(sourceBindings) > 1 { + err := fmt.Errorf("batch mode does not yet support multi-source pipelines (%d sources bound); please reduce to one source or use streaming mode", len(sourceBindings)) + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchUnsupported", err.Error()) + if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { + logf.FromContext(ctx).Error(statusErr, "Failed to update status after multi-source batch rejection") + } + return ctrl.Result{}, err + } + pipelineSource := sourceBindings[0].Source log := logf.FromContext(ctx) // Ensure counts object exists before initialization 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..f1257dd 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,9 +136,14 @@ 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 { + // Step 3: Check for idle timeout. Multi-source pipelines anchor the + // idle-timeout policy to the first 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. + idleSource := sourceBindings[0].Source + 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) r.setCondition(pipelineInstance, ConditionTypeSucceeded, metav1.ConditionTrue, "IdleTimeout", reason) r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionFalse, "IdleTimeout", reason) @@ -176,8 +186,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 +216,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 +304,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 deprecated singular `Spec.SourceRef` 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 +350,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 legacy 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 +399,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, diff --git a/internal/controller/pipelineinstance_controller_test.go b/internal/controller/pipelineinstance_controller_test.go index be279ad..dc657b4 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{ @@ -516,7 +516,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -603,7 +603,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -661,7 +661,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: "nonexistent-pipeline", }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -705,7 +705,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: "nonexistent-pipeline", }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -746,7 +746,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 +832,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 +849,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 +887,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: "nonexistent-source", }, }, @@ -969,7 +969,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 +1012,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1128,7 +1128,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1219,7 +1219,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1322,7 +1322,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1378,7 +1378,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1456,7 +1456,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1512,7 +1512,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1577,7 +1577,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1616,7 +1616,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1660,7 +1660,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1738,7 +1738,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1805,7 +1805,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1873,7 +1873,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -1942,7 +1942,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2021,7 +2021,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2144,7 +2144,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2207,7 +2207,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2299,7 +2299,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2376,7 +2376,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, }, @@ -2462,7 +2462,7 @@ var _ = Describe("PipelineInstance Controller", func() { PipelineRef: pipelinesv1alpha1.PipelineReference{ Name: pipelineName, }, - SourceRef: pipelinesv1alpha1.SourceReference{ + SourceRef: &pipelinesv1alpha1.SourceReference{ Name: pipelineSourceName, }, Execution: &pipelinesv1alpha1.ExecutionConfig{ @@ -2553,7 +2553,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 +2668,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 +2737,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 +2820,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..84203bd --- /dev/null +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -0,0 +1,207 @@ +package controller + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + + pipelinesv1alpha1 "github.com/PlainsightAI/openfilter-pipelines-controller/api/v1alpha1" +) + +// 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 deprecated +// `Spec.SourceRef` path 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() + // Deprecated field intentionally exercised: this test guards the + // legacy-CR compatibility path that the SA1019 deprecation comment + // asks new callers to avoid. + //nolint:staticcheck // SA1019: legacy SourceRef path is the system under test. + 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) + } + } +} + +// 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{ + //nolint:staticcheck // SA1019: deprecated SourceRef is the system under test. + 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) + } + }) +} 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 From fd81830ef0fca88d94b13ac26e8e031526dae044 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Tue, 9 Jun 2026 22:25:57 -0300 Subject: [PATCH 02/19] docs(samples): document the openfilter +1 port reservation for multi-source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated end-to-end against a docker-desktop cluster with a real RTSP stream: two videoin filters bound to distinct PipelineSources successfully co-tenant in one Pod and both stream at 24fps. The first smoke attempt crashed back-cam with `ZMQError: Address already in use (addr='tcp://*:5551')` because front-cam's `outputs: tcp://*:5550` reserves both 5550 and 5551 — openfilter binds the configured port AND port+1 for its ZMQ control channel (https://docs.plainsight.ai/docs/openfilter/your-first-filter/: "There is one idiosyncrasy with TCP connections to keep in mind, TWO ports are used, the port number you specify and that port number + 1"). Spacing outputs by 2 (5550, 5552, …) fixed the collision. Adding the warning to the multi-source sample so anyone copying it as a template gets the spacing right on the first try. Controller-side behavior is unaffected — port selection lives in the user-authored `pipeline.spec.filters[].config` strings; the controller just plumbs them through verbatim. --- ...v1alpha1_pipelineinstance_stream_multisource.yaml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml index 90d44da..7f57492 100644 --- a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml @@ -3,14 +3,22 @@ # 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 +# 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`. +# named `front-cam`, `back-cam`, and `webvis`. # - Two PipelineSources named `pipelinesource-front` and `pipelinesource-back`. # +# IMPORTANT — port spacing for multi-source pipelines: OpenFilter binds +# **two consecutive ports** per `outputs` entry (the port you set and N+1; the +# +1 carries ZMQ control-plane traffic). When authoring a multi-VideoIn +# Pipeline, stagger output ports by at least 2 (e.g. 5550, 5552, ...) — using +# adjacent ports (5550, 5551) causes the second VideoIn to fail with +# `ZMQError: Address already in use` because the first VideoIn already +# reserved 5551. See https://docs.plainsight.ai/docs/openfilter/your-first-filter/. +# # Use `sourceRef` for single-source pipelines instead — `sources` is for the # multi-camera case. apiVersion: filter.plainsight.ai/v1alpha1 From d744e68f0f78f719875a2017b5ee3e88d0f5b003 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Tue, 9 Jun 2026 22:40:45 -0300 Subject: [PATCH 03/19] docs(samples): document distinct per-VideoIn topic tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second openfilter authoring constraint surfaced while empirically validating PR2 against two real RTSP streams (vod2stream-car-truck-person and vod2stream): every VideoIn must tag its output topic via the `;topic` suffix in its `sources` config, otherwise both VideoIns publish to the default topic 'main' and webvis (or any downstream fan-in filter) crashes with: RuntimeError: duplicate topic 'main' from: VideoIn-… @ tcp://… Fix is the `sources: "$(RTSP_URL);"` form on each VideoIn plus matching `tcp://…;` references in the downstream filter's sources. Sample comment now covers both authoring constraints (port spacing + distinct topics) with a complete worked example. Validated end-to-end: two distinct streams streaming concurrently in one Pod, webvis /data endpoint returns the multi-topic shape with each entry pinned to its own `meta.src` URI. Controller code unchanged — both constraints live in the pipeline graph that the user (or plainsight-api export) authors. --- ...1_pipelineinstance_stream_multisource.yaml | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml index 7f57492..04d7150 100644 --- a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml @@ -11,13 +11,41 @@ # named `front-cam`, `back-cam`, and `webvis`. # - Two PipelineSources named `pipelinesource-front` and `pipelinesource-back`. # -# IMPORTANT — port spacing for multi-source pipelines: OpenFilter binds -# **two consecutive ports** per `outputs` entry (the port you set and N+1; the -# +1 carries ZMQ control-plane traffic). When authoring a multi-VideoIn -# Pipeline, stagger output ports by at least 2 (e.g. 5550, 5552, ...) — using -# adjacent ports (5550, 5551) causes the second VideoIn to fail with -# `ZMQError: Address already in use` because the first VideoIn already -# reserved 5551. See https://docs.plainsight.ai/docs/openfilter/your-first-filter/. +# 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. **Tag every VideoIn output topic** 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 (webvis, +# recorder, …) crashes with `RuntimeError: duplicate topic 'main'`. +# Convention: use the filter `name` (== this binding's filterName) as +# the topic so wiring is deterministic. +# +# 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. From ee7d32b4870d424eeaa477b3ffaab74d24293507 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Tue, 9 Jun 2026 23:01:11 -0300 Subject: [PATCH 04/19] feat(crd): admission rule + design doc for multi-source binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small follow-ups to the PR2 main commit, surfaced while running the docker-desktop smoke tests against real RTSP streams. CRD admission rule (`api/v1alpha1/pipelineinstance_types.go`) +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)" surfaces the contract violation at apply time instead of silently letting `EffectiveSources()` pick `Sources` (when both are set) or returning nil (when neither is set). Verified live against docker-desktop: a CR with neither field is rejected, a CR with both is rejected, a CR with only `sources` is accepted. DESIGN_DOCUMENT.md - 3.3 PipelineInstance: documents both `sourceRef` (legacy) and `sources` (multi-source) shapes with the admission constraint. - New 5.4 "Multi-source contract" — covers EffectiveSources normalization, the per-container env injection rule, and the two authoring constraints (port spacing + per-VideoIn topic tags) that the smoke tests proved out empirically. - New 5.5 "Multi-source batch — V1 status" — documents the `MultiSourceBatchUnsupported` condition the batch reconciler surfaces today. Generated CRDs regenerated via `make manifests generate helm-update-crds`. make test + make lint both green; coverage unchanged at 60.9%. --- DESIGN_DOCUMENT.md | 71 ++++++++++++++++++- api/v1alpha1/pipelineinstance_types.go | 7 +- ...ilter.plainsight.ai_pipelineinstances.yaml | 5 ++ ...ilter.plainsight.ai_pipelineinstances.yaml | 5 ++ 4 files changed, 85 insertions(+), 3 deletions(-) diff --git a/DESIGN_DOCUMENT.md b/DESIGN_DOCUMENT.md index 74c6229..613aa99 100644 --- a/DESIGN_DOCUMENT.md +++ b/DESIGN_DOCUMENT.md @@ -145,11 +145,34 @@ 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. See section 5.4 for the per-container env injection contract. + # Batch mode currently rejects multi-source bindings (>1 entry); the + # rejection surfaces as a `MultiSourceBatchUnsupported` condition. + 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 +393,52 @@ 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: rejected, 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. +- 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. + +### 5.5 Multi-source batch — V1 status + +Batch mode (`pipeline.spec.mode: batch`) currently accepts only one +source binding. A PipelineInstance with `len(Sources) > 1` and a batch +Pipeline is rejected with a `Degraded=MultiSourceBatchUnsupported` +condition; the message names the count and points at streaming or +single-source as the workaround. Multi-source batch (for benchmarking +multi-camera pipelines against per-media GT) needs an N-init-claimer +reshape and is tracked as a focused follow-up. + ## 6. Controller Responsibilities The PipelineInstance controller (`internal/controller/pipelineinstance_controller.go`) is the orchestrator for all pipeline execution logic. diff --git a/api/v1alpha1/pipelineinstance_types.go b/api/v1alpha1/pipelineinstance_types.go index fafc461..f12e674 100644 --- a/api/v1alpha1/pipelineinstance_types.go +++ b/api/v1alpha1/pipelineinstance_types.go @@ -46,7 +46,8 @@ type ExecutionConfig struct { // // 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: +// Exactly one of the following must be set (enforced by the XValidation +// rule on this type): // // - sourceRef (legacy, single-source): the bound source URL is broadcast // to every filter container as RTSP_URL (streaming) or as a single @@ -60,7 +61,9 @@ type ExecutionConfig struct { // 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. +// 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 diff --git a/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml index 544f1db..c91cd79 100644 --- a/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml +++ b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml @@ -1122,6 +1122,11 @@ spec: required: - pipelineRef 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_pipelineinstances.yaml b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml index 544f1db..c91cd79 100644 --- a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml +++ b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml @@ -1122,6 +1122,11 @@ spec: required: - pipelineRef 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: From 6c9fcaf839fb0da438d6e6a80e76987b8000923e Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Tue, 9 Jun 2026 23:36:33 -0300 Subject: [PATCH 05/19] =?UTF-8?q?feat(batch):=20multi-source=20batch=20?= =?UTF-8?q?=E2=80=94=20N=20init=20claimers,=20one=20Pod=20per=20run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the multi-source batch gap that PR2 originally left as a follow-up. Multi-camera benchmark runs from plainsight-api (PLAT-1140) now have an end-to-end controller path; the rejection condition is gone. Behavior Batch mode now branches on `len(Sources)`: - Single-source (legacy SourceRef or one-entry Sources): unchanged. The existing queue-based parallel-files-per-bucket flow keeps working bit-for-bit. List bucket → enqueue → N pods pop one file each via Valkey → init claimer downloads → filter chain runs. - Multi-source (≥2 entries in Sources): `reconcileBatchMultiSource` emits a single Pod (parallelism=1, completions=1) with one init claimer per binding in **direct mode** plus the user's filter containers. Per-VideoIn `VIDEO_INPUT_PATH` is injected into the matching container only; downstream containers are untouched. Completion is the Job's own Succeeded/Failed status — no per-file work-queue accounting. CRD `BucketSource.Object` (optional) names a specific S3 object key. Set this for every binding in multi-source batch so each direct-mode claimer has a deterministic target; the legacy queue path stays prefix-based and ignores the field. Two new Degraded conditions surface configuration errors: - `MultiSourceBatchInvalidSource`: a binding's PipelineSource isn't a Bucket (RTSP makes no sense for batch). - `MultiSourceBatchMissingObject`: a binding's Bucket.Object is unset. Claimer `cmd/claimer/main.go` learns a direct mode triggered by the new `S3_OBJECT_KEY` env var: skip Valkey entirely, download exactly that key to `VIDEO_INPUT_PATH`, exit. Legacy queue mode is unchanged when the env is empty. `STREAM` and `GROUP` are now required only in queue mode. Reconciler `reconcileBatch` lost the `MultiSourceBatchUnsupported` rejection and gained a one-line branch into the new path. The single-source code path is otherwise untouched. New file `pipelineinstance_controller_batch_multisource.go` carries: - `reconcileBatchMultiSource` (validation, Job ensure, status sync) - `buildMultiSourceBatchJob` (Job builder) - `buildDirectClaimerEnv` (per-binding direct-mode env) - `buildBatchFilterContainersForMultiSource` (per-container env injection) - `perFilterInputPath` (download path naming) Tests `TestBuildMultiSourceBatchJob_PerBindingInitClaimersAndEnv` pins: - One init claimer per binding, named `claimer-` - Each claimer's S3_OBJECT_KEY + VIDEO_INPUT_PATH match its binding - No Valkey env on direct-mode claimers - Each VideoIn filter container has VIDEO_INPUT_PATH set; downstream filters do not - Job topology: parallelism=1, completions=1 Make test + make lint both green. Validated empirically on docker-desktop K8s with a fake S3 bucket (claimers can't actually download but the controller-side shape is the validation target): 2 init claimers named correctly, each with its specific S3_OBJECT_KEY + path, per-VideoIn env on both VideoIn filter containers, downstream `image-out` container left clean. DESIGN_DOCUMENT.md sections 3.3 and 5.5 updated to reflect that batch multi-source is now supported and documents the `MultiSourceBatchInvalidSource` / `MultiSourceBatchMissingObject` operator-actionable failure modes. --- DESIGN_DOCUMENT.md | 53 ++- api/v1alpha1/pipeline_types.go | 11 + cmd/claimer/main.go | 43 +- .../filter.plainsight.ai_pipelinesources.yaml | 11 + .../filter.plainsight.ai_pipelinesources.yaml | 11 + .../pipelineinstance_controller_batch.go | 30 +- ...neinstance_controller_batch_multisource.go | 366 ++++++++++++++++++ .../pipelineinstance_multisource_test.go | 121 ++++++ 8 files changed, 615 insertions(+), 31 deletions(-) create mode 100644 internal/controller/pipelineinstance_controller_batch_multisource.go diff --git a/DESIGN_DOCUMENT.md b/DESIGN_DOCUMENT.md index 613aa99..893a369 100644 --- a/DESIGN_DOCUMENT.md +++ b/DESIGN_DOCUMENT.md @@ -162,9 +162,12 @@ spec: # 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. See section 5.4 for the per-container env injection contract. - # Batch mode currently rejects multi-source bindings (>1 entry); the - # rejection surfaces as a `MultiSourceBatchUnsupported` condition. + # 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 needs Bucket.Object set so the + # direct-mode claimer has a deterministic key). sources: - filterName: front-cam sourceRef: @@ -429,15 +432,41 @@ source plumbing: `pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml` sample for a complete worked example. -### 5.5 Multi-source batch — V1 status - -Batch mode (`pipeline.spec.mode: batch`) currently accepts only one -source binding. A PipelineInstance with `len(Sources) > 1` and a batch -Pipeline is rejected with a `Degraded=MultiSourceBatchUnsupported` -condition; the message names the count and points at streaming or -single-source as the workaround. Multi-source batch (for benchmarking -multi-camera pipelines against per-media GT) needs an N-init-claimer -reshape and is tracked as a focused follow-up. +### 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.Object` must be set to a specific S3 object + key. 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. ## 6. Controller Responsibilities diff --git a/api/v1alpha1/pipeline_types.go b/api/v1alpha1/pipeline_types.go index b8ea150..2f97050 100644 --- a/api/v1alpha1/pipeline_types.go +++ b/api/v1alpha1/pipeline_types.go @@ -47,6 +47,17 @@ type BucketSource struct { // +optional Prefix string `json:"prefix,omitempty"` + // object names a specific S3 object key inside the bucket. Set this for + // multi-source batch bindings (PLAT-1071): the claimer downloads exactly + // this key to the bound VideoIn's per-container input path, skipping the + // Valkey work-queue (queue-based fan-out only makes sense when one + // PipelineSource feeds many parallel Pods, which multi-source batch is + // not). When `object` is empty the legacy queue-based behavior applies: + // the bucket+prefix is listed and each file is enqueued as a Valkey + // work item processed by parallel pods. + // +optional + Object string `json:"object,omitempty"` + // endpoint is the S3-compatible endpoint URL (required for non-AWS S3) // Examples: // - MinIO: "http://minio.example.com:9000" 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/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml b/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml index 00299a8..1e96552 100644 --- a/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml +++ b/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml @@ -85,6 +85,17 @@ spec: description: name is the name of the S3-compatible bucket minLength: 1 type: string + object: + description: |- + object names a specific S3 object key inside the bucket. Set this for + multi-source batch bindings (PLAT-1071): the claimer downloads exactly + this key to the bound VideoIn's per-container input path, skipping the + Valkey work-queue (queue-based fan-out only makes sense when one + PipelineSource feeds many parallel Pods, which multi-source batch is + not). When `object` is empty the legacy queue-based behavior applies: + the bucket+prefix is listed and each file is enqueued as a Valkey + work item processed by parallel pods. + type: string prefix: description: prefix is an optional path prefix within the bucket (e.g., "input-data/") 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..1e96552 100644 --- a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml +++ b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml @@ -85,6 +85,17 @@ spec: description: name is the name of the S3-compatible bucket minLength: 1 type: string + object: + description: |- + object names a specific S3 object key inside the bucket. Set this for + multi-source batch bindings (PLAT-1071): the claimer downloads exactly + this key to the bound VideoIn's per-container input path, skipping the + Valkey work-queue (queue-based fan-out only makes sense when one + PipelineSource feeds many parallel Pods, which multi-source batch is + not). When `object` is empty the legacy queue-based behavior applies: + the bucket+prefix is listed and each file is enqueued as a Valkey + work item processed by parallel pods. + type: string prefix: description: prefix is an optional path prefix within the bucket (e.g., "input-data/") diff --git a/internal/controller/pipelineinstance_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index 4f6173d..727c676 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -40,21 +40,25 @@ import ( // reconcileBatch handles the batch (Job-based) reconciliation path. // -// V1 scope (PLAT-1071): batch mode currently supports a single source -// binding. Multi-source batch — used for benchmarking multi-camera -// pipelines against per-media GT — needs N parallel init claimers and a -// reshaped work-queue model. That work is tracked as a focused follow-up; -// today multi-source batch is rejected at the top of this function with a -// clear operator-actionable error. Single-source batch (legacy SourceRef -// or a one-entry Sources array) keeps working exactly as before. +// 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. +// +// - Multi-source: ≥2 PipelineSources bound, each pinned to a +// specific filter container by name. Per-binding the source must +// name a specific S3 object via `Bucket.Object` 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. func (r *PipelineInstanceReconciler) reconcileBatch(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipeline *pipelinesv1alpha1.Pipeline, sourceBindings []ResolvedSourceBinding) (ctrl.Result, error) { if len(sourceBindings) > 1 { - err := fmt.Errorf("batch mode does not yet support multi-source pipelines (%d sources bound); please reduce to one source or use streaming mode", len(sourceBindings)) - r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchUnsupported", err.Error()) - if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { - logf.FromContext(ctx).Error(statusErr, "Failed to update status after multi-source batch rejection") - } - return ctrl.Result{}, err + return r.reconcileBatchMultiSource(ctx, pipelineInstance, pipeline, sourceBindings) } pipelineSource := sourceBindings[0].Source log := logf.FromContext(ctx) diff --git a/internal/controller/pipelineinstance_controller_batch_multisource.go b/internal/controller/pipelineinstance_controller_batch_multisource.go new file mode 100644 index 0000000..0486395 --- /dev/null +++ b/internal/controller/pipelineinstance_controller_batch_multisource.go @@ -0,0 +1,366 @@ +/* +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" + 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's `Bucket.Object` must be set so the per-binding +// direct-mode claimer has a deterministic key to download. +// +// 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) + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchInvalidSource", msg) + if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { + log.Error(statusErr, "Failed to update status after multi-source batch validation") + } + return ctrl.Result{}, nil + } + if b.Source.Spec.Bucket.Object == "" { + msg := fmt.Sprintf("multi-source batch requires every binding's PipelineSource Bucket.Object to be set; %q does not name a specific object", b.FilterName) + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchMissingObject", msg) + if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { + log.Error(statusErr, "Failed to update status after multi-source batch validation") + } + return ctrl.Result{}, nil + } + } + + // 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. + r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionTrue, "Processing", "Multi-source batch Job is running") + 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 per-binding VIDEO_INPUT_PATH and FILTER_VIDEO_IN +// env vars 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, b.Source.Spec.Bucket.Object) + } + + // 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: fmt.Sprintf("claimer-%s", 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", b.Source.Spec.Bucket.Object, "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: bucket.Object}, + {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, + }) + } + + env := append([]corev1.EnvVar{}, configEnv...) + + // Per-binding env: VideoIn containers whose name matches a + // binding pick up VIDEO_INPUT_PATH and we also expose it as + // the FILTER_SOURCES alias so authors can write either + // `sources: file://$(VIDEO_INPUT_PATH)` (explicit) or + // `sources: $(FILTER_SOURCES)` (parallel to RTSP streaming + // where downstream filters do this). + if path, ok := downloadPath[filter.Name]; ok { + env = append(env, + corev1.EnvVar{Name: "VIDEO_INPUT_PATH", Value: path}, + ) + } + + // 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 +} + +// 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) +} diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 84203bd..1e6cad4 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -171,6 +171,127 @@ func TestBuildStreamingDeployment_LegacyBroadcast(t *testing.T) { } } +// 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", + Object: "front.mp4", + CredentialsSecret: &pipelinesv1alpha1.SecretReference{ + Name: "s3-creds", + }, + }, + }, + } + backSource := &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{ + Name: "media", + Object: "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") + } + 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). From 84a9d88cb2e4ba01cb7c116b58f46e85d89e9e78 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Tue, 9 Jun 2026 23:50:35 -0300 Subject: [PATCH 06/19] test: fill multi-source coverage gaps (reconciler, claimer, resolver) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds tests for the parts of the PR that were happy-path-only after the PLAT-1141 work. Coverage gains: - reconcileBatchMultiSource: 0% → 73.9% - resolveSourceBindings: 80% → 86.7% - cmd/claimer/loadConfig: 68.8% → 87.5% Controller package overall: 58.6% → 63.0%. Claimer package: 17.6% → 19.1%. New test files / functions internal/controller/pipelineinstance_controller_batch_multisource_test.go Pins the six branches of reconcileBatchMultiSource against a fake client with status-subresource support: 1. RejectsNonBucketSource → MultiSourceBatchInvalidSource 2. RejectsMissingObject → MultiSourceBatchMissingObject 3. CreatesJobAndStampsStartTime → happy-path Job create 4. JobCompleteMarksSucceeded → JobComplete → PI Succeeded 5. JobFailedMarksDegraded → JobFailed → PI Degraded (Reason/Message propagate) 6. JobProgressingStaysProgressing → active Job → PI Progressing internal/controller/pipelineinstance_multisource_test.go Two new tests for the streaming + resolver paths: * MultiSource_WithCredentials — credentials-bearing per-binding RTSP source must inject _RTSP_USERNAME / _RTSP_PASSWORD as secretKeyRefs into ONLY the matching VideoIn container; one-with, one-without coexist; credential secret must not leak into the credential-less sibling. * ResolveSourceBindings_SourceMissingSurfacesActionableError + ResolveSourceBindings_LegacySourceRefMissingSurfacesError — operator-quality error wording for both shapes. cmd/claimer/main_test.go TestLoadConfig_DirectMode (table) — five sub-tests covering the S3_OBJECT_KEY conditional STREAM/GROUP relaxation, queue-mode-still- requires-STREAM, queue-mode-still-requires-GROUP, S3_BUCKET still required in either mode, and VIDEO_INPUT_PATH default in direct mode. Known gap: cmd/claimer/main.go:run() direct-download branch stays at 0% unit coverage. Testing it would require mocking the minio client or spinning up a fake S3 server inside the test process; the function is small delegation (loadConfig → createMinIOClient → downloadFile) and is validated end-to-end by the docker-desktop smoke test that proved the init claimers receive the correct env vars and execute. make test + make lint both green. --- cmd/claimer/main_test.go | 64 ++++ ...tance_controller_batch_multisource_test.go | 344 ++++++++++++++++++ .../pipelineinstance_multisource_test.go | 167 +++++++++ 3 files changed, 575 insertions(+) create mode 100644 internal/controller/pipelineinstance_controller_batch_multisource_test.go 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/internal/controller/pipelineinstance_controller_batch_multisource_test.go b/internal/controller/pipelineinstance_controller_batch_multisource_test.go new file mode 100644 index 0000000..ff371e8 --- /dev/null +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -0,0 +1,344 @@ +/* +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" + "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.Object 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", Object: "front.mp4"}, + }, + }}, + {FilterName: "back-cam", Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Object: "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) + } + + // 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 but no Object key surfaces a +// `MultiSourceBatchMissingObject` Degraded condition. +func TestReconcileBatchMultiSource_RejectsMissingObject(t *testing.T) { + pi, pipeline, bindings := makeMultiSourcePI(t) + // Strip the object on back-cam. + bindings[1].Source.Spec.Bucket.Object = "" + 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) + } +} + +// 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) + } +} diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 1e6cad4..622ee58 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -2,13 +2,20 @@ package controller import ( "context" + "strings" "testing" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" 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 @@ -326,3 +333,163 @@ func TestEffectiveSources(t *testing.T) { } }) } + +// 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{ + //nolint:staticcheck // SA1019: legacy SourceRef is the system under test. + 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) + } +} From a4f943cbd3cae83f0fcb858889c923c355a9fa52 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Wed, 10 Jun 2026 19:54:34 -0300 Subject: [PATCH 07/19] fix(PLAT-1141): valid multisource sample names + uniform VIDEO_INPUT_PATH contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes from the cross-repo contract review: - Stream multisource sample used `filterName: front_cam` / `back_cam` — rejected at admission by the CRD's own DNS-1123 pattern, and mismatching the comment's `front-cam` / `back-cam` Pipeline filters. All names (filterName, filter names, topics) now align hyphenated so there is one name per camera end-to-end, matching the sample's stated convention. - Single-source batch now injects VIDEO_INPUT_PATH (= the claimer's download destination, spec.videoInputPath) into every filter container, mirroring the per-binding injection the multi-source path already does. This lets exported Pipelines author VideoIn sources as `file://$(VIDEO_INPUT_PATH)` uniformly — the plainsight-api export (PLAT-1140) switches to that form so the same exported YAML serves 1..N sources. Existing CRDs with hardcoded /ws/input.mp4 keep working (env injection is additive); injected before filter.Env so a user-supplied value still wins. - Added the batch multisource sample the design doc calls for (direct-mode Job, per-binding claimers, Bucket.Object constraints, $(VIDEO_INPUT_PATH) authoring) and registered both multisource samples in kustomization. --- config/samples/kustomization.yaml | 1 + ...a1_pipelineinstance_batch_multisource.yaml | 65 +++++++++++++++++++ ...1_pipelineinstance_stream_multisource.yaml | 16 +++-- .../pipelineinstance_controller_batch.go | 14 ++++ .../pipelineinstance_controller_test.go | 4 ++ 5 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index a17c144..476957c 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -6,4 +6,5 @@ resources: - 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..5e5e21c --- /dev/null +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml @@ -0,0 +1,65 @@ +# 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 must name a specific `object` (no prefix scans). +# +# 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 +# tag each VideoIn's output topic. Example pipeline `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 naming a +# specific object, e.g.: +# +# spec: +# bucket: +# endpoint: https://storage.googleapis.com +# name: my-benchmark-clips +# object: 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 index 04d7150..52a17c7 100644 --- a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml @@ -25,7 +25,11 @@ # topic `main` and any downstream that fans-in two upstreams (webvis, # recorder, …) crashes with `RuntimeError: duplicate topic 'main'`. # Convention: use the filter `name` (== this binding's filterName) as -# the topic so wiring is deterministic. +# the topic so wiring is deterministic. Note filter names (and therefore +# `sources[].filterName` below) must be DNS-1123 labels — lowercase +# alphanumerics and hyphens only, no underscores — because they flow +# into container names; using them as topics too keeps one name per +# camera end-to-end. # # Example pipeline `pipeline-rtsp-multi` config that satisfies both: # @@ -33,19 +37,19 @@ # - name: front-cam # config: # - name: sources -# value: "$(RTSP_URL);front_cam" # ;front_cam tags the topic +# 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" +# 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" +# 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. @@ -57,9 +61,9 @@ spec: pipelineRef: name: pipeline-rtsp-multi sources: - - filterName: front_cam + - filterName: front-cam sourceRef: name: pipelinesource-front - - filterName: back_cam + - filterName: back-cam sourceRef: name: pipelinesource-back diff --git a/internal/controller/pipelineinstance_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index 727c676..18434e1 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -549,6 +549,20 @@ func (r *PipelineInstanceReconciler) buildJob(ctx context.Context, pipelineInsta containerEnv := make([]corev1.EnvVar, 0, len(configEnvVars)+len(filter.Env)) containerEnv = append(containerEnv, configEnvVars...) + // Expose the claimer's download destination to every filter + // container so VideoIn sources can be authored as + // `sources: file://$(VIDEO_INPUT_PATH)` — the same contract the + // multi-source path provides per-binding (see + // buildBatchFilterContainersForMultiSource). Single-source has no + // per-filter binding to key on (legacy broadcast), so all + // containers get it; only VideoIn entries reference it. Injected + // before filter.Env so a user-supplied value wins on duplicate + // names (kubelet keeps the last entry). + containerEnv = append(containerEnv, corev1.EnvVar{ + Name: "VIDEO_INPUT_PATH", + Value: videoInputPath, + }) + // Inject GPU env vars before user env vars so users can override if needed. // Skipped when the respective path field is empty (e.g. on EKS). if filter.Resources != nil && containerResourcesRequireGPU(*filter.Resources) { diff --git a/internal/controller/pipelineinstance_controller_test.go b/internal/controller/pipelineinstance_controller_test.go index dc657b4..70ff887 100644 --- a/internal/controller/pipelineinstance_controller_test.go +++ b/internal/controller/pipelineinstance_controller_test.go @@ -504,6 +504,10 @@ 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. + Expect(job.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{Name: "VIDEO_INPUT_PATH", Value: "/ws/custom-input.mp4"})) }) It("should inject per-namespace Valkey credentials from namespace secret in claimer", func() { From 30dcfc7ec3c9264ccd4732029674469a1a787a45 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 13:53:12 -0300 Subject: [PATCH 08/19] =?UTF-8?q?fix(PLAT-1141):=20review=20criticals=20?= =?UTF-8?q?=E2=80=94=20env=20ordering=20+=20object-key=20resolution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes from the cross-repo PLAT-1071 review: 1. VIDEO_INPUT_PATH now precedes the FILTER_* config env in BOTH batch builders. Kubernetes dependent-env expansion only resolves $(VAR) references to variables defined earlier in the list — with the old config-first ordering, the `sources: file://$(VIDEO_INPUT_PATH)` authoring contract arrived in the container as a literal unexpanded string and multi-source batch was dead on arrival (the streaming builder already ordered RTSP env first and documents why). Ordering assertions added to the multisource unit test and the single-source envtest — the prior tests only asserted presence/value, which is how this slipped through. 2. Direct-mode claimers resolve their object key as Bucket.Object when set, else Bucket.Prefix used as the full key (new bindingObjectKey). The platform's media model has no separate object concept — the agent has always shipped single-file object keys in `prefix`, and the legacy queue path relied on prefix-as-key listing. The previous hard requirement on Bucket.Object (a field this PR itself introduced) would have Degraded every agent-deployed multi-source batch (MultiSourceBatchMissingObject) since the agent never populates it. Degrade now only when neither is set. Relatedly, the queue seeder honors an explicit Object (enqueue exactly that key, no prefix scan) so a hand-authored single-binding CR with Object doesn't silently process the whole prefix. --- ...a1_pipelineinstance_batch_multisource.yaml | 4 +- .../controller/pipelineinstance_controller.go | 11 +++ .../pipelineinstance_controller_batch.go | 29 ++++---- ...neinstance_controller_batch_multisource.go | 53 ++++++++++----- .../pipelineinstance_controller_test.go | 7 +- .../pipelineinstance_multisource_test.go | 67 +++++++++++++++++++ 6 files changed, 140 insertions(+), 31 deletions(-) diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml index 5e5e21c..854307e 100644 --- a/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml @@ -12,7 +12,9 @@ # # Constraints enforced by the controller (surfaced as Degraded conditions): # - every binding's PipelineSource must be a Bucket source (no RTSP), and -# - every Bucket source must name a specific `object` (no prefix scans). +# - every Bucket source must resolve a specific object key: `object` when +# set, else `prefix` is used as the full 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 diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index afef55d..c69d9f3 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -615,6 +615,17 @@ func (r *PipelineInstanceReconciler) listBucketFiles(ctx context.Context, pipeli bucket := pipelineSource.Spec.Bucket + // A source naming a specific object (PLAT-1071) is exactly one work + // item — enqueue it directly instead of listing the prefix. Without + // this, a single-binding batch whose PipelineSource carries Object + // would silently process every object under Prefix (the multi-source + // path honors Object via direct-mode claimers; the queue path must + // match). Existence is verified by the claimer's download, which + // surfaces a missing object as a per-file failure. + if bucket.Object != "" { + return []string{bucket.Object}, nil + } + endpoint := bucket.Endpoint useSSL := true if endpoint != "" { diff --git a/internal/controller/pipelineinstance_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index 18434e1..507d85b 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -544,24 +544,25 @@ 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)) - containerEnv = append(containerEnv, configEnvVars...) - - // Expose the claimer's download destination to every filter - // container so VideoIn sources can be authored as - // `sources: file://$(VIDEO_INPUT_PATH)` — the same contract the - // multi-source path provides per-binding (see - // buildBatchFilterContainersForMultiSource). Single-source has no - // per-filter binding to key on (legacy broadcast), so all - // containers get it; only VideoIn entries reference it. Injected - // before filter.Env so a user-supplied value wins on duplicate - // names (kubelet keeps the last entry). + // 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. // Skipped when the respective path field is empty (e.g. on EKS). diff --git a/internal/controller/pipelineinstance_controller_batch_multisource.go b/internal/controller/pipelineinstance_controller_batch_multisource.go index 0486395..8a34faa 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource.go @@ -49,8 +49,10 @@ import ( // Constraints enforced here (V1): // - Every binding's PipelineSource must have a Bucket source set // (RTSP makes no sense for batch). -// - Every binding's `Bucket.Object` must be set so the per-binding -// direct-mode claimer has a deterministic key to download. +// - Every binding must resolve a deterministic object key for its +// direct-mode claimer: `Bucket.Object` when set, else `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 @@ -74,8 +76,8 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte } return ctrl.Result{}, nil } - if b.Source.Spec.Bucket.Object == "" { - msg := fmt.Sprintf("multi-source batch requires every binding's PipelineSource Bucket.Object to be set; %q does not name a specific object", b.FilterName) + if bindingObjectKey(b) == "" { + msg := fmt.Sprintf("multi-source batch requires every binding's PipelineSource to name a specific object (Bucket.Object or a Bucket.Prefix that is a full object key); %q has neither", b.FilterName) r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchMissingObject", msg) if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { log.Error(statusErr, "Failed to update status after multi-source batch validation") @@ -185,7 +187,7 @@ func (r *PipelineInstanceReconciler) buildMultiSourceBatchJob(ctx context.Contex // (where their VideoIn reads). downloadPath := make(map[string]string, len(sourceBindings)) for _, b := range sourceBindings { - downloadPath[b.FilterName] = perFilterInputPath(b.FilterName, b.Source.Spec.Bucket.Object) + downloadPath[b.FilterName] = perFilterInputPath(b.FilterName, bindingObjectKey(b)) } // One init claimer per binding (direct mode). @@ -199,7 +201,7 @@ func (r *PipelineInstanceReconciler) buildMultiSourceBatchJob(ctx context.Contex {Name: "workspace", MountPath: "/ws"}, }, }) - log.V(1).Info("Added direct-mode claimer", "index", i, "filter", b.FilterName, "object", b.Source.Spec.Bucket.Object, "destination", downloadPath[b.FilterName]) + 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. @@ -259,7 +261,7 @@ func (r *PipelineInstanceReconciler) buildDirectClaimerEnv(b ResolvedSourceBindi bucket := b.Source.Spec.Bucket env := []corev1.EnvVar{ {Name: "S3_BUCKET", Value: bucket.Name}, - {Name: "S3_OBJECT_KEY", Value: bucket.Object}, + {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}, @@ -308,19 +310,22 @@ func (r *PipelineInstanceReconciler) buildBatchFilterContainersForMultiSource(pi }) } - env := append([]corev1.EnvVar{}, configEnv...) - - // Per-binding env: VideoIn containers whose name matches a - // binding pick up VIDEO_INPUT_PATH and we also expose it as - // the FILTER_SOURCES alias so authors can write either - // `sources: file://$(VIDEO_INPUT_PATH)` (explicit) or - // `sources: $(FILTER_SOURCES)` (parallel to RTSP streaming - // where downstream filters do this). + // 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) { @@ -353,6 +358,24 @@ func (r *PipelineInstanceReconciler) buildBatchFilterContainersForMultiSource(pi return containers } +// bindingObjectKey resolves the exact S3 object a binding's claimer must +// download: Bucket.Object when set (explicit override), else Bucket.Prefix. +// The platform's media model has no separate object concept — single-file +// medias put the 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 "" + } + if b.Source.Spec.Bucket.Object != "" { + return b.Source.Spec.Bucket.Object + } + return b.Source.Spec.Bucket.Prefix +} + // 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 diff --git a/internal/controller/pipelineinstance_controller_test.go b/internal/controller/pipelineinstance_controller_test.go index 70ff887..6982b54 100644 --- a/internal/controller/pipelineinstance_controller_test.go +++ b/internal/controller/pipelineinstance_controller_test.go @@ -507,7 +507,12 @@ var _ = Describe("PipelineInstance Controller", func() { // 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. - Expect(job.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{Name: "VIDEO_INPUT_PATH", Value: "/ws/custom-input.mp4"})) + // 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() { diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 622ee58..fcf82f1 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -276,6 +276,15 @@ func TestBuildMultiSourceBatchJob_PerBindingInitClaimersAndEnv(t *testing.T) { 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") } @@ -493,3 +502,61 @@ func TestResolveSourceBindings_LegacySourceRefMissingSurfacesError(t *testing.T) 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) + } +} + +// TestBindingObjectKey pins the object-key resolution the direct-mode +// claimers depend on: explicit Bucket.Object wins; otherwise Bucket.Prefix +// is the full key (the platform's media model carries single-file object +// keys in prefix — there is no separate object concept upstream); neither → +// empty (reconcile Degrades). +func TestBindingObjectKey(t *testing.T) { + mk := func(object, prefix string) ResolvedSourceBinding { + return ResolvedSourceBinding{ + FilterName: "front-cam", + Source: &pipelinesv1alpha1.PipelineSource{ + Spec: pipelinesv1alpha1.PipelineSourceSpec{ + Bucket: &pipelinesv1alpha1.BucketSource{ + Name: "b", + Object: object, + Prefix: prefix, + }, + }, + }, + } + } + if got := bindingObjectKey(mk("clips/exact.mp4", "clips/")); got != "clips/exact.mp4" { + t.Errorf("explicit Object must win, got %q", got) + } + if got := bindingObjectKey(mk("", "clips/front_001.mp4")); got != "clips/front_001.mp4" { + t.Errorf("Prefix must serve as the full key when Object empty, got %q", got) + } + if got := bindingObjectKey(mk("", "")); got != "" { + t.Errorf("neither set must resolve empty, got %q", got) + } + if got := bindingObjectKey(ResolvedSourceBinding{FilterName: "x"}); got != "" { + t.Errorf("nil source must resolve empty, got %q", got) + } +} From 43f53cb8af4a380da52d962aad4baf9ee7329664 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 14:10:39 -0300 Subject: [PATCH 09/19] =?UTF-8?q?refactor(PLAT-1141):=20drop=20BucketSourc?= =?UTF-8?q?e.Object=20=E2=80=94=20Prefix=20is=20the=20object=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Object field was invented by this PR and never had a producer: the platform's media model carries single-file object keys in `prefix` (the agent parses the media URL path into it) and the legacy queue path has always relied on prefix-as-key listing. Keeping an optional field nobody sets is exactly the drift trap the cross-repo review caught (the multi-source path hard-required it while the agent never populated it). Since the field never shipped (unmerged PR), removing it is free. - bindingObjectKey resolves Bucket.Prefix as the full object key; multi-source batch Degrades (MultiSourceBatchMissingObject) only when the prefix is empty. - Queue-seeder early return removed with the field. - CRD bases + Helm chart copies regenerated in lockstep; sample and design comments updated to the prefix-as-full-key contract. --- api/v1alpha1/pipeline_types.go | 10 ------- .../filter.plainsight.ai_pipelinesources.yaml | 11 -------- ...a1_pipelineinstance_batch_multisource.yaml | 12 ++++----- .../filter.plainsight.ai_pipelinesources.yaml | 11 -------- .../controller/pipelineinstance_controller.go | 11 -------- .../pipelineinstance_controller_batch.go | 4 +-- ...neinstance_controller_batch_multisource.go | 25 ++++++++---------- ...tance_controller_batch_multisource_test.go | 12 ++++----- .../pipelineinstance_multisource_test.go | 26 ++++++++----------- 9 files changed, 36 insertions(+), 86 deletions(-) diff --git a/api/v1alpha1/pipeline_types.go b/api/v1alpha1/pipeline_types.go index 2f97050..3258b84 100644 --- a/api/v1alpha1/pipeline_types.go +++ b/api/v1alpha1/pipeline_types.go @@ -47,16 +47,6 @@ type BucketSource struct { // +optional Prefix string `json:"prefix,omitempty"` - // object names a specific S3 object key inside the bucket. Set this for - // multi-source batch bindings (PLAT-1071): the claimer downloads exactly - // this key to the bound VideoIn's per-container input path, skipping the - // Valkey work-queue (queue-based fan-out only makes sense when one - // PipelineSource feeds many parallel Pods, which multi-source batch is - // not). When `object` is empty the legacy queue-based behavior applies: - // the bucket+prefix is listed and each file is enqueued as a Valkey - // work item processed by parallel pods. - // +optional - Object string `json:"object,omitempty"` // endpoint is the S3-compatible endpoint URL (required for non-AWS S3) // Examples: diff --git a/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml b/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml index 1e96552..00299a8 100644 --- a/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml +++ b/config/crd/bases/filter.plainsight.ai_pipelinesources.yaml @@ -85,17 +85,6 @@ spec: description: name is the name of the S3-compatible bucket minLength: 1 type: string - object: - description: |- - object names a specific S3 object key inside the bucket. Set this for - multi-source batch bindings (PLAT-1071): the claimer downloads exactly - this key to the bound VideoIn's per-container input path, skipping the - Valkey work-queue (queue-based fan-out only makes sense when one - PipelineSource feeds many parallel Pods, which multi-source batch is - not). When `object` is empty the legacy queue-based behavior applies: - the bucket+prefix is listed and each file is enqueued as a Valkey - work item processed by parallel pods. - type: string prefix: description: prefix is an optional path prefix within the bucket (e.g., "input-data/") diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml index 854307e..0ab9fa6 100644 --- a/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml @@ -12,9 +12,9 @@ # # Constraints enforced by the controller (surfaced as Degraded conditions): # - every binding's PipelineSource must be a Bucket source (no RTSP), and -# - every Bucket source must resolve a specific object key: `object` when -# set, else `prefix` is used as the full key (how single-file medias -# ship from the platform — no prefix scans in multi-source mode). +# - 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 @@ -43,14 +43,14 @@ # - name: sources # value: "tcp://localhost:5550;front-cam, tcp://localhost:5552;back-cam" # -# This sample assumes two PipelineSources, each a Bucket source naming a -# specific object, e.g.: +# 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 -# object: clips/front_lobby_001.mp4 +# prefix: clips/front_lobby_001.mp4 apiVersion: filter.plainsight.ai/v1alpha1 kind: PipelineInstance metadata: 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 1e96552..00299a8 100644 --- a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml +++ b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelinesources.yaml @@ -85,17 +85,6 @@ spec: description: name is the name of the S3-compatible bucket minLength: 1 type: string - object: - description: |- - object names a specific S3 object key inside the bucket. Set this for - multi-source batch bindings (PLAT-1071): the claimer downloads exactly - this key to the bound VideoIn's per-container input path, skipping the - Valkey work-queue (queue-based fan-out only makes sense when one - PipelineSource feeds many parallel Pods, which multi-source batch is - not). When `object` is empty the legacy queue-based behavior applies: - the bucket+prefix is listed and each file is enqueued as a Valkey - work item processed by parallel pods. - type: string prefix: description: prefix is an optional path prefix within the bucket (e.g., "input-data/") diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index c69d9f3..afef55d 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -615,17 +615,6 @@ func (r *PipelineInstanceReconciler) listBucketFiles(ctx context.Context, pipeli bucket := pipelineSource.Spec.Bucket - // A source naming a specific object (PLAT-1071) is exactly one work - // item — enqueue it directly instead of listing the prefix. Without - // this, a single-binding batch whose PipelineSource carries Object - // would silently process every object under Prefix (the multi-source - // path honors Object via direct-mode claimers; the queue path must - // match). Existence is verified by the claimer's download, which - // surfaces a missing object as a per-file failure. - if bucket.Object != "" { - return []string{bucket.Object}, nil - } - endpoint := bucket.Endpoint useSSL := true if endpoint != "" { diff --git a/internal/controller/pipelineinstance_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index 507d85b..097daf3 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -49,8 +49,8 @@ import ( // the filter chain. Unchanged from before PR2. // // - Multi-source: ≥2 PipelineSources bound, each pinned to a -// specific filter container by name. Per-binding the source must -// name a specific S3 object via `Bucket.Object` so each VideoIn's +// 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 diff --git a/internal/controller/pipelineinstance_controller_batch_multisource.go b/internal/controller/pipelineinstance_controller_batch_multisource.go index 8a34faa..90c0f20 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource.go @@ -50,9 +50,9 @@ import ( // - 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.Object` when set, else `Bucket.Prefix` -// used as the full key (the platform's media model carries single-file -// keys in prefix — see bindingObjectKey). +// 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 @@ -77,7 +77,7 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte return ctrl.Result{}, nil } if bindingObjectKey(b) == "" { - msg := fmt.Sprintf("multi-source batch requires every binding's PipelineSource to name a specific object (Bucket.Object or a Bucket.Prefix that is a full object key); %q has neither", b.FilterName) + 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) r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchMissingObject", msg) if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { log.Error(statusErr, "Failed to update status after multi-source batch validation") @@ -359,20 +359,17 @@ func (r *PipelineInstanceReconciler) buildBatchFilterContainersForMultiSource(pi } // bindingObjectKey resolves the exact S3 object a binding's claimer must -// download: Bucket.Object when set (explicit override), else Bucket.Prefix. -// The platform's media model has no separate object concept — single-file -// medias put the 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. +// 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 "" } - if b.Source.Spec.Bucket.Object != "" { - return b.Source.Spec.Bucket.Object - } return b.Source.Spec.Bucket.Prefix } diff --git a/internal/controller/pipelineinstance_controller_batch_multisource_test.go b/internal/controller/pipelineinstance_controller_batch_multisource_test.go index ff371e8..37cee37 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource_test.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -72,12 +72,12 @@ func makeMultiSourcePI(t *testing.T) (*pipelinesv1alpha1.PipelineInstance, *pipe bindings := []ResolvedSourceBinding{ {FilterName: "front-cam", Source: &pipelinesv1alpha1.PipelineSource{ Spec: pipelinesv1alpha1.PipelineSourceSpec{ - Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Object: "front.mp4"}, + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "front.mp4"}, }, }}, {FilterName: "back-cam", Source: &pipelinesv1alpha1.PipelineSource{ Spec: pipelinesv1alpha1.PipelineSourceSpec{ - Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Object: "back.mp4"}, + Bucket: &pipelinesv1alpha1.BucketSource{Name: "media", Prefix: "back.mp4"}, }, }}, } @@ -165,12 +165,12 @@ func TestReconcileBatchMultiSource_RejectsNonBucketSource(t *testing.T) { } // TestReconcileBatchMultiSource_RejectsMissingObject pins branch 2: a -// binding with a Bucket source but no Object key surfaces a -// `MultiSourceBatchMissingObject` Degraded condition. +// 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 on back-cam. - bindings[1].Source.Spec.Bucket.Object = "" + // 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 { diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index fcf82f1..e808652 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -206,7 +206,7 @@ func TestBuildMultiSourceBatchJob_PerBindingInitClaimersAndEnv(t *testing.T) { Spec: pipelinesv1alpha1.PipelineSourceSpec{ Bucket: &pipelinesv1alpha1.BucketSource{ Name: "media", - Object: "front.mp4", + Prefix: "front.mp4", CredentialsSecret: &pipelinesv1alpha1.SecretReference{ Name: "s3-creds", }, @@ -217,7 +217,7 @@ func TestBuildMultiSourceBatchJob_PerBindingInitClaimersAndEnv(t *testing.T) { Spec: pipelinesv1alpha1.PipelineSourceSpec{ Bucket: &pipelinesv1alpha1.BucketSource{ Name: "media", - Object: "back.mp4", + Prefix: "back.mp4", CredentialsSecret: &pipelinesv1alpha1.SecretReference{ Name: "s3-creds", }, @@ -528,33 +528,29 @@ func assertEnvPrecedesFilterConfig(t *testing.T, c corev1.Container, name string } // TestBindingObjectKey pins the object-key resolution the direct-mode -// claimers depend on: explicit Bucket.Object wins; otherwise Bucket.Prefix -// is the full key (the platform's media model carries single-file object -// keys in prefix — there is no separate object concept upstream); neither → -// empty (reconcile Degrades). +// 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(object, prefix string) ResolvedSourceBinding { + mk := func(prefix string) ResolvedSourceBinding { return ResolvedSourceBinding{ FilterName: "front-cam", Source: &pipelinesv1alpha1.PipelineSource{ Spec: pipelinesv1alpha1.PipelineSourceSpec{ Bucket: &pipelinesv1alpha1.BucketSource{ Name: "b", - Object: object, Prefix: prefix, }, }, }, } } - if got := bindingObjectKey(mk("clips/exact.mp4", "clips/")); got != "clips/exact.mp4" { - t.Errorf("explicit Object must win, got %q", got) + 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("", "clips/front_001.mp4")); got != "clips/front_001.mp4" { - t.Errorf("Prefix must serve as the full key when Object empty, got %q", got) - } - if got := bindingObjectKey(mk("", "")); got != "" { - t.Errorf("neither set must resolve empty, 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) From 582bd0bc448040a8e2d82711e2b522e88a05c5ba Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 14:26:23 -0300 Subject: [PATCH 10/19] docs(PLAT-1141): sample documents both fan-in topic schemes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stream multisource sample presented publisher-side ;topic tags as the only way to avoid the duplicate-topic fan-in crash. The plainsight-api export actually emits the other equivalent form — VideoIns stay on default `main`, the fan-in filter remaps per upstream (;main>_main) — now empirically validated end-to-end on docker-desktop (both remapped topics at full frame rate). The comment presents both forms and says which one the platform produces, so hand-authors and reviewers don't conclude exported pipelines are miswired. --- ...1_pipelineinstance_stream_multisource.yaml | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml index 52a17c7..bb5dc2c 100644 --- a/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_stream_multisource.yaml @@ -20,16 +20,20 @@ # the second VideoIn with `Address already in use`). See # https://docs.plainsight.ai/docs/openfilter/your-first-filter/. # -# 2. **Tag every VideoIn output topic** 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 (webvis, -# recorder, …) crashes with `RuntimeError: duplicate topic 'main'`. -# Convention: use the filter `name` (== this binding's filterName) as -# the topic so wiring is deterministic. Note filter names (and therefore -# `sources[].filterName` below) must be DNS-1123 labels — lowercase -# alphanumerics and hyphens only, no underscores — because they flow -# into container names; using them as topics too keeps one name per -# camera end-to-end. +# 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: # From c4f47ab0ae32fc7fca8f6a81012f7e55efa86f9f Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 14:30:32 -0300 Subject: [PATCH 11/19] docs(PLAT-1141): batch multisource sample cross-references both topic schemes --- ...pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml index 0ab9fa6..910aaf8 100644 --- a/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml +++ b/config/samples/pipelines_v1alpha1_pipelineinstance_batch_multisource.yaml @@ -23,7 +23,9 @@ # # The same two openfilter authoring constraints as the streaming # multi-source sample apply: stagger output ports by 2 (5550, 5552, …) and -# tag each VideoIn's output topic. Example pipeline `pipeline-batch-multi`: +# 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 From 7961a28aa14e12e206830c0ae6b9ba8d65297cc9 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 14:51:56 -0300 Subject: [PATCH 12/19] =?UTF-8?q?fix(PLAT-1141):=20review=20operability=20?= =?UTF-8?q?=E2=80=94=20self-healing=20Degraded,=20unmatched=20bindings,=20?= =?UTF-8?q?name=20caps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings 2.2-2.9: - 2.2 Degraded validation states self-heal: SetupWithManager gains a PipelineSource→PipelineInstance watch (maps source events to every referencing instance via EffectiveSources, covering sources[] and the legacy singular ref), and clearDegradedReason (now variadic) flips Degraded back to False once multi-source batch validation passes — previously a fixed PipelineSource never re-triggered reconcile and instances reported Degraded=True + Succeeded=True forever. - 2.3 A sources[].filterName matching no pipeline.spec.filters[].name now Degrades with SourceBindingUnmatched (message lists unmatched + available names) in ALL modes — was a silent no-op leaving the mistargeted VideoIn inputless while the instance reported Running. Cleared automatically once bindings validate. - 2.5 claimer- container names cap at 63 chars (deterministic 55-char filterName budget, trailing-hyphen trim) — a CRD-valid 56+ char filterName previously produced an apiserver- rejected Job and an error-requeue loop. - 2.6 Stale comment promising uninjected env fixed. - 2.7/2.8 DESIGN_DOCUMENT: §5.4 no longer claims batch multi-source is rejected (contradicting §5.5); documents the unmatched-binding validation, the idle-timeout sourceBindings[0] anchoring caveat, and the prefix-as-key contract (stale Bucket.Object reference removed). - 2.9 Multisource unit tests use CRD-valid hyphenated filter names. New tests: Degraded-clears-on-recovery reconcile cycle, claimer name truncation, watch mapping fn, unmatched-binding Degradation in both modes + clear-on-fix. --- DESIGN_DOCUMENT.md | 32 ++- api/v1alpha1/pipeline_types.go | 1 - .../controller/pipelineinstance_controller.go | 160 ++++++++++- ...neinstance_controller_batch_multisource.go | 41 ++- ...tance_controller_batch_multisource_test.go | 83 ++++++ .../pipelineinstance_multisource_test.go | 268 +++++++++++++++++- 6 files changed, 548 insertions(+), 37 deletions(-) diff --git a/DESIGN_DOCUMENT.md b/DESIGN_DOCUMENT.md index 893a369..ea01a7a 100644 --- a/DESIGN_DOCUMENT.md +++ b/DESIGN_DOCUMENT.md @@ -400,8 +400,8 @@ The claimer is a standalone Go binary (`cmd/claimer/main.go`) that runs as an in 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: rejected, see 5.5). Per-container -source plumbing: +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 @@ -415,6 +415,20 @@ source plumbing: 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 @@ -458,15 +472,17 @@ Constraints on multi-source batch: - Every binding's PipelineSource must be a Bucket source — RTSP makes no sense for batch. -- Every binding's `Bucket.Object` must be set to a specific S3 object - key. 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. +- 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. +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 diff --git a/api/v1alpha1/pipeline_types.go b/api/v1alpha1/pipeline_types.go index 3258b84..b8ea150 100644 --- a/api/v1alpha1/pipeline_types.go +++ b/api/v1alpha1/pipeline_types.go @@ -47,7 +47,6 @@ type BucketSource struct { // +optional Prefix string `json:"prefix,omitempty"` - // endpoint is the S3-compatible endpoint URL (required for non-AWS S3) // Examples: // - MinIO: "http://minio.example.com:9000" diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index afef55d..923f3fb 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 @@ -500,6 +516,43 @@ 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) + if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { + log.Error(statusErr, "Failed to update status after unmatched source binding validation") + } + return ctrl.Result{}, nil + } + + // Bindings validate — clear any stale Degraded/SourceBindingUnmatched + // left by a previous failing pass so external watchers see the recovery. + // Same conflict semantics as the PipelineNotFound clear above. + if err := r.clearDegradedReason(ctx, pipelineInstance, ReasonSourceBindingUnmatched); 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 == "" { @@ -576,6 +629,28 @@ func (r *PipelineInstanceReconciler) resolveSourceBindings(ctx context.Context, return bindings, 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. func (r *PipelineInstanceReconciler) getCredentials(ctx context.Context, pipelineInstance *pipelinesv1alpha1.PipelineInstance, pipelineSource *pipelinesv1alpha1.PipelineSource) (string, string, error) { if pipelineSource.Spec.Bucket == nil { @@ -863,15 +938,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) @@ -893,6 +978,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) } @@ -945,3 +1039,53 @@ func (r *PipelineInstanceReconciler) pipelineInstancesForPipeline(ctx context.Co } return requests } + +// pipelineInstancesForPipelineSource returns reconcile requests for every +// PipelineInstance in the PipelineSource's namespace that references it — +// 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: same-namespace only, mirroring pipelineInstancesForPipeline. A +// cross-namespace sourceRef still recovers via a CR-spec edit or controller +// restart, just without the watch-driven instant wake; if cross-ns refs ever +// become common, switch to a field-indexed cross-namespace list keyed by +// sourceRef.{namespace,name}. +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, client.InNamespace(source.Namespace)); 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_multisource.go b/internal/controller/pipelineinstance_controller_batch_multisource.go index 90c0f20..616cc6f 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource.go @@ -70,7 +70,7 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte 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) - r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchInvalidSource", msg) + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, ReasonMultiSourceBatchInvalidSource, msg) if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { log.Error(statusErr, "Failed to update status after multi-source batch validation") } @@ -78,7 +78,7 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte } 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) - r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, "MultiSourceBatchMissingObject", msg) + r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, ReasonMultiSourceBatchMissingObject, msg) if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { log.Error(statusErr, "Failed to update status after multi-source batch validation") } @@ -86,6 +86,18 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte } } + // 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 { @@ -176,8 +188,8 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte // buildMultiSourceBatchJob constructs the Job for the multi-source batch // path. One init claimer per binding (direct mode), then the user's -// filter containers with per-binding VIDEO_INPUT_PATH and FILTER_VIDEO_IN -// env vars injected into the matching VideoIn container only. +// 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() @@ -194,7 +206,7 @@ func (r *PipelineInstanceReconciler) buildMultiSourceBatchJob(ctx context.Contex initContainers := make([]corev1.Container, 0, len(sourceBindings)) for i, b := range sourceBindings { initContainers = append(initContainers, corev1.Container{ - Name: fmt.Sprintf("claimer-%s", b.FilterName), + Name: claimerContainerName(b.FilterName), Image: r.ClaimerImage, Env: r.buildDirectClaimerEnv(b, downloadPath[b.FilterName]), VolumeMounts: []corev1.VolumeMount{ @@ -373,6 +385,25 @@ func bindingObjectKey(b ResolvedSourceBinding) string { 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 diff --git a/internal/controller/pipelineinstance_controller_batch_multisource_test.go b/internal/controller/pipelineinstance_controller_batch_multisource_test.go index 37cee37..635768f 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource_test.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -12,6 +12,7 @@ package controller import ( "context" + "strings" "testing" batchv1 "k8s.io/api/batch/v1" @@ -187,6 +188,88 @@ func TestReconcileBatchMultiSource_RejectsMissingObject(t *testing.T) { } } +// 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) + } +} + // TestReconcileBatchMultiSource_CreatesJobAndStampsStartTime pins branch // 3: with valid bindings, the reconciler creates the Job (named // "-job"), stamps StartTime, sets Status.JobName, and leaves a diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index e808652..2736584 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -5,9 +5,13 @@ import ( "strings" "testing" + appsv1 "k8s.io/api/apps/v1" + 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" + "sigs.k8s.io/controller-runtime/pkg/reconcile" pipelinesv1alpha1 "github.com/PlainsightAI/openfilter-pipelines-controller/api/v1alpha1" ) @@ -59,13 +63,13 @@ func hasEnv(envs []corev1.EnvVar, name string) bool { // 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 +// (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 +// - 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 +// - 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 @@ -74,15 +78,15 @@ 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"}}, + {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: "front-cam", Image: "videoin:latest"}, + {Name: "back-cam", Image: "videoin:latest"}, {Name: "webvis", Image: "webvis:latest"}, }, }, @@ -99,25 +103,25 @@ func TestBuildStreamingDeployment_MultiSource_PerContainerRTSP(t *testing.T) { }, } bindings := []ResolvedSourceBinding{ - {FilterName: "front_cam", Source: frontSource}, - {FilterName: "back_cam", Source: backSource}, + {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") + 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) + 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) + 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) @@ -127,10 +131,10 @@ func TestBuildStreamingDeployment_MultiSource_PerContainerRTSP(t *testing.T) { // 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") + 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") + t.Errorf("back-cam ended up with front source URL — bindings swapped") } } @@ -527,6 +531,240 @@ func assertEnvPrecedesFilterConfig(t *testing.T, c corev1.Container, name string } } +// 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"}, + //nolint:staticcheck // SA1019: legacy SourceRef path is under test. + 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"}, + //nolint:staticcheck // SA1019: legacy SourceRef path is under test. + 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. + piCrossNS := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-cross-ns", Namespace: "default"}, + Spec: pipelinesv1alpha1.PipelineInstanceSpec{ + PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, + Sources: []pipelinesv1alpha1.NamedSourceRef{ + {FilterName: "front-cam", SourceRef: pipelinesv1alpha1.SourceReference{Name: "shared-src", Namespace: &otherNS}}, + }, + }, + } + + r := &PipelineInstanceReconciler{ + Client: fake.NewClientBuilder().WithScheme(sch). + WithObjects(src, piMulti, piLegacy, piUnrelated, piCrossNS).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) != 2 || !got["pi-multi"] || !got["pi-legacy"] { + t.Errorf("expected exactly [pi-multi pi-legacy], 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 !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) + } +} + // 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 From 1dd8d7329d08a01ebeae25cdca6023fa0dce65de Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 15:55:23 -0300 Subject: [PATCH 13/19] test(PLAT-1141): pin CRD admission contract; un-deprecate sourceRef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - envtest specs for the PipelineInstance admission rules the agent and portal PRs build on: the sourceRef/sources CEL xor (both, neither, empty list), listMapKey uniqueness on sources[].filterName, and the DNS-1123 filterName pattern (16 specs against a real API server). - table test for perFilterInputPath (claimer↔filter path contract: extension preservation, .mp4 default, edge cases). - drop the Deprecated marker on spec.sourceRef: it is the first-class convenience form for single-source pipelines (broadcast semantics, no filter names needed), supported indefinitely alongside sources[]. Removes the now-stale SA1019 lint suppressions; CRDs regenerated and Helm copies synced. --- api/v1alpha1/pipelineinstance_types.go | 23 ++- ...ilter.plainsight.ai_pipelineinstances.yaml | 11 +- ...ilter.plainsight.ai_pipelineinstances.yaml | 11 +- .../pipelineinstance_admission_test.go | 157 ++++++++++++++++++ .../controller/pipelineinstance_controller.go | 2 +- ...tance_controller_batch_multisource_test.go | 29 ++++ .../pipelineinstance_controller_streaming.go | 4 +- .../pipelineinstance_multisource_test.go | 16 +- 8 files changed, 214 insertions(+), 39 deletions(-) create mode 100644 internal/controller/pipelineinstance_admission_test.go diff --git a/api/v1alpha1/pipelineinstance_types.go b/api/v1alpha1/pipelineinstance_types.go index f12e674..2b8cf31 100644 --- a/api/v1alpha1/pipelineinstance_types.go +++ b/api/v1alpha1/pipelineinstance_types.go @@ -49,10 +49,10 @@ type ExecutionConfig struct { // Exactly one of the following must be set (enforced by the XValidation // rule on this type): // -// - sourceRef (legacy, single-source): the bound 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 -// and demos continue to work unchanged. +// - 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 @@ -70,12 +70,11 @@ type PipelineInstanceSpec struct { PipelineRef PipelineReference `json:"pipelineRef"` // sourceRef references the PipelineSource resource for input - // configuration. Use this for single-source pipelines; the source URL - // is broadcast to every filter container. For multi-source pipelines, - // use `sources` instead. - // - // Deprecated: prefer `sources` for new pipelines. Retained for legacy - // single-source CRs and existing demos. + // 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"` @@ -164,8 +163,8 @@ type NamedSourceRef struct { } // EffectiveSources returns the canonical list of (filterName, source) bindings -// for this PipelineInstance, normalizing the legacy `SourceRef` field into the -// new shape. When `SourceRef` is set, the returned list has a single entry +// 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 diff --git a/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml index c91cd79..66e055d 100644 --- a/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml +++ b/config/crd/bases/filter.plainsight.ai_pipelineinstances.yaml @@ -1005,12 +1005,11 @@ spec: sourceRef: description: |- sourceRef references the PipelineSource resource for input - configuration. Use this for single-source pipelines; the source URL - is broadcast to every filter container. For multi-source pipelines, - use `sources` instead. - - Deprecated: prefer `sources` for new pipelines. Retained for legacy - single-source CRs and existing demos. + 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 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 c91cd79..66e055d 100644 --- a/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml +++ b/deployment/openfilter-pipelines-controller/crds/filter.plainsight.ai_pipelineinstances.yaml @@ -1005,12 +1005,11 @@ spec: sourceRef: description: |- sourceRef references the PipelineSource resource for input - configuration. Use this for single-source pipelines; the source URL - is broadcast to every filter container. For multi-source pipelines, - use `sources` instead. - - Deprecated: prefer `sources` for new pipelines. Retained for legacy - single-source CRs and existing demos. + 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 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 923f3fb..2957fce 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -585,7 +585,7 @@ func (r *PipelineInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Req // 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 deprecated singular +// 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 { diff --git a/internal/controller/pipelineinstance_controller_batch_multisource_test.go b/internal/controller/pipelineinstance_controller_batch_multisource_test.go index 635768f..89f0a77 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource_test.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -270,6 +270,35 @@ func TestClaimerContainerName(t *testing.T) { } } +// 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 diff --git a/internal/controller/pipelineinstance_controller_streaming.go b/internal/controller/pipelineinstance_controller_streaming.go index f1257dd..3844bbb 100644 --- a/internal/controller/pipelineinstance_controller_streaming.go +++ b/internal/controller/pipelineinstance_controller_streaming.go @@ -310,7 +310,7 @@ func (r *PipelineInstanceReconciler) ensureStreamingDeployment(ctx context.Conte // 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 deprecated singular `Spec.SourceRef` resolve to one binding with +// 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 { @@ -368,7 +368,7 @@ func (r *PipelineInstanceReconciler) buildStreamingDeployment(ctx context.Contex // 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 legacy SourceRef coexists with a Sources entry that targets + // when a singular SourceRef coexists with a Sources entry that targets // this container, which validation rejects — the last write wins. var envVars []corev1.EnvVar for _, src := range perContainerSources { diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 2736584..279574b 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -138,18 +138,14 @@ func TestBuildStreamingDeployment_MultiSource_PerContainerRTSP(t *testing.T) { } } -// TestBuildStreamingDeployment_LegacyBroadcast confirms the deprecated -// `Spec.SourceRef` path still broadcasts RTSP_URL to every container, +// 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() - // Deprecated field intentionally exercised: this test guards the - // legacy-CR compatibility path that the SA1019 deprecation comment - // asks new callers to avoid. - //nolint:staticcheck // SA1019: legacy SourceRef path is the system under test. pi.Spec.SourceRef = &pipelinesv1alpha1.SourceReference{Name: "legacy-source"} pipeline := &pipelinesv1alpha1.Pipeline{ @@ -318,7 +314,6 @@ func envValue(envs []corev1.EnvVar, name string) string { func TestEffectiveSources(t *testing.T) { t.Run("legacy_source_ref_becomes_broadcast_sentinel", func(t *testing.T) { spec := pipelinesv1alpha1.PipelineInstanceSpec{ - //nolint:staticcheck // SA1019: deprecated SourceRef is the system under test. SourceRef: &pipelinesv1alpha1.SourceReference{Name: "src1"}, } got := spec.EffectiveSources() @@ -486,7 +481,6 @@ func TestResolveSourceBindings_LegacySourceRefMissingSurfacesError(t *testing.T) pi := &pipelinesv1alpha1.PipelineInstance{ ObjectMeta: metav1.ObjectMeta{Name: "test-pi", Namespace: "default"}, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ - //nolint:staticcheck // SA1019: legacy SourceRef is the system under test. SourceRef: &pipelinesv1alpha1.SourceReference{Name: "src-missing"}, }, } @@ -560,16 +554,14 @@ func TestPipelineInstancesForPipelineSource_MapsReferencingInstances(t *testing. ObjectMeta: metav1.ObjectMeta{Name: "pi-legacy", Namespace: "default"}, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, - //nolint:staticcheck // SA1019: legacy SourceRef path is under test. - SourceRef: &pipelinesv1alpha1.SourceReference{Name: "shared-src"}, + 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"}, - //nolint:staticcheck // SA1019: legacy SourceRef path is under test. - SourceRef: &pipelinesv1alpha1.SourceReference{Name: "different-src"}, + SourceRef: &pipelinesv1alpha1.SourceReference{Name: "different-src"}, }, } // Same name but the ref explicitly points at another namespace — the From f54935f2f9cb8d5279d49885f762d67ae8b9f516 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 17:12:10 -0300 Subject: [PATCH 14/19] fix(PLAT-1141): clear stale Degraded/PipelineSourceNotFound once sources resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found live in the cross-PR smoke test: the deployment agent applies the PipelineInstance before its PipelineSources (intentional — TOCTOU fix), so the first reconcile sets Degraded/PipelineSourceNotFound. The PipelineSource watch re-reconciled and built the deployment, but the stale condition was never cleared (clear-on-pass only covered SourceBindingUnmatched) — and the agent's provisioning monitor tears the instance down on any lingering Degraded. Add the reason to the existing variadic clear and pin the two-phase race in a regression test. --- .../controller/pipelineinstance_controller.go | 13 +++-- .../pipelineinstance_multisource_test.go | 54 +++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index 2957fce..cd0cfd3 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -540,10 +540,17 @@ func (r *PipelineInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } - // Bindings validate — clear any stale Degraded/SourceBindingUnmatched - // left by a previous failing pass so external watchers see the recovery. + // 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); err != nil { + 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) diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 279574b..a56e381 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -757,6 +757,60 @@ func TestReconcile_UnmatchedSourceBinding_ClearsOnFix(t *testing.T) { } } +// 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 From a5697a5f6c7ae2566268144aa4f30f88581b1057 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Thu, 11 Jun 2026 23:32:38 -0300 Subject: [PATCH 15/19] feat(PLAT-1141): direct-download mode for single-binding singleObject buckets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New BucketSource.singleObject field: declares the prefix is a FULL object key (set by the deployment agent for single-file media kinds). A single named binding whose bucket declares it now routes through the direct-download path — one init claimer with S3_OBJECT_KEY, no Valkey, and the extension-preserving /ws/ input path — instead of queue mode, whose fixed input.mp4 destination destroys the file extension and silently breaks extension-sensitive entry filters (image-in finds zero images and exits; the Job hangs forever on the waiting downstream). Found empirically testing batch image pipelines. Backward compatible: bindings without the flag (hand-authored CRs, fileset prefixes) keep the queue flow; the legacy broadcast sentinel (singular sourceRef) is never routed direct. CRDs regenerated, Helm copies synced. --- api/v1alpha1/pipeline_types.go | 13 ++ .../filter.plainsight.ai_pipelinesources.yaml | 13 ++ .../filter.plainsight.ai_pipelinesources.yaml | 13 ++ .../pipelineinstance_controller_batch.go | 10 +- ...neinstance_controller_batch_multisource.go | 18 ++ ...tance_controller_batch_multisource_test.go | 177 ++++++++++++++++++ 6 files changed, 243 insertions(+), 1 deletion(-) 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/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/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_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index 097daf3..eb2f872 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -56,8 +56,16 @@ import ( // 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 { + if len(sourceBindings) > 1 || isDirectModeSingleBinding(sourceBindings) { return r.reconcileBatchMultiSource(ctx, pipelineInstance, pipeline, sourceBindings) } pipelineSource := sourceBindings[0].Source diff --git a/internal/controller/pipelineinstance_controller_batch_multisource.go b/internal/controller/pipelineinstance_controller_batch_multisource.go index 616cc6f..8318b80 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource.go @@ -415,3 +415,21 @@ func perFilterInputPath(filterName, objectKey string) string { } 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 +} diff --git a/internal/controller/pipelineinstance_controller_batch_multisource_test.go b/internal/controller/pipelineinstance_controller_batch_multisource_test.go index 89f0a77..7e5ea02 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource_test.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -454,3 +454,180 @@ func TestReconcileBatchMultiSource_JobProgressingStaysProgressing(t *testing.T) 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") + } + } + } + } +} From d24524f0f68cc5e4438ecaecf4003730bfa208a1 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Fri, 12 Jun 2026 00:13:39 -0300 Subject: [PATCH 16/19] fix(PLAT-1141): skip directory-placeholder objects in bucket listing Zero-byte keys ending in "/" (GCS/S3 console "folders") were enqueued as work items in queue mode; the placeholder then failed its download/decode attempts, landed in the DLQ, and failed the whole run. Observed empirically: an external_videos fileset over a console-created GCS folder processed both real videos (2/3 completions) and then failed the run on the "videos/" placeholder. Skip them at listing time; zero-byte regular files and non-empty trailing-slash keys are kept. --- .../controller/pipelineinstance_controller.go | 12 ++++++++++ .../pipelineinstance_multisource_test.go | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index cd0cfd3..1bb8f47 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -742,12 +742,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 diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index a56e381..7717954 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -840,3 +840,26 @@ func TestBindingObjectKey(t *testing.T) { 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) + } + } +} From c7310c9882899672c50e40a6e98ac84e1499e900 Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Fri, 12 Jun 2026 01:33:29 -0300 Subject: [PATCH 17/19] =?UTF-8?q?fix(PLAT-1141):=20review=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20condition=20hygiene,=20status-error=20propagation,?= =?UTF-8?q?=20cross-ns=20watch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #74: - Validation failures (unmatched filterName, multi-source batch invalid source / missing object key) now set Progressing=False alongside Degraded so the condition set reads consistently for automation; Succeeded is left untouched (it records a past terminal outcome). - Status-update errors on those validation paths are propagated (conflict → requeue, else return the error) instead of being logged and swallowed — these paths intentionally don't requeue, so a swallowed write failure left the server-side status stale forever. Batch validation degrade centralized in degradeBatchValidation. - pipelineInstancesForPipelineSource lists cluster-wide: a cross-namespace sourceRef (ref.namespace set) now wakes its instance on source events exactly like a same-namespace one; the per-ref namespace resolution already handled correctness. - Steady-state batch reconcile (30s loop) skips the no-op status write when the Progressing condition is unchanged. - Docs: DESIGN_DOCUMENT notes the port/topic ownership pivot (authoring-time, not controller-owned); reconcileBatch documents that the queue path's filterName is broadcast-equivalent. --- DESIGN_DOCUMENT.md | 10 ++++ .../controller/pipelineinstance_controller.go | 25 ++++++--- .../pipelineinstance_controller_batch.go | 5 +- ...neinstance_controller_batch_multisource.go | 56 ++++++++++++++----- ...tance_controller_batch_multisource_test.go | 6 ++ .../pipelineinstance_multisource_test.go | 35 ++++++++++-- 6 files changed, 109 insertions(+), 28 deletions(-) diff --git a/DESIGN_DOCUMENT.md b/DESIGN_DOCUMENT.md index ea01a7a..c1e2e66 100644 --- a/DESIGN_DOCUMENT.md +++ b/DESIGN_DOCUMENT.md @@ -446,6 +446,16 @@ claimer per binding — see 5.5). Per-container source plumbing: `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 diff --git a/internal/controller/pipelineinstance_controller.go b/internal/controller/pipelineinstance_controller.go index 1bb8f47..d04a6c7 100644 --- a/internal/controller/pipelineinstance_controller.go +++ b/internal/controller/pipelineinstance_controller.go @@ -534,8 +534,20 @@ func (r *PipelineInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Req 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 } @@ -1060,25 +1072,24 @@ func (r *PipelineInstanceReconciler) pipelineInstancesForPipeline(ctx context.Co } // pipelineInstancesForPipelineSource returns reconcile requests for every -// PipelineInstance in the PipelineSource's namespace that references it — +// 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: same-namespace only, mirroring pipelineInstancesForPipeline. A -// cross-namespace sourceRef still recovers via a CR-spec edit or controller -// restart, just without the watch-driven instant wake; if cross-ns refs ever -// become common, switch to a field-indexed cross-namespace list keyed by -// sourceRef.{namespace,name}. +// 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, client.InNamespace(source.Namespace)); err != nil { + 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 diff --git a/internal/controller/pipelineinstance_controller_batch.go b/internal/controller/pipelineinstance_controller_batch.go index eb2f872..1dd67f5 100644 --- a/internal/controller/pipelineinstance_controller_batch.go +++ b/internal/controller/pipelineinstance_controller_batch.go @@ -46,7 +46,10 @@ import ( // `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. +// 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 diff --git a/internal/controller/pipelineinstance_controller_batch_multisource.go b/internal/controller/pipelineinstance_controller_batch_multisource.go index 8318b80..c65ebff 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource.go @@ -25,6 +25,7 @@ import ( 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" @@ -70,19 +71,11 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte 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) - r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, ReasonMultiSourceBatchInvalidSource, msg) - if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { - log.Error(statusErr, "Failed to update status after multi-source batch validation") - } - return ctrl.Result{}, nil + 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) - r.setCondition(pipelineInstance, ConditionTypeDegraded, metav1.ConditionTrue, ReasonMultiSourceBatchMissingObject, msg) - if statusErr := r.Status().Update(ctx, pipelineInstance); statusErr != nil { - log.Error(statusErr, "Failed to update status after multi-source batch validation") - } - return ctrl.Result{}, nil + return r.degradeBatchValidation(ctx, pipelineInstance, ReasonMultiSourceBatchMissingObject, msg) } } @@ -177,11 +170,23 @@ func (r *PipelineInstanceReconciler) reconcileBatchMultiSource(ctx context.Conte } } - // Still progressing. - r.setCondition(pipelineInstance, ConditionTypeProgressing, metav1.ConditionTrue, "Processing", "Multi-source batch Job is running") - if err := r.Status().Update(ctx, pipelineInstance); err != nil { - log.Error(err, "Failed to update status") - return ctrl.Result{}, err + // 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 } @@ -433,3 +438,24 @@ func isDirectModeSingleBinding(bindings []ResolvedSourceBinding) bool { 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 index 7e5ea02..5a7fd2d 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource_test.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -156,6 +156,9 @@ func TestReconcileBatchMultiSource_RejectsNonBucketSource(t *testing.T) { 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{} @@ -186,6 +189,9 @@ func TestReconcileBatchMultiSource_RejectsMissingObject(t *testing.T) { 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 diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 7717954..8b7b951 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -566,8 +566,8 @@ func TestPipelineInstancesForPipelineSource_MapsReferencingInstances(t *testing. } // Same name but the ref explicitly points at another namespace — the // event for default/shared-src must NOT wake this instance. - piCrossNS := &pipelinesv1alpha1.PipelineInstance{ - ObjectMeta: metav1.ObjectMeta{Name: "pi-cross-ns", Namespace: "default"}, + piCrossNSAway := &pipelinesv1alpha1.PipelineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "pi-cross-ns-away", Namespace: "default"}, Spec: pipelinesv1alpha1.PipelineInstanceSpec{ PipelineRef: pipelinesv1alpha1.PipelineReference{Name: "p"}, Sources: []pipelinesv1alpha1.NamedSourceRef{ @@ -575,10 +575,32 @@ func TestPipelineInstancesForPipelineSource_MapsReferencingInstances(t *testing. }, }, } + // 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, piCrossNS).Build(), + WithObjects(src, piMulti, piLegacy, piUnrelated, piCrossNSAway, piCrossNSToward, piOtherNSLocal).Build(), Scheme: sch, } @@ -587,8 +609,8 @@ func TestPipelineInstancesForPipelineSource_MapsReferencingInstances(t *testing. for _, req := range requests { got[req.Name] = true } - if len(requests) != 2 || !got["pi-multi"] || !got["pi-legacy"] { - t.Errorf("expected exactly [pi-multi pi-legacy], got %v", requests) + 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) } } @@ -668,6 +690,9 @@ func TestReconcile_UnmatchedSourceBinding_DegradesStreaming(t *testing.T) { 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) } From 6c53951daa1fd009b92236a44bbc417517bf05be Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Fri, 12 Jun 2026 07:40:43 -0300 Subject: [PATCH 18/19] fix(PLAT-1141): deterministic idle-timeout anchor; purge stale Bucket.Object references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review (Copilot round 2): - The V1 idle-timeout policy anchored to sourceBindings[0], but spec.sources is listType=map — element order is not semantically stable across patches. Anchor to the lexicographically-smallest filterName instead (idleAnchorBinding, table-tested), and log which binding anchored the timeout. - DESIGN_DOCUMENT.md and a test comment still referenced the removed Bucket.Object field — corrected to the prefix-is-full-object-key contract. --- DESIGN_DOCUMENT.md | 4 +-- ...tance_controller_batch_multisource_test.go | 2 +- .../pipelineinstance_controller_streaming.go | 33 +++++++++++++++---- .../pipelineinstance_multisource_test.go | 26 +++++++++++++++ 4 files changed, 56 insertions(+), 9 deletions(-) diff --git a/DESIGN_DOCUMENT.md b/DESIGN_DOCUMENT.md index c1e2e66..76d31f6 100644 --- a/DESIGN_DOCUMENT.md +++ b/DESIGN_DOCUMENT.md @@ -166,8 +166,8 @@ spec: # 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 needs Bucket.Object set so the - # direct-mode claimer has a deterministic key). + # (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: diff --git a/internal/controller/pipelineinstance_controller_batch_multisource_test.go b/internal/controller/pipelineinstance_controller_batch_multisource_test.go index 5a7fd2d..4ec5e94 100644 --- a/internal/controller/pipelineinstance_controller_batch_multisource_test.go +++ b/internal/controller/pipelineinstance_controller_batch_multisource_test.go @@ -29,7 +29,7 @@ import ( // 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.Object is empty) +// 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 diff --git a/internal/controller/pipelineinstance_controller_streaming.go b/internal/controller/pipelineinstance_controller_streaming.go index 3844bbb..b3e0183 100644 --- a/internal/controller/pipelineinstance_controller_streaming.go +++ b/internal/controller/pipelineinstance_controller_streaming.go @@ -137,14 +137,19 @@ func (r *PipelineInstanceReconciler) reconcileStreaming(ctx context.Context, pip } // Step 3: Check for idle timeout. Multi-source pipelines anchor the - // idle-timeout policy to the first 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. - idleSource := sourceBindings[0].Source + // 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) + 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) @@ -760,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_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 8b7b951..99db2ef 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -888,3 +888,29 @@ func TestIsDirectoryPlaceholder(t *testing.T) { } } } + +// 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") + } +} From 842b671829c5e209b19e1c7fb6af335204fcfade Mon Sep 17 00:00:00 2001 From: Lucas Mundim Date: Fri, 12 Jun 2026 10:48:59 -0300 Subject: [PATCH 19/19] test(PLAT-1141): GPU coexistence on multi-source + streaming env-ordering assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review (shingonoide): two coverage gaps closed — - TestBuildStreamingDeployment_MultiSource_GPUCoexistence pins the 3-cams + 1-GPU-filter topology on a Spec.Sources instance: exactly one container holds the nvidia.com/gpu limit, the GPU filter carries NVIDIA_VISIBLE_DEVICES (and no RTSP_URL), the camera containers carry their per-binding RTSP_URL and no GPU env. - TestBuildStreamingDeployment_RTSPURLPrecedesFilterConfig asserts the streaming-side RTSP_URL-before-FILTER_* ordering via the same assertEnvPrecedesFilterConfig helper that already guards the batch VIDEO_INPUT_PATH ordering. --- .../pipelineinstance_multisource_test.go | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/internal/controller/pipelineinstance_multisource_test.go b/internal/controller/pipelineinstance_multisource_test.go index 99db2ef..923bdcc 100644 --- a/internal/controller/pipelineinstance_multisource_test.go +++ b/internal/controller/pipelineinstance_multisource_test.go @@ -8,6 +8,7 @@ import ( 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" @@ -914,3 +915,112 @@ func TestIdleAnchorBinding(t *testing.T) { 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") +}