Custom PVC source specified in ExtraVolumes in the fluentd definition is not honoured

[bbyreddy]

Describe the bug:
Custom PVC source details specified in the fluentd.extraVolumes.volume.pvc.source.claimName of the fluentd definition are not honoured by the logging-operator. It always tries to create a new PVC and expects a non-empty fluentd.extraVolumes.volume.pvc.spec.
Operator Version: 4.4.3

I suppose the issue is with the following code snippet in the pkg.resources.fluentd.statefulset.go (line# 51 to 63). The else block will never be executed as the n.Volume.PersistentVolumeClaim will not ne null in both the cases (when user wants to provide pvc.spec or pvc.source).

	for _, n := range r.Logging.Spec.FluentdSpec.ExtraVolumes {
		if n.Volume != nil && n.Volume.PersistentVolumeClaim != nil {
			if err := n.Volume.ApplyPVCForStatefulSet(n.ContainerName, n.Path, spec, func(name string) metav1.ObjectMeta {
				return r.FluentdObjectMeta(name, ComponentFluentd)
			}); err != nil {
				return nil, reconciler.StatePresent, err
			}
		} else {
			if err := n.ApplyVolumeForPodSpec(&spec.Template.Spec); err != nil {
				return nil, reconciler.StatePresent, err
			}
		}
	}

Expected behaviour:
Expectation according to the documentation is to either provide a volume.pvc.spec or volume.pvc.source. If the volume.pvc.source is provided, then the same should be mounted instead of creating a new PVC.

Steps to reproduce the bug:

1. Deploy the logging-operator and create the logging custom resource as defined below

apiVersion: logging.banzaicloud.io/v1beta1
kind: Logging
metadata:
  name: logging
spec:
  controlNamespace: c8yedge
  fluentbit:
    bufferStorageVolume:
      hostPath:
        path: ""
  fluentd:
    disablePvc: true
    bufferStorageVolume:
      emptyDir:
        medium: Memory
    extraArgs:
    - --log-rotate-age
    - "10"
    extraVolumes:
    - containerName: fluentd
      path: /var/log/
      volume:
        pvc:
          source:
            claimName: edge-logs
            readOnly: false
      volumeName: edge-logs
  watchNamespaces:
  - c8yedge

2. logging-fluentd statefulset fails to deploy with the following error

   Warning FailedCreate 43s (x13 over 64s) statefulset-controller create Claim logging-edge-logs-logging-fluentd-0 for Pod logging-fluentd-0 in StatefulSet logging-fluentd failed error: PersistentVolumeClaim "logging-edge-logs-logging-fluentd-0" is invalid: [spec.accessModes: Required value: at least 1 access mode is required, spec.resources[storage]: Required value]

Additional context:
None

Environment details:

• Kubernetes version: K3s version v1.25.11+k3s1
• Cloud-provider/provisioner: NA
• logging-operator version: 4.4.3
• Install method: Helm + manual creation of the logging CRD
• Logs from the misbehaving component: No logs, as the fluentd pod didn't start
• Resource definition (possibly in YAML format) that caused the issue, without sensitive data: Please refer to the steps to reproduce.

kind: bug

[pepov]

@bbyreddy the purpose of extraVolumes is to always create a new PVC. If you just want to configure the original PVC used for the buffers, then please use

spec:
  fluentd:
    bufferStorageVolume:
      pvc:
        ...

Does that help?

[pepov]

I think I understand the original problem and I will try to phrase it, you can correct me if I'm wrong:

You want to use an existing persistent volume in the statefulset as a destination for some of your logs, that is managed outside of the logging operator.

I think we have never really tried to support this use case, so we have to think about how we would do that normally.

Please feel free to join the project discord so that we can discuss further: https://discord.gg/JKGg2wDBp8

[bbyreddy]

@bbyreddy the purpose of extraVolumes is to always create a new PVC. If you just want to configure the original PVC used for the buffers, then please use

spec:
  fluentd:
    bufferStorageVolume:
      pvc:
        ...

Does that help?

@pepov
Hi Peter,
Thank you for looking into this issue.
I tried this as well and it doesn't work as the PVC for fluentd is needed and it's automatically created if not provided in the ExtraVolumes.

[bbyreddy]

@pepov
I made some changes to the code, which seem to work. Please check the PR: #1617

[bbyreddy]

Thank you very much, @pepov , @OverOrion , @aslafy-z for your timely support. This issue is resolved and enabled us adopt the latest version of the logging operator.

[bbyreddy]

Hi @pepov ,
How can we get this fix rolled into 4.5.x fix instead of waiting for a new release of the operator? I was expecting this change to be part of 4.5.2 release, but it wasn't picked. Do you expect me to graft these changes into the 4.5.0 release branch?
Thanks!

[pepov]

I've prepared it here: #1653

[bbyreddy]

Thank you, @pepov

[pepov]

4.5.3 is available with the fix