Fluentd ConfigCheck pod misses PodSecurityContext if configCheck is set in Logging resource

[mdraijer]

Describe the bug:
When you add

spec:
  configCheck:
    timeoutSeconds: 10

in the Logging custom resource, then the code in newCheckPod is somehow not executed which results in a fluentd-checkconfig pod that misses certain elements.
Many of them may be missing but one of them is the PodSecurityContext, which, in combination with the configuration workers>1 in the FluentdConfig causes the configcheck to crash with the error described in #1023

Expected behaviour:
If the mentioned configCheck configuration in Logging is added, then the runtime pod configuration of the fluentd-configcheck pod should still contain the configured pod spec. For example PodSecurityContext.

Steps to reproduce the bug:
Add the mentioned configCheck configuration in Logging resource; also set some pod spec in the FluentdConfig resource, e.g. .spec.security.podSecurityContext.runAsUser = 1001.

Additional context:
The bug was discovered after analysis regarding the problem described in #1023

Environment details:

• Kubernetes version (e.g. v1.15.2): 1.33.9
• Cloud-provider/provisioner (e.g. AKS, GKE, EKS, PKE etc): on premise Openshift
• logging-operator version (e.g. 2.1.1): 6.2.2
• Install method (e.g. helm or static manifests): static manifests
• Logs from the misbehaving component (and any other relevant logs): not necessary, just run kubectl get pod ...-fluentd-configcheck-... -oyaml
• Resource definition (possibly in YAML format) that caused the issue, without sensitive data:

# Logging:
spec:
  ...
  configCheck:
    timeoutSeconds: 10
  ...
# FluentdConfig:
spec:
  ...
  security:
    podSecurityContext:
      runAsUser: 1001
  workers: 8

/kind bug

[vyncint]

Took a look at this. Short version: I could not reproduce the podSecurityContext symptom on master, but the general complaint is real and I found two concrete instances of it. One is fixed in #2285.

podSecurityContext specifically looks fine on master. newCheckPod does set it:

Spec: corev1.PodSpec{
	...
	SecurityContext: fluentdSpec.Security.PodSecurityContext,

I built a Reconciler with security.podSecurityContext.runAsUser set and confirmed it lands on the pod, and I saw the same on a kind cluster with the released 6.7.0 chart. This report is against 6.2.2, so either it was fixed since or something else was going on. Worth confirming against a recent version before spending more time on that part.

dnsPolicy is a real instance of the same gap. The configcheck pod copies dnsConfig from the Fluentd spec but not dnsPolicy, so it always ran with ClusterFirst while the aggregator ran with the configured policy. On a kind cluster with spec.fluentd.dnsPolicy: None:

	aggregator StatefulSet	configcheck pod
released 6.7.0	None	ClusterFirst

That matters because None resolves only through dnsConfig while ClusterFirst merges it on top of cluster DNS, so the configcheck pod resolves names differently from the aggregator it is validating. #2285 fixes it.

spec.configCheck from the Logging resource is silently discarded when the aggregator sets its own. This one is closer to your original scenario and I have not opened a PR for it, since it is a judgement call rather than an obvious slip.

func (l *Logging) AggregatorLevelConfigCheck(check *ConfigCheck) {
	if check != nil {
		l.Spec.ConfigCheck = *check
		l.configCheckDefaults()
	}
}

That replaces the whole struct. If a FluentdConfig sets any configCheck field, every field set on the Logging resource is dropped. Concretely, with configCheck: {strategy: StartWithTimeout, timeoutSeconds: 42, labels: {from: logging}} on the Logging and only {strategy: DryRun} on the FluentdConfig, you end up with timeoutSeconds: 10 (re-defaulted) and no labels. Since ConfigCheck.Labels feeds configCheckPodObjectMeta, that is directly a configcheck pod missing configured settings.

The LoggingSpec.ConfigCheck doc comment says "Can be overridden on the fluentd / syslog-ng level", which reads to me as per-field override rather than whole-struct replacement, and the Labels field being dropped seems unlikely to be intended. But the aggregator-side comment says "just copied over the field in the logging resource if defined", which does describe the current behaviour, so this is a maintainer call. Happy to send a patch making it a field-wise merge if you want it.

One thing worth checking for your original report: the configcheck is keyed only by a hash of the generated fluentd config (configHash() hashes *r.config and nothing else), and the result is cached in status.configCheckResults. Editing the Logging resource in a way that does not change the generated config — adding configCheck.timeoutSeconds, or changing podSecurityContext — leaves the hash identical, so the existing result is reused and no new configcheck pod is created. If you looked at a pod that predated the change, that would explain "the code in newCheckPod is somehow not executed". I did not chase this one down properly, so treat it as a hypothesis.