diff --git a/api/v1/exec.go b/api/v1/exec.go index f2670fd..1d3d6a4 100644 --- a/api/v1/exec.go +++ b/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"` diff --git a/api/v1/storedebuginstance_types.go b/api/v1/storedebuginstance_types.go index f661efe..9a1fa2a 100644 --- a/api/v1/storedebuginstance_types.go +++ b/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 diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 6d12979..f84f313 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -869,6 +869,7 @@ func (in *StoreDebugInstanceList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StoreDebugInstanceSpec) DeepCopyInto(out *StoreDebugInstanceSpec) { *out = *in + out.Duration = in.Duration if in.ExtraLabels != nil { in, out := &in.ExtraLabels, &out.ExtraLabels *out = make(map[string]string, len(*in)) @@ -977,6 +978,8 @@ func (in *StoreExecList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StoreExecSpec) DeepCopyInto(out *StoreExecSpec) { *out = *in + out.CleanupPeriodSuccessfulExec = in.CleanupPeriodSuccessfulExec + out.CleanupPeriodErrorExec = in.CleanupPeriodErrorExec if in.ExtraEnvs != nil { in, out := &in.ExtraEnvs, &out.ExtraEnvs *out = make([]corev1.EnvVar, len(*in)) diff --git a/cmd/main.go b/cmd/main.go index 8be275f..c037390 100644 --- a/cmd/main.go +++ b/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) diff --git a/helm/templates/deployment.yaml b/helm/templates/deployment.yaml index aa3aee1..b35ae9d 100644 --- a/helm/templates/deployment.yaml +++ b/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 }}" diff --git a/helm/values.yaml b/helm/values.yaml index e6eb0db..dc7a892 100644 --- a/helm/values.yaml +++ b/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 diff --git a/internal/config/config.go b/internal/config/config.go index f599bc8..ea87fec 100644 --- a/internal/config/config.go +++ b/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 { diff --git a/internal/controller/storedebuginstance_controller.go b/internal/controller/storedebuginstance_controller.go index 116d829..fda3959 100644 --- a/internal/controller/storedebuginstance_controller.go +++ b/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( diff --git a/internal/controller/storedebuginstance_status.go b/internal/controller/storedebuginstance_status.go index 716aae7..c9d5328 100644 --- a/internal/controller/storedebuginstance_status.go +++ b/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 diff --git a/internal/controller/storeexec_controller.go b/internal/controller/storeexec_controller.go index 77ec9ff..82c8454 100644 --- a/internal/controller/storeexec_controller.go +++ b/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) { diff --git a/internal/controller/successful_cleanup_test.go b/internal/controller/successful_cleanup_test.go index 010107c..f5a02a8 100644 --- a/internal/controller/successful_cleanup_test.go +++ b/internal/controller/successful_cleanup_test.go @@ -17,54 +17,86 @@ import ( const cleanupTestNamespace = "default" -func TestStoreExecSuccessfulCleanupDeletesDoneOneShotAfterGracePeriod(t *testing.T) { +func TestStoreExecCleanupDeletesFinishedExecAfterCleanupPeriod(t *testing.T) { ctx := context.Background() - ex := storeExecForCleanup("done-old", shopv1.ExecStateDone, time.Now().Add(-2*time.Hour)) - cl := fake.NewClientBuilder(). - WithScheme(cleanupTestScheme(t)). - WithObjects(ex). - Build() + scheme := cleanupTestScheme(t) - reconciler := StoreExecReconciler{ - Client: cl, - CleanupGracePeriod: time.Hour, + tests := []struct { + name string + state shopv1.StatefulState + successfulIn time.Duration + errorIn time.Duration + }{ + {name: "done", state: shopv1.ExecStateDone, successfulIn: time.Hour}, + {name: "error", state: shopv1.ExecStateError, errorIn: time.Hour}, } - result, handled, err := reconciler.reconcileSuccessfulStoreExecCleanup(ctx, ex) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ex := storeExecForCleanup(tt.name+"-old", tt.state, time.Now().Add(-2*time.Hour)) + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: tt.successfulIn} + ex.Spec.CleanupPeriodErrorExec = metav1.Duration{Duration: tt.errorIn} - require.NoError(t, err) - assert.True(t, handled) - assert.Zero(t, result) + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(ex). + Build() + + reconciler := StoreExecReconciler{Client: cl} + + result, handled, err := reconciler.reconcileStoreExecCleanup(ctx, ex) + + require.NoError(t, err) + assert.True(t, handled) + assert.Zero(t, result) - got := &shopv1.StoreExec{} - err = cl.Get(ctx, types.NamespacedName{Namespace: cleanupTestNamespace, Name: ex.Name}, got) - assert.True(t, k8serrors.IsNotFound(err)) + got := &shopv1.StoreExec{} + err = cl.Get(ctx, types.NamespacedName{Namespace: cleanupTestNamespace, Name: ex.Name}, got) + assert.True(t, k8serrors.IsNotFound(err)) + }) + } } -func TestStoreExecSuccessfulCleanupRetainsDoneOneShotBeforeGracePeriod(t *testing.T) { +func TestStoreExecCleanupRetainsFinishedExecBeforeCleanupPeriod(t *testing.T) { ctx := context.Background() - ex := storeExecForCleanup("done-new", shopv1.ExecStateDone, time.Now().Add(-30*time.Minute)) - cl := fake.NewClientBuilder(). - WithScheme(cleanupTestScheme(t)). - WithObjects(ex). - Build() + scheme := cleanupTestScheme(t) - reconciler := StoreExecReconciler{ - Client: cl, - CleanupGracePeriod: time.Hour, + tests := []struct { + name string + state shopv1.StatefulState + successfulIn time.Duration + errorIn time.Duration + }{ + {name: "done", state: shopv1.ExecStateDone, successfulIn: time.Hour}, + {name: "error", state: shopv1.ExecStateError, errorIn: time.Hour}, } - result, handled, err := reconciler.reconcileSuccessfulStoreExecCleanup(ctx, ex) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ex := storeExecForCleanup(tt.name+"-new", tt.state, time.Now().Add(-30*time.Minute)) + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: tt.successfulIn} + ex.Spec.CleanupPeriodErrorExec = metav1.Duration{Duration: tt.errorIn} - require.NoError(t, err) - assert.True(t, handled) - assert.Greater(t, result.RequeueAfter, time.Duration(0)) + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(ex). + Build() - got := &shopv1.StoreExec{} - require.NoError(t, cl.Get(ctx, types.NamespacedName{Namespace: cleanupTestNamespace, Name: ex.Name}, got)) + reconciler := StoreExecReconciler{Client: cl} + + result, handled, err := reconciler.reconcileStoreExecCleanup(ctx, ex) + + require.NoError(t, err) + assert.True(t, handled) + assert.Greater(t, result.RequeueAfter, time.Duration(0)) + + got := &shopv1.StoreExec{} + require.NoError(t, cl.Get(ctx, types.NamespacedName{Namespace: cleanupTestNamespace, Name: ex.Name}, got)) + }) + } } -func TestStoreExecSuccessfulCleanupRetainsCronAndFailedResources(t *testing.T) { +func TestStoreExecCleanupSkipsUnaffectedResources(t *testing.T) { ctx := context.Background() scheme := cleanupTestScheme(t) @@ -77,12 +109,48 @@ func TestStoreExecSuccessfulCleanupRetainsCronAndFailedResources(t *testing.T) { ex: func() *shopv1.StoreExec { ex := storeExecForCleanup("cron-done", shopv1.ExecStateDone, time.Now().Add(-2*time.Hour)) ex.Spec.CronSchedule = "*/5 * * * *" + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: time.Hour} return ex }(), }, { - name: "error", - ex: storeExecForCleanup("error", shopv1.ExecStateError, time.Now().Add(-2*time.Hour)), + name: "still running", + ex: func() *shopv1.StoreExec { + ex := storeExecForCleanup("running", shopv1.ExecStateRunning, time.Now().Add(-2*time.Hour)) + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: time.Hour} + ex.Spec.CleanupPeriodErrorExec = metav1.Duration{Duration: time.Hour} + return ex + }(), + }, + { + name: "done with successful period disabled", + ex: func() *shopv1.StoreExec { + ex := storeExecForCleanup("done-no-period", shopv1.ExecStateDone, time.Now().Add(-2*time.Hour)) + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: 0} + ex.Spec.CleanupPeriodErrorExec = metav1.Duration{Duration: time.Hour} + return ex + }(), + }, + { + name: "error with error period disabled", + ex: func() *shopv1.StoreExec { + ex := storeExecForCleanup("error-no-period", shopv1.ExecStateError, time.Now().Add(-2*time.Hour)) + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: time.Hour} + ex.Spec.CleanupPeriodErrorExec = metav1.Duration{Duration: 0} + return ex + }(), + }, + { + name: "done without usable finish time", + ex: func() *shopv1.StoreExec { + ex := storeExecForCleanup("done-no-timestamp", shopv1.ExecStateDone, time.Now().Add(-2*time.Hour)) + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: time.Hour} + for i := range ex.Status.Conditions { + ex.Status.Conditions[i].LastTransitionTime = metav1.Time{} + ex.Status.Conditions[i].LastUpdateTime = metav1.Time{} + } + return ex + }(), }, } @@ -93,12 +161,9 @@ func TestStoreExecSuccessfulCleanupRetainsCronAndFailedResources(t *testing.T) { WithObjects(tt.ex). Build() - reconciler := StoreExecReconciler{ - Client: cl, - CleanupGracePeriod: time.Hour, - } + reconciler := StoreExecReconciler{Client: cl} - result, handled, err := reconciler.reconcileSuccessfulStoreExecCleanup(ctx, tt.ex) + result, handled, err := reconciler.reconcileStoreExecCleanup(ctx, tt.ex) require.NoError(t, err) assert.False(t, handled) @@ -110,7 +175,7 @@ func TestStoreExecSuccessfulCleanupRetainsCronAndFailedResources(t *testing.T) { } } -func TestStoreDebugInstanceSuccessfulCleanupUsesDurationAndGracePeriod(t *testing.T) { +func TestStoreDebugInstanceSuccessfulCleanupUsesDuration(t *testing.T) { ctx := context.Background() scheme := cleanupTestScheme(t) @@ -121,15 +186,15 @@ func TestStoreDebugInstanceSuccessfulCleanupUsesDurationAndGracePeriod(t *testin expectDeleted bool }{ { - name: "after duration and grace", + name: "after duration", objectName: "debug-done-old", - creationTime: time.Now().Add(-3 * time.Hour), + creationTime: time.Now().Add(-2 * time.Hour), expectDeleted: true, }, { - name: "before duration and grace", + name: "before duration", objectName: "debug-done-new", - creationTime: time.Now().Add(-90 * time.Minute), + creationTime: time.Now().Add(-30 * time.Minute), expectDeleted: false, }, } @@ -142,10 +207,7 @@ func TestStoreDebugInstanceSuccessfulCleanupUsesDurationAndGracePeriod(t *testin WithObjects(debugInstance). Build() - reconciler := StoreDebugInstanceReconciler{ - Client: cl, - CleanupGracePeriod: time.Hour, - } + reconciler := StoreDebugInstanceReconciler{Client: cl} result, handled, err := reconciler.reconcileSuccessfulStoreDebugInstanceCleanup(ctx, debugInstance) @@ -166,19 +228,22 @@ func TestStoreDebugInstanceSuccessfulCleanupUsesDurationAndGracePeriod(t *testin } } -func TestSuccessfulCleanupDisabledWithZeroGracePeriod(t *testing.T) { +func TestSuccessfulCleanupDisabledWithZeroPeriods(t *testing.T) { ctx := context.Background() scheme := cleanupTestScheme(t) ex := storeExecForCleanup("disabled-exec", shopv1.ExecStateDone, time.Now().Add(-2*time.Hour)) + ex.Spec.CleanupPeriodSuccessfulExec = metav1.Duration{Duration: 0} + ex.Spec.CleanupPeriodErrorExec = metav1.Duration{Duration: 0} debugInstance := storeDebugInstanceForCleanup("disabled-debug", time.Now().Add(-3*time.Hour)) + debugInstance.Spec.Duration = metav1.Duration{Duration: 0} cl := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(ex, debugInstance). Build() execReconciler := StoreExecReconciler{Client: cl} - execResult, execHandled, err := execReconciler.reconcileSuccessfulStoreExecCleanup(ctx, ex) + execResult, execHandled, err := execReconciler.reconcileStoreExecCleanup(ctx, ex) require.NoError(t, err) assert.False(t, execHandled) assert.Equal(t, time.Duration(0), execResult.RequeueAfter) @@ -220,7 +285,7 @@ func storeExecForCleanup(name string, state shopv1.StatefulState, finishedAt tim State: state, Conditions: []shopv1.ExecCondition{ { - Type: shopv1.ExecStateRunning, + Type: state, LastTransitionTime: metav1.NewTime(finishedAt), LastUpdateTime: metav1.NewTime(finishedAt), Message: "Command finished", @@ -240,7 +305,7 @@ func storeDebugInstanceForCleanup(name string, creationTime time.Time) *shopv1.S }, Spec: shopv1.StoreDebugInstanceSpec{ StoreRef: "test", - Duration: "1h", + Duration: metav1.Duration{Duration: time.Hour}, }, Status: shopv1.StoreDebugInstanceStatus{ State: shopv1.StoreDebugInstanceStateDone, diff --git a/internal/util/labels.go b/internal/util/labels.go index aea62bc..a097717 100644 --- a/internal/util/labels.go +++ b/internal/util/labels.go @@ -3,7 +3,6 @@ package util import ( "fmt" "maps" - "time" v1 "github.com/shopware/shopware-operator/api/v1" ) @@ -52,9 +51,7 @@ func GetDefaultStoreSnapshotLabels(store v1.Store, overwrite map[string]string, func GetDefaultStoreInstanceDebugLabels(store v1.Store, storeDebugInstance v1.StoreDebugInstance) map[string]string { labels := GetDefaultContainerStoreLabels(store, storeDebugInstance.Spec.ExtraLabels) - // we don't need to check for errors here, because the duration is validated in the controller - duration, _ := time.ParseDuration(storeDebugInstance.Spec.Duration) - validUntil := storeDebugInstance.CreationTimestamp.Add(duration) + validUntil := storeDebugInstance.CreationTimestamp.Add(storeDebugInstance.Spec.Duration.Duration) labels[ShopwareKey("store.debug")] = "true" labels[ShopwareKey("store.debug.instance")] = storeDebugInstance.Name