Share the configcheck abnormal-failure retry and cover it

pkg/resources/configcheck/failure.go

@@ -0,0 +1,66 @@
+// Copyright © 2026 Kube logging authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package configcheck
+
+import (
+	"context"
+	"fmt"
+
+	"emperror.dev/errors"
+	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// RetryAbnormalFailure deletes a failed configcheck pod that did not fail on the config itself and
+// returns the message to report while it is retried, or an empty message when the failure is a
+// genuine verdict on the config.
+func RetryAbnormalFailure(ctx context.Context, c client.Client, log logr.Logger, pod *corev1.Pod) (string, error) {
+	// A pod-level Reason (DeadlineExceeded, Evicted, Shutdown, NodeAffinity, ...) means the pod was
+	// killed around the check; an invalid config only makes the container exit non-zero, which leaves
+	// the pod-level Reason empty.
+	if pod.Status.Reason == "" {
+		return "", nil
+	}
+
+	log.Info("configcheck pod did not complete normally, deleting it to retry",
+		"pod", pod.Name,
+		"reason", pod.Status.Reason,
+		"activeDeadlineSeconds", pod.Spec.ActiveDeadlineSeconds,
+		"runningContainer", stillRunningContainer(pod))
+
+	if err := client.IgnoreNotFound(c.Delete(ctx, pod)); err != nil {
+		return "", errors.WrapIf(err, "failed to delete configcheck pod that did not complete normally")
+	}
+
+	return fmt.Sprintf("configcheck pod %s did not complete normally (reason: %s), deleted for retry",
+		pod.Name, pod.Status.Reason), nil
+}
+
+// stillRunningContainer names the container that was still running when the pod stopped, so a
+// helper that held up the check can be identified.
+func stillRunningContainer(pod *corev1.Pod) string {
+	for _, cs := range pod.Status.InitContainerStatuses {
+		if cs.State.Running != nil {
+			return cs.Name
+		}
+	}
+	for _, cs := range pod.Status.ContainerStatuses {
+		if cs.State.Running != nil {
+			return cs.Name
+		}
+	}
+	return ""
+}

pkg/resources/configcheck/failure_test.go

@@ -0,0 +1,175 @@
+// Copyright © 2026 Kube logging authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package configcheck
+
+import (
+	"context"
+	"testing"
+
+	"emperror.dev/errors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+)
+
+func failedPod(reason string) *corev1.Pod {
+	return &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{Name: "fluentd-configcheck-deadbeef", Namespace: "logging"},
+		Status: corev1.PodStatus{
+			Phase:  corev1.PodFailed,
+			Reason: reason,
+		},
+	}
+}
+
+func newClient(t *testing.T, pod *corev1.Pod, opts ...interceptor.Funcs) client.Client {
+	t.Helper()
+
+	scheme := runtime.NewScheme()
+	require.NoError(t, corev1.AddToScheme(scheme))
+
+	builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(pod)
+	if len(opts) > 0 {
+		builder = builder.WithInterceptorFuncs(opts[0])
+	}
+	return builder.Build()
+}
+
+// TestRetryAbnormalFailure pins which failed pods are a verdict on the config. Only a container
+// exiting non-zero is, and that leaves the pod-level Reason empty; a pod-level Reason means the pod
+// was killed around the check, so it has to be cleared out and tried again.
+func TestRetryAbnormalFailure(t *testing.T) {
+	tests := []struct {
+		name         string
+		reason       string
+		expectRetry  bool
+		expectDelete bool
+	}{
+		{name: "InvalidConfigIsAVerdict", reason: ""},
+		{name: "DeadlineExceeded", reason: "DeadlineExceeded", expectRetry: true, expectDelete: true},
+		{name: "Evicted", reason: "Evicted", expectRetry: true, expectDelete: true},
+		{name: "NodeAffinity", reason: "NodeAffinity", expectRetry: true, expectDelete: true},
+		{name: "UnexpectedAdmissionError", reason: "UnexpectedAdmissionError", expectRetry: true, expectDelete: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			pod := failedPod(tt.reason)
+			c := newClient(t, pod)
+			ctx := context.Background()
+
+			message, err := RetryAbnormalFailure(ctx, c, log.Log, pod)
+			require.NoError(t, err)
+
+			if !tt.expectRetry {
+				assert.Empty(t, message, "a config verdict must be left to the caller to record")
+			} else {
+				assert.Contains(t, message, tt.reason)
+				assert.Contains(t, message, pod.Name)
+			}
+
+			err = c.Get(ctx, client.ObjectKeyFromObject(pod), &corev1.Pod{})
+			assert.Equal(t, tt.expectDelete, apierrors.IsNotFound(err),
+				"deleting the pod is what lets the next reconcile create a fresh one")
+		})
+	}
+}
+
+// TestRetryAbnormalFailureAlreadyDeleted covers the pod being reaped between the read and the
+// delete: there is nothing left to clean up, so the retry still stands.
+func TestRetryAbnormalFailureAlreadyDeleted(t *testing.T) {
+	pod := failedPod("DeadlineExceeded")
+	c := newClient(t, pod)
+	ctx := context.Background()
+	require.NoError(t, c.Delete(ctx, pod))
+
+	message, err := RetryAbnormalFailure(ctx, c, log.Log, pod)
+	require.NoError(t, err)
+	assert.Contains(t, message, "DeadlineExceeded")
+}
+
+// TestRetryAbnormalFailureDeleteFails pins that a failed delete is reported instead of being
+// reported as a retry that never happened.
+func TestRetryAbnormalFailureDeleteFails(t *testing.T) {
+	pod := failedPod("DeadlineExceeded")
+	c := newClient(t, pod, interceptor.Funcs{
+		Delete: func(context.Context, client.WithWatch, client.Object, ...client.DeleteOption) error {
+			return errors.New("boom")
+		},
+	})
+
+	message, err := RetryAbnormalFailure(context.Background(), c, log.Log, pod)
+	require.Error(t, err)
+	assert.Empty(t, message)
+}
+
+func TestStillRunningContainer(t *testing.T) {
+	running := corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}
+	terminated := corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{}}
+
+	tests := []struct {
+		name     string
+		pod      *corev1.Pod
+		expected string
+	}{
+		{
+			name:     "NoStatuses",
+			pod:      &corev1.Pod{},
+			expected: "",
+		},
+		{
+			name: "EverythingTerminated",
+			pod: &corev1.Pod{Status: corev1.PodStatus{
+				InitContainerStatuses: []corev1.ContainerStatus{{Name: "tmp-dir-hack", State: terminated}},
+				ContainerStatuses:     []corev1.ContainerStatus{{Name: "fluentd", State: terminated}},
+			}},
+			expected: "",
+		},
+		{
+			// The case the log line exists for: a native sidecar is an init container, so a
+			// helper that never exits shows up there while fluentd has already finished.
+			name: "NativeSidecarStillRunning",
+			pod: &corev1.Pod{Status: corev1.PodStatus{
+				InitContainerStatuses: []corev1.ContainerStatus{
+					{Name: "tmp-dir-hack", State: terminated},
+					{Name: "geoip-refresh", State: running},
+				},
+				ContainerStatuses: []corev1.ContainerStatus{{Name: "fluentd", State: terminated}},
+			}},
+			expected: "geoip-refresh",
+		},
+		{
+			name: "DryRunItselfStillRunning",
+			pod: &corev1.Pod{Status: corev1.PodStatus{
+				InitContainerStatuses: []corev1.ContainerStatus{{Name: "tmp-dir-hack", State: terminated}},
+				ContainerStatuses:     []corev1.ContainerStatus{{Name: "fluentd", State: running}},
+			}},
+			expected: "fluentd",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, stillRunningContainer(tt.pod))
+		})
+	}
+}

pkg/resources/fluentd/appconfigmap.go

@@ -191,23 +191,14 @@ func (r *Reconciler) configCheck(ctx context.Context) (*ConfigCheckResult, error
 		case corev1.PodRunning:
 			return &ConfigCheckResult{}, nil
 		case corev1.PodFailed:
-			if pod.Status.Reason != "" {
-				// A pod that fails with a Reason (DeadlineExceeded, Evicted, Shutdown,
-				// NodeAffinity, ...) didn't fail because of an invalid config - a real
-				// config validation failure leaves Status.Reason empty. Delete it so a
-				// fresh one is created and retried instead of latching a false
-				// "config is invalid" verdict.
-				r.Log.Info("configcheck pod did not complete normally, deleting it to retry",
-					"pod", pod.Name,
-					"reason", pod.Status.Reason,
-					"activeDeadlineSeconds", pod.Spec.ActiveDeadlineSeconds,
-					"runningContainer", stillRunningContainer(pod))
-				if err := client.IgnoreNotFound(r.Client.Delete(ctx, pod)); err != nil {
-					return nil, errors.WrapIf(err, "failed to delete configcheck pod that did not complete normally")
-				}
+			message, err := configcheck.RetryAbnormalFailure(ctx, r.Client, r.Log, pod)
+			if err != nil {
+				return nil, err
+			}
+			if message != "" {
 				return &ConfigCheckResult{
 					Ready:   false,
-					Message: fmt.Sprintf("configcheck pod %s did not complete normally (reason: %s), deleted for retry", pod.Name, pod.Status.Reason),
+					Message: message,
 				}, nil
 			}
 			return &ConfigCheckResult{
@@ -233,24 +224,6 @@ func (r *Reconciler) configCheck(ctx context.Context) (*ConfigCheckResult, error
 	return &ConfigCheckResult{}, nil
 }
 
-// stillRunningContainer returns the name of the container that was still
-// running when its pod stopped, if any - useful for diagnosing why a
-// configcheck pod with a helper container (e.g. a native sidecar) didn't
-// complete in time.
-func stillRunningContainer(pod *corev1.Pod) string {
-	for _, cs := range pod.Status.InitContainerStatuses {
-		if cs.State.Running != nil {
-			return cs.Name
-		}
-	}
-	for _, cs := range pod.Status.ContainerStatuses {
-		if cs.State.Running != nil {
-			return cs.Name
-		}
-	}
-	return ""
-}
-
 func (r *Reconciler) newCheckSecret(hashKey string, fluentdSpec v1beta1.FluentdSpec) (*corev1.Secret, error) {
 	data, err := r.generateConfigSecret(fluentdSpec)
 	if err != nil {
@@ -357,18 +330,18 @@ func (r *Reconciler) newCheckPod(hashKey string, fluentdSpec v1beta1.FluentdSpec
 		pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, volumeMount)
 	}
 	for _, n := range fluentdSpec.ExtraVolumes {
-		// The check pod is a plain, one-shot Pod, not part of the StatefulSet, so a
-		// PVC-backed extraVolume meant to be provisioned via volumeClaimTemplates
-		// (statefulset.go) never has a matching PVC here; mount an emptyDir instead
-		// - the dry-run only needs the mount path to resolve, not real data.
-		if n.Volume != nil && n.Volume.PersistentVolumeClaim != nil && !isPersistentVolumeClaimSpecEmpty(n.Volume.PersistentVolumeClaim.PersistentVolumeClaimSpec) {
-			emptyDirVolume := volume.KubernetesVolume{EmptyDir: &corev1.EmptyDirVolumeSource{}}
-			if err := emptyDirVolume.ApplyVolumeForPodSpec(n.VolumeName, n.ContainerName, n.Path, &pod.Spec); err != nil {
-				r.Log.Error(err, "Fluentd Config check pod extraVolume attachment failed.")
-			}
-			continue
+		checkVolume := n.Volume
+		// The check pod is a plain, one-shot Pod, so a PVC-backed extraVolume - which the
+		// StatefulSet only ever gets through volumeClaimTemplates - has no claim to bind to
+		// here. The dry-run needs the mount path to resolve, not the data behind it.
+		if isPVCBacked(n.Volume) {
+			r.Log.Info("extraVolume is PVC-backed, mounting an empty dir on the configcheck pod instead",
+				"volume", n.VolumeName,
+				"path", n.Path,
+				"hint", "set fluentd.configCheckPod.volumes with the same name if the check needs its contents")
+			checkVolume = &volume.KubernetesVolume{EmptyDir: &corev1.EmptyDirVolumeSource{}}
 		}
-		if err := n.ApplyVolumeForPodSpec(&pod.Spec); err != nil {
+		if err := checkVolume.ApplyVolumeForPodSpec(n.VolumeName, n.ContainerName, n.Path, &pod.Spec); err != nil {
 			r.Log.Error(err, "Fluentd Config check pod extraVolume attachment failed.")
 		}
 	}