From 27aef2b88781d5dc89abafec492d5be6563f2868 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 | 13 ++++- internal/collector/collector.go | 58 +++++++++++-------- internal/collector/collector_test.go | 28 +++++++++ 9 files changed, 115 insertions(+), 28 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 40dd5f5590..3112e7b942 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 41e558a548..5e392c1a0f 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 7ca26e6cf2..736efb4dcd 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 3a6d0222ba..3d9bfa04e4 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 a00289955e..c928e18af5 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 @@ -2665,7 +2672,7 @@ Type:: object The list represents only the initial set used by the collector's Kafka client for the first connection only. The collector's Kafka client fetches constantly an updated list from Kafka. These updates are not reconciled back to the collector configuration. -If provided, it must be a valid URL with a 'tcp' or 'tls' scheme. +If provided, it must be a valid URL with a 'tcp' or 'tls' scheme and include a port number. If none is provided, the target URL from the OutputSpec is used as fallback. |topic|string| Topic specifies the target topic to send logs to. The value when not specified is 'topic' @@ -2687,7 +2694,7 @@ Example: |tuning|object| Tuning specs tuning for the output |url|string| URL to send log records to. This field is optional. -If provided, it must be a valid URL with a 'tcp' or 'tls' scheme, for example: 'tls://kafka.secure.com:9093/app-topic'. +If provided, it must be a valid URL with a 'tcp' or 'tls' scheme and include a port number, for example: 'tls://kafka.secure.com:9093/app-topic'. The 'username@password' part of `url` is ignored. |====================== @@ -3895,7 +3902,7 @@ Example: |tuning|object| Tuning specs tuning for the output -|url|string| An absolute URL, with a scheme. Valid schemes are: `tcp`, `tls`, `udp` +|url|string| An absolute URL, with a scheme and a port number. Valid schemes are: `tcp`, `tls`, `udp` For example, to send syslog records using UDP: url: udp://syslog.example.com:514 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}}}}))