From bec772b0ad2161f2701602b96a6752a7cdb3b257 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Tue, 11 Mar 2025 17:22:54 +0200 Subject: [PATCH 1/8] feat: add DevWorkspace pruner Signed-off-by: Oleksii Kurinnyi --- .../devworkspaceoperatorconfig_types.go | 25 ++ controllers/pruner/pruner_controller.go | 338 ++++++++++++++++++ go.mod | 2 + go.sum | 9 + main.go | 9 + pkg/cache/cache.go | 7 + pkg/config/defaults.go | 6 + pkg/config/sync.go | 32 ++ pkg/constants/metadata.go | 4 + 9 files changed, 432 insertions(+) create mode 100644 controllers/pruner/pruner_controller.go diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 04bd20dc4..19395ee15 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -49,6 +49,29 @@ type OperatorConfiguration struct { EnableExperimentalFeatures *bool `json:"enableExperimentalFeatures,omitempty"` } +type CleanupCronJobConfig struct { + // Enable determines whether the cleanup cron job is enabled. + // Defaults to false if not specified. + // +kubebuilder:validation:Optional + Enable *bool `json:"enable,omitempty"` + // RetainTime specifies the minimum time (in seconds) since a DevWorkspace was last started before it is considered stale and eligible for cleanup. + // For example, a value of 2592000 (30 days) would mean that any DevWorkspace that has not been started in the last 30 days will be deleted. + // Defaults to 2592000 seconds (30 days) if not specified. + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:default:=2592000 + // +kubebuilder:validation:Optional + RetainTime *int32 `json:"retainTime,omitempty"` + // DryRun determines whether the cleanup cron job should be run in dry-run mode. + // If set to true, the cron job will not delete any DevWorkspaces, but will log the DevWorkspaces that would have been deleted. + // Defaults to false if not specified. + // +kubebuilder:validation:Optional + DryRun *bool `json:"dryRun,omitempty"` + // Schedule specifies the cron schedule for the cleanup cron job. + // +kubebuilder:default:="0 0 1 * *" + // +kubebuilder:validation:Optional + Schedule string `json:"schedule,omitempty"` +} + type RoutingConfig struct { // DefaultRoutingClass specifies the routingClass to be used when a DevWorkspace // specifies an empty `.spec.routingClass`. Supported routingClasses can be defined @@ -161,6 +184,8 @@ type WorkspaceConfig struct { PodAnnotations map[string]string `json:"podAnnotations,omitempty"` // RuntimeClassName defines the spec.runtimeClassName for DevWorkspace pods created by the DevWorkspace Operator. RuntimeClassName *string `json:"runtimeClassName,omitempty"` + // CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. + CleanupCronJob *CleanupCronJobConfig `json:"cleanupCronJob,omitempty"` } type WebhookConfig struct { diff --git a/controllers/pruner/pruner_controller.go b/controllers/pruner/pruner_controller.go new file mode 100644 index 000000000..ce01d0f65 --- /dev/null +++ b/controllers/pruner/pruner_controller.go @@ -0,0 +1,338 @@ +// +// Copyright (c) 2019-2024 Red Hat, Inc. +// 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 controllers + +import ( + "context" + "fmt" + "time" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/conditions" + "github.com/devfile/devworkspace-operator/pkg/config" + + "github.com/operator-framework/operator-lib/prune" + "github.com/robfig/cron/v3" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// DevWorkspacePrunerReconciler reconciles DevWorkspace objects for pruning purposes. +type DevWorkspacePrunerReconciler struct { + client.Client + Log logr.Logger + Scheme *runtime.Scheme + + cron *cron.Cron +} + +func shouldReconcileOnUpdate(e event.UpdateEvent, log logr.Logger) bool { + log.Info("DevWorkspaceOperatorConfig update event received") + oldConfig, ok := e.ObjectOld.(*controllerv1alpha1.DevWorkspaceOperatorConfig) + if !ok { + return false + } + newConfig, ok := e.ObjectNew.(*controllerv1alpha1.DevWorkspaceOperatorConfig) + if !ok { + return false + } + + oldCleanup := oldConfig.Config.Workspace.CleanupCronJob + newCleanup := newConfig.Config.Workspace.CleanupCronJob + + differentBool := func(a, b *bool) bool { + switch { + case a == nil && b == nil: + return false + case a == nil || b == nil: + return true + default: + return *a != *b + } + } + differentInt32 := func(a, b *int32) bool { + switch { + case a == nil && b == nil: + return false + case a == nil || b == nil: + return true + default: + return *a != *b + } + } + + if oldCleanup == nil && newCleanup == nil { + return false + } + if (oldCleanup == nil && newCleanup != nil) || (oldCleanup != nil && newCleanup == nil) { + return true + } + if differentBool(oldCleanup.Enable, newCleanup.Enable) { + return true + } + if differentBool(oldCleanup.DryRun, newCleanup.DryRun) { + return true + } + if differentInt32(oldCleanup.RetainTime, newCleanup.RetainTime) { + return true + } + return oldCleanup.Schedule != newCleanup.Schedule +} + +// SetupWithManager sets up the controller with the Manager. +func (r *DevWorkspacePrunerReconciler) SetupWithManager(mgr ctrl.Manager) error { + log := r.Log.WithName("setupWithManager") + log.Info("Setting up DevWorkspacePrunerReconciler") + + configPredicate := predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + return shouldReconcileOnUpdate(e, log) + }, + CreateFunc: func(e event.CreateEvent) bool { return true }, + DeleteFunc: func(e event.DeleteEvent) bool { return false }, + GenericFunc: func(e event.GenericEvent) bool { return false }, + } + + maxConcurrentReconciles, err := config.GetMaxConcurrentReconciles() + if err != nil { + return err + } + + // Initialize cron scheduler + r.cron = cron.New() + + return ctrl.NewControllerManagedBy(mgr). + WithOptions(controller.Options{MaxConcurrentReconciles: maxConcurrentReconciles}). + For(&controllerv1alpha1.DevWorkspaceOperatorConfig{}). + WithEventFilter(configPredicate). + Complete(r) +} + +// +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list;delete +// +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;watch + +// Reconcile is the main reconciliation loop for the pruner controller. +func (r *DevWorkspacePrunerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := r.Log + log.Info("Reconciling DevWorkspacePruner", "DWOC", req.NamespacedName) + + dwOperatorConfig := &controllerv1alpha1.DevWorkspaceOperatorConfig{} + err := r.Get(ctx, req.NamespacedName, dwOperatorConfig) + if err != nil { + log.Error(err, "Failed to get DevWorkspaceOperatorConfig") + r.stopCron(log) + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + cleanupConfig := dwOperatorConfig.Config.Workspace.CleanupCronJob + log = log.WithValues("CleanupCronJob", cleanupConfig) + + if cleanupConfig == nil { + log.Info("DevWorkspaceOperatorConfig does not have cleanup configuration, stopping cron schedler and skipping reconciliation") + r.stopCron(log) + return ctrl.Result{}, nil + } + if cleanupConfig.Enable == nil || !*cleanupConfig.Enable { + log.Info("DevWorkspace pruning is disabled, stopping cron scheduler and skipping reconciliation") + r.stopCron(log) + return ctrl.Result{}, nil + } + if cleanupConfig.Schedule == "" { + log.Info("DevWorkspace pruning schedule is not defined, stopping cron scheduler and skipping reconciliation") + r.stopCron(log) + return ctrl.Result{}, nil + } + + r.startCron(ctx, cleanupConfig, log) + + return ctrl.Result{}, nil +} + +func (r *DevWorkspacePrunerReconciler) startCron(ctx context.Context, cleanupConfig *controllerv1alpha1.CleanupCronJobConfig, logger logr.Logger) { + log := logger.WithName("cron") + log.Info("Starting cron scheduler") + + // remove existing cronjob tasks + // we cannot update the existing tasks, so we need to remove them and add new ones + entries := r.cron.Entries() + for _, entry := range entries { + r.cron.Remove(entry.ID) + } + + // add cronjob task + _, err := r.cron.AddFunc(cleanupConfig.Schedule, func() { + taskLog := logger.WithName("cronTask") + + // define pruning parameters + retainTime := time.Duration(*cleanupConfig.RetainTime) * time.Second + + dryRun := false + if cleanupConfig.DryRun != nil { + dryRun = *cleanupConfig.DryRun + } + + taskLog.Info("Starting DevWorkspace pruning job") + if err := r.pruneDevWorkspaces(ctx, retainTime, dryRun, logger); err != nil { + taskLog.Error(err, "Failed to prune DevWorkspaces") + } + taskLog.Info("DevWorkspace pruning job finished") + }) + if err != nil { + log.Error(err, "Failed to add cronjob function") + return + } + + r.cron.Start() +} + +func (r *DevWorkspacePrunerReconciler) stopCron(logger logr.Logger) { + log := logger.WithName("cron") + log.Info("Stopping cron scheduler") + + // remove existing cronjob tasks + entries := r.cron.Entries() + for _, entry := range entries { + r.cron.Remove(entry.ID) + } + + ctx := r.cron.Stop() + ctx.Done() + + log.Info("Cron scheduler stopped") +} + +func (r *DevWorkspacePrunerReconciler) pruneDevWorkspaces(ctx context.Context, retainTime time.Duration, dryRun bool, logger logr.Logger) error { + log := logger.WithName("pruner") + + // create a prune strategy based on the configuration + var pruneStrategy prune.StrategyFunc + if dryRun { + pruneStrategy = r.dryRunPruneStrategy(retainTime, log) + } else { + pruneStrategy = r.pruneStrategy(retainTime, log) + } + + gvk := schema.GroupVersionKind{ + Group: dwv2.SchemeGroupVersion.Group, + Version: dwv2.SchemeGroupVersion.Version, + Kind: "DevWorkspace", + } + + // create pruner that uses our custom strategy + pruner, err := prune.NewPruner(r.Client, gvk, pruneStrategy) + if err != nil { + return fmt.Errorf("failed to create pruner: %w", err) + } + + deletedObjects, err := pruner.Prune(ctx) + if err != nil { + return fmt.Errorf("failed to prune objects: %w", err) + } + log.Info(fmt.Sprintf("Pruned %d DevWorkspaces", len(deletedObjects))) + + for _, obj := range deletedObjects { + devWorkspace, ok := obj.(*dwv2.DevWorkspace) + if !ok { + log.Error(err, fmt.Sprintf("failed to convert %v to DevWorkspace", obj)) + continue + } + log.Info(fmt.Sprintf("Pruned DevWorkspace '%s' in namespace '%s'", devWorkspace.Name, devWorkspace.Namespace)) + } + + return nil +} + +// pruneStrategy returns a StrategyFunc that will return a list of +// DevWorkspaces to prune based on the lastTransitionTime of the 'Started' condition. +func (r *DevWorkspacePrunerReconciler) pruneStrategy(retainTime time.Duration, logger logr.Logger) prune.StrategyFunc { + log := logger.WithName("pruneStrategy") + + return func(ctx context.Context, objs []client.Object) ([]client.Object, error) { + filteredObjs := filterByInactivityTime(objs, retainTime, log) + log.Info(fmt.Sprintf("Found %d DevWorkspaces to prune", len(filteredObjs))) + return filteredObjs, nil + } +} + +// dryRunPruneStrategy returns a StrategyFunc that will always return an empty list of DevWorkspaces to prune. +// This is used for dry-run mode. +func (r *DevWorkspacePrunerReconciler) dryRunPruneStrategy(retainTime time.Duration, logger logr.Logger) prune.StrategyFunc { + log := logger.WithName("dryRunPruneStrategy") + + return func(ctx context.Context, objs []client.Object) ([]client.Object, error) { + filteredObjs := filterByInactivityTime(objs, retainTime, log) + log.Info(fmt.Sprintf("Found %d DevWorkspaces to prune", len(filteredObjs))) + + // Return an empty list of DevWorkspaces because this is a dry-run + return []client.Object{}, nil + } +} + +// filterByInactivityTime filters DevWorkspaces based on the lastTransitionTime of the 'Started' condition. +func filterByInactivityTime(objs []client.Object, retainTime time.Duration, log logr.Logger) []client.Object { + var filteredObjs []client.Object + for _, obj := range objs { + devWorkspace, ok := obj.(*dwv2.DevWorkspace) + if !ok { + log.Error(nil, fmt.Sprintf("failed to convert %v to DevWorkspace", obj)) + continue + } + + if canPrune(*devWorkspace, retainTime, log) { + filteredObjs = append(filteredObjs, devWorkspace) + log.Info(fmt.Sprintf("Adding DevWorkspace '%s/%s' to prune list", devWorkspace.Namespace, devWorkspace.Name)) + } else { + log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': not eligible for pruning", devWorkspace.Namespace, devWorkspace.Name)) + } + } + return filteredObjs +} + +// canPrune returns true if the DevWorkspace is eligible for pruning. +func canPrune(dw dwv2.DevWorkspace, retainTime time.Duration, log logr.Logger) bool { + // Skip started and running DevWorkspaces + if dw.Spec.Started { + log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': already started", dw.Namespace, dw.Name)) + return false + } + + var startTime *metav1.Time + for _, condition := range dw.Status.Conditions { + if condition.Type == conditions.Started { + startTime = &condition.LastTransitionTime + break + } + } + if startTime == nil { + log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': missing 'Started' condition", dw.Namespace, dw.Name)) + return false + } + if time.Since(startTime.Time) <= retainTime { + log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': not eligible for pruning", dw.Namespace, dw.Name)) + return false + } + return true +} diff --git a/go.mod b/go.mod index c3fe57402..f11d0ea86 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,9 @@ require ( github.com/onsi/ginkgo/v2 v2.19.0 github.com/onsi/gomega v1.34.1 github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87 + github.com/operator-framework/operator-lib v0.11.0 github.com/prometheus/client_golang v1.14.0 + github.com/robfig/cron/v3 v3.0.0 github.com/stretchr/testify v1.10.0 golang.org/x/crypto v0.31.0 golang.org/x/net v0.33.0 diff --git a/go.sum b/go.sum index 50bcaa183..c35a9c03b 100644 --- a/go.sum +++ b/go.sum @@ -298,9 +298,13 @@ github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRW github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= +github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= +github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/onsi/ginkgo v0.0.0-20170829012221-11459a886d9c/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.10.1/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= +github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= +github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/ginkgo/v2 v2.19.0 h1:9Cnnf7UHo57Hy3k6/m5k3dRfGTMXGvxhHFvkDTCTpvA= github.com/onsi/ginkgo/v2 v2.19.0/go.mod h1:rlwLi9PilAFJ8jCg9UE1QP6VBpd6/xj3SRC0d6TU0To= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= @@ -309,6 +313,8 @@ github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k= github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY= github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87 h1:L/fZlWB7DdYCd09r9LvBa44xRH42Dx80ybxfN1h5C8Y= github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87/go.mod h1:fT6U/JfG8uZzemTRwZA2kBDJP5nWz7v05UHnty/D+pk= +github.com/operator-framework/operator-lib v0.11.0 h1:eYzqpiOfq9WBI4Trddisiq/X9BwCisZd3rIzmHRC9Z8= +github.com/operator-framework/operator-lib v0.11.0/go.mod h1:RpyKhFAoG6DmKTDIwMuO6pI3LRc8IE9rxEYWy476o6g= github.com/pjbgf/sha1cd v0.3.0 h1:4D5XXmUUBUl/xQ6IjCkEAbqXskkq/4O7LmGn0AqMDs4= github.com/pjbgf/sha1cd v0.3.0/go.mod h1:nZ1rrWOcGJ5uZgEEVL1VUM9iRQiZvWdbZjkKyFzPPsI= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -345,6 +351,8 @@ github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1 github.com/prometheus/procfs v0.8.0 h1:ODq8ZFEaYeCaZOJlZZdJA2AbQR98dSHSM1KW/You5mo= github.com/prometheus/procfs v0.8.0/go.mod h1:z7EfXMXOkbkqb9IINtpCn86r/to3BnA0uaxHdg830/4= github.com/remyoudompheng/bigfft v0.0.0-20170806203942-52369c62f446/go.mod h1:uYEyJGbgTkfkS4+E/PavXkNJcbFIpEtjt2B0KDQ5+9M= +github.com/robfig/cron/v3 v3.0.0 h1:kQ6Cb7aHOHTSzNVNEhmp8EcWKLb4CbiMW9h9VyIhO4E= +github.com/robfig/cron/v3 v3.0.0/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= @@ -716,6 +724,7 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= +gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME= gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI= diff --git a/main.go b/main.go index 2984f4f1f..d50a515d6 100644 --- a/main.go +++ b/main.go @@ -35,6 +35,7 @@ import ( dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + prunercontroller "github.com/devfile/devworkspace-operator/controllers/pruner" workspacecontroller "github.com/devfile/devworkspace-operator/controllers/workspace" configv1 "github.com/openshift/api/config/v1" @@ -169,6 +170,14 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "DevWorkspace") os.Exit(1) } + if err = (&prunercontroller.DevWorkspacePrunerReconciler{ + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("DevWorkspacePruner"), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "DevWorkspacePruner") + os.Exit(1) + } // +kubebuilder:scaffold:builder // Get a config to talk to the apiserver diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index a29e6d868..9521d0122 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -46,6 +46,10 @@ func GetCacheFunc() (cache.NewCacheFunc, error) { if err != nil { return nil, err } + cronJobObjectSelector, err := labels.Parse(fmt.Sprintf("%s=true", constants.DevWorkspaceWatchCronJobLabel)) + if err != nil { + return nil, err + } rbacObjectSelector, err := labels.Parse("controller.devfile.io/workspace-rbac=true") if err != nil { return nil, err @@ -70,6 +74,9 @@ func GetCacheFunc() (cache.NewCacheFunc, error) { &networkingv1.Ingress{}: { Label: devworkspaceObjectSelector, }, + &batchv1.CronJob{}: { + Label: cronJobObjectSelector, + }, &corev1.ConfigMap{}: { Label: configmapObjectSelector, }, diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 5e1c2eba0..9247f0bbc 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -76,6 +76,12 @@ var defaultConfig = &v1alpha1.OperatorConfiguration{ corev1.ResourceMemory: resource.MustParse("64Mi"), }, }, + CleanupCronJob: &v1alpha1.CleanupCronJobConfig{ + Enable: pointer.Bool(false), + DryRun: pointer.Bool(false), + RetainTime: pointer.Int32(2592000), + Schedule: "0 0 1 * *", + }, }, } diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 483c820ec..9f438e555 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -410,6 +410,24 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { to.Workspace.PodAnnotations[key] = value } } + + if from.Workspace.CleanupCronJob != nil { + if to.Workspace.CleanupCronJob == nil { + to.Workspace.CleanupCronJob = &controller.CleanupCronJobConfig{} + } + if from.Workspace.CleanupCronJob.Enable != nil { + to.Workspace.CleanupCronJob.Enable = from.Workspace.CleanupCronJob.Enable + } + if from.Workspace.CleanupCronJob.DryRun != nil { + to.Workspace.CleanupCronJob.DryRun = from.Workspace.CleanupCronJob.DryRun + } + if from.Workspace.CleanupCronJob.RetainTime != nil { + to.Workspace.CleanupCronJob.RetainTime = from.Workspace.CleanupCronJob.RetainTime + } + if from.Workspace.CleanupCronJob.Schedule != "" { + to.Workspace.CleanupCronJob.Schedule = from.Workspace.CleanupCronJob.Schedule + } + } } } @@ -638,6 +656,20 @@ func GetCurrentConfigString(currConfig *controller.OperatorConfiguration) string if !reflect.DeepEqual(workspace.PodAnnotations, defaultConfig.Workspace.PodAnnotations) { config = append(config, "workspace.podAnnotations is set") } + if workspace.CleanupCronJob != nil { + if workspace.CleanupCronJob.Enable != nil && *workspace.CleanupCronJob.Enable != *defaultConfig.Workspace.CleanupCronJob.Enable { + config = append(config, fmt.Sprintf("workspace.cleanupCronJob.enable=%t", *workspace.CleanupCronJob.Enable)) + } + if workspace.CleanupCronJob.DryRun != nil && *workspace.CleanupCronJob.DryRun != *defaultConfig.Workspace.CleanupCronJob.DryRun { + config = append(config, fmt.Sprintf("workspace.cleanupCronJob.dryRun=%t", *workspace.CleanupCronJob.DryRun)) + } + if workspace.CleanupCronJob.RetainTime != nil && *workspace.CleanupCronJob.RetainTime != *defaultConfig.Workspace.CleanupCronJob.RetainTime { + config = append(config, fmt.Sprintf("workspace.cleanupCronJob.retainTime=%d", *workspace.CleanupCronJob.RetainTime)) + } + if workspace.CleanupCronJob.Schedule != defaultConfig.Workspace.CleanupCronJob.Schedule { + config = append(config, fmt.Sprintf("workspace.cleanupCronJob.cronJobScript=%s", workspace.CleanupCronJob.Schedule)) + } + } } if currConfig.EnableExperimentalFeatures != nil && *currConfig.EnableExperimentalFeatures { config = append(config, "enableExperimentalFeatures=true") diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 20ffd09a6..6715e15d6 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -34,6 +34,10 @@ const ( // DevWorkspaceNameLabel is the label key to store workspace name DevWorkspaceNameLabel = "controller.devfile.io/devworkspace_name" + // DevWorkspaceWatchCronJobLabel marks a cronjob so that it is watched by the controller. This label is required on all + // cronjobs that should be seen by the controller + DevWorkspaceWatchCronJobLabel = "controller.devfile.io/watch-cronjob" + // DevWorkspaceWatchConfigMapLabel marks a configmap so that it is watched by the controller. This label is required on all // configmaps that should be seen by the controller DevWorkspaceWatchConfigMapLabel = "controller.devfile.io/watch-configmap" From 48475a81510e9c55390cbf5c66c69907337a69a3 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Tue, 11 Mar 2025 17:24:42 +0200 Subject: [PATCH 2/8] chore: run make update_devworkspace_api update_devworkspace_crds generate_all Signed-off-by: Oleksii Kurinnyi --- .../v1alpha1/zz_generated.deepcopy.go | 35 ++++++++++++++ ...evfile.io_devworkspaceoperatorconfigs.yaml | 28 +++++++++++ ...kspace-operator.clusterserviceversion.yaml | 16 +++++++ deploy/deployment/kubernetes/combined.yaml | 46 +++++++++++++++++++ ...workspace-controller-role.ClusterRole.yaml | 16 +++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 30 ++++++++++++ deploy/deployment/openshift/combined.yaml | 46 +++++++++++++++++++ ...workspace-controller-role.ClusterRole.yaml | 16 +++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 30 ++++++++++++ deploy/templates/components/rbac/role.yaml | 16 +++++++ ...evfile.io_devworkspaceoperatorconfigs.yaml | 30 ++++++++++++ 11 files changed, 309 insertions(+) diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index 687b2aa2a..9381d38b4 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -46,6 +46,36 @@ func (in Attributes) DeepCopy() Attributes { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CleanupCronJobConfig) DeepCopyInto(out *CleanupCronJobConfig) { + *out = *in + if in.Enable != nil { + in, out := &in.Enable, &out.Enable + *out = new(bool) + **out = **in + } + if in.RetainTime != nil { + in, out := &in.RetainTime, &out.RetainTime + *out = new(int32) + **out = **in + } + if in.DryRun != nil { + in, out := &in.DryRun, &out.DryRun + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CleanupCronJobConfig. +func (in *CleanupCronJobConfig) DeepCopy() *CleanupCronJobConfig { + if in == nil { + return nil + } + out := new(CleanupCronJobConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConfigmapReference) DeepCopyInto(out *ConfigmapReference) { *out = *in @@ -748,6 +778,11 @@ func (in *WorkspaceConfig) DeepCopyInto(out *WorkspaceConfig) { *out = new(string) **out = **in } + if in.CleanupCronJob != nil { + in, out := &in.CleanupCronJob, &out.CleanupCronJob + *out = new(CleanupCronJobConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkspaceConfig. diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 705998fb8..114d477af 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -185,6 +185,34 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + cleanupCronJob: + description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. + properties: + dryRun: + description: |- + DryRun determines whether the cleanup cron job should be run in dry-run mode. + If set to true, the cron job will not delete any DevWorkspaces, but will log the DevWorkspaces that would have been deleted. + Defaults to false if not specified. + type: boolean + enable: + description: |- + Enable determines whether the cleanup cron job is enabled. + Defaults to false if not specified. + type: boolean + retainTime: + default: 2592000 + description: |- + RetainTime specifies the minimum time (in seconds) since a DevWorkspace was last started before it is considered stale and eligible for cleanup. + For example, a value of 2592000 (30 days) would mean that any DevWorkspace that has not been started in the last 30 days will be deleted. + Defaults to 2592000 seconds (30 days) if not specified. + format: int32 + minimum: 0 + type: integer + schedule: + default: 0 0 1 * * + description: Schedule specifies the cron schedule for the cleanup cron job. + type: string + type: object cleanupOnStop: description: |- CleanupOnStop governs how the Operator handles stopped DevWorkspaces. If set to diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index a3e65beaf..4ad37dffb 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -216,6 +216,14 @@ spec: - '*' verbs: - '*' + - apiGroups: + - controller.devfile.io + resources: + - devworkspaceoperatorconfigs + verbs: + - get + - list + - watch - apiGroups: - controller.devfile.io resources: @@ -313,6 +321,14 @@ spec: - '*' verbs: - '*' + - apiGroups: + - workspace.devfile.io + resources: + - devworkspaces + verbs: + - delete + - get + - list serviceAccountName: devworkspace-controller-serviceaccount deployments: - name: devworkspace-controller-manager diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index d44882e7b..d47eba2e4 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -186,6 +186,36 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + cleanupCronJob: + description: CleanupCronJobConfig defines configuration options + for a cron job that automatically cleans up stale DevWorkspaces. + properties: + dryRun: + description: |- + DryRun determines whether the cleanup cron job should be run in dry-run mode. + If set to true, the cron job will not delete any DevWorkspaces, but will log the DevWorkspaces that would have been deleted. + Defaults to false if not specified. + type: boolean + enable: + description: |- + Enable determines whether the cleanup cron job is enabled. + Defaults to false if not specified. + type: boolean + retainTime: + default: 2592000 + description: |- + RetainTime specifies the minimum time (in seconds) since a DevWorkspace was last started before it is considered stale and eligible for cleanup. + For example, a value of 2592000 (30 days) would mean that any DevWorkspace that has not been started in the last 30 days will be deleted. + Defaults to 2592000 seconds (30 days) if not specified. + format: int32 + minimum: 0 + type: integer + schedule: + default: 0 0 1 * * + description: Schedule specifies the cron schedule for the + cleanup cron job. + type: string + type: object cleanupOnStop: description: |- CleanupOnStop governs how the Operator handles stopped DevWorkspaces. If set to @@ -25071,6 +25101,14 @@ rules: - '*' verbs: - '*' +- apiGroups: + - controller.devfile.io + resources: + - devworkspaceoperatorconfigs + verbs: + - get + - list + - watch - apiGroups: - controller.devfile.io resources: @@ -25168,6 +25206,14 @@ rules: - '*' verbs: - '*' +- apiGroups: + - workspace.devfile.io + resources: + - devworkspaces + verbs: + - delete + - get + - list --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index e9f2e742f..50ebea935 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -139,6 +139,14 @@ rules: - '*' verbs: - '*' +- apiGroups: + - controller.devfile.io + resources: + - devworkspaceoperatorconfigs + verbs: + - get + - list + - watch - apiGroups: - controller.devfile.io resources: @@ -236,3 +244,11 @@ rules: - '*' verbs: - '*' +- apiGroups: + - workspace.devfile.io + resources: + - devworkspaces + verbs: + - delete + - get + - list diff --git a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index ed68ed014..506c2bcea 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -186,6 +186,36 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + cleanupCronJob: + description: CleanupCronJobConfig defines configuration options + for a cron job that automatically cleans up stale DevWorkspaces. + properties: + dryRun: + description: |- + DryRun determines whether the cleanup cron job should be run in dry-run mode. + If set to true, the cron job will not delete any DevWorkspaces, but will log the DevWorkspaces that would have been deleted. + Defaults to false if not specified. + type: boolean + enable: + description: |- + Enable determines whether the cleanup cron job is enabled. + Defaults to false if not specified. + type: boolean + retainTime: + default: 2592000 + description: |- + RetainTime specifies the minimum time (in seconds) since a DevWorkspace was last started before it is considered stale and eligible for cleanup. + For example, a value of 2592000 (30 days) would mean that any DevWorkspace that has not been started in the last 30 days will be deleted. + Defaults to 2592000 seconds (30 days) if not specified. + format: int32 + minimum: 0 + type: integer + schedule: + default: 0 0 1 * * + description: Schedule specifies the cron schedule for the + cleanup cron job. + type: string + type: object cleanupOnStop: description: |- CleanupOnStop governs how the Operator handles stopped DevWorkspaces. If set to diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 58edcf702..77ce338b3 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -186,6 +186,36 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + cleanupCronJob: + description: CleanupCronJobConfig defines configuration options + for a cron job that automatically cleans up stale DevWorkspaces. + properties: + dryRun: + description: |- + DryRun determines whether the cleanup cron job should be run in dry-run mode. + If set to true, the cron job will not delete any DevWorkspaces, but will log the DevWorkspaces that would have been deleted. + Defaults to false if not specified. + type: boolean + enable: + description: |- + Enable determines whether the cleanup cron job is enabled. + Defaults to false if not specified. + type: boolean + retainTime: + default: 2592000 + description: |- + RetainTime specifies the minimum time (in seconds) since a DevWorkspace was last started before it is considered stale and eligible for cleanup. + For example, a value of 2592000 (30 days) would mean that any DevWorkspace that has not been started in the last 30 days will be deleted. + Defaults to 2592000 seconds (30 days) if not specified. + format: int32 + minimum: 0 + type: integer + schedule: + default: 0 0 1 * * + description: Schedule specifies the cron schedule for the + cleanup cron job. + type: string + type: object cleanupOnStop: description: |- CleanupOnStop governs how the Operator handles stopped DevWorkspaces. If set to @@ -25071,6 +25101,14 @@ rules: - '*' verbs: - '*' +- apiGroups: + - controller.devfile.io + resources: + - devworkspaceoperatorconfigs + verbs: + - get + - list + - watch - apiGroups: - controller.devfile.io resources: @@ -25168,6 +25206,14 @@ rules: - '*' verbs: - '*' +- apiGroups: + - workspace.devfile.io + resources: + - devworkspaces + verbs: + - delete + - get + - list --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index e9f2e742f..50ebea935 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -139,6 +139,14 @@ rules: - '*' verbs: - '*' +- apiGroups: + - controller.devfile.io + resources: + - devworkspaceoperatorconfigs + verbs: + - get + - list + - watch - apiGroups: - controller.devfile.io resources: @@ -236,3 +244,11 @@ rules: - '*' verbs: - '*' +- apiGroups: + - workspace.devfile.io + resources: + - devworkspaces + verbs: + - delete + - get + - list diff --git a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index ed68ed014..506c2bcea 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -186,6 +186,36 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + cleanupCronJob: + description: CleanupCronJobConfig defines configuration options + for a cron job that automatically cleans up stale DevWorkspaces. + properties: + dryRun: + description: |- + DryRun determines whether the cleanup cron job should be run in dry-run mode. + If set to true, the cron job will not delete any DevWorkspaces, but will log the DevWorkspaces that would have been deleted. + Defaults to false if not specified. + type: boolean + enable: + description: |- + Enable determines whether the cleanup cron job is enabled. + Defaults to false if not specified. + type: boolean + retainTime: + default: 2592000 + description: |- + RetainTime specifies the minimum time (in seconds) since a DevWorkspace was last started before it is considered stale and eligible for cleanup. + For example, a value of 2592000 (30 days) would mean that any DevWorkspace that has not been started in the last 30 days will be deleted. + Defaults to 2592000 seconds (30 days) if not specified. + format: int32 + minimum: 0 + type: integer + schedule: + default: 0 0 1 * * + description: Schedule specifies the cron schedule for the + cleanup cron job. + type: string + type: object cleanupOnStop: description: |- CleanupOnStop governs how the Operator handles stopped DevWorkspaces. If set to diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index 73d0895e2..44428170a 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -137,6 +137,14 @@ rules: - '*' verbs: - '*' +- apiGroups: + - controller.devfile.io + resources: + - devworkspaceoperatorconfigs + verbs: + - get + - list + - watch - apiGroups: - controller.devfile.io resources: @@ -234,3 +242,11 @@ rules: - '*' verbs: - '*' +- apiGroups: + - workspace.devfile.io + resources: + - devworkspaces + verbs: + - delete + - get + - list diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index cbc5e9fe2..3a0c66fc5 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -184,6 +184,36 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + cleanupCronJob: + description: CleanupCronJobConfig defines configuration options + for a cron job that automatically cleans up stale DevWorkspaces. + properties: + dryRun: + description: |- + DryRun determines whether the cleanup cron job should be run in dry-run mode. + If set to true, the cron job will not delete any DevWorkspaces, but will log the DevWorkspaces that would have been deleted. + Defaults to false if not specified. + type: boolean + enable: + description: |- + Enable determines whether the cleanup cron job is enabled. + Defaults to false if not specified. + type: boolean + retainTime: + default: 2592000 + description: |- + RetainTime specifies the minimum time (in seconds) since a DevWorkspace was last started before it is considered stale and eligible for cleanup. + For example, a value of 2592000 (30 days) would mean that any DevWorkspace that has not been started in the last 30 days will be deleted. + Defaults to 2592000 seconds (30 days) if not specified. + format: int32 + minimum: 0 + type: integer + schedule: + default: 0 0 1 * * + description: Schedule specifies the cron schedule for the + cleanup cron job. + type: string + type: object cleanupOnStop: description: |- CleanupOnStop governs how the Operator handles stopped DevWorkspaces. If set to From cdbf38815a5dc9461e8e6a98bd1ae3d85299ac8d Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Wed, 19 Mar 2025 17:02:00 +0200 Subject: [PATCH 3/8] test: add unit tests for the pruner controller Signed-off-by: Oleksii Kurinnyi --- controllers/pruner/pruner_controller_test.go | 550 +++++++++++++++++++ controllers/pruner/suite_test.go | 26 + 2 files changed, 576 insertions(+) create mode 100644 controllers/pruner/pruner_controller_test.go create mode 100644 controllers/pruner/suite_test.go diff --git a/controllers/pruner/pruner_controller_test.go b/controllers/pruner/pruner_controller_test.go new file mode 100644 index 000000000..cb5219340 --- /dev/null +++ b/controllers/pruner/pruner_controller_test.go @@ -0,0 +1,550 @@ +// Copyright (c) 2019-2024 Red Hat, Inc. +// 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 controllers + +import ( + "context" + "time" + + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/robfig/cron/v3" + + 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" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/conditions" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +var _ = Describe("DevWorkspacePrunerReconciler", func() { + var ( + ctx context.Context + fakeClient client.Client + reconciler DevWorkspacePrunerReconciler + nameNamespace types.NamespacedName + log logr.Logger + ) + + BeforeEach(func() { + ctx = context.Background() + scheme := runtime.NewScheme() + Expect(controllerv1alpha1.AddToScheme(scheme)).To(Succeed()) + Expect(dwv2.AddToScheme(scheme)).To(Succeed()) + fakeClient = fake.NewClientBuilder().WithScheme(scheme).Build() + log = zap.New(zap.UseDevMode(true)).WithName("prunerController") + + reconciler = DevWorkspacePrunerReconciler{ + Client: fakeClient, + Log: log, + Scheme: scheme, + cron: cron.New(), + } + + nameNamespace = types.NamespacedName{ + Name: "devworkspace-operator-config", + Namespace: "devworkspace-controller", + } + }) + + AfterEach(func() { + reconciler.stopCron(log) // Ensure cron is stopped after each test + }) + + Context("Helper Functions", func() { + var ( + retainTime time.Duration + dw1, dw2, dw3 dwv2.DevWorkspace + ) + + BeforeEach(func() { + retainTime = 1 * time.Minute + + // Create DevWorkspaces + // dw1 is running + dw1 = *createDevWorkspace("dw1", "test-ns", true, metav1.Now()) + // dw2 is inactive for 2 minutes + dw2 = *createDevWorkspace("dw2", "test-ns", false, metav1.NewTime(time.Now().Add(-2*time.Minute))) + // dw3 was recently active + dw3 = *createDevWorkspace("dw3", "test-ns", false, metav1.NewTime(time.Now().Add(-30*time.Second))) + }) + + Describe("canPrune", func() { + It("Should return false if DevWorkspace is started", func() { + result := canPrune(dw1, retainTime, log) + Expect(result).To(BeFalse()) + }) + + It("Should return false if 'Started' condition is missing", func() { + dw4 := dwv2.DevWorkspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dw4", + Namespace: "test-ns", + }, + Spec: dwv2.DevWorkspaceSpec{ + Started: false, + }, + Status: dwv2.DevWorkspaceStatus{ + Conditions: []dwv2.DevWorkspaceCondition{}, // Empty conditions + }, + } + result := canPrune(dw4, retainTime, log) + Expect(result).To(BeFalse()) + }) + + It("Should return false if DevWorkspace was recently active", func() { + result := canPrune(dw3, retainTime, log) + Expect(result).To(BeFalse()) + }) + + It("Should return true if DevWorkspace is inactive", func() { + result := canPrune(dw2, retainTime, log) + Expect(result).To(BeTrue()) + }) + }) + + Describe("filterByInactivityTime", func() { + It("Should return only inactive DevWorkspaces", func() { + objs := []client.Object{ + &dw1, + &dw2, + &dw3, + } + filteredObjs := filterByInactivityTime(objs, retainTime, log) + Expect(filteredObjs).To(HaveLen(1)) + Expect(filteredObjs[0].GetName()).To(Equal("dw2")) + }) + }) + }) + + Context("Reconcile", func() { + It("Should do nothing if DevWorkspaceOperatorConfig is not found", func() { + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(BeEmpty()) + }) + + It("Should not start cron if CleanupCronJob is nil", func() { + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{}, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(BeEmpty()) + }) + + It("Should do not start cron if pruning is disabled", func() { + enabled := false + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: &enabled, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(BeEmpty()) + }) + + It("Should do not start cron if schedule is missing", func() { + enabled := true + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: &enabled, + Schedule: "", + }, + }, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(BeEmpty()) + }) + + It("Should start cron if pruning is enabled and schedule is defined", func() { + enabled := true + schedule := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(HaveLen(1)) + }) + + It("Should update cron schedule if DevWorkspaceOperatorConfig is updated", func() { + enabled := true + schedule1 := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: &enabled, + Schedule: schedule1, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(HaveLen(1)) + entryID := reconciler.cron.Entries()[0].ID + + schedule2 := "1 * * * *" + dwoc.Config.Workspace.CleanupCronJob.Schedule = schedule2 + Expect(fakeClient.Update(ctx, dwoc)).To(Succeed()) + + result, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(HaveLen(1)) + Expect(reconciler.cron.Entries()[0].ID).NotTo(Equal(entryID)) + }) + + It("Should stop cron if DevWorkspaceOperatorConfig is deleted", func() { + enabled := true + schedule := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(HaveLen(1)) + + Expect(fakeClient.Delete(ctx, dwoc)).To(Succeed()) + + result, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: nameNamespace}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()). + To(HaveLen(0)) + }) + }) + + Context("Prune DevWorkspaces", func() { + var ( + retainTime time.Duration + dryRun bool + dw1, dw2, dw3 *dwv2.DevWorkspace + ) + + BeforeEach(func() { + // Set up test parameters + retainTime = 1 * time.Minute + dryRun = false + + // Create DevWorkspaces + // dw1 is running + dw1 = createDevWorkspace("dw1", "test-ns", true, metav1.Now()) + // dw2 is inactive for 2 minutes + dw2 = createDevWorkspace("dw2", "test-ns", false, metav1.NewTime(time.Now().Add(-2*time.Minute))) + // dw3 was recently active + dw3 = createDevWorkspace("dw3", "test-ns", false, metav1.NewTime(time.Now().Add(-30*time.Second))) + + Expect(fakeClient.Create(ctx, dw1)).To(Succeed()) + Expect(fakeClient.Create(ctx, dw2)).To(Succeed()) + Expect(fakeClient.Create(ctx, dw3)).To(Succeed()) + }) + + AfterEach(func() { + Expect(fakeClient.Delete(ctx, dw1)).To(Succeed()) + // Check if dw2 exists before deleting + if err := fakeClient.Get(ctx, types.NamespacedName{Name: "dw2", Namespace: "test-ns"}, &dwv2.DevWorkspace{}); err == nil { + Expect(fakeClient.Delete(ctx, dw2)).To(Succeed()) + } + Expect(fakeClient.Delete(ctx, dw3)).To(Succeed()) + }) + + It("Should prune inactive DevWorkspaces", func() { + err := reconciler.pruneDevWorkspaces(ctx, retainTime, dryRun, log) + Expect(err).ToNot(HaveOccurred()) + + // Check if dw2 is deleted + err = fakeClient.Get(ctx, types.NamespacedName{Name: "dw2", Namespace: "test-ns"}, &dwv2.DevWorkspace{}) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + + // Check if dw1 and dw3 still exist + err = fakeClient.Get(ctx, types.NamespacedName{Name: "dw1", Namespace: "test-ns"}, &dwv2.DevWorkspace{}) + Expect(err).ToNot(HaveOccurred()) + + err = fakeClient.Get(ctx, types.NamespacedName{Name: "dw3", Namespace: "test-ns"}, &dwv2.DevWorkspace{}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("Should not prune any DevWorkspaces in dryRun mode", func() { + dryRun := true + err := reconciler.pruneDevWorkspaces(ctx, retainTime, dryRun, log) + Expect(err).ToNot(HaveOccurred()) + + // Check that all DevWorkspaces still exist + err = fakeClient.Get(ctx, types.NamespacedName{Name: "dw1", Namespace: "test-ns"}, &dwv2.DevWorkspace{}) + Expect(err).ToNot(HaveOccurred()) + + err = fakeClient.Get(ctx, types.NamespacedName{Name: "dw2", Namespace: "test-ns"}, &dwv2.DevWorkspace{}) + Expect(err).ToNot(HaveOccurred()) + + err = fakeClient.Get(ctx, types.NamespacedName{Name: "dw3", Namespace: "test-ns"}, &dwv2.DevWorkspace{}) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("Cron management functions", func() { + It("Should start cron", func() { + enabled := true + schedule := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + }, + }, + }, + } + + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + Expect(reconciler.cron.Entries()).To(BeEmpty()) + + reconciler.startCron(ctx, dwoc.Config.Workspace.CleanupCronJob, log) + + Expect(reconciler.cron.Entries()).To(HaveLen(1)) + }) + + It("Should stop cron", func() { + enabled := true + schedule := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + }, + }, + }, + } + + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + Expect(reconciler.cron.Entries()).To(BeEmpty()) + + reconciler.startCron(ctx, dwoc.Config.Workspace.CleanupCronJob, log) + + Expect(reconciler.cron.Entries()).To(HaveLen(1)) + + reconciler.stopCron(log) + + Expect(reconciler.cron.Entries()).To(BeEmpty()) + }) + }) +}) + +// Helper function to create a DevWorkspace +func createDevWorkspace(name, namespace string, started bool, lastTransitionTime metav1.Time) *dwv2.DevWorkspace { + dw := &dwv2.DevWorkspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: dwv2.DevWorkspaceSpec{ + Started: started, + }, + Status: dwv2.DevWorkspaceStatus{ + Conditions: []dwv2.DevWorkspaceCondition{}, + }, + } + + if !lastTransitionTime.IsZero() { + condition := dwv2.DevWorkspaceCondition{ + Type: conditions.Started, + Status: corev1.ConditionTrue, + LastTransitionTime: lastTransitionTime, + Reason: "Test", + Message: "Test", + } + if !started { + condition.Status = corev1.ConditionFalse + } + dw.Status.Conditions = append(dw.Status.Conditions, condition) + } + + return dw +} + +var _ = Describe("DevWorkspaceOperatorConfig UpdateFunc Tests", func() { + var configPredicate predicate.Funcs + + BeforeEach(func() { + configPredicate = predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + return shouldReconcileOnUpdate(e, zap.New(zap.UseDevMode(true))) + }, + } + }) + + DescribeTable("Testing UpdateFunc for cleanup configuration changes", + func(oldCleanup, newCleanup *controllerv1alpha1.CleanupCronJobConfig, expected bool) { + oldCfg := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: oldCleanup, + }, + }, + } + newCfg := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: newCleanup, + }, + }, + } + updateEvent := event.UpdateEvent{ + ObjectOld: oldCfg, + ObjectNew: newCfg, + } + result := configPredicate.Update(updateEvent) + Expect(result).To(Equal(expected)) + }, + + Entry("Both nil => no change", nil, nil, false), + Entry("OldCleanup==nil, NewCleanup!=nil => changed", nil, &controllerv1alpha1.CleanupCronJobConfig{}, true), + Entry("OldCleanup!=nil, NewCleanup==nil => changed", &controllerv1alpha1.CleanupCronJobConfig{}, nil, true), + Entry("OldCleanup.Enable==nil, NewCleanup.Enable==nil => no change", + &controllerv1alpha1.CleanupCronJobConfig{Enable: nil}, + &controllerv1alpha1.CleanupCronJobConfig{Enable: nil}, + false, + ), + Entry("OldCleanup.Enable==nil, NewCleanup.Enable!=nil => changed", + &controllerv1alpha1.CleanupCronJobConfig{Enable: nil}, + &controllerv1alpha1.CleanupCronJobConfig{Enable: pointer.Bool(true)}, + true, + ), + Entry("OldCleanup.Enable!=nil, NewCleanup.Enable==nil => changed", + &controllerv1alpha1.CleanupCronJobConfig{Enable: pointer.Bool(true)}, + &controllerv1alpha1.CleanupCronJobConfig{Enable: nil}, + true, + ), + Entry("Enable differs => changed", + &controllerv1alpha1.CleanupCronJobConfig{Enable: pointer.Bool(true)}, + &controllerv1alpha1.CleanupCronJobConfig{Enable: pointer.Bool(false)}, + true, + ), + Entry("OldCleanup.DryRun==nil, NewCleanup.DryRun==nil => no change", + &controllerv1alpha1.CleanupCronJobConfig{DryRun: nil}, + &controllerv1alpha1.CleanupCronJobConfig{DryRun: nil}, + false, + ), + Entry("OldCleanup.DryRun==nil, NewCleanup.DryRun!=nil => changed", + &controllerv1alpha1.CleanupCronJobConfig{DryRun: nil}, + &controllerv1alpha1.CleanupCronJobConfig{DryRun: pointer.Bool(true)}, + true, + ), + Entry("OldCleanup.DryRun!=nil, NewCleanup.DryRun==nil => changed", + &controllerv1alpha1.CleanupCronJobConfig{DryRun: pointer.Bool(true)}, + &controllerv1alpha1.CleanupCronJobConfig{DryRun: nil}, + true, + ), + Entry("DryRun differs => changed", + &controllerv1alpha1.CleanupCronJobConfig{DryRun: pointer.Bool(true)}, + &controllerv1alpha1.CleanupCronJobConfig{DryRun: pointer.Bool(false)}, + true, + ), + Entry("RetainTime differs => changed", + &controllerv1alpha1.CleanupCronJobConfig{RetainTime: pointer.Int32(1)}, + &controllerv1alpha1.CleanupCronJobConfig{RetainTime: pointer.Int32(2)}, + true, + ), + Entry("Schedule differs => changed", + &controllerv1alpha1.CleanupCronJobConfig{Schedule: "0 * * * *"}, + &controllerv1alpha1.CleanupCronJobConfig{Schedule: "1 * * * *"}, + true, + ), + Entry("All fields match => no change", + &controllerv1alpha1.CleanupCronJobConfig{ + Enable: pointer.Bool(true), + DryRun: pointer.Bool(false), + RetainTime: pointer.Int32(5), + Schedule: "0 * * * *", + }, + &controllerv1alpha1.CleanupCronJobConfig{ + Enable: pointer.Bool(true), + DryRun: pointer.Bool(false), + RetainTime: pointer.Int32(5), + Schedule: "0 * * * *", + }, + false, + ), + ) +}) diff --git a/controllers/pruner/suite_test.go b/controllers/pruner/suite_test.go new file mode 100644 index 000000000..60dd38c41 --- /dev/null +++ b/controllers/pruner/suite_test.go @@ -0,0 +1,26 @@ +// Copyright (c) 2019-2024 Red Hat, Inc. +// 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 controllers_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestPruner(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Pruner Suite") +} From 3218747c43587920b2e89960e9476aba9399d316 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Tue, 1 Apr 2025 12:02:23 +0300 Subject: [PATCH 4/8] fixup! feat: add DevWorkspace pruner Signed-off-by: Oleksii Kurinnyi --- controllers/pruner/pruner_controller.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/controllers/pruner/pruner_controller.go b/controllers/pruner/pruner_controller.go index ce01d0f65..79d09d6af 100644 --- a/controllers/pruner/pruner_controller.go +++ b/controllers/pruner/pruner_controller.go @@ -112,7 +112,7 @@ func (r *DevWorkspacePrunerReconciler) SetupWithManager(mgr ctrl.Manager) error return shouldReconcileOnUpdate(e, log) }, CreateFunc: func(e event.CreateEvent) bool { return true }, - DeleteFunc: func(e event.DeleteEvent) bool { return false }, + DeleteFunc: func(e event.DeleteEvent) bool { return true }, GenericFunc: func(e event.GenericEvent) bool { return false }, } @@ -287,6 +287,7 @@ func (r *DevWorkspacePrunerReconciler) dryRunPruneStrategy(retainTime time.Durat log.Info(fmt.Sprintf("Found %d DevWorkspaces to prune", len(filteredObjs))) // Return an empty list of DevWorkspaces because this is a dry-run + log.Info("Dry run mode: no DevWorkspaces will be pruned") return []client.Object{}, nil } } @@ -320,11 +321,9 @@ func canPrune(dw dwv2.DevWorkspace, retainTime time.Duration, log logr.Logger) b } var startTime *metav1.Time - for _, condition := range dw.Status.Conditions { - if condition.Type == conditions.Started { - startTime = &condition.LastTransitionTime - break - } + startedCondition := conditions.GetConditionByType(dw.Status.Conditions, conditions.Started) + if startedCondition != nil { + startTime = &startedCondition.LastTransitionTime } if startTime == nil { log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': missing 'Started' condition", dw.Namespace, dw.Name)) From f977dfbeea80f677aae7d997182234b83c41b7c0 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Mon, 14 Apr 2025 11:18:53 +0300 Subject: [PATCH 5/8] fix: accept events from global DWOC only Signed-off-by: Oleksii Kurinnyi --- controllers/pruner/pruner_controller.go | 26 +++++++++++++++++++- controllers/pruner/pruner_controller_test.go | 22 +++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/controllers/pruner/pruner_controller.go b/controllers/pruner/pruner_controller.go index 79d09d6af..2dd2f13dd 100644 --- a/controllers/pruner/pruner_controller.go +++ b/controllers/pruner/pruner_controller.go @@ -27,12 +27,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/source" dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/pkg/conditions" "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" "github.com/operator-framework/operator-lib/prune" "github.com/robfig/cron/v3" @@ -126,7 +129,28 @@ func (r *DevWorkspacePrunerReconciler) SetupWithManager(mgr ctrl.Manager) error return ctrl.NewControllerManagedBy(mgr). WithOptions(controller.Options{MaxConcurrentReconciles: maxConcurrentReconciles}). - For(&controllerv1alpha1.DevWorkspaceOperatorConfig{}). + Named("DevWorkspacePruner"). + Watches(&source.Kind{Type: &controllerv1alpha1.DevWorkspaceOperatorConfig{}}, + handler.EnqueueRequestsFromMapFunc(func(object client.Object) []ctrl.Request { + objectNamespace := object.GetNamespace() + operatorNamespace, err := infrastructure.GetNamespace() + + // Ignore events from other namespaces + if err != nil || objectNamespace != operatorNamespace { + log.Info("Received event from different namespace, ignoring", "namespace", objectNamespace) + return []ctrl.Request{} + } + + return []ctrl.Request{ + { + NamespacedName: client.ObjectKey{ + Name: object.GetName(), + Namespace: object.GetNamespace(), + }, + }, + } + }), + ). WithEventFilter(configPredicate). Complete(r) } diff --git a/controllers/pruner/pruner_controller_test.go b/controllers/pruner/pruner_controller_test.go index cb5219340..840530a31 100644 --- a/controllers/pruner/pruner_controller_test.go +++ b/controllers/pruner/pruner_controller_test.go @@ -148,6 +148,28 @@ var _ = Describe("DevWorkspacePrunerReconciler", func() { Expect(reconciler.cron.Entries()).To(BeEmpty()) }) + It("Should not start cron if received event from different namespace", func() { + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: "other-namespace"}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + CleanupCronJob: &controllerv1alpha1.CleanupCronJobConfig{ + Enable: pointer.Bool(true), + Schedule: "* * * * *", + }, + }, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{ + Name: nameNamespace.Name, + Namespace: nameNamespace.Namespace, + }}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(reconciler.cron.Entries()).To(BeEmpty()) + }) + It("Should not start cron if CleanupCronJob is nil", func() { dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, From 11916fbd43b0a685ab7f80a5a366ac9bad32acda Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Wed, 12 Mar 2025 09:55:45 +0200 Subject: [PATCH 6/8] fixes Signed-off-by: Oleksii Kurinnyi --- controllers/pruner/pruner_controller.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/controllers/pruner/pruner_controller.go b/controllers/pruner/pruner_controller.go index 2dd2f13dd..1ee9413d2 100644 --- a/controllers/pruner/pruner_controller.go +++ b/controllers/pruner/pruner_controller.go @@ -25,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -119,25 +118,18 @@ func (r *DevWorkspacePrunerReconciler) SetupWithManager(mgr ctrl.Manager) error GenericFunc: func(e event.GenericEvent) bool { return false }, } - maxConcurrentReconciles, err := config.GetMaxConcurrentReconciles() - if err != nil { - return err - } - // Initialize cron scheduler r.cron = cron.New() return ctrl.NewControllerManagedBy(mgr). - WithOptions(controller.Options{MaxConcurrentReconciles: maxConcurrentReconciles}). Named("DevWorkspacePruner"). Watches(&source.Kind{Type: &controllerv1alpha1.DevWorkspaceOperatorConfig{}}, handler.EnqueueRequestsFromMapFunc(func(object client.Object) []ctrl.Request { - objectNamespace := object.GetNamespace() operatorNamespace, err := infrastructure.GetNamespace() // Ignore events from other namespaces - if err != nil || objectNamespace != operatorNamespace { - log.Info("Received event from different namespace, ignoring", "namespace", objectNamespace) + if err != nil || object.GetNamespace() != operatorNamespace || object.GetName() != config.OperatorConfigName { + log.Info("Received event from different namespace, ignoring", "namespace", object.GetNamespace()) return []ctrl.Request{} } @@ -328,9 +320,6 @@ func filterByInactivityTime(objs []client.Object, retainTime time.Duration, log if canPrune(*devWorkspace, retainTime, log) { filteredObjs = append(filteredObjs, devWorkspace) - log.Info(fmt.Sprintf("Adding DevWorkspace '%s/%s' to prune list", devWorkspace.Namespace, devWorkspace.Name)) - } else { - log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': not eligible for pruning", devWorkspace.Namespace, devWorkspace.Name)) } } return filteredObjs @@ -354,8 +343,10 @@ func canPrune(dw dwv2.DevWorkspace, retainTime time.Duration, log logr.Logger) b return false } if time.Since(startTime.Time) <= retainTime { - log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': not eligible for pruning", dw.Namespace, dw.Name)) + log.Info(fmt.Sprintf("Skipping DevWorkspace '%s/%s': last transition time is within retain time", dw.Namespace, dw.Name)) return false } + + log.Info(fmt.Sprintf("DevWorkspace '%s/%s' is eligible for pruning", dw.Namespace, dw.Name)) return true } From d15691cdbe59e8870cfeddcaae236d24071bc70c Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Thu, 17 Apr 2025 15:52:37 +0300 Subject: [PATCH 7/8] fix: rename controller Signed-off-by: Oleksii Kurinnyi --- .../cleanupcronjob_controller.go} | 26 +++++++++---------- .../cleanupcronjob_controller_test.go} | 8 +++--- .../{pruner => cleanupcronjob}/suite_test.go | 4 +-- main.go | 8 +++--- 4 files changed, 23 insertions(+), 23 deletions(-) rename controllers/{pruner/pruner_controller.go => cleanupcronjob/cleanupcronjob_controller.go} (89%) rename controllers/{pruner/pruner_controller_test.go => cleanupcronjob/cleanupcronjob_controller_test.go} (98%) rename controllers/{pruner => cleanupcronjob}/suite_test.go (90%) diff --git a/controllers/pruner/pruner_controller.go b/controllers/cleanupcronjob/cleanupcronjob_controller.go similarity index 89% rename from controllers/pruner/pruner_controller.go rename to controllers/cleanupcronjob/cleanupcronjob_controller.go index 1ee9413d2..d56d5d1e5 100644 --- a/controllers/pruner/pruner_controller.go +++ b/controllers/cleanupcronjob/cleanupcronjob_controller.go @@ -42,8 +42,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// DevWorkspacePrunerReconciler reconciles DevWorkspace objects for pruning purposes. -type DevWorkspacePrunerReconciler struct { +// CleanupCronJobReconciler reconciles `CleanupCronJob` configuration for the purpose of pruning stale DevWorkspaces. +type CleanupCronJobReconciler struct { client.Client Log logr.Logger Scheme *runtime.Scheme @@ -105,9 +105,9 @@ func shouldReconcileOnUpdate(e event.UpdateEvent, log logr.Logger) bool { } // SetupWithManager sets up the controller with the Manager. -func (r *DevWorkspacePrunerReconciler) SetupWithManager(mgr ctrl.Manager) error { +func (r *CleanupCronJobReconciler) SetupWithManager(mgr ctrl.Manager) error { log := r.Log.WithName("setupWithManager") - log.Info("Setting up DevWorkspacePrunerReconciler") + log.Info("Setting up CleanupCronJobReconciler") configPredicate := predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { @@ -122,7 +122,7 @@ func (r *DevWorkspacePrunerReconciler) SetupWithManager(mgr ctrl.Manager) error r.cron = cron.New() return ctrl.NewControllerManagedBy(mgr). - Named("DevWorkspacePruner"). + Named("CleanupCronJob"). Watches(&source.Kind{Type: &controllerv1alpha1.DevWorkspaceOperatorConfig{}}, handler.EnqueueRequestsFromMapFunc(func(object client.Object) []ctrl.Request { operatorNamespace, err := infrastructure.GetNamespace() @@ -150,10 +150,10 @@ func (r *DevWorkspacePrunerReconciler) SetupWithManager(mgr ctrl.Manager) error // +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list;delete // +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;watch -// Reconcile is the main reconciliation loop for the pruner controller. -func (r *DevWorkspacePrunerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +// Reconcile is the main reconciliation loop for the CleanupCronJob controller. +func (r *CleanupCronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := r.Log - log.Info("Reconciling DevWorkspacePruner", "DWOC", req.NamespacedName) + log.Info("Reconciling CleanupCronJob", "DWOC", req.NamespacedName) dwOperatorConfig := &controllerv1alpha1.DevWorkspaceOperatorConfig{} err := r.Get(ctx, req.NamespacedName, dwOperatorConfig) @@ -187,7 +187,7 @@ func (r *DevWorkspacePrunerReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, nil } -func (r *DevWorkspacePrunerReconciler) startCron(ctx context.Context, cleanupConfig *controllerv1alpha1.CleanupCronJobConfig, logger logr.Logger) { +func (r *CleanupCronJobReconciler) startCron(ctx context.Context, cleanupConfig *controllerv1alpha1.CleanupCronJobConfig, logger logr.Logger) { log := logger.WithName("cron") log.Info("Starting cron scheduler") @@ -224,7 +224,7 @@ func (r *DevWorkspacePrunerReconciler) startCron(ctx context.Context, cleanupCon r.cron.Start() } -func (r *DevWorkspacePrunerReconciler) stopCron(logger logr.Logger) { +func (r *CleanupCronJobReconciler) stopCron(logger logr.Logger) { log := logger.WithName("cron") log.Info("Stopping cron scheduler") @@ -240,7 +240,7 @@ func (r *DevWorkspacePrunerReconciler) stopCron(logger logr.Logger) { log.Info("Cron scheduler stopped") } -func (r *DevWorkspacePrunerReconciler) pruneDevWorkspaces(ctx context.Context, retainTime time.Duration, dryRun bool, logger logr.Logger) error { +func (r *CleanupCronJobReconciler) pruneDevWorkspaces(ctx context.Context, retainTime time.Duration, dryRun bool, logger logr.Logger) error { log := logger.WithName("pruner") // create a prune strategy based on the configuration @@ -283,7 +283,7 @@ func (r *DevWorkspacePrunerReconciler) pruneDevWorkspaces(ctx context.Context, r // pruneStrategy returns a StrategyFunc that will return a list of // DevWorkspaces to prune based on the lastTransitionTime of the 'Started' condition. -func (r *DevWorkspacePrunerReconciler) pruneStrategy(retainTime time.Duration, logger logr.Logger) prune.StrategyFunc { +func (r *CleanupCronJobReconciler) pruneStrategy(retainTime time.Duration, logger logr.Logger) prune.StrategyFunc { log := logger.WithName("pruneStrategy") return func(ctx context.Context, objs []client.Object) ([]client.Object, error) { @@ -295,7 +295,7 @@ func (r *DevWorkspacePrunerReconciler) pruneStrategy(retainTime time.Duration, l // dryRunPruneStrategy returns a StrategyFunc that will always return an empty list of DevWorkspaces to prune. // This is used for dry-run mode. -func (r *DevWorkspacePrunerReconciler) dryRunPruneStrategy(retainTime time.Duration, logger logr.Logger) prune.StrategyFunc { +func (r *CleanupCronJobReconciler) dryRunPruneStrategy(retainTime time.Duration, logger logr.Logger) prune.StrategyFunc { log := logger.WithName("dryRunPruneStrategy") return func(ctx context.Context, objs []client.Object) ([]client.Object, error) { diff --git a/controllers/pruner/pruner_controller_test.go b/controllers/cleanupcronjob/cleanupcronjob_controller_test.go similarity index 98% rename from controllers/pruner/pruner_controller_test.go rename to controllers/cleanupcronjob/cleanupcronjob_controller_test.go index 840530a31..3bf5f58f7 100644 --- a/controllers/pruner/pruner_controller_test.go +++ b/controllers/cleanupcronjob/cleanupcronjob_controller_test.go @@ -40,11 +40,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ) -var _ = Describe("DevWorkspacePrunerReconciler", func() { +var _ = Describe("CleanupCronJobReconciler", func() { var ( ctx context.Context fakeClient client.Client - reconciler DevWorkspacePrunerReconciler + reconciler CleanupCronJobReconciler nameNamespace types.NamespacedName log logr.Logger ) @@ -55,9 +55,9 @@ var _ = Describe("DevWorkspacePrunerReconciler", func() { Expect(controllerv1alpha1.AddToScheme(scheme)).To(Succeed()) Expect(dwv2.AddToScheme(scheme)).To(Succeed()) fakeClient = fake.NewClientBuilder().WithScheme(scheme).Build() - log = zap.New(zap.UseDevMode(true)).WithName("prunerController") + log = zap.New(zap.UseDevMode(true)).WithName("cleanupCronJobController") - reconciler = DevWorkspacePrunerReconciler{ + reconciler = CleanupCronJobReconciler{ Client: fakeClient, Log: log, Scheme: scheme, diff --git a/controllers/pruner/suite_test.go b/controllers/cleanupcronjob/suite_test.go similarity index 90% rename from controllers/pruner/suite_test.go rename to controllers/cleanupcronjob/suite_test.go index 60dd38c41..d6041638d 100644 --- a/controllers/pruner/suite_test.go +++ b/controllers/cleanupcronjob/suite_test.go @@ -20,7 +20,7 @@ import ( . "github.com/onsi/gomega" ) -func TestPruner(t *testing.T) { +func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Pruner Suite") + RunSpecs(t, "CleanupCronJob Controller Suite") } diff --git a/main.go b/main.go index d50a515d6..b6409abeb 100644 --- a/main.go +++ b/main.go @@ -35,7 +35,7 @@ import ( dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - prunercontroller "github.com/devfile/devworkspace-operator/controllers/pruner" + "github.com/devfile/devworkspace-operator/controllers/pruner" workspacecontroller "github.com/devfile/devworkspace-operator/controllers/workspace" configv1 "github.com/openshift/api/config/v1" @@ -170,12 +170,12 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "DevWorkspace") os.Exit(1) } - if err = (&prunercontroller.DevWorkspacePrunerReconciler{ + if err = (&controllers.CleanupCronJobReconciler{ Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("DevWorkspacePruner"), + Log: ctrl.Log.WithName("controllers").WithName("CleanupCronJob"), Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "DevWorkspacePruner") + setupLog.Error(err, "unable to create controller", "controller", "CleanupCronJob") os.Exit(1) } // +kubebuilder:scaffold:builder From 0decf40efe70f0bcd7639ecef55fbecbba277dd1 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Fri, 18 Apr 2025 09:48:13 +0300 Subject: [PATCH 8/8] fixup! fix: rename controller Signed-off-by: Oleksii Kurinnyi --- main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index b6409abeb..4236d2c19 100644 --- a/main.go +++ b/main.go @@ -35,7 +35,7 @@ import ( dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/controllers/pruner" + cleanupCronJobController "github.com/devfile/devworkspace-operator/controllers/cleanupcronjob" workspacecontroller "github.com/devfile/devworkspace-operator/controllers/workspace" configv1 "github.com/openshift/api/config/v1" @@ -170,7 +170,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "DevWorkspace") os.Exit(1) } - if err = (&controllers.CleanupCronJobReconciler{ + if err = (&cleanupCronJobController.CleanupCronJobReconciler{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("CleanupCronJob"), Scheme: mgr.GetScheme(),