feat: make StoreExec and StoreDebugInstance cleanup configurable per CR

api/v1/exec.go

@@ -26,6 +26,18 @@ type StoreExecSpec struct {
 	// +kubebuilder:default=3
 	MaxRetries int32 `json:"maxRetries,omitempty"`
 
+	// How long a successfully finished StoreExec is kept before it is deleted.
+	// Zero disables cleanup for successful executions.
+	// +kubebuilder:default="5m"
+	// +kubebuilder:validation:XValidation:rule="self.matches('^(0|([0-9]+([.][0-9]+)?(ns|us|ms|s|m|h))+)$')",message="must be a valid duration, e.g. 30s, 5m or 1h"
+	CleanupPeriodSuccessfulExec metav1.Duration `json:"cleanupPeriodSuccessfulExec,omitempty"`
+
+	// How long a failed StoreExec is kept before it is deleted.
+	// Zero disables cleanup for failed executions.
+	// +kubebuilder:default="1h"
+	// +kubebuilder:validation:XValidation:rule="self.matches('^(0|([0-9]+([.][0-9]+)?(ns|us|ms|s|m|h))+)$')",message="must be a valid duration, e.g. 30s, 5m or 1h"
+	CleanupPeriodErrorExec metav1.Duration `json:"cleanupPeriodErrorExec,omitempty"`
+
 	ExtraEnvs []corev1.EnvVar `json:"extraEnvs,omitempty"`
 
 	Container ContainerSpec `json:"container,omitempty"`

api/v1/storedebuginstance_types.go

@@ -29,9 +29,12 @@ type StoreDebugInstanceSpec struct {
 	// StoreRef is the reference to the store to debug
 	StoreRef string `json:"storeRef,omitempty"`
 	// Duration is the duration of the debug instance after which it will be deleted
-	// e.g. 1h or 30m
+	// e.g. 1h or 30m. Zero keeps the instance until it is deleted manually.
+	// The pattern is enforced by the API server: a value the Go duration parser
+	// rejects would otherwise break the informer for every StoreDebugInstance.
 	// +default="1h"
-	Duration string `json:"duration,omitempty"`
+	// +kubebuilder:validation:XValidation:rule="self.matches('^(0|([0-9]+([.][0-9]+)?(ns|us|ms|s|m|h))+)$')",message="must be a valid duration, e.g. 30s, 5m or 1h"
+	Duration metav1.Duration `json:"duration,omitempty"`
 	// ExtraLabels is the extra labels to add to the debug instance
 	ExtraLabels map[string]string `json:"extraLabels,omitempty"`
 	// ExtraContainerPorts is the extra ports to add to the debug instance

cmd/main.go

@@ -240,11 +240,10 @@ func main() {
 		os.Exit(1)
 	}
 	if err = (&controller.StoreExecReconciler{
-		Client:             nsClient,
-		Logger:             logger.With(zapz.String("component", "store-exec-reconciler")),
-		Scheme:             mgr.GetScheme(),
-		Recorder:           mgr.GetEventRecorderFor(fmt.Sprintf("shopware-controller-%s", cfg.Namespace)),
-		CleanupGracePeriod: cfg.SuccessfulCRCleanupGracePeriod,
+		Client:   nsClient,
+		Logger:   logger.With(zapz.String("component", "store-exec-reconciler")),
+		Scheme:   mgr.GetScheme(),
+		Recorder: mgr.GetEventRecorderFor(fmt.Sprintf("shopware-controller-%s", cfg.Namespace)),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create exec controller", "controller", "StoreExec")
 		os.Exit(1)
@@ -274,11 +273,10 @@ func main() {
 		os.Exit(1)
 	}
 	if err = (&controller.StoreDebugInstanceReconciler{
-		Client:             nsClient,
-		Logger:             logger.With(zapz.String("component", "store-debug-instance-reconciler")),
-		Scheme:             mgr.GetScheme(),
-		Recorder:           mgr.GetEventRecorderFor(fmt.Sprintf("shopware-controller-%s", cfg.Namespace)),
-		CleanupGracePeriod: cfg.SuccessfulCRCleanupGracePeriod,
+		Client:   nsClient,
+		Logger:   logger.With(zapz.String("component", "store-debug-instance-reconciler")),
+		Scheme:   mgr.GetScheme(),
+		Recorder: mgr.GetEventRecorderFor(fmt.Sprintf("shopware-controller-%s", cfg.Namespace)),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create instance controller", "controller", "StoreDebugInstance")
 		os.Exit(1)

helm/templates/deployment.yaml

@@ -91,8 +91,6 @@ spec:
           value: "{{ .Values.keda.enabled | default "false" }}"
         - name: ENABLE_SERVICE_MONITOR
           value: "{{ and .Values.metrics.enabled .Values.metrics.serviceMonitor.enabled }}"
-        - name: SUCCESSFUL_CR_CLEANUP_GRACE_PERIOD
-          value: "{{ .Values.successfulCRCleanupGracePeriod | default "1h" }}"
         {{- if .Values.metrics.enabled }}
         - name: METRICS_BIND_ADDRESS
           value: ":{{ .Values.metrics.port | default 8080 }}"

helm/values.yaml

@@ -105,8 +105,6 @@ logFormat: json
 # Disable check for s3/database/fastly and Opensearch checks. Useful if network access is not given for one of the services.
 # This is a global level. You can also control this per store.
 disableChecks: false
-# Grace period before successful StoreExec and StoreDebugInstance CRs are deleted. Set to "0" to disable cleanup.
-successfulCRCleanupGracePeriod: 1h
 
 # keda: when enabled, the operator creates a KEDA ScaledObject for every store
 # queue worker deployment. The ScaledObject scales the workers based on the

internal/config/config.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
-	"time"
 
 	"github.com/sethvargo/go-envconfig"
 )
@@ -77,8 +76,6 @@ type StoreConfig struct {
 	EnableServiceMonitor bool   `env:"ENABLE_SERVICE_MONITOR, default=false"`
 	DisableChecks        bool   `env:"DISABLE_CHECKS, default=false"`
 	Namespace            string `env:"NAMESPACE, default=default"`
-
-	SuccessfulCRCleanupGracePeriod time.Duration `env:"SUCCESSFUL_CR_CLEANUP_GRACE_PERIOD, default=1h"`
 }
 
 type Config struct {

internal/controller/storedebuginstance_controller.go

@@ -40,10 +40,9 @@ import (
 // StoreDebugInstanceReconciler reconciles a StoreDebugInstance object
 type StoreDebugInstanceReconciler struct {
 	client.Client
-	Scheme             *runtime.Scheme
-	Recorder           record.EventRecorder
-	Logger             *zap.SugaredLogger
-	CleanupGracePeriod time.Duration
+	Scheme   *runtime.Scheme
+	Recorder record.EventRecorder
+	Logger   *zap.SugaredLogger
 }
 
 // +kubebuilder:rbac:groups=shop.shopware.com,namespace=default,resources=storedebuginstances,verbs=get;list;watch;create;update;patch;delete
@@ -80,12 +79,6 @@ func (r *StoreDebugInstanceReconciler) Reconcile(ctx context.Context, req ctrl.R
 		log.Errorw("get CR store debug instance", zap.Error(err))
 	}
 
-	// validate duration
-	_, err = time.ParseDuration(storeDebugInstance.Spec.Duration)
-	if err != nil {
-		return rr, fmt.Errorf("invalid duration: %w", err)
-	}
-
 	if result, deleted, cleanupErr := r.deleteSuccessfulStoreDebugInstanceIfCleanupDue(ctx, storeDebugInstance); deleted || cleanupErr != nil {
 		if cleanupErr != nil {
 			log.Errorw("failed to cleanup successful store debug instance", zap.Error(cleanupErr))
@@ -238,17 +231,16 @@ func (r *StoreDebugInstanceReconciler) reconcileSuccessfulStoreDebugInstanceClea
 func (r *StoreDebugInstanceReconciler) isStoreDebugInstanceCleanupEligible(
 	storeDebugInstance *shopv1.StoreDebugInstance,
 ) bool {
-	return r.CleanupGracePeriod > 0 &&
+	return storeDebugInstance.Spec.Duration.Duration > 0 &&
 		storeDebugInstance.DeletionTimestamp == nil &&
-		storeDebugInstance.IsState(shopv1.StoreDebugInstanceStateDone)
+		(storeDebugInstance.IsState(shopv1.StoreDebugInstanceStateDone) ||
+			storeDebugInstance.IsState(shopv1.StoreDebugInstanceStateError))
 }
 
 func (r *StoreDebugInstanceReconciler) storeDebugInstanceCleanupRemaining(
 	storeDebugInstance *shopv1.StoreDebugInstance,
 ) time.Duration {
-	duration, _ := time.ParseDuration(storeDebugInstance.Spec.Duration)
-	deleteAfter := storeDebugInstance.CreationTimestamp.Add(duration).Add(r.CleanupGracePeriod)
-	return time.Until(deleteAfter)
+	return time.Until(storeDebugInstance.CreationTimestamp.Add(storeDebugInstance.Spec.Duration.Duration))
 }
 
 func (r *StoreDebugInstanceReconciler) deleteSuccessfulStoreDebugInstance(

internal/controller/storedebuginstance_status.go

@@ -99,8 +99,8 @@ func (r *StoreDebugInstanceReconciler) stateRunning(ctx context.Context, store *
 		storeDebugInstance.Status.AddCondition(con)
 	}()
 
-	duration, _ := time.ParseDuration(storeDebugInstance.Spec.Duration)
-	if time.Now().After(storeDebugInstance.CreationTimestamp.Add(duration)) {
+	duration := storeDebugInstance.Spec.Duration.Duration
+	if duration > 0 && time.Now().After(storeDebugInstance.CreationTimestamp.Add(duration)) {
 		con.Message = "Store debug instance expired"
 		con.Status = string(v1.StoreDebugInstanceStateDone)
 		return v1.StoreDebugInstanceStateDone

internal/controller/storeexec_controller.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"slices"
 	"time"
 
 	v1 "github.com/shopware/shopware-operator/api/v1"
@@ -20,10 +21,9 @@ import (
 
 type StoreExecReconciler struct {
 	client.Client
-	Scheme             *runtime.Scheme
-	Recorder           record.EventRecorder
-	Logger             *zap.SugaredLogger
-	CleanupGracePeriod time.Duration
+	Scheme   *runtime.Scheme
+	Recorder record.EventRecorder
+	Logger   *zap.SugaredLogger
 }
 
 // +kubebuilder:rbac:groups=shop.shopware.com,namespace=default,resources=storeexecs,verbs=get;list;watch;create;update;patch;delete
@@ -63,9 +63,9 @@ func (r *StoreExecReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return rr, nil
 	}
 
-	if result, handled, cleanupErr := r.reconcileSuccessfulStoreExecCleanup(ctx, ex); handled || cleanupErr != nil {
+	if result, handled, cleanupErr := r.reconcileStoreExecCleanup(ctx, ex); handled || cleanupErr != nil {
 		if cleanupErr != nil {
-			log.Errorw("failed to cleanup successful store-exec", zap.Error(cleanupErr))
+			log.Errorw("failed to cleanup finished store-exec", zap.Error(cleanupErr))
 			skipStatusUpdate = true
 			return rr, nil
 		}
@@ -147,47 +147,62 @@ func (r *StoreExecReconciler) reconcileJob(ctx context.Context, store *v1.Store,
 	return nil
 }
 
-func (r *StoreExecReconciler) reconcileSuccessfulStoreExecCleanup(
+func (r *StoreExecReconciler) reconcileStoreExecCleanup(
 	ctx context.Context,
 	ex *v1.StoreExec,
 ) (ctrl.Result, bool, error) {
-	if r.CleanupGracePeriod <= 0 ||
-		ex.DeletionTimestamp != nil ||
-		ex.Spec.CronSchedule != "" ||
-		!ex.IsState(v1.ExecStateDone) {
+	period, ok := cleanupPeriodFor(ex)
+	if !ok {
 		return ctrl.Result{}, false, nil
 	}
 
-	deleteAfter := storeExecFinishedAt(ex).Add(r.CleanupGracePeriod)
+	finishedAt, ok := storeExecFinishedAt(ex)
+	if !ok {
+		return ctrl.Result{}, false, nil
+	}
+
+	deleteAfter := finishedAt.Add(period)
 	if remaining := time.Until(deleteAfter); remaining > 0 {
 		return ctrl.Result{RequeueAfter: remaining}, true, nil
 	}
 
 	if err := r.Delete(ctx, ex); err != nil && !k8serrors.IsNotFound(err) {
-		return ctrl.Result{}, false, fmt.Errorf("delete successful StoreExec: %w", err)
+		return ctrl.Result{}, false, fmt.Errorf("delete finished StoreExec: %w", err)
 	}
 
 	return ctrl.Result{}, true, nil
 }
 
-func storeExecFinishedAt(ex *v1.StoreExec) time.Time {
-	for i := len(ex.Status.Conditions) - 1; i >= 0; i-- {
-		if !ex.Status.Conditions[i].LastTransitionTime.IsZero() {
-			return ex.Status.Conditions[i].LastTransitionTime.Time
-		}
+func storeExecFinishedAt(ex *v1.StoreExec) (time.Time, bool) {
+	if !ex.IsState(v1.ExecStateDone, v1.ExecStateError) {
+		return time.Time{}, false
 	}
 
-	for i := len(ex.Status.Conditions) - 1; i >= 0; i-- {
-		if !ex.Status.Conditions[i].LastUpdateTime.IsZero() {
-			return ex.Status.Conditions[i].LastUpdateTime.Time
+	for _, v := range slices.Backward(ex.Status.Conditions) {
+		if t := v.LastTransitionTime; !t.IsZero() {
+			return t.Time, true
 		}
 	}
 
-	if !ex.CreationTimestamp.IsZero() {
-		return ex.CreationTimestamp.Time
+	return time.Time{}, false
+}
+
+func cleanupPeriodFor(ex *v1.StoreExec) (time.Duration, bool) {
+	if ex.DeletionTimestamp != nil || ex.Spec.CronSchedule != "" {
+		return 0, false
+	}
+
+	var period time.Duration
+	switch {
+	case ex.IsState(v1.ExecStateDone):
+		period = ex.Spec.CleanupPeriodSuccessfulExec.Duration
+	case ex.IsState(v1.ExecStateError):
+		period = ex.Spec.CleanupPeriodErrorExec.Duration
+	default:
+		return 0, false
 	}
 
-	return time.Now()
+	return period, period > 0
 }
 
 func (r *StoreExecReconciler) reconcileCronJob(ctx context.Context, store *v1.Store, exec *v1.StoreExec) (err error) {