From 9c9700f2f72d754650590fdcdc6abbf9647fcbae Mon Sep 17 00:00:00 2001 From: Vitalii Parfonov Date: Tue, 7 Jul 2026 22:30:10 +0300 Subject: [PATCH] feat: make terminationGracePeriodSeconds configurable via field of CollectorSpec Signed-off-by: Vitalii Parfonov --- .../v1/clusterlogforwarder_types.go | 9 +++ api/observability/v1/zz_generated.deepcopy.go | 5 ++ ...cluster-logging.clusterserviceversion.yaml | 7 +++ ...ity.openshift.io_clusterlogforwarders.yaml | 8 +++ ...ity.openshift.io_clusterlogforwarders.yaml | 8 +++ ...cluster-logging.clusterserviceversion.yaml | 7 +++ .../operator/api_observability_v1.adoc | 18 ++++++ internal/collector/collector.go | 58 +++++++++++-------- internal/collector/collector_test.go | 28 +++++++++ 9 files changed, 123 insertions(+), 25 deletions(-) diff --git a/api/observability/v1/clusterlogforwarder_types.go b/api/observability/v1/clusterlogforwarder_types.go index de9007761c..c1bba40474 100644 --- a/api/observability/v1/clusterlogforwarder_types.go +++ b/api/observability/v1/clusterlogforwarder_types.go @@ -143,6 +143,15 @@ type CollectorSpec struct { // +kubebuilder:validation:Pattern="^(?:[0-9]{1,2}|100)%?$" // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Max Unavailable",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:text"} MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"` + + // TerminationGracePeriodSeconds defines the termination grace period for collector pods + // in seconds. If not specified, the default is 10 seconds. + // + // +nullable + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Minimum:=1 + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Termination Grace Period Seconds",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:number"} + TerminationGracePeriodSeconds *int64 `json:"terminationGracePeriodSeconds,omitempty"` } type NetworkPolicy struct { diff --git a/api/observability/v1/zz_generated.deepcopy.go b/api/observability/v1/zz_generated.deepcopy.go index dd3ee2235d..90be0320ef 100644 --- a/api/observability/v1/zz_generated.deepcopy.go +++ b/api/observability/v1/zz_generated.deepcopy.go @@ -495,6 +495,11 @@ func (in *CollectorSpec) DeepCopyInto(out *CollectorSpec) { *out = new(intstr.IntOrString) **out = **in } + if in.TerminationGracePeriodSeconds != nil { + in, out := &in.TerminationGracePeriodSeconds, &out.TerminationGracePeriodSeconds + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CollectorSpec. diff --git a/bundle/manifests/cluster-logging.clusterserviceversion.yaml b/bundle/manifests/cluster-logging.clusterserviceversion.yaml index b3d1bd35c0..bbc7bbfdbe 100644 --- a/bundle/manifests/cluster-logging.clusterserviceversion.yaml +++ b/bundle/manifests/cluster-logging.clusterserviceversion.yaml @@ -172,6 +172,13 @@ spec: path: collector.resources x-descriptors: - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: |- + TerminationGracePeriodSeconds defines the termination grace period for collector pods + in seconds. If not specified, the default is 10 seconds. + displayName: Termination Grace Period Seconds + path: collector.terminationGracePeriodSeconds + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:number - description: Define the tolerations the collector pods will accept displayName: Tolerations path: collector.tolerations diff --git a/bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml b/bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml index a5dfbdbe0f..d18433e9e4 100644 --- a/bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml +++ b/bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml @@ -1033,6 +1033,14 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + terminationGracePeriodSeconds: + description: |- + TerminationGracePeriodSeconds defines the termination grace period for collector pods + in seconds. If not specified, the default is 10 seconds. + format: int64 + minimum: 1 + nullable: true + type: integer tolerations: description: Define the tolerations the collector pods will accept items: diff --git a/config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml b/config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml index 148ad69729..1e627b9940 100644 --- a/config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml +++ b/config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml @@ -1033,6 +1033,14 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + terminationGracePeriodSeconds: + description: |- + TerminationGracePeriodSeconds defines the termination grace period for collector pods + in seconds. If not specified, the default is 10 seconds. + format: int64 + minimum: 1 + nullable: true + type: integer tolerations: description: Define the tolerations the collector pods will accept items: diff --git a/config/manifests/bases/cluster-logging.clusterserviceversion.yaml b/config/manifests/bases/cluster-logging.clusterserviceversion.yaml index b46185c951..8f32e32492 100644 --- a/config/manifests/bases/cluster-logging.clusterserviceversion.yaml +++ b/config/manifests/bases/cluster-logging.clusterserviceversion.yaml @@ -95,6 +95,13 @@ spec: path: collector.resources x-descriptors: - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: |- + TerminationGracePeriodSeconds defines the termination grace period for collector pods + in seconds. If not specified, the default is 10 seconds. + displayName: Termination Grace Period Seconds + path: collector.terminationGracePeriodSeconds + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:number - description: Define the tolerations the collector pods will accept displayName: Tolerations path: collector.tolerations diff --git a/docs/reference/operator/api_observability_v1.adoc b/docs/reference/operator/api_observability_v1.adoc index ce9b850882..e6bee6d4f5 100644 --- a/docs/reference/operator/api_observability_v1.adoc +++ b/docs/reference/operator/api_observability_v1.adoc @@ -82,6 +82,9 @@ Value can be a number (e.g., 50) or a percentage string (e.g., "50%"). |resources|object| The resource requirements for the collector +|terminationGracePeriodSeconds|int| TerminationGracePeriodSeconds defines the termination grace period for collector pods +in seconds. If not specified, the default is 10 seconds. + |tolerations|array| Define the tolerations the collector pods will accept |====================== @@ -958,6 +961,10 @@ Type:: object Type:: object +=== .spec.collector.terminationGracePeriodSeconds + +Type:: int + === .spec.collector.tolerations[] Type:: array @@ -1279,6 +1286,8 @@ NOTE1: `In` CANNOT contain `.log_type`, `.log_source` or `.message` as those fie NOTE2: If this filter is used in a pipeline with GoogleCloudLogging, `.hostname` CANNOT be added to this list as it is a required field. +NOTE3: If used in a pipeline with a Lokistack output type, see Lokistack output documentation for additional fields that cannot be pruned. + |notIn|array| `NotIn` is an array of dot-delimited field paths. All fields besides the ones listed here are removed from the log record. Each field path expression must start with a "." @@ -1303,6 +1312,8 @@ NOTE1: `NotIn` MUST contain `.log_type`, `.log_source` and `.message` as those f NOTE2: If this filter is used in a pipeline with GoogleCloudLogging, `.hostname` MUST be added to this list as it is a required field. +NOTE3: If used in a pipeline with a Lokistack output type, see Lokistack output documentation for additional fields that cannot be pruned. + |====================== === .spec.filters[].prune.in[] @@ -1860,6 +1871,13 @@ Type:: array |lokiStack|object| LokiStack configures forwarding log events to a Red Hat managed Loki deployment using the Red Hat tenancy model +The listed labelKeys cannot be pruned as they are required as default stream labels for LokiStack. +1. .kubernetes.container_name +2. .kubernetes.namespace_name +3. .kubernetes.pod_name + +If these fields are not present in the log record, they will be set to the empty string. + |name|string| Name used to refer to the output from a `pipeline`. |otlp|object| OTLP configures forwarding log events to a receiver using the OpenTelemetry Protocol diff --git a/internal/collector/collector.go b/internal/collector/collector.go index 569b6b57d4..d6c821c9c6 100644 --- a/internal/collector/collector.go +++ b/internal/collector/collector.go @@ -26,30 +26,31 @@ const ( //DefaultMaxUnavailable is the maxUnavailable collector setting when not defined by spec.collector.maxUnavailable DefaultMaxUnavailable = "100%" - defaultAudience = "openshift" - clusterLoggingPriorityClassName = "system-node-critical" - metricsVolumeName = "metrics" - metricsVolumePath = "/etc/collector/metrics" - saTokenVolumeName = "sa-token" - saTokenExpirationSecs = 3600 //1 hour - sourcePodsName = "varlogpods" - sourcePodsPath = "/var/log/pods" - sourceJournalName = "varlogjournal" - sourceJournalPath = "/var/log/journal" - sourceAuditdName = "varlogaudit" - sourceAuditdPath = "/var/log/audit" - sourceAuditOVNName = "varlogovn" - sourceOVNPath = "/var/log/ovn" - sourceOAuthServerName = "varlogoauthserver" - sourceOAuthServerPath = "/var/log/oauth-server" - sourceOAuthAPIServerName = "varlogoauthapiserver" - sourceOAuthAPIServerPath = "/var/log/oauth-apiserver" - sourceOpenshiftAPIServerName = "varlogopenshiftapiserver" - sourceOpenshiftAPIServerPath = "/var/log/openshift-apiserver" - sourceKubeAPIServerName = "varlogkubeapiserver" - sourceKubeAPIServerPath = "/var/log/kube-apiserver" - tmpVolumeName = "tmp" - tmpPath = "/tmp" + defaultAudience = "openshift" + clusterLoggingPriorityClassName = "system-node-critical" + metricsVolumeName = "metrics" + metricsVolumePath = "/etc/collector/metrics" + saTokenVolumeName = "sa-token" + saTokenExpirationSecs = 3600 //1 hour + defaultTerminationGracePeriodSeconds int64 = 10 + sourcePodsName = "varlogpods" + sourcePodsPath = "/var/log/pods" + sourceJournalName = "varlogjournal" + sourceJournalPath = "/var/log/journal" + sourceAuditdName = "varlogaudit" + sourceAuditdPath = "/var/log/audit" + sourceAuditOVNName = "varlogovn" + sourceOVNPath = "/var/log/ovn" + sourceOAuthServerName = "varlogoauthserver" + sourceOAuthServerPath = "/var/log/oauth-server" + sourceOAuthAPIServerName = "varlogoauthapiserver" + sourceOAuthAPIServerPath = "/var/log/oauth-apiserver" + sourceOpenshiftAPIServerName = "varlogopenshiftapiserver" + sourceOpenshiftAPIServerPath = "/var/log/openshift-apiserver" + sourceKubeAPIServerName = "varlogkubeapiserver" + sourceKubeAPIServerPath = "/var/log/kube-apiserver" + tmpVolumeName = "tmp" + tmpPath = "/tmp" ) type Visitor func(collector *v1.Container, podSpec *v1.PodSpec, resNames *factory.ForwarderResourceNames, namespace, logLevel string) @@ -142,11 +143,18 @@ func (f *Factory) NewDeployment(namespace, name string, trustedCABundle *v1.Conf func (f *Factory) NewPodSpec(trustedCABundle *v1.ConfigMap, spec obs.ClusterLogForwarderSpec, clusterID string, tlsProfileSpec configv1.TLSProfileSpec, namespace string) *v1.PodSpec { + var gracePeriod *int64 + if f.CollectorSpec.TerminationGracePeriodSeconds != nil { + gracePeriod = f.CollectorSpec.TerminationGracePeriodSeconds + } else { + gracePeriod = utils.GetPtr(defaultTerminationGracePeriodSeconds) + } + podSpec := &v1.PodSpec{ NodeSelector: utils.EnsureLinuxNodeSelector(f.NodeSelector()), PriorityClassName: clusterLoggingPriorityClassName, ServiceAccountName: f.ResourceNames.ServiceAccount, - TerminationGracePeriodSeconds: utils.GetPtr[int64](10), + TerminationGracePeriodSeconds: gracePeriod, Tolerations: append(constants.DefaultTolerations(), f.Tolerations()...), Affinity: f.Affinity(), diff --git a/internal/collector/collector_test.go b/internal/collector/collector_test.go index 239976cf99..79dfe7ef5e 100644 --- a/internal/collector/collector_test.go +++ b/internal/collector/collector_test.go @@ -169,6 +169,20 @@ var _ = Describe("Factory#Daemonset", func() { Describe("when creating the podSpec", func() { + Context("and evaluating terminationGracePeriodSeconds", func() { + It("should default to 10 when not specified", func() { + Expect(*podSpec.TerminationGracePeriodSeconds).To(Equal(int64(10))) + }) + + It("should use spec value when specified", func() { + factory.CollectorSpec = obs.CollectorSpec{ + TerminationGracePeriodSeconds: utils.GetPtr[int64](60), + } + podSpec = *factory.NewPodSpec(nil, obs.ClusterLogForwarderSpec{}, "1234", tls.GetClusterTLSProfileSpec(nil), constants.OpenshiftNS) + Expect(*podSpec.TerminationGracePeriodSeconds).To(Equal(int64(60))) + }) + }) + Context("and evaluating tolerations", func() { It("should add only defaults when none are defined", func() { Expect(podSpec.Tolerations).To(Equal(constants.DefaultTolerations())) @@ -528,6 +542,20 @@ var _ = Describe("Factory#Deployment", func() { Describe("when creating the podSpec", func() { + Context("and evaluating terminationGracePeriodSeconds", func() { + It("should default to 10 when not specified", func() { + Expect(*podSpec.TerminationGracePeriodSeconds).To(Equal(int64(10))) + }) + + It("should use spec value when specified", func() { + factory.CollectorSpec = obs.CollectorSpec{ + TerminationGracePeriodSeconds: utils.GetPtr[int64](60), + } + podSpec = *factory.NewPodSpec(nil, obs.ClusterLogForwarderSpec{}, "1234", tls.GetClusterTLSProfileSpec(nil), constants.OpenshiftNS) + Expect(*podSpec.TerminationGracePeriodSeconds).To(Equal(int64(60))) + }) + }) + Context("and mounting volumes", func() { It("should not mount host path or sa token volumes", func() { Expect(podSpec.Volumes).NotTo(ContainElement(v1.Volume{Name: saTokenVolumeName, VolumeSource: v1.VolumeSource{Projected: &v1.ProjectedVolumeSource{Sources: []v1.VolumeProjection{saToken}}}}))