diff --git a/pkg/resources/configcheck/failure.go b/pkg/resources/configcheck/failure.go
new file mode 100644
index 000000000..fc28f5ba7
--- /dev/null
+++ b/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 ""
+}
diff --git a/pkg/resources/configcheck/failure_test.go b/pkg/resources/configcheck/failure_test.go
new file mode 100644
index 000000000..13751d425
--- /dev/null
+++ b/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))
+ })
+ }
+}
diff --git a/pkg/resources/fluentd/appconfigmap.go b/pkg/resources/fluentd/appconfigmap.go
index 7b64bcc2c..b16d67eb6 100644
--- a/pkg/resources/fluentd/appconfigmap.go
+++ b/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.")
}
}
diff --git a/pkg/resources/fluentd/appconfigmap_test.go b/pkg/resources/fluentd/appconfigmap_test.go
index dc605e4fe..91bc30023 100644
--- a/pkg/resources/fluentd/appconfigmap_test.go
+++ b/pkg/resources/fluentd/appconfigmap_test.go
@@ -26,6 +26,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"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/log"
@@ -34,7 +35,8 @@ import (
// newCheckPodReconciler builds a Reconciler that is just complete enough for
// newCheckPod, which only needs the logging resource and the fluentd spec.
-func newCheckPodReconciler(t *testing.T, fluentdSpec *v1beta1.FluentdSpec) *Reconciler {
+// Pass a client only for the paths that talk to the API server, e.g. configCheck.
+func newCheckPodReconciler(t *testing.T, fluentdSpec *v1beta1.FluentdSpec, c client.Client) *Reconciler {
t.Helper()
logging := &v1beta1.Logging{}
@@ -45,7 +47,18 @@ func newCheckPodReconciler(t *testing.T, fluentdSpec *v1beta1.FluentdSpec) *Reco
config := "\n\n"
- return New(nil, log.Log, logging, logging.Spec.FluentdSpec, nil, &config, nil, reconciler.ReconcilerOpts{})
+ return New(c, log.Log, logging, logging.Spec.FluentdSpec, nil, &config, nil, reconciler.ReconcilerOpts{})
+}
+
+// newFakeClient returns a client backed by an empty in-memory store for the
+// configcheck paths that create and delete pods.
+func newFakeClient(t *testing.T) client.Client {
+ t.Helper()
+
+ scheme := runtime.NewScheme()
+ require.NoError(t, corev1.AddToScheme(scheme))
+
+ return fake.NewClientBuilder().WithScheme(scheme).Build()
}
func TestNewCheckPodDNSSettings(t *testing.T) {
@@ -93,7 +106,7 @@ func TestNewCheckPodDNSSettings(t *testing.T) {
DNSPolicy: tt.dnsPolicy,
DNSConfig: tt.dnsConfig,
}
- r := newCheckPodReconciler(t, spec)
+ r := newCheckPodReconciler(t, spec, nil)
pod, err := r.newCheckPod("deadbeef", *r.fluentdSpec)
require.NoError(t, err)
@@ -116,7 +129,7 @@ func TestNewCheckPodDNSSettingsMatchStatefulSet(t *testing.T) {
Options: []corev1.PodDNSConfigOption{{Name: "ndots", Value: &ndots}},
},
}
- r := newCheckPodReconciler(t, spec)
+ r := newCheckPodReconciler(t, spec, nil)
checkPod, err := r.newCheckPod("deadbeef", *r.fluentdSpec)
require.NoError(t, err)
@@ -145,7 +158,7 @@ func TestNewCheckPodDoesNotMutateSharedAffinity(t *testing.T) {
},
},
}
- r := newCheckPodReconciler(t, spec)
+ r := newCheckPodReconciler(t, spec, nil)
checkPod, err := r.newCheckPod("deadbeef", *r.fluentdSpec)
require.NoError(t, err)
@@ -184,7 +197,7 @@ func TestNewCheckPodConfigCheckPodOverrides(t *testing.T) {
ActiveDeadlineSeconds: &deadline,
},
}
- r := newCheckPodReconciler(t, spec)
+ r := newCheckPodReconciler(t, spec, nil)
checkPod, err := r.newCheckPod("deadbeef", *r.fluentdSpec)
require.NoError(t, err)
@@ -216,11 +229,12 @@ func TestNewCheckPodConfigCheckPodOverridesAreAdditive(t *testing.T) {
compressConfigFile bool
tlsEnabled bool
configCheckPod *v1beta1.ConfigCheckPodOverrides
+ expectNoOp bool
}{
- {name: "NilOverride"},
+ {name: "NilOverride", expectNoOp: true},
+ {name: "EmptyOverride", configCheckPod: &v1beta1.ConfigCheckPodOverrides{}, expectNoOp: true},
{name: "WithOverride", configCheckPod: overrides},
- {name: "WithOverrideAndCompress", compressConfigFile: true, configCheckPod: overrides},
- {name: "WithOverrideAndTLS", tlsEnabled: true, configCheckPod: overrides},
+ {name: "WithOverrideCompressAndTLS", compressConfigFile: true, tlsEnabled: true, configCheckPod: overrides},
}
for _, tt := range tests {
@@ -229,28 +243,32 @@ func TestNewCheckPodConfigCheckPodOverridesAreAdditive(t *testing.T) {
if tt.tlsEnabled {
baseSpec.TLS = v1beta1.FluentdTLS{Enabled: true, SecretName: "fluentd-tls-secret"}
}
- baseReconciler := newCheckPodReconciler(t, baseSpec)
+ baseReconciler := newCheckPodReconciler(t, baseSpec, nil)
basePod, err := baseReconciler.newCheckPod("deadbeef", *baseReconciler.fluentdSpec)
require.NoError(t, err)
spec := baseSpec.DeepCopy()
spec.ConfigCheckPod = tt.configCheckPod
- r := newCheckPodReconciler(t, spec)
+ r := newCheckPodReconciler(t, spec, nil)
pod, err := r.newCheckPod("deadbeef", *r.fluentdSpec)
require.NoError(t, err)
+ if tt.expectNoOp {
+ // Nothing to merge must mean nothing changed: this is the whole
+ // backwards-compatibility claim of the configCheckPod field.
+ assert.Equal(t, basePod.Spec, pod.Spec, "an empty configCheckPod must leave the pod spec untouched")
+ return
+ }
+
for _, c := range basePod.Spec.InitContainers {
assert.Contains(t, pod.Spec.InitContainers, c, "configCheckPod must not drop the operator's own init containers")
}
for _, v := range basePod.Spec.Volumes {
assert.Contains(t, pod.Spec.Volumes, v, "configCheckPod must not drop the operator's own volumes")
}
-
- if tt.configCheckPod != nil {
- assert.Contains(t, pod.Spec.InitContainers, overrideInitContainer)
- assert.Contains(t, pod.Spec.Volumes, overrideVolume)
- }
+ assert.Contains(t, pod.Spec.InitContainers, overrideInitContainer)
+ assert.Contains(t, pod.Spec.Volumes, overrideVolume)
})
}
}
@@ -258,84 +276,131 @@ func TestNewCheckPodConfigCheckPodOverridesAreAdditive(t *testing.T) {
// TestConfigCheckDeadlineExceededIsRetried pins that a configcheck pod failed
// by activeDeadlineSeconds (Status.Reason == "DeadlineExceeded") is deleted
// and reported not-ready for retry, rather than latched as an invalid config.
-func TestConfigCheckDeadlineExceededIsRetried(t *testing.T) {
- scheme := runtime.NewScheme()
- require.NoError(t, corev1.AddToScheme(scheme))
+func TestConfigCheckFailedPodVerdict(t *testing.T) {
+ tests := []struct {
+ name string
+ reason string
+ expectRetry bool
+ }{
+ {name: "InvalidConfig", reason: ""},
+ {name: "DeadlineExceeded", reason: "DeadlineExceeded", expectRetry: true},
+ {name: "Evicted", reason: "Evicted", expectRetry: true},
+ {name: "NodeAffinity", reason: "NodeAffinity", expectRetry: true},
+ {name: "Shutdown", reason: "Shutdown", expectRetry: true},
+ }
- logging := &v1beta1.Logging{}
- logging.Name = "test"
- logging.Spec.ControlNamespace = "logging"
- logging.Spec.FluentdSpec = &v1beta1.FluentdSpec{}
- require.NoError(t, logging.SetDefaults())
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ fakeClient := newFakeClient(t)
+ r := newCheckPodReconciler(t, &v1beta1.FluentdSpec{}, fakeClient)
+ ctx := context.Background()
- config := "\n\n"
- fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
- r := New(fakeClient, log.Log, logging, logging.Spec.FluentdSpec, nil, &config, nil, reconciler.ReconcilerOpts{})
- ctx := context.Background()
+ _, err := r.configCheck(ctx)
+ require.NoError(t, err)
- // first pass creates the configcheck pod
- _, err := r.configCheck(ctx)
- require.NoError(t, err)
+ pods := &corev1.PodList{}
+ require.NoError(t, fakeClient.List(ctx, pods))
+ require.Len(t, pods.Items, 1)
- pods := &corev1.PodList{}
- require.NoError(t, fakeClient.List(ctx, pods))
- require.Len(t, pods.Items, 1)
- pod := &pods.Items[0]
+ pod := &pods.Items[0]
+ pod.Status.Phase = corev1.PodFailed
+ pod.Status.Reason = tt.reason
+ require.NoError(t, fakeClient.Status().Update(ctx, pod))
- pod.Status.Phase = corev1.PodFailed
- pod.Status.Reason = "DeadlineExceeded"
- require.NoError(t, fakeClient.Status().Update(ctx, pod))
+ result, err := r.configCheck(ctx)
+ require.NoError(t, err)
- result, err := r.configCheck(ctx)
- require.NoError(t, err)
- assert.False(t, result.Ready)
- assert.False(t, result.Valid)
- assert.Contains(t, result.Message, "DeadlineExceeded")
+ remaining := &corev1.PodList{}
+ require.NoError(t, fakeClient.List(ctx, remaining))
+
+ if !tt.expectRetry {
+ assert.True(t, result.Ready, "an invalid config is a final verdict, not something to retry")
+ assert.False(t, result.Valid)
+ assert.Len(t, remaining.Items, 1, "the pod that proved the config invalid must be kept for diagnosis")
+ return
+ }
- remaining := &corev1.PodList{}
- require.NoError(t, fakeClient.List(ctx, remaining))
- assert.Empty(t, remaining.Items, "the pod that exceeded its deadline should have been deleted so a fresh one can be created")
+ assert.False(t, result.Ready, "a pod killed around the check says nothing about the config")
+ assert.False(t, result.Valid)
+ assert.Contains(t, result.Message, tt.reason)
+ assert.Empty(t, remaining.Items, "the pod must be deleted so a fresh check can be created")
+ })
+ }
}
-// TestNewCheckPodPVCBackedExtraVolumeBecomesEmptyDir pins that a PVC-backed
-// extraVolume (one whose PersistentVolumeClaimSpec is set, meaning it is only
-// ever realized via volumeClaimTemplates on the StatefulSet) is downgraded to
-// an emptyDir on the check pod, since the check pod is a plain Pod with no
-// matching PVC - referencing the claim name outright would make the pod
-// permanently uncreatable.
-func TestNewCheckPodPVCBackedExtraVolumeBecomesEmptyDir(t *testing.T) {
- spec := &v1beta1.FluentdSpec{
- ExtraVolumes: []v1beta1.ExtraVolume{
- {
- VolumeName: "geoip-data",
- ContainerName: "fluentd",
- Path: "/geoip",
- Volume: &volume.KubernetesVolume{
- PersistentVolumeClaim: &volume.PersistentVolumeClaim{
- PersistentVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{
- AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
- Resources: corev1.VolumeResourceRequirements{
- Requests: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse("1Gi")},
- },
- },
- PersistentVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ClaimName: "geoip-data"},
- },
- },
+// TestNewCheckPodExtraVolumeClaims pins which extraVolumes the check pod can mount as-is. A spec
+// is only ever realized as a volumeClaimTemplate on the StatefulSet, so on a plain Pod it names a
+// claim that never exists and must become an emptyDir; a bare claimName refers to a claim the user
+// brought themselves, which does exist and must be left alone.
+func TestNewCheckPodExtraVolumeClaims(t *testing.T) {
+ pvcSpec := corev1.PersistentVolumeClaimSpec{
+ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
+ Resources: corev1.VolumeResourceRequirements{
+ Requests: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse("1Gi")},
+ },
+ }
+
+ tests := []struct {
+ name string
+ claim *volume.PersistentVolumeClaim
+ expectEmptyDir bool
+ expectClaimName string
+ }{
+ {
+ name: "TemplatedClaimBecomesEmptyDir",
+ claim: &volume.PersistentVolumeClaim{
+ PersistentVolumeClaimSpec: pvcSpec,
+ PersistentVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ClaimName: "geoip-data"},
+ },
+ expectEmptyDir: true,
+ },
+ {
+ name: "ExistingClaimIsKept",
+ claim: &volume.PersistentVolumeClaim{
+ PersistentVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ClaimName: "geoip-data"},
},
+ expectClaimName: "geoip-data",
},
}
- r := newCheckPodReconciler(t, spec)
- pod, err := r.newCheckPod("deadbeef", *r.fluentdSpec)
- require.NoError(t, err)
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ spec := &v1beta1.FluentdSpec{
+ ExtraVolumes: []v1beta1.ExtraVolume{
+ {
+ VolumeName: "geoip-data",
+ ContainerName: "fluentd",
+ Path: "/geoip",
+ Volume: &volume.KubernetesVolume{PersistentVolumeClaim: tt.claim},
+ },
+ },
+ }
+ r := newCheckPodReconciler(t, spec, nil)
- var got *corev1.Volume
- for i := range pod.Spec.Volumes {
- if pod.Spec.Volumes[i].Name == "geoip-data" {
- got = &pod.Spec.Volumes[i]
- }
+ pod, err := r.newCheckPod("deadbeef", *r.fluentdSpec)
+ require.NoError(t, err)
+
+ var got *corev1.Volume
+ for i := range pod.Spec.Volumes {
+ if pod.Spec.Volumes[i].Name == "geoip-data" {
+ got = &pod.Spec.Volumes[i]
+ }
+ }
+ require.NotNil(t, got, "extraVolume must still be attached to the check pod")
+
+ if tt.expectEmptyDir {
+ assert.NotNil(t, got.EmptyDir)
+ assert.Nil(t, got.PersistentVolumeClaim, "must not reference a claim that will never exist for the check pod")
+ } else {
+ require.NotNil(t, got.PersistentVolumeClaim, "a claim the user brought themselves must be mounted as-is")
+ assert.Equal(t, tt.expectClaimName, got.PersistentVolumeClaim.ClaimName)
+ }
+
+ // The substitution must not lose the mount it exists for.
+ assert.Contains(t, pod.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{
+ Name: "geoip-data",
+ MountPath: "/geoip",
+ })
+ })
}
- require.NotNil(t, got, "extraVolume must still be attached to the check pod")
- assert.NotNil(t, got.EmptyDir, "a PVC meant for volumeClaimTemplates has no matching PVC on a plain Pod, so the check pod must fall back to an emptyDir")
- assert.Nil(t, got.PersistentVolumeClaim, "must not reference a claim name that will never exist for the check pod")
}
diff --git a/pkg/resources/fluentd/statefulset.go b/pkg/resources/fluentd/statefulset.go
index 7e7e77638..bcfd5ae57 100644
--- a/pkg/resources/fluentd/statefulset.go
+++ b/pkg/resources/fluentd/statefulset.go
@@ -23,6 +23,7 @@ import (
"github.com/cisco-open/operator-tools/pkg/reconciler"
"github.com/cisco-open/operator-tools/pkg/types"
util "github.com/cisco-open/operator-tools/pkg/utils"
+ "github.com/cisco-open/operator-tools/pkg/volume"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -57,7 +58,7 @@ func (r *Reconciler) statefulset() (runtime.Object, reconciler.DesiredState, err
r.Log.Info("volume definition missing from extraVolume, ignoring", "path", n.Path, "containerName", n.ContainerName, "volumeName", n.VolumeName)
continue
}
- if n.Volume.PersistentVolumeClaim == nil || isPersistentVolumeClaimSpecEmpty(n.Volume.PersistentVolumeClaim.PersistentVolumeClaimSpec) {
+ if !isPVCBacked(n.Volume) {
if err := n.ApplyVolumeForPodSpec(&spec.Template.Spec); err != nil {
return nil, reconciler.StatePresent, err
}
@@ -85,6 +86,13 @@ func isPersistentVolumeClaimSpecEmpty(pvcSpec corev1.PersistentVolumeClaimSpec)
return reflect.DeepEqual(pvcSpec, empty)
}
+// isPVCBacked reports whether an extraVolume is realized as a volumeClaimTemplate on the
+// StatefulSet, which is what makes it unusable as-is on the one-shot configcheck pod.
+func isPVCBacked(v *volume.KubernetesVolume) bool {
+ return v != nil && v.PersistentVolumeClaim != nil &&
+ !isPersistentVolumeClaimSpecEmpty(v.PersistentVolumeClaim.PersistentVolumeClaimSpec)
+}
+
func (r *Reconciler) statefulsetSpec() *appsv1.StatefulSetSpec {
var initContainers []corev1.Container
diff --git a/pkg/resources/syslogng/configcheck.go b/pkg/resources/syslogng/configcheck.go
index 86aa5d526..2bb6654b2 100644
--- a/pkg/resources/syslogng/configcheck.go
+++ b/pkg/resources/syslogng/configcheck.go
@@ -142,23 +142,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{
@@ -184,23 +175,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 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) (*corev1.Secret, error) { //nolint: unparam
meta := r.SyslogNGObjectMeta(configCheckResourceName(hashKey), ComponentConfigCheck)
meta.Labels = utils.MergeLabels(