feat: add terminationGracePeriodSeconds field to CollectorSpec

[vparfonov]

Description

Problem

The collector pods have terminationGracePeriodSeconds hardcoded to 10 seconds. When deliveryMode: AtLeastOnce is configured, Vector uses disk buffers to persist log data. During shutdown, Vector needs time to flush in-memory events to disk and close connections to remote sinks. If the grace period is too short, Kubernetes sends SIGKILL while a write is in progress, corrupting the disk buffer and causing CrashLoopBackOff with InvalidProtobufPayload errors on restart.

Solution

Expose terminationGracePeriodSeconds as a configurable field in spec.collector, allowing users to set the appropriate grace period for their environment.

• New optional field: spec.collector.terminationGracePeriodSeconds
• Default remains 10 seconds (backward compatible — no change for existing users)

Example

apiVersion: observability.openshift.io/v1
 kind: ClusterLogForwarder
 metadata:
   name: my-forwarder
   namespace: openshift-logging
 spec:
   collector:
     terminationGracePeriodSeconds: 60
   ...

/cc @Clee2691
/assign @jcantrill

Links

• Depending on PR(s):
• GitHub issue:
• JIRA: https://redhat.atlassian.net/browse/LOG-9636
• Enhancement proposal:

Summary by CodeRabbit

• New Features

  • Added an optional termination grace period setting for collector pods, with validation for valid values.
  • When not set, the collector continues using the default 10-second grace period.
  • The new setting is now shown in the resource schema and UI metadata.

• Bug Fixes

  • Collector pod configuration now respects the user-provided termination grace period instead of always using a fixed value.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f0fcc340-8658-42c6-b625-c1d10bbb3545

📥 Commits

Reviewing files that changed from the base of the PR and between 296d8c4 and 809a5d5.

📒 Files selected for processing (8)

• api/observability/v1/clusterlogforwarder_types.go
• api/observability/v1/zz_generated.deepcopy.go
• bundle/manifests/cluster-logging.clusterserviceversion.yaml
• bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml
• config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml
• config/manifests/bases/cluster-logging.clusterserviceversion.yaml
• internal/collector/collector.go
• internal/collector/collector_test.go

---

📝 Walkthrough

Walkthrough

Adds an optional TerminationGracePeriodSeconds field to CollectorSpec, with deepcopy support and CRD/CSV manifest updates. Collector pod-spec construction now uses this field when set, falling back to a new default constant. Corresponding Ginkgo tests verify default and configured behavior.

Changes

Termination grace period feature

Layer / File(s)	Summary
API field, deepcopy, and pod-spec wiring api/observability/v1/clusterlogforwarder_types.go, api/observability/v1/zz_generated.deepcopy.go, internal/collector/collector.go	Adds nullable, min-1 validated TerminationGracePeriodSeconds *int64 to CollectorSpec, extends deepcopy to copy the pointer, and updates Factory.NewPodSpec to use the spec value or a new defaultTerminationGracePeriodSeconds constant instead of a hardcoded 10.
Pod spec tests for grace period internal/collector/collector_test.go	Adds test contexts for DaemonSet and Deployment NewPodSpec flows asserting default of 10 and correct use of a configured value (e.g., 60).
CRD and CSV manifest updates bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml, config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml, bundle/manifests/cluster-logging.clusterserviceversion.yaml, config/manifests/bases/cluster-logging.clusterserviceversion.yaml	Adds terminationGracePeriodSeconds OpenAPI schema property (int64, nullable, minimum 1) and corresponding CSV specDescriptors entries; updates CSV createdAt timestamp.

Estimated code review effort: 2 (Simple) | ~12 minutes

Related PRs: None identified.

Suggested labels: api-review, enhancement

Suggested reviewers: None identified.

Poem:
A grace period, once fixed at ten,
Now bends to the spec's own pen.
Deepcopy and pod spec agree,
CRDs describe it clearly and free,
Tests confirm defaults hold again.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: adding terminationGracePeriodSeconds to CollectorSpec.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)

---

_{Comment @coderabbitai help to get the list of available commands.}

[jcantrill]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcantrill, vparfonov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [jcantrill]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vparfonov]

/retest-required

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 296d8c4 and 2 for PR HEAD 809a5d5 in total

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 6c2ec7a and 1 for PR HEAD 809a5d5 in total

[openshift-ci]

@vparfonov: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-using-bundle	809a5d5	link	false	/test e2e-using-bundle

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.