From 1480c980b93f0465f858f4765361e34bb40676d4 Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Fri, 17 Oct 2025 10:11:40 +0200 Subject: [PATCH 01/12] feat(23570): Add controller for workspace backup A new backup controller orchestrates a backup process for workspace PVC. A new configuration option is added to DevWorkspaceOperatorConfig that enables running regular cronjob that is responsible for backup mechanism. The job executes following steps: - Find a workspaces - Finds out that workspace has been recently stopped - Detect a workspace PVC - Execute a job in the same namespace that does the backup The last step is currently not fully implemented as it requires running a buildah inside the container and it will be delivered as a separate feature. Issue: https://github.com/eclipse-che/che/issues/23570 Signed-off-by: Ales Raszka --- .../devworkspaceoperatorconfig_types.go | 14 + .../v1alpha1/zz_generated.deepcopy.go | 25 ++ .../backupcronjob/backupcronjob_controller.go | 366 +++++++++++++++++ .../backupcronjob_controller_test.go | 369 ++++++++++++++++++ controllers/backupcronjob/suite_test.go | 28 ++ ...evfile.io_devworkspaceoperatorconfigs.yaml | 15 + ...kspace-operator.clusterserviceversion.yaml | 18 +- deploy/deployment/kubernetes/combined.yaml | 34 +- ...workspace-controller-role.ClusterRole.yaml | 18 +- ...r.devfile.io.CustomResourceDefinition.yaml | 16 + deploy/deployment/openshift/combined.yaml | 34 +- ...workspace-controller-role.ClusterRole.yaml | 18 +- ...r.devfile.io.CustomResourceDefinition.yaml | 16 + deploy/templates/components/rbac/role.yaml | 18 +- ...evfile.io_devworkspaceoperatorconfigs.yaml | 16 + main.go | 9 + pkg/config/defaults.go | 4 + pkg/config/sync.go | 20 + 18 files changed, 1032 insertions(+), 6 deletions(-) create mode 100644 controllers/backupcronjob/backupcronjob_controller.go create mode 100644 controllers/backupcronjob/backupcronjob_controller_test.go create mode 100644 controllers/backupcronjob/suite_test.go diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 9ffff832f..683b34278 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -72,6 +72,18 @@ type CleanupCronJobConfig struct { Schedule string `json:"schedule,omitempty"` } +type BackupCronJobConfig struct { + // Enable determines whether backup CronJobs should be created for workspace PVCs. + // Defaults to false if not specified. + // +kubebuilder:validation:Optional + Enable *bool `json:"enable,omitempty"` + // Schedule specifies the cron schedule for the backup cron job. + // For example, "0 2 * * *" runs daily at 2 AM. + // +kubebuilder:default:="0 2 * * *" + // +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 @@ -189,6 +201,8 @@ type WorkspaceConfig struct { RuntimeClassName *string `json:"runtimeClassName,omitempty"` // CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. CleanupCronJob *CleanupCronJobConfig `json:"cleanupCronJob,omitempty"` + // BackupCronJobConfig defines configuration options for a cron job that automatically backs up workspace PVCs. + BackupCronJob *BackupCronJobConfig `json:"backupCronJob,omitempty"` // PostStartTimeout defines the maximum duration the PostStart hook can run // before it is automatically failed. This timeout is used for the postStart lifecycle hook // that is used to run commands in the workspace container. The timeout is specified in seconds. diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index f31eb7604..8fa733dd2 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -46,6 +46,26 @@ 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 *BackupCronJobConfig) DeepCopyInto(out *BackupCronJobConfig) { + *out = *in + if in.Enable != nil { + in, out := &in.Enable, &out.Enable + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupCronJobConfig. +func (in *BackupCronJobConfig) DeepCopy() *BackupCronJobConfig { + if in == nil { + return nil + } + out := new(BackupCronJobConfig) + in.DeepCopyInto(out) + 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 @@ -788,6 +808,11 @@ func (in *WorkspaceConfig) DeepCopyInto(out *WorkspaceConfig) { *out = new(CleanupCronJobConfig) (*in).DeepCopyInto(*out) } + if in.BackupCronJob != nil { + in, out := &in.BackupCronJob, &out.BackupCronJob + *out = new(BackupCronJobConfig) + (*in).DeepCopyInto(*out) + } if in.HostUsers != nil { in, out := &in.HostUsers, &out.HostUsers *out = new(bool) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go new file mode 100644 index 000000000..f07de7490 --- /dev/null +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -0,0 +1,366 @@ +// +// Copyright (c) 2019-2025 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" + + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + dw "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/constants" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + "github.com/go-logr/logr" + "github.com/robfig/cron/v3" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +const ( + defaultBackupImage = "registry.access.redhat.com/ubi8/ubi-minimal:latest" +) + +// BackupCronJobReconciler reconciles `BackupCronJob` configuration for the purpose of backing up workspace PVCs. +type BackupCronJobReconciler struct { + client.Client + Log logr.Logger + Scheme *runtime.Scheme + + cron *cron.Cron +} + +// shouldReconcileOnUpdate determines whether the BackupCronJobReconciler should reconcile +// based on changes in the DevWorkspaceOperatorConfig object. +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 + } + + oldBackup := oldConfig.Config.Workspace.BackupCronJob + newBackup := newConfig.Config.Workspace.BackupCronJob + + 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 + } + } + + if oldBackup == nil && newBackup == nil { + return false + } + if (oldBackup == nil && newBackup != nil) || (oldBackup != nil && newBackup == nil) { + return true + } + if differentBool(oldBackup.Enable, newBackup.Enable) { + return true + } + + if oldBackup.Schedule != newBackup.Schedule { + return true + } + + return false +} + +// SetupWithManager sets up the controller with the Manager. +func (r *BackupCronJobReconciler) SetupWithManager(mgr ctrl.Manager) error { + log := r.Log.WithName("setupWithManager") + log.Info("Setting up BackupCronJobReconciler") + + 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 true }, + GenericFunc: func(e event.GenericEvent) bool { return false }, + } + + r.cron = cron.New() + + return ctrl.NewControllerManagedBy(mgr). + Named("BackupCronJob"). + Watches(&controllerv1alpha1.DevWorkspaceOperatorConfig{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { + operatorNamespace, err := infrastructure.GetNamespace() + // Ignore events from other namespaces + if err != nil || object.GetNamespace() != operatorNamespace || object.GetName() != config.OperatorConfigName { + log.Info("Received event from different namespace, ignoring", "namespace", object.GetNamespace()) + return []ctrl.Request{} + } + + return []ctrl.Request{ + { + NamespacedName: client.ObjectKey{ + Name: object.GetName(), + Namespace: object.GetNamespace(), + }, + }, + } + }), + ). + WithEventFilter(configPredicate). + Complete(r) +} + +// +kubebuilder:rbac:groups=batch,resources=cronjobs,verbs=get;list;create;update;patch;delete +// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list +// +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;watch +// +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list +// +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list + +// Reconcile is the main reconciliation loop for the BackupCronJob controller. +func (r *BackupCronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := r.Log + log.Info("Reconciling BackupCronJob", "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) + } + + isCronConfigured := r.isBackupEnabled(dwOperatorConfig) + if !isCronConfigured { + log.Info("DevWorkspace backup is not configured, stopping cron scheduler and skipping reconciliation") + r.stopCron(log) + return ctrl.Result{}, nil + } + + backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob + r.startCron(ctx, backUpConfig, log) + + return ctrl.Result{}, nil +} + +// isBackupEnabled checks if the backup cron job is enabled in the configuration. +func (r *BackupCronJobReconciler) isBackupEnabled(config *controllerv1alpha1.DevWorkspaceOperatorConfig) bool { + if config.Config != nil && config.Config.Workspace != nil && config.Config.Workspace.BackupCronJob != nil { + if config.Config.Workspace.BackupCronJob.Enable != nil && *config.Config.Workspace.BackupCronJob.Enable { + return true + } + } + return false +} + +// startCron starts the cron scheduler with the backup job according to the provided configuration. +func (r *BackupCronJobReconciler) startCron(ctx context.Context, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) { + log := logger.WithName("backup cron") + log.Info("Starting backup 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 { + log.Info("Removing existing cronjob task", "entryID", entry.ID) + r.cron.Remove(entry.ID) + } + + // add cronjob task + log.Info("Adding cronjob task", "schedule", backUpConfig.Schedule) + _, err := r.cron.AddFunc(backUpConfig.Schedule, func() { + taskLog := logger.WithName("cronTask") + + taskLog.Info("Starting DevWorkspace backup job") + if err := r.executeBackupSync(ctx, logger); err != nil { + taskLog.Error(err, "Failed to execute backup job for DevWorkspaces") + } + taskLog.Info("DevWorkspace backup job finished") + }) + if err != nil { + log.Error(err, "Failed to add cronjob function") + return + } + + log.Info("Starting cron scheduler") + r.cron.Start() +} + +// stopCron stops the cron scheduler and removes all existing cronjob tasks. +func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) { + log := logger.WithName("backup 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") +} + +// executeBackupSync executes the backup job for all DevWorkspaces in the cluster that +// have been stopped in the last N minutes. +func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, logger logr.Logger) error { + log := logger.WithName("executeBackupSync") + log.Info("Executing backup sync for all DevWorkspaces") + devWorkspaces := &dw.DevWorkspaceList{} + err := r.List(ctx, devWorkspaces) + if err != nil { + log.Error(err, "Failed to list DevWorkspaces") + return err + } + for _, dw := range devWorkspaces.Items { + if !r.wasStoppedInTimeRange(&dw, 30, ctx, logger) { + log.Info("Skipping backup for DevWorkspace that wasn't stopped recently", "namespace", dw.Namespace, "name", dw.Name) + continue + } + dwID := dw.Status.DevWorkspaceId + log.Info("Found DevWorkspace", "namespace", dw.Namespace, "devworkspace", dw.Name, "id", dwID) + + if err := r.createBackupJob(&dw, ctx, logger); err != nil { + log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID) + continue + } + log.Info("Backup Job created for DevWorkspace", "id", dwID) + + } + return nil +} + +// wasStoppedInTimeRange checks if the DevWorkspace was stopped in the last N minutes. +func (r *BackupCronJobReconciler) wasStoppedInTimeRange(workspace *dw.DevWorkspace, timeRangeInMinute float64, ctx context.Context, logger logr.Logger) bool { + log := logger.WithName("wasStoppedInTimeRange") + if workspace.Status.Phase != dw.DevWorkspaceStatusStopped { + return false + } + log.Info("DevWorkspace is currently stopped, checking if it was stopped recently", "namespace", workspace.Namespace, "name", workspace.Name) + // Check if the workspace was stopped in the last N minutes + if workspace.Status.Conditions != nil { + lastTimeStopped := metav1.Time{} + for _, condition := range workspace.Status.Conditions { + if condition.Type == conditions.Started && condition.Status == corev1.ConditionFalse { + lastTimeStopped = condition.LastTransitionTime + } + } + // Calculate the time difference + if !lastTimeStopped.IsZero() { + timeDiff := metav1.Now().Sub(lastTimeStopped.Time) + if timeDiff.Minutes() <= timeRangeInMinute { + log.Info("DevWorkspace was stopped recently", "namespace", workspace.Namespace, "name", workspace.Name) + return true + } + } + } + return false +} + +// createBackupJob creates a Kubernetes Job to back up the workspace's PVC data. +func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, logger logr.Logger) error { + log := logger.WithName("createBackupJob") + dwID := workspace.Status.DevWorkspaceId + + // Find a PVC with the name "claim-devworkspace" + pvc := &corev1.PersistentVolumeClaim{} + err := r.Get(ctx, client.ObjectKey{Name: "claim-devworkspace", Namespace: workspace.Namespace}, pvc) + if err != nil { + log.Error(err, "Failed to get PVC for DevWorkspace", "id", dwID) + return err + } + + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "backup-job-", + Namespace: workspace.Namespace, + }, + Spec: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{ + { + Name: "backup", + Image: defaultBackupImage, + Command: []string{"/bin/sh", "-c"}, + Env: []corev1.EnvVar{ + { + Name: "DEVWORKSPACE_NAME", + Value: workspace.Name, + }, + { + Name: "DEVWORKSPACE_NAMESPACE", + Value: workspace.Namespace, + }, + { + Name: "WORKSPACE_ID", + Value: dwID, + }, + { + Name: "BACKUP_SOURCE_PATH", + Value: "/workspace/" + dwID + "/" + constants.DefaultProjectsSourcesRoot, + }, + }, + // TODO: Replace the following command with actual backup logic + Args: []string{ + "echo \"Starting backup for workspace $WORKSPACE_ID\" && ls -l \"$BACKUP_SOURCE_PATH\" && sleep 1 && echo Backup completed for workspace " + dwID, + }, + VolumeMounts: []corev1.VolumeMount{ + { + MountPath: "/workspace", + Name: "workspace-data", + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "workspace-data", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: pvc.Name, + }, + }, + }, + }, + }, + }, + }, + } + err = r.Create(ctx, job) + if err != nil { + log.Error(err, "Failed to create backup Job for DevWorkspace", "devworkspace", workspace.Name) + return err + } + log.Info("Created backup Job for DevWorkspace", "jobName", job.Name, "devworkspace", workspace.Name) + return nil +} diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go new file mode 100644 index 000000000..18731e03d --- /dev/null +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -0,0 +1,369 @@ +// Copyright (c) 2019-2025 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" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + 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/event" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "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" +) + +var _ = Describe("BackupCronJobReconciler", func() { + var ( + ctx context.Context + fakeClient client.Client + reconciler BackupCronJobReconciler + 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()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + Expect(batchv1.AddToScheme(scheme)).To(Succeed()) + fakeClient = fake.NewClientBuilder().WithScheme(scheme).Build() + log = zap.New(zap.UseDevMode(true)).WithName("BackupCronJobReconcilerTest") + + reconciler = BackupCronJobReconciler{ + 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("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 dwOperatorConfig.Config.Workspace.BackupCronJob is nil", func() { + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: nil, + }, + }, + } + 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 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{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + 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 start cron if 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{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + 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{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + 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.BackupCronJob.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{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + 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("executeBackupSync", func() { + It("creates a Job for a DevWorkspace stopped within last 30 minutes", func() { + dw := createDevWorkspace("dw-recent", "ns-a", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.Phase = dwv2.DevWorkspaceStatusStopped + dw.Status.DevWorkspaceId = "id-recent" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} + Expect(fakeClient.Create(ctx, pvc)).To(Succeed()) + + Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed()) + + jobList := &batchv1.JobList{} + Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) + Expect(jobList.Items).To(HaveLen(1)) + }) + + It("does not create a Job when the DevWorkspace was stopped beyond time range", func() { + dw := createDevWorkspace("dw-old", "ns-b", false, metav1.NewTime(time.Now().Add(-60*time.Minute))) + dw.Status.Phase = dwv2.DevWorkspaceStatusStopped + dw.Status.DevWorkspaceId = "id-old" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} + Expect(fakeClient.Create(ctx, pvc)).To(Succeed()) + + Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed()) + + jobList := &batchv1.JobList{} + Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) + Expect(jobList.Items).To(HaveLen(0)) + }) + + It("does not create a Job for a running DevWorkspace", func() { + dw := createDevWorkspace("dw-running", "ns-c", true, metav1.NewTime(time.Now().Add(-5*time.Minute))) + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} + Expect(fakeClient.Create(ctx, pvc)).To(Succeed()) + + Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed()) + + jobList := &batchv1.JobList{} + Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) + Expect(jobList.Items).To(HaveLen(0)) + }) + }) + +}) + +// 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 backup configuration changes", + func(oldBackup, newBackup *controllerv1alpha1.BackupCronJobConfig, expected bool) { + oldCfg := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: oldBackup, + }, + }, + } + newCfg := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: newBackup, + }, + }, + } + 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("OldBackup==nil, NewBackup!=nil => changed", nil, &controllerv1alpha1.BackupCronJobConfig{}, true), + Entry("OldBackup!=nil, NewBackup==nil => changed", &controllerv1alpha1.BackupCronJobConfig{}, nil, true), + Entry("OldBackup.Enable==nil, NewBackup.Enable==nil => no change", + &controllerv1alpha1.BackupCronJobConfig{Enable: nil}, + &controllerv1alpha1.BackupCronJobConfig{Enable: nil}, + false, + ), + Entry("OldBackup.Enable==nil, NewBackup.Enable!=nil => changed", + &controllerv1alpha1.BackupCronJobConfig{Enable: nil}, + &controllerv1alpha1.BackupCronJobConfig{Enable: pointer.Bool(true)}, + true, + ), + Entry("OldBackup.Enable!=nil, NewBackup.Enable==nil => changed", + &controllerv1alpha1.BackupCronJobConfig{Enable: pointer.Bool(true)}, + &controllerv1alpha1.BackupCronJobConfig{Enable: nil}, + true, + ), + Entry("Enable differs => changed", + &controllerv1alpha1.BackupCronJobConfig{Enable: pointer.Bool(true)}, + &controllerv1alpha1.BackupCronJobConfig{Enable: pointer.Bool(false)}, + true, + ), + Entry("Schedule differs => changed", + &controllerv1alpha1.BackupCronJobConfig{Schedule: "0 * * * *"}, + &controllerv1alpha1.BackupCronJobConfig{Schedule: "1 * * * *"}, + true, + ), + Entry("All fields match => no change", + &controllerv1alpha1.BackupCronJobConfig{ + Enable: pointer.Bool(true), + Schedule: "0 * * * *", + }, + &controllerv1alpha1.BackupCronJobConfig{ + Enable: pointer.Bool(true), + Schedule: "0 * * * *", + }, + false, + ), + ) +}) diff --git a/controllers/backupcronjob/suite_test.go b/controllers/backupcronjob/suite_test.go new file mode 100644 index 000000000..9c26322a3 --- /dev/null +++ b/controllers/backupcronjob/suite_test.go @@ -0,0 +1,28 @@ +// +// 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 ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestBackupCronJob(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "BackupCronJob Controller Suite") +} diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 471d552c8..4950def26 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -181,6 +181,21 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + backupCronJob: + description: BackupCronJobConfig defines configuration options for a cron job that automatically backs up workspace PVCs. + properties: + enable: + description: |- + Enable determines whether backup CronJobs should be created for workspace PVCs. + Defaults to false if not specified. + type: boolean + schedule: + default: 0 2 * * * + description: |- + Schedule specifies the cron schedule for the backup cron job. + For example, "0 2 * * *" runs daily at 2 AM. + type: string + type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. properties: diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index 51be04001..8cbfe2d07 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -101,7 +101,6 @@ spec: - "" resources: - configmaps - - persistentvolumeclaims - pods - secrets - serviceaccounts @@ -117,6 +116,12 @@ spec: - get - list - watch + - apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - '*' - apiGroups: - "" resources: @@ -170,6 +175,17 @@ spec: - subjectaccessreviews verbs: - create + - apiGroups: + - batch + resources: + - cronjobs + verbs: + - create + - delete + - get + - list + - patch + - update - apiGroups: - batch resources: diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 34953ec1e..91ac5ec7f 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -182,6 +182,22 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + backupCronJob: + description: BackupCronJobConfig defines configuration options + for a cron job that automatically backs up workspace PVCs. + properties: + enable: + description: |- + Enable determines whether backup CronJobs should be created for workspace PVCs. + Defaults to false if not specified. + type: boolean + schedule: + default: 0 2 * * * + description: |- + Schedule specifies the cron schedule for the backup cron job. + For example, "0 2 * * *" runs daily at 2 AM. + type: string + type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. @@ -25809,7 +25825,6 @@ rules: - "" resources: - configmaps - - persistentvolumeclaims - pods - secrets - serviceaccounts @@ -25825,6 +25840,12 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - '*' - apiGroups: - "" resources: @@ -25878,6 +25899,17 @@ rules: - subjectaccessreviews verbs: - create +- apiGroups: + - batch + resources: + - cronjobs + verbs: + - create + - delete + - get + - list + - patch + - update - apiGroups: - batch resources: diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index cb43b0b1e..541f6c71a 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -21,7 +21,6 @@ rules: - "" resources: - configmaps - - persistentvolumeclaims - pods - secrets - serviceaccounts @@ -37,6 +36,12 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - '*' - apiGroups: - "" resources: @@ -90,6 +95,17 @@ rules: - subjectaccessreviews verbs: - create +- apiGroups: + - batch + resources: + - cronjobs + verbs: + - create + - delete + - get + - list + - patch + - update - apiGroups: - batch resources: 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 910c7f6b5..461504b0f 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -182,6 +182,22 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + backupCronJob: + description: BackupCronJobConfig defines configuration options + for a cron job that automatically backs up workspace PVCs. + properties: + enable: + description: |- + Enable determines whether backup CronJobs should be created for workspace PVCs. + Defaults to false if not specified. + type: boolean + schedule: + default: 0 2 * * * + description: |- + Schedule specifies the cron schedule for the backup cron job. + For example, "0 2 * * *" runs daily at 2 AM. + type: string + type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 31bb95d7f..2f8a87609 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -182,6 +182,22 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + backupCronJob: + description: BackupCronJobConfig defines configuration options + for a cron job that automatically backs up workspace PVCs. + properties: + enable: + description: |- + Enable determines whether backup CronJobs should be created for workspace PVCs. + Defaults to false if not specified. + type: boolean + schedule: + default: 0 2 * * * + description: |- + Schedule specifies the cron schedule for the backup cron job. + For example, "0 2 * * *" runs daily at 2 AM. + type: string + type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. @@ -25809,7 +25825,6 @@ rules: - "" resources: - configmaps - - persistentvolumeclaims - pods - secrets - serviceaccounts @@ -25825,6 +25840,12 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - '*' - apiGroups: - "" resources: @@ -25878,6 +25899,17 @@ rules: - subjectaccessreviews verbs: - create +- apiGroups: + - batch + resources: + - cronjobs + verbs: + - create + - delete + - get + - list + - patch + - update - apiGroups: - batch resources: diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index cb43b0b1e..541f6c71a 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -21,7 +21,6 @@ rules: - "" resources: - configmaps - - persistentvolumeclaims - pods - secrets - serviceaccounts @@ -37,6 +36,12 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - '*' - apiGroups: - "" resources: @@ -90,6 +95,17 @@ rules: - subjectaccessreviews verbs: - create +- apiGroups: + - batch + resources: + - cronjobs + verbs: + - create + - delete + - get + - list + - patch + - update - apiGroups: - batch resources: 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 910c7f6b5..461504b0f 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -182,6 +182,22 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + backupCronJob: + description: BackupCronJobConfig defines configuration options + for a cron job that automatically backs up workspace PVCs. + properties: + enable: + description: |- + Enable determines whether backup CronJobs should be created for workspace PVCs. + Defaults to false if not specified. + type: boolean + schedule: + default: 0 2 * * * + description: |- + Schedule specifies the cron schedule for the backup cron job. + For example, "0 2 * * *" runs daily at 2 AM. + type: string + type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index 03157dd56..d43fbca4f 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -19,7 +19,6 @@ rules: - "" resources: - configmaps - - persistentvolumeclaims - pods - secrets - serviceaccounts @@ -35,6 +34,12 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - '*' - apiGroups: - "" resources: @@ -88,6 +93,17 @@ rules: - subjectaccessreviews verbs: - create +- apiGroups: + - batch + resources: + - cronjobs + verbs: + - create + - delete + - get + - list + - patch + - update - apiGroups: - batch resources: diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index d03cd57bd..35670172c 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -180,6 +180,22 @@ spec: Workspace defines configuration options related to how DevWorkspaces are managed properties: + backupCronJob: + description: BackupCronJobConfig defines configuration options + for a cron job that automatically backs up workspace PVCs. + properties: + enable: + description: |- + Enable determines whether backup CronJobs should be created for workspace PVCs. + Defaults to false if not specified. + type: boolean + schedule: + default: 0 2 * * * + description: |- + Schedule specifies the cron schedule for the backup cron job. + For example, "0 2 * * *" runs daily at 2 AM. + type: string + type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. diff --git a/main.go b/main.go index 6f355a4d2..4f41c5e5a 100644 --- a/main.go +++ b/main.go @@ -37,6 +37,7 @@ import ( dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + backupCronJobController "github.com/devfile/devworkspace-operator/controllers/backupcronjob" cleanupCronJobController "github.com/devfile/devworkspace-operator/controllers/cleanupcronjob" workspacecontroller "github.com/devfile/devworkspace-operator/controllers/workspace" @@ -188,6 +189,14 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "CleanupCronJob") os.Exit(1) } + if err = (&backupCronJobController.BackupCronJobReconciler{ + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("BackupCronJob"), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "BackupCronJob") + os.Exit(1) + } // +kubebuilder:scaffold:builder // Get a config to talk to the apiserver diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 03fec33ad..ab2f7b26a 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -82,6 +82,10 @@ var defaultConfig = &v1alpha1.OperatorConfiguration{ RetainTime: pointer.Int32(2592000), Schedule: "0 0 1 * *", }, + BackupCronJob: &v1alpha1.BackupCronJobConfig{ + Enable: pointer.Bool(false), + Schedule: "0 0 1 * *", + }, // Do not declare a default value for this field. // Setting a default leads to an endless reconcile loop when UserNamespacesSupport is disabled, // because in that case the field is ignored and always set to nil. diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 76ffd89a5..f3ed4e384 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -431,6 +431,17 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { to.Workspace.CleanupCronJob.Schedule = from.Workspace.CleanupCronJob.Schedule } } + if from.Workspace.BackupCronJob != nil { + if to.Workspace.BackupCronJob == nil { + to.Workspace.BackupCronJob = &controller.BackupCronJobConfig{} + } + if from.Workspace.BackupCronJob.Enable != nil { + to.Workspace.BackupCronJob.Enable = from.Workspace.BackupCronJob.Enable + } + if from.Workspace.BackupCronJob.Schedule != "" { + to.Workspace.BackupCronJob.Schedule = from.Workspace.BackupCronJob.Schedule + } + } if from.Workspace.PostStartTimeout != "" { to.Workspace.PostStartTimeout = from.Workspace.PostStartTimeout @@ -684,6 +695,15 @@ func GetCurrentConfigString(currConfig *controller.OperatorConfiguration) string config = append(config, fmt.Sprintf("workspace.cleanupCronJob.cronJobScript=%s", workspace.CleanupCronJob.Schedule)) } } + if workspace.BackupCronJob != nil { + if workspace.BackupCronJob.Enable != nil && *workspace.BackupCronJob.Enable != *defaultConfig.Workspace.BackupCronJob.Enable { + config = append(config, fmt.Sprintf("workspace.backupCronJob.enable=%t", *workspace.BackupCronJob.Enable)) + } + + if workspace.BackupCronJob.Schedule != defaultConfig.Workspace.BackupCronJob.Schedule { + config = append(config, fmt.Sprintf("workspace.backupCronJob.cronJobScript=%s", workspace.BackupCronJob.Schedule)) + } + } if workspace.HostUsers != nil { config = append(config, fmt.Sprintf("workspace.hostUsers=%t", *workspace.HostUsers)) } From d1a8f946baf22c72ce97595592862709a4da6f57 Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Fri, 24 Oct 2025 11:36:52 +0200 Subject: [PATCH 02/12] feat: Create a backup mechanism using Buildah image A backup of workspace is done using Buildah and storing a content of the workspace PVC into a container image. The image is later stored in a registry and can be used to recover data. A prototype script was updated and stored under project-backup directory and is build alongside the controller. The backup job calls the script and execute following steps: - mount a volume with workspace data - build container image using buildah - push image to registry configured by the operator admin Signed-off-by: Ales Raszka --- .github/workflows/next-build.yml | 12 ++++ .github/workflows/pr.yml | 3 + Makefile | 3 + .../devworkspaceoperatorconfig_types.go | 4 ++ build/scripts/generate_deployment.sh | 12 +++- .../backupcronjob/backupcronjob_controller.go | 71 ++++++++++++------- ...evfile.io_devworkspaceoperatorconfigs.yaml | 5 ++ ...kspace-operator.clusterserviceversion.yaml | 17 ++--- deploy/deployment/kubernetes/combined.yaml | 20 +++--- ...rkspace-controller-manager.Deployment.yaml | 2 + ...workspace-controller-role.ClusterRole.yaml | 13 +--- ...r.devfile.io.CustomResourceDefinition.yaml | 5 ++ deploy/deployment/openshift/combined.yaml | 20 +++--- ...rkspace-controller-manager.Deployment.yaml | 2 + ...workspace-controller-role.ClusterRole.yaml | 13 +--- ...r.devfile.io.CustomResourceDefinition.yaml | 5 ++ .../templates/base/manager_image_patch.yaml | 4 +- .../components/csv/clusterserviceversion.yaml | 2 + .../templates/components/manager/manager.yaml | 2 + deploy/templates/components/rbac/role.yaml | 13 +--- ...evfile.io_devworkspaceoperatorconfigs.yaml | 5 ++ docs/release/README.md | 4 ++ internal/images/image.go | 10 +++ make-release.sh | 12 ++++ project-backup/Containerfile | 29 ++++++++ project-backup/entrypoint.sh | 7 ++ project-backup/workspace-recovery.sh | 61 ++++++++++++++++ 27 files changed, 262 insertions(+), 94 deletions(-) create mode 100644 project-backup/Containerfile create mode 100644 project-backup/entrypoint.sh create mode 100644 project-backup/workspace-recovery.sh diff --git a/.github/workflows/next-build.yml b/.github/workflows/next-build.yml index 0e2c05941..a9f40b1dc 100644 --- a/.github/workflows/next-build.yml +++ b/.github/workflows/next-build.yml @@ -73,6 +73,17 @@ jobs: quay.io/devfile/project-clone:sha-${{ steps.git-sha.outputs.sha }} file: ./project-clone/Dockerfile + - name: Build and push + uses: docker/build-push-action@0a97817b6ade9f46837855d676c4cca3a2471fc9 #v4.2.1 + with: + context: ./project-backup + push: true + platforms: linux/amd64, linux/arm64, linux/ppc64le, linux/s390x + tags: | + quay.io/devfile/project-backup:next + quay.io/devfile/project-backup:sha-${{ steps.git-sha.outputs.sha }} + file: ./project-backup/Dockerfile + build-next-olm-imgs: runs-on: ubuntu-latest needs: build-next-imgs @@ -147,6 +158,7 @@ jobs: export TAG="sha-${{ needs.build-next-imgs.outputs.git-sha }}" export DEFAULT_DWO_IMG="quay.io/devfile/devworkspace-controller:$TAG" export PROJECT_CLONE_IMG="quay.io/devfile/project-clone:$TAG" + export PROJECT_BACKUP_IMG="quay.io/devfile/project-backup:$TAG" # Next builds are not rolled out unless the version is incremented. We want to use semver # prerelease tags to make sure each new build increments on the previous one, e.g. diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 09b35f69a..df77e3725 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -130,3 +130,6 @@ jobs: - name: Check if project-clone dockerimage build is working run: docker build -f ./project-clone/Dockerfile . + - + name: Check if project-backup containerimage build is working + run: docker build -f ./project-backup/Containerfile project-backup/ diff --git a/Makefile b/Makefile index feff1b2e9..89a6a8f97 100644 --- a/Makefile +++ b/Makefile @@ -26,6 +26,7 @@ export DWO_IMG ?= quay.io/devfile/devworkspace-controller:next export DWO_BUNDLE_IMG ?= quay.io/devfile/devworkspace-operator-bundle:next export DWO_INDEX_IMG ?= quay.io/devfile/devworkspace-operator-index:next export PROJECT_CLONE_IMG ?= quay.io/devfile/project-clone:next +export PROJECT_BACKUP_IMG ?= quay.io/devfile/project-clone:next export PULL_POLICY ?= Always export DEFAULT_ROUTING ?= basic export KUBECONFIG ?= ${HOME}/.kube/config @@ -128,6 +129,7 @@ _print_vars: @echo " DWO_BUNDLE_IMG=$(DWO_BUNDLE_IMG)" @echo " DWO_INDEX_IMG=$(DWO_INDEX_IMG)" @echo " PROJECT_CLONE_IMG=$(PROJECT_CLONE_IMG)" + @echo " PROJECT_BACKUP_IMG=$(PROJECT_BACKUP_IMG)" @echo " PULL_POLICY=$(PULL_POLICY)" @echo " ROUTING_SUFFIX=$(ROUTING_SUFFIX)" @echo " DEFAULT_ROUTING=$(DEFAULT_ROUTING)" @@ -369,6 +371,7 @@ help: Makefile @echo 'Supported environment variables:' @echo ' DWO_IMG - Image used for controller' @echo ' PROJECT_CLONE_IMG - Image used for project-clone init container' + @echo ' PROJECT_BACKUP_IMG - Image used for project-backup workspace backup container' @echo ' NAMESPACE - Namespace to use for deploying controller' @echo ' KUBECONFIG - Kubeconfig which should be used for accessing to the cluster. Currently is: $(KUBECONFIG)' @echo ' ROUTING_SUFFIX - Cluster routing suffix (e.g. $$(minikube ip).nip.io, apps-crc.testing)' diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 683b34278..1914ec7e7 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -82,6 +82,10 @@ type BackupCronJobConfig struct { // +kubebuilder:default:="0 2 * * *" // +kubebuilder:validation:Optional Schedule string `json:"schedule,omitempty"` + // A registry where backup images are stored. Images are stored + // in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} + // +kubebuilder:validation:Optional + Registry string `json:"registry,omitempty"` } type RoutingConfig struct { diff --git a/build/scripts/generate_deployment.sh b/build/scripts/generate_deployment.sh index a10b30e72..e4a6e740f 100755 --- a/build/scripts/generate_deployment.sh +++ b/build/scripts/generate_deployment.sh @@ -31,7 +31,7 @@ set -e # List of environment variables that will be replaced by envsubst -SUBST_VARS='$NAMESPACE $DWO_IMG $PROJECT_CLONE_IMG $ROUTING_SUFFIX $DEFAULT_ROUTING $PULL_POLICY' +SUBST_VARS='$NAMESPACE $DWO_IMG $PROJECT_CLONE_IMG $PROJECT_BACKUP_IMG $ROUTING_SUFFIX $DEFAULT_ROUTING $PULL_POLICY' SCRIPT_DIR=$(cd "$(dirname "$0")"; pwd) DEPLOY_DIR="$SCRIPT_DIR/../../deploy/" @@ -58,6 +58,11 @@ Arguments: '--use-defaults' is passed; otherwise, the value of the PROJECT_CLONE_IMG environment variable is used. If unspecifed, the default value of 'quay.io/devfile/project-clone:next' is used. + --project-backup-image + Image to use for the project backup workspace. Used only when + '--use-defaults' is passed; otherwise, the value of the PROJECT_BACKUP_IMG + environment variable is used. If unspecifed, the default value of + 'quay.io/devfile/project-backup:next' is used. --split-yaml Parse output file combined.yaml into a yaml file for each record in combined yaml. Files are output to the 'objects' subdirectory @@ -96,6 +101,10 @@ while [[ "$#" -gt 0 ]]; do PROJECT_CLONE_IMG=$2 shift ;; + --project-backup-image) + PROJECT_BACKUP_IMG=$2 + shift + ;; --split-yamls) SPLIT_YAMLS=true ;; @@ -118,6 +127,7 @@ if $USE_DEFAULT_ENV; then export NAMESPACE=devworkspace-controller export DWO_IMG=${DEFAULT_DWO_IMG:-"quay.io/devfile/devworkspace-controller:next"} export PROJECT_CLONE_IMG=${PROJECT_CLONE_IMG:-"quay.io/devfile/project-clone:next"} + export PROJECT_BACKUP_IMG=${PROJECT_BACKUP_IMG:-"quay.io/devfile/project-backup:next"} export PULL_POLICY=Always export DEFAULT_ROUTING=basic export DEVWORKSPACE_API_VERSION=a6ec0a38307b63a29fad2eea945cc69bee97a683 diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index f07de7490..64415ed11 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -22,6 +22,7 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/internal/images" "github.com/devfile/devworkspace-operator/pkg/conditions" "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" @@ -137,11 +138,10 @@ func (r *BackupCronJobReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// +kubebuilder:rbac:groups=batch,resources=cronjobs,verbs=get;list;create;update;patch;delete -// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list -// +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;watch -// +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list +// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;create;update;patch;delete +// +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;update;patch;watch +// +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list // Reconcile is the main reconciliation loop for the BackupCronJob controller. func (r *BackupCronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -180,7 +180,7 @@ func (r *BackupCronJobReconciler) isBackupEnabled(config *controllerv1alpha1.Dev } // startCron starts the cron scheduler with the backup job according to the provided configuration. -func (r *BackupCronJobReconciler) startCron(ctx context.Context, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) { +func (r *BackupCronJobReconciler) startCron(ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) { log := logger.WithName("backup cron") log.Info("Starting backup cron scheduler") @@ -198,7 +198,7 @@ func (r *BackupCronJobReconciler) startCron(ctx context.Context, backUpConfig *c taskLog := logger.WithName("cronTask") taskLog.Info("Starting DevWorkspace backup job") - if err := r.executeBackupSync(ctx, logger); err != nil { + if err := r.executeBackupSync(ctx, req, backUpConfig, logger); err != nil { taskLog.Error(err, "Failed to execute backup job for DevWorkspaces") } taskLog.Info("DevWorkspace backup job finished") @@ -231,7 +231,7 @@ func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) { // executeBackupSync executes the backup job for all DevWorkspaces in the cluster that // have been stopped in the last N minutes. -func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, logger logr.Logger) error { +func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error { log := logger.WithName("executeBackupSync") log.Info("Executing backup sync for all DevWorkspaces") devWorkspaces := &dw.DevWorkspaceList{} @@ -248,7 +248,7 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, logger dwID := dw.Status.DevWorkspaceId log.Info("Found DevWorkspace", "namespace", dw.Namespace, "devworkspace", dw.Name, "id", dwID) - if err := r.createBackupJob(&dw, ctx, logger); err != nil { + if err := r.createBackupJob(&dw, ctx, req, backUpConfig, logger); err != nil { log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID) continue } @@ -285,8 +285,12 @@ func (r *BackupCronJobReconciler) wasStoppedInTimeRange(workspace *dw.DevWorkspa return false } +func ptrInt64(i int64) *int64 { return &i } +func ptrInt32(i int32) *int32 { return &i } +func ptrBool(b bool) *bool { return &b } + // createBackupJob creates a Kubernetes Job to back up the workspace's PVC data. -func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, logger logr.Logger) error { +func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error { log := logger.WithName("createBackupJob") dwID := workspace.Status.DevWorkspaceId @@ -307,38 +311,47 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, + SecurityContext: &corev1.PodSecurityContext{ + FSGroup: ptrInt64(0), + }, Containers: []corev1.Container{ { - Name: "backup", - Image: defaultBackupImage, - Command: []string{"/bin/sh", "-c"}, + Name: "backup-workspace", Env: []corev1.EnvVar{ - { - Name: "DEVWORKSPACE_NAME", - Value: workspace.Name, - }, - { - Name: "DEVWORKSPACE_NAMESPACE", - Value: workspace.Namespace, - }, - { - Name: "WORKSPACE_ID", - Value: dwID, - }, + {Name: "DEVWORKSPACE_NAME", Value: workspace.Name}, + {Name: "DEVWORKSPACE_NAMESPACE", Value: workspace.Namespace}, + {Name: "WORKSPACE_ID", Value: dwID}, { Name: "BACKUP_SOURCE_PATH", Value: "/workspace/" + dwID + "/" + constants.DefaultProjectsSourcesRoot, }, + {Name: "STORAGE_DRIVER", Value: "overlay"}, + {Name: "BUILDAH_ISOLATION", Value: "chroot"}, + {Name: "DEVWORKSPACE_BACKUP_REGISTRY", Value: backUpConfig.Registry}, + {Name: "BUILDAH_PUSH_OPTIONS", Value: "--tls-verify=false"}, }, - // TODO: Replace the following command with actual backup logic + Image: images.GetProjectBackupImage(), + // Image: "localhost:5001/workspace-backup-image:v3", Args: []string{ - "echo \"Starting backup for workspace $WORKSPACE_ID\" && ls -l \"$BACKUP_SOURCE_PATH\" && sleep 1 && echo Backup completed for workspace " + dwID, + "/workspace-recovery.sh", + "--backup", }, VolumeMounts: []corev1.VolumeMount{ { MountPath: "/workspace", Name: "workspace-data", }, + { + MountPath: "/var/lib/containers", + Name: "build-storage", + }, + }, + SecurityContext: &corev1.SecurityContext{ + RunAsUser: ptrInt64(0), + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"SYS_ADMIN", "SYS_CHROOT"}, + }, + AllowPrivilegeEscalation: ptrBool(false), }, }, }, @@ -351,6 +364,12 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct }, }, }, + { + Name: "build-storage", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, }, }, }, diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 4950def26..e4d7af119 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -189,6 +189,11 @@ spec: Enable determines whether backup CronJobs should be created for workspace PVCs. Defaults to false if not specified. type: boolean + registry: + description: |- + A registry where backup images are stored. Images are stored + in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} + type: string schedule: default: 0 2 * * * description: |- diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index 8cbfe2d07..a2b00d28c 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -175,17 +175,6 @@ spec: - subjectaccessreviews verbs: - create - - apiGroups: - - batch - resources: - - cronjobs - verbs: - - create - - delete - - get - - list - - patch - - update - apiGroups: - batch resources: @@ -220,6 +209,8 @@ spec: verbs: - get - list + - patch + - update - watch - apiGroups: - controller.devfile.io @@ -348,6 +339,8 @@ spec: value: quay.io/devfile/devworkspace-controller:next - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next + - name: RELATED_IMAGE_project_backup + value: quay.io/devfile/project-clone:next - name: WATCH_NAMESPACE valueFrom: fieldRef: @@ -481,6 +474,8 @@ spec: name: devworkspace_webhook_server - image: quay.io/devfile/project-clone:next name: project_clone + - image: quay.io/devfile/project-clone:next + name: project_backup - image: registry.access.redhat.com/ubi9/ubi-micro:9.5-1733126338 name: pvc_cleanup_job - image: quay.io/eclipse/che-workspace-data-sync-storage:0.0.1 diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 91ac5ec7f..679ea7fd7 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -191,6 +191,11 @@ spec: Enable determines whether backup CronJobs should be created for workspace PVCs. Defaults to false if not specified. type: boolean + registry: + description: |- + A registry where backup images are stored. Images are stored + in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} + type: string schedule: default: 0 2 * * * description: |- @@ -25899,17 +25904,6 @@ rules: - subjectaccessreviews verbs: - create -- apiGroups: - - batch - resources: - - cronjobs - verbs: - - create - - delete - - get - - list - - patch - - update - apiGroups: - batch resources: @@ -25944,6 +25938,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - controller.devfile.io @@ -26190,6 +26186,8 @@ spec: value: quay.io/devfile/devworkspace-controller:next - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next + - name: RELATED_IMAGE_project_backup + value: quay.io/devfile/project-clone:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml index cd6b29efc..cb31391bf 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml @@ -30,6 +30,8 @@ spec: value: quay.io/devfile/devworkspace-controller:next - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next + - name: RELATED_IMAGE_project_backup + value: quay.io/devfile/project-clone:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index 541f6c71a..311097a15 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -95,17 +95,6 @@ rules: - subjectaccessreviews verbs: - create -- apiGroups: - - batch - resources: - - cronjobs - verbs: - - create - - delete - - get - - list - - patch - - update - apiGroups: - batch resources: @@ -140,6 +129,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - controller.devfile.io 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 461504b0f..daabd4f63 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -191,6 +191,11 @@ spec: Enable determines whether backup CronJobs should be created for workspace PVCs. Defaults to false if not specified. type: boolean + registry: + description: |- + A registry where backup images are stored. Images are stored + in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} + type: string schedule: default: 0 2 * * * description: |- diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 2f8a87609..d928112a9 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -191,6 +191,11 @@ spec: Enable determines whether backup CronJobs should be created for workspace PVCs. Defaults to false if not specified. type: boolean + registry: + description: |- + A registry where backup images are stored. Images are stored + in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} + type: string schedule: default: 0 2 * * * description: |- @@ -25899,17 +25904,6 @@ rules: - subjectaccessreviews verbs: - create -- apiGroups: - - batch - resources: - - cronjobs - verbs: - - create - - delete - - get - - list - - patch - - update - apiGroups: - batch resources: @@ -25944,6 +25938,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - controller.devfile.io @@ -26192,6 +26188,8 @@ spec: value: quay.io/devfile/devworkspace-controller:next - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next + - name: RELATED_IMAGE_project_backup + value: quay.io/devfile/project-clone:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml index 5b226b7c9..95da7453d 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml @@ -30,6 +30,8 @@ spec: value: quay.io/devfile/devworkspace-controller:next - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next + - name: RELATED_IMAGE_project_backup + value: quay.io/devfile/project-clone:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index 541f6c71a..311097a15 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -95,17 +95,6 @@ rules: - subjectaccessreviews verbs: - create -- apiGroups: - - batch - resources: - - cronjobs - verbs: - - create - - delete - - get - - list - - patch - - update - apiGroups: - batch resources: @@ -140,6 +129,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - controller.devfile.io 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 461504b0f..daabd4f63 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -191,6 +191,11 @@ spec: Enable determines whether backup CronJobs should be created for workspace PVCs. Defaults to false if not specified. type: boolean + registry: + description: |- + A registry where backup images are stored. Images are stored + in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} + type: string schedule: default: 0 2 * * * description: |- diff --git a/deploy/templates/base/manager_image_patch.yaml b/deploy/templates/base/manager_image_patch.yaml index 4fddefc60..845df5b9a 100644 --- a/deploy/templates/base/manager_image_patch.yaml +++ b/deploy/templates/base/manager_image_patch.yaml @@ -15,4 +15,6 @@ spec: - name: RELATED_IMAGE_devworkspace_webhook_server value: ${DWO_IMG} - name: RELATED_IMAGE_project_clone - value: ${PROJECT_CLONE_IMG} \ No newline at end of file + value: ${PROJECT_CLONE_IMG} + - name: RELATED_IMAGE_project_backup + value: ${PROJECT_BACKUP_IMG} diff --git a/deploy/templates/components/csv/clusterserviceversion.yaml b/deploy/templates/components/csv/clusterserviceversion.yaml index 8f51953be..12e771e86 100644 --- a/deploy/templates/components/csv/clusterserviceversion.yaml +++ b/deploy/templates/components/csv/clusterserviceversion.yaml @@ -104,6 +104,8 @@ spec: name: devworkspace_webhook_server - image: quay.io/devfile/project-clone:next name: project_clone + - image: quay.io/devfile/project-clone:next + name: project_backup - image: registry.access.redhat.com/ubi9/ubi-micro:9.5-1733126338 name: pvc_cleanup_job - image: quay.io/eclipse/che-workspace-data-sync-storage:0.0.1 diff --git a/deploy/templates/components/manager/manager.yaml b/deploy/templates/components/manager/manager.yaml index 331391d50..9a50a0fb4 100644 --- a/deploy/templates/components/manager/manager.yaml +++ b/deploy/templates/components/manager/manager.yaml @@ -86,3 +86,5 @@ spec: value: "quay.io/eclipse/che-sidecar-workspace-data-sync:0.0.1" - name: RELATED_IMAGE_project_clone value: "quay.io/devfile/project-clone:next" + - name: RELATED_IMAGE_project_backup + value: "quay.io/devfile/project-backup:next" diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index d43fbca4f..6d9cc6886 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -93,17 +93,6 @@ rules: - subjectaccessreviews verbs: - create -- apiGroups: - - batch - resources: - - cronjobs - verbs: - - create - - delete - - get - - list - - patch - - update - apiGroups: - batch resources: @@ -138,6 +127,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - controller.devfile.io diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 35670172c..334f38d7c 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -189,6 +189,11 @@ spec: Enable determines whether backup CronJobs should be created for workspace PVCs. Defaults to false if not specified. type: boolean + registry: + description: |- + A registry where backup images are stored. Images are stored + in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} + type: string schedule: default: 0 2 * * * description: |- diff --git a/docs/release/README.md b/docs/release/README.md index c377a9e98..c3d95b487 100644 --- a/docs/release/README.md +++ b/docs/release/README.md @@ -40,9 +40,13 @@ This means that to test commits in a release branch before running the release j ```bash export DWO_IMG=quay.io/yourrepo/devworkspace-controller:prerelease export PROJECT_CLONE_IMG=quay.io/yourrepo/project-clone:prerelease +export PROJECT_BACKUP_IMG=quay.io/yourrepo/project-backup:prerelease # build and push project clone image podman build -t "$PROJECT_CLONE_IMG" -f ./project-clone/Dockerfile . podman push "$PROJECT_CLONE_IMG" +# build and push project backup image +podman build -t "$PROJECT_BACKUP_IMG" -f ./project-backup/Containerfile ./project-backup/ +podman push "$PROJECT_BACKUP_IMG" # build and push DevWorkspace Operator image export DOCKER=podman # optional make docker diff --git a/internal/images/image.go b/internal/images/image.go index 11bd46202..2ec548e16 100644 --- a/internal/images/image.go +++ b/internal/images/image.go @@ -38,6 +38,7 @@ const ( asyncStorageServerImageEnvVar = "RELATED_IMAGE_async_storage_server" asyncStorageSidecarImageEnvVar = "RELATED_IMAGE_async_storage_sidecar" projectCloneImageEnvVar = "RELATED_IMAGE_project_clone" + projectBackupImageEnvVar = "RELATED_IMAGE_project_backup" ) // GetWebhookServerImage returns the image reference for the webhook server image. Returns @@ -88,3 +89,12 @@ func GetProjectCloneImage() string { } return val } + +func GetProjectBackupImage() string { + val, ok := os.LookupEnv(projectBackupImageEnvVar) + if !ok { + log.Info(fmt.Sprintf("Could not get project backup image: environment variable %s is not set", projectBackupImageEnvVar)) + return "" + } + return val +} diff --git a/make-release.sh b/make-release.sh index 9d4079ac9..912419092 100755 --- a/make-release.sh +++ b/make-release.sh @@ -19,6 +19,7 @@ set -e DWO_REPO="${DWO_REPO:-git@github.com:devfile/devworkspace-operator}" DWO_QUAY_REPO="${DWO_QUAY_REPO:-quay.io/devfile/devworkspace-controller}" PROJECT_CLONE_QUAY_REPO="${PROJECT_CLONE_QUAY_REPO:-quay.io/devfile/project-clone}" +PROJECT_BACKUP_QUAY_REPO="${PROJECT_BACKUP_QUAY_REPO:-quay.io/devfile/project-backup}" DWO_BUNDLE_QUAY_REPO="${DWO_BUNDLE_QUAY_REPO:-quay.io/devfile/devworkspace-operator-bundle}" DWO_INDEX_IMAGE="${DWO_INDEX_IMAGE:-quay.io/devfile/devworkspace-operator-index:release}" DWO_DIGEST_INDEX_IMAGE="${DWO_DIGEST_INDEX_IMAGE:-quay.io/devfile/devworkspace-operator-index:release-digest}" @@ -127,12 +128,14 @@ update_images() { # Get image tags DWO_QUAY_IMG="${DWO_QUAY_REPO}:${VERSION}" PROJECT_CLONE_QUAY_IMG="${PROJECT_CLONE_QUAY_REPO}:${VERSION}" + PROJECT_BACKUP_QUAY_IMG="${PROJECT_BACKUP_QUAY_REPO}:${VERSION}" DWO_BUNDLE_QUAY_IMG="${DWO_BUNDLE_QUAY_REPO}:${VERSION}" # Update defaults in Makefile sed -i Makefile -r \ -e "s|quay.io/devfile/devworkspace-controller:[0-9a-zA-Z._-]+|${DWO_QUAY_IMG}|g" \ -e "s|quay.io/devfile/project-clone:[0-9a-zA-Z._-]+|${PROJECT_CLONE_QUAY_IMG}|g" \ + -e "s|quay.io/devfile/project-backup:[0-9a-zA-Z._-]+|${PROJECT_BACKUP_QUAY_IMG}|g" \ -e "s|quay.io/devfile/devworkspace-operator-bundle:[0-9a-zA-Z._-]+|${DWO_BUNDLE_QUAY_IMG}|g" \ -e "s|quay.io/devfile/devworkspace-operator-index:[0-9a-zA-Z._-]+|${DWO_INDEX_IMAGE}|g" @@ -140,12 +143,15 @@ update_images() { sed -i build/scripts/generate_deployment.sh -r \ -e "s|quay.io/devfile/devworkspace-controller:[0-9a-zA-Z._-]+|${DWO_QUAY_IMG}|g" \ -e "s|quay.io/devfile/project-clone:[0-9a-zA-Z._-]+|${PROJECT_CLONE_QUAY_IMG}|g" + -e "s|quay.io/devfile/project-backup:[0-9a-zA-Z._-]+|${PROJECT_PROJECT_BACKUP_QUAY_IMGCLONE_QUAY_IMG}|g" local DEFAULT_DWO_IMG="$DWO_QUAY_IMG" local PROJECT_CLONE_IMG="$PROJECT_CLONE_QUAY_IMG" + local PROJECT_BACKUP_IMG="$PROJECT_BACKUP_QUAY_IMG" export DEFAULT_DWO_IMG export PROJECT_CLONE_IMG + export PROJECT_BACKUP_IMG make generate_all } @@ -157,12 +163,15 @@ update_images() { build_and_push_images() { DWO_QUAY_IMG="${DWO_QUAY_REPO}:${VERSION}" PROJECT_CLONE_QUAY_IMG="${PROJECT_CLONE_QUAY_REPO}:${VERSION}" + PROJECT_BACKUP_QUAY_IMG="${PROJECT_BACKUP_QUAY_REPO}:${VERSION}" if [ "$DRY_RUN" == "dryrun" ]; then docker buildx build . -t "${DWO_QUAY_IMG}" -f ./build/Dockerfile \ --platform "$ARCHITECTURES" docker buildx build . -t "${PROJECT_CLONE_QUAY_IMG}" -f ./project-clone/Dockerfile \ --platform "$ARCHITECTURES" + docker buildx build . -t "${PROJECT_BACKUP_QUAY_IMG}" -f ./project-backup/Containerfile \ + --platform "$ARCHITECTURES" else docker buildx build . -t "${DWO_QUAY_IMG}" -f ./build/Dockerfile \ --platform "$ARCHITECTURES" \ @@ -170,6 +179,9 @@ build_and_push_images() { docker buildx build . -t "${PROJECT_CLONE_QUAY_IMG}" -f ./project-clone/Dockerfile \ --platform "$ARCHITECTURES" \ --push + docker buildx build . -t "${PROJECT_BACKUP_QUAY_IMG}" -f ./project-backup/Containerfile \ + --platform "$ARCHITECTURES" \ + --push fi } diff --git a/project-backup/Containerfile b/project-backup/Containerfile new file mode 100644 index 000000000..4e1ae0136 --- /dev/null +++ b/project-backup/Containerfile @@ -0,0 +1,29 @@ +FROM registry.access.redhat.com/ubi9:9.5 + +ARG USER_HOME_DIR="/home/user" +ARG INSTALL_PACKAGES="shadow-utils bash jq podman buildah ca-certificates fuse-overlayfs" + +ENV HOME=${USER_HOME_DIR} +ENV BUILDAH_ISOLATION=chroot + + +COPY --chown=0:0 entrypoint.sh / +COPY --chown=0:0 workspace-recovery.sh / + +RUN set -e ; \ + dnf install -y ${INSTALL_PACKAGES} ; \ + dnf update -y ; \ + dnf clean all ; \ + mkdir -p ${USER_HOME_DIR} ; \ + mkdir -p ${USER_HOME_DIR}/.config/containers ; \ + (echo '[storage]';echo 'driver = "overlay"';echo 'graphroot = "/tmp/graphroot"';echo '[storage.options.overlay]';echo 'mount_program = "/usr/bin/fuse-overlayfs"') > ${USER_HOME_DIR}/.config/containers/storage.conf ; \ + chown -R 1000:1000 ${USER_HOME_DIR} ; \ + chmod +x /entrypoint.sh ; \ + chmod +x /workspace-recovery.sh ; \ + echo "user:x:1000:0:devspaces user:${USER_HOME_DIR}:/bin/bash" >> /etc/passwd ; \ + echo "user:x:1000:" >> /etc/group + + +USER 1000 +WORKDIR ${USER_HOME_DIR} +ENTRYPOINT ["/usr/libexec/podman/catatonit","--","/entrypoint.sh"] diff --git a/project-backup/entrypoint.sh b/project-backup/entrypoint.sh new file mode 100644 index 000000000..58bb1bf96 --- /dev/null +++ b/project-backup/entrypoint.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +set -x +set -e + +exec "$@" + diff --git a/project-backup/workspace-recovery.sh b/project-backup/workspace-recovery.sh new file mode 100644 index 000000000..266cbe9f8 --- /dev/null +++ b/project-backup/workspace-recovery.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +set -euo pipefail +set -x + +# --- Configuration --- +: "${DEVWORKSPACE_BACKUP_REGISTRY:?Missing DEVWORKSPACE_BACKUP_REGISTRY}" +: "${DEVWORKSPACE_NAMESPACE:?Missing DEVWORKSPACE_NAMESPACE}" +: "${DEVWORKSPACE_NAME:?Missing DEVWORKSPACE_NAME}" +: "${PROJECTS_ROOT:?Missing PROJECTS_ROOT}" +: "${BACKUP_SOURCE_PATH:?Missing BACKUP_SOURCE_PATH}" + +BACKUP_IMAGE="${DEVWORKSPACE_BACKUP_REGISTRY}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME}:latest" + +# --- Functions --- +backup() { + local new_image + new_image=$(buildah from scratch) + + buildah copy "$new_image" "$BACKUP_SOURCE_PATH" / + buildah config --label DEVWORKSPACE="$DEVWORKSPACE_NAME" "$new_image" + buildah config --label NAMESPACE="$DEVWORKSPACE_NAMESPACE" "$new_image" + buildah commit "$new_image" "$BACKUP_IMAGE" + + buildah umount "$new_image" + buildah push ${BUILDAH_PUSH_OPTIONS:-} "$BACKUP_IMAGE" +} + +restore() { + local container_name="workspace-restore" + + podman create --name "$container_name" "$BACKUP_IMAGE" + rm -rf "${PROJECTS_ROOT:?}"/* + podman cp "$container_name":/. "$PROJECTS_ROOT" + podman rm "$container_name" +} + +usage() { + echo "Usage: $0 [--backup|--restore]" + exit 1 +} +echo + +# --- Main --- +if [[ $# -eq 0 ]]; then + usage +fi + +for arg in "$@"; do + case "$arg" in + --backup) + backup + ;; + --restore) + restore + ;; + *) + echo "Unknown option: $arg" + usage + ;; + esac +done From b5bbebda3b99c6db29bf574874293d118a6654aa Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Wed, 29 Oct 2025 08:26:09 +0100 Subject: [PATCH 03/12] feat: determine backup need based on LastBackupTime A new sub-object was added to the operator config that reflect a current status of the backup controller and stores a last time the backup was executed. This value is used to determine whether a backup of the workspace is needed or if it already has been executed. Signed-off-by: Ales Raszka --- .../devworkspaceoperatorconfig_types.go | 12 ++++ .../v1alpha1/zz_generated.deepcopy.go | 32 +++++++++ .../backupcronjob/backupcronjob_controller.go | 62 +++++++++++----- .../backupcronjob_controller_test.go | 51 +++++++++++-- ...evfile.io_devworkspaceoperatorconfigs.yaml | 70 ++++++++++++++++++ deploy/deployment/kubernetes/combined.yaml | 72 +++++++++++++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 72 +++++++++++++++++++ deploy/deployment/openshift/combined.yaml | 72 +++++++++++++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 72 +++++++++++++++++++ ...evfile.io_devworkspaceoperatorconfigs.yaml | 72 +++++++++++++++++++ pkg/config/defaults.go | 1 + pkg/config/sync.go | 3 + pkg/constants/metadata.go | 6 ++ 13 files changed, 575 insertions(+), 22 deletions(-) diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 1914ec7e7..c85d4e211 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -349,14 +349,26 @@ type ConfigmapReference struct { Namespace string `json:"namespace"` } +type OperatorConfigurationStatus struct { + // Conditions represent the latest available observations of the OperatorConfiguration's state + Conditions []metav1.Condition `json:"conditions,omitempty"` + // LastBackupTime is the timestamp of the last successful backup. Nil if + // no backup is configured or no backup has yet succeeded. + LastBackupTime *metav1.Time `json:"lastBackupTime,omitempty"` +} + // DevWorkspaceOperatorConfig is the Schema for the devworkspaceoperatorconfigs API // +kubebuilder:object:root=true +// +kubebuilder:subresource:status // +kubebuilder:resource:path=devworkspaceoperatorconfigs,scope=Namespaced,shortName=dwoc type DevWorkspaceOperatorConfig struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` Config *OperatorConfiguration `json:"config,omitempty"` + // Status represents the current status of the DevWorkspaceOperatorConfig + // automatically managed by the DevWorkspace Operator. + Status *OperatorConfigurationStatus `json:"status,omitempty"` } // DevWorkspaceOperatorConfigList contains a list of DevWorkspaceOperatorConfig diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index 8fa733dd2..ebc70ca6e 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ package v1alpha1 import ( "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -121,6 +122,11 @@ func (in *DevWorkspaceOperatorConfig) DeepCopyInto(out *DevWorkspaceOperatorConf *out = new(OperatorConfiguration) (*in).DeepCopyInto(*out) } + if in.Status != nil { + in, out := &in.Status, &out.Status + *out = new(OperatorConfigurationStatus) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DevWorkspaceOperatorConfig. @@ -453,6 +459,32 @@ func (in *OperatorConfiguration) DeepCopy() *OperatorConfiguration { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OperatorConfigurationStatus) DeepCopyInto(out *OperatorConfigurationStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.LastBackupTime != nil { + in, out := &in.LastBackupTime, &out.LastBackupTime + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OperatorConfigurationStatus. +func (in *OperatorConfigurationStatus) DeepCopy() *OperatorConfigurationStatus { + if in == nil { + return nil + } + out := new(OperatorConfigurationStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PersistentHomeConfig) DeepCopyInto(out *PersistentHomeConfig) { *out = *in diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 64415ed11..8993bc962 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" @@ -163,8 +164,7 @@ func (r *BackupCronJobReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } - backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob - r.startCron(ctx, backUpConfig, log) + r.startCron(ctx, dwOperatorConfig, log) return ctrl.Result{}, nil } @@ -180,7 +180,7 @@ func (r *BackupCronJobReconciler) isBackupEnabled(config *controllerv1alpha1.Dev } // startCron starts the cron scheduler with the backup job according to the provided configuration. -func (r *BackupCronJobReconciler) startCron(ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) { +func (r *BackupCronJobReconciler) startCron(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) { log := logger.WithName("backup cron") log.Info("Starting backup cron scheduler") @@ -193,12 +193,13 @@ func (r *BackupCronJobReconciler) startCron(ctx context.Context, req ctrl.Reques } // add cronjob task + backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob log.Info("Adding cronjob task", "schedule", backUpConfig.Schedule) _, err := r.cron.AddFunc(backUpConfig.Schedule, func() { taskLog := logger.WithName("cronTask") taskLog.Info("Starting DevWorkspace backup job") - if err := r.executeBackupSync(ctx, req, backUpConfig, logger); err != nil { + if err := r.executeBackupSync(ctx, dwOperatorConfig, logger); err != nil { taskLog.Error(err, "Failed to execute backup job for DevWorkspaces") } taskLog.Info("DevWorkspace backup job finished") @@ -224,47 +225,63 @@ func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) { } ctx := r.cron.Stop() - ctx.Done() + <-ctx.Done() log.Info("Cron scheduler stopped") } // executeBackupSync executes the backup job for all DevWorkspaces in the cluster that // have been stopped in the last N minutes. -func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error { +func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) error { log := logger.WithName("executeBackupSync") log.Info("Executing backup sync for all DevWorkspaces") + backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob devWorkspaces := &dw.DevWorkspaceList{} err := r.List(ctx, devWorkspaces) if err != nil { log.Error(err, "Failed to list DevWorkspaces") return err } + var lastBackupTime *metav1.Time + if dwOperatorConfig.Status != nil && dwOperatorConfig.Status.LastBackupTime != nil { + lastBackupTime = dwOperatorConfig.Status.LastBackupTime + } for _, dw := range devWorkspaces.Items { - if !r.wasStoppedInTimeRange(&dw, 30, ctx, logger) { + if !r.wasStoppedSinceLastBackup(&dw, lastBackupTime, logger) { log.Info("Skipping backup for DevWorkspace that wasn't stopped recently", "namespace", dw.Namespace, "name", dw.Name) continue } dwID := dw.Status.DevWorkspaceId log.Info("Found DevWorkspace", "namespace", dw.Namespace, "devworkspace", dw.Name, "id", dwID) - if err := r.createBackupJob(&dw, ctx, req, backUpConfig, logger); err != nil { + if err := r.createBackupJob(&dw, ctx, backUpConfig, logger); err != nil { log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID) continue } log.Info("Backup Job created for DevWorkspace", "id", dwID) } + origConfig := client.MergeFrom(dwOperatorConfig.DeepCopy()) + if dwOperatorConfig.Status == nil { + dwOperatorConfig.Status = &controllerv1alpha1.OperatorConfigurationStatus{} + } + dwOperatorConfig.Status.LastBackupTime = &metav1.Time{Time: metav1.Now().Time} + + err = r.Status().Patch(ctx, dwOperatorConfig, origConfig) + if err != nil { + log.Error(err, "Failed to update DevWorkspaceOperatorConfig status with last backup time") + // Not returning error as the backup jobs were created successfully + } return nil } -// wasStoppedInTimeRange checks if the DevWorkspace was stopped in the last N minutes. -func (r *BackupCronJobReconciler) wasStoppedInTimeRange(workspace *dw.DevWorkspace, timeRangeInMinute float64, ctx context.Context, logger logr.Logger) bool { - log := logger.WithName("wasStoppedInTimeRange") +// wasStoppedSinceLastBackup checks if the DevWorkspace was stopped since the last backup time. +func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWorkspace, lastBackupTime *metav1.Time, logger logr.Logger) bool { + log := logger.WithName("wasStoppedSinceLastBackup") if workspace.Status.Phase != dw.DevWorkspaceStatusStopped { return false } - log.Info("DevWorkspace is currently stopped, checking if it was stopped recently", "namespace", workspace.Namespace, "name", workspace.Name) + log.Info("DevWorkspace is currently stopped, checking if it was stopped since last backup", "namespace", workspace.Namespace, "name", workspace.Name) // Check if the workspace was stopped in the last N minutes if workspace.Status.Conditions != nil { lastTimeStopped := metav1.Time{} @@ -273,11 +290,13 @@ func (r *BackupCronJobReconciler) wasStoppedInTimeRange(workspace *dw.DevWorkspa lastTimeStopped = condition.LastTransitionTime } } - // Calculate the time difference if !lastTimeStopped.IsZero() { - timeDiff := metav1.Now().Sub(lastTimeStopped.Time) - if timeDiff.Minutes() <= timeRangeInMinute { - log.Info("DevWorkspace was stopped recently", "namespace", workspace.Namespace, "name", workspace.Name) + if lastBackupTime == nil { + // No previous backup, so consider it stopped since last backup + return true + } + if lastTimeStopped.Time.After(lastBackupTime.Time) { + log.Info("DevWorkspace was stopped since last backup", "namespace", workspace.Namespace, "name", workspace.Name) return true } } @@ -290,7 +309,7 @@ func ptrInt32(i int32) *int32 { return &i } func ptrBool(b bool) *bool { return &b } // createBackupJob creates a Kubernetes Job to back up the workspace's PVC data. -func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error { +func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error { log := logger.WithName("createBackupJob") dwID := workspace.Status.DevWorkspaceId @@ -304,8 +323,12 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "backup-job-", + GenerateName: constants.DevWorkspaceBackupJobNamePrefix, Namespace: workspace.Namespace, + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: dwID, + constants.DevWorkspaceBackupJobLabel: "true", + }, }, Spec: batchv1.JobSpec{ Template: corev1.PodTemplateSpec{ @@ -375,6 +398,9 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct }, }, } + if err := controllerutil.SetControllerReference(workspace, job, r.Scheme); err != nil { + return err + } err = r.Create(ctx, job) if err != nil { log.Error(err, "Failed to create backup Job for DevWorkspace", "devworkspace", workspace.Name) diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index 18731e03d..b245f2cd6 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -210,7 +210,20 @@ var _ = Describe("BackupCronJobReconciler", func() { }) Context("executeBackupSync", func() { - It("creates a Job for a DevWorkspace stopped within last 30 minutes", func() { + It("creates a Job for a DevWorkspace stopped with no previsou backup", func() { + enabled := true + schedule := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + }, + }, + }, + } dw := createDevWorkspace("dw-recent", "ns-a", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) dw.Status.Phase = dwv2.DevWorkspaceStatusStopped dw.Status.DevWorkspaceId = "id-recent" @@ -219,7 +232,7 @@ var _ = Describe("BackupCronJobReconciler", func() { pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} Expect(fakeClient.Create(ctx, pvc)).To(Succeed()) - Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed()) + Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed()) jobList := &batchv1.JobList{} Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) @@ -227,6 +240,23 @@ var _ = Describe("BackupCronJobReconciler", func() { }) It("does not create a Job when the DevWorkspace was stopped beyond time range", func() { + enabled := true + schedule := "* * * * *" + lastBackupTime := metav1.NewTime(time.Now().Add(-15 * time.Minute)) + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + }, + }, + }, + Status: &controllerv1alpha1.OperatorConfigurationStatus{ + LastBackupTime: &lastBackupTime, + }, + } dw := createDevWorkspace("dw-old", "ns-b", false, metav1.NewTime(time.Now().Add(-60*time.Minute))) dw.Status.Phase = dwv2.DevWorkspaceStatusStopped dw.Status.DevWorkspaceId = "id-old" @@ -235,7 +265,7 @@ var _ = Describe("BackupCronJobReconciler", func() { pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} Expect(fakeClient.Create(ctx, pvc)).To(Succeed()) - Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed()) + Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed()) jobList := &batchv1.JobList{} Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) @@ -243,13 +273,26 @@ var _ = Describe("BackupCronJobReconciler", func() { }) It("does not create a Job for a running DevWorkspace", func() { + enabled := true + schedule := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + }, + }, + }, + } dw := createDevWorkspace("dw-running", "ns-c", true, metav1.NewTime(time.Now().Add(-5*time.Minute))) Expect(fakeClient.Create(ctx, dw)).To(Succeed()) pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} Expect(fakeClient.Create(ctx, pvc)).To(Succeed()) - Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed()) + Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed()) jobList := &batchv1.JobList{} Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index e4d7af119..d592b7897 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -3050,9 +3050,79 @@ spec: type: string metadata: type: object + status: + description: |- + Status represents the current status of the DevWorkspaceOperatorConfig + automatically managed by the DevWorkspace Operator. + properties: + conditions: + description: Conditions represent the latest available observations of the OperatorConfiguration's state + items: + description: Condition contains details for one aspect of the current state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + lastBackupTime: + description: |- + LastBackupTime is the timestamp of the last successful backup. Nil if + no backup is configured or no backup has yet succeeded. + format: date-time + type: string + type: object type: object served: true storage: true + subresources: + status: {} status: acceptedNames: kind: "" diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 679ea7fd7..72c26350e 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -3202,9 +3202,81 @@ spec: type: string metadata: type: object + status: + description: |- + Status represents the current status of the DevWorkspaceOperatorConfig + automatically managed by the DevWorkspace Operator. + properties: + conditions: + description: Conditions represent the latest available observations + of the OperatorConfiguration's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + lastBackupTime: + description: |- + LastBackupTime is the timestamp of the last successful backup. Nil if + no backup is configured or no backup has yet succeeded. + format: date-time + type: string + type: object type: object served: true storage: true + subresources: + status: {} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition 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 daabd4f63..3e1a1b1f5 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -3202,6 +3202,78 @@ spec: type: string metadata: type: object + status: + description: |- + Status represents the current status of the DevWorkspaceOperatorConfig + automatically managed by the DevWorkspace Operator. + properties: + conditions: + description: Conditions represent the latest available observations + of the OperatorConfiguration's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + lastBackupTime: + description: |- + LastBackupTime is the timestamp of the last successful backup. Nil if + no backup is configured or no backup has yet succeeded. + format: date-time + type: string + type: object type: object served: true storage: true + subresources: + status: {} diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index d928112a9..ae3d589ed 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -3202,9 +3202,81 @@ spec: type: string metadata: type: object + status: + description: |- + Status represents the current status of the DevWorkspaceOperatorConfig + automatically managed by the DevWorkspace Operator. + properties: + conditions: + description: Conditions represent the latest available observations + of the OperatorConfiguration's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + lastBackupTime: + description: |- + LastBackupTime is the timestamp of the last successful backup. Nil if + no backup is configured or no backup has yet succeeded. + format: date-time + type: string + type: object type: object served: true storage: true + subresources: + status: {} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition 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 daabd4f63..3e1a1b1f5 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -3202,6 +3202,78 @@ spec: type: string metadata: type: object + status: + description: |- + Status represents the current status of the DevWorkspaceOperatorConfig + automatically managed by the DevWorkspace Operator. + properties: + conditions: + description: Conditions represent the latest available observations + of the OperatorConfiguration's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + lastBackupTime: + description: |- + LastBackupTime is the timestamp of the last successful backup. Nil if + no backup is configured or no backup has yet succeeded. + format: date-time + type: string + type: object type: object served: true storage: true + subresources: + status: {} diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 334f38d7c..1e4a7de84 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -3200,6 +3200,78 @@ spec: type: string metadata: type: object + status: + description: |- + Status represents the current status of the DevWorkspaceOperatorConfig + automatically managed by the DevWorkspace Operator. + properties: + conditions: + description: Conditions represent the latest available observations + of the OperatorConfiguration's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + lastBackupTime: + description: |- + LastBackupTime is the timestamp of the last successful backup. Nil if + no backup is configured or no backup has yet succeeded. + format: date-time + type: string + type: object type: object served: true storage: true + subresources: + status: {} diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index ab2f7b26a..5ddd8cd3b 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -85,6 +85,7 @@ var defaultConfig = &v1alpha1.OperatorConfiguration{ BackupCronJob: &v1alpha1.BackupCronJobConfig{ Enable: pointer.Bool(false), Schedule: "0 0 1 * *", + Registry: "sample.com", }, // Do not declare a default value for this field. // Setting a default leads to an endless reconcile loop when UserNamespacesSupport is disabled, diff --git a/pkg/config/sync.go b/pkg/config/sync.go index f3ed4e384..a7844fa35 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -441,6 +441,9 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { if from.Workspace.BackupCronJob.Schedule != "" { to.Workspace.BackupCronJob.Schedule = from.Workspace.BackupCronJob.Schedule } + if from.Workspace.BackupCronJob.Registry != "" { + to.Workspace.BackupCronJob.Registry = from.Workspace.BackupCronJob.Registry + } } if from.Workspace.PostStartTimeout != "" { diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 092dddeab..3f8f1d33e 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -172,4 +172,10 @@ const ( // NamespaceNodeSelectorAnnotation is an annotation applied to a namespace to configure the node selector for all workspaces // in that namespace. Value should be json-encoded map[string]string NamespaceNodeSelectorAnnotation = "controller.devfile.io/node-selector" + + // DevWorkspaceBackupJobNamePrefix is the prefix used for backup jobs created for DevWorkspaces + DevWorkspaceBackupJobNamePrefix = "devworkspace-backup-" + + // DevWorkspaceBackupJobLabel is the label key to identify backup jobs created for DevWorkspaces + DevWorkspaceBackupJobLabel = "controller.devfile.io/backup-job" ) From 307145c15b85063ca79d2892859b7950f36674f7 Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Wed, 29 Oct 2025 12:09:57 +0100 Subject: [PATCH 04/12] feat: Use PVC name from config if provided A backup job use a PVC name from a default value or from the config if user configured custom name. Signed-off-by: Ales Raszka --- .../backupcronjob/backupcronjob_controller.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 8993bc962..7b1baf22f 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -235,7 +235,6 @@ func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) { func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) error { log := logger.WithName("executeBackupSync") log.Info("Executing backup sync for all DevWorkspaces") - backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob devWorkspaces := &dw.DevWorkspaceList{} err := r.List(ctx, devWorkspaces) if err != nil { @@ -254,7 +253,7 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera dwID := dw.Status.DevWorkspaceId log.Info("Found DevWorkspace", "namespace", dw.Namespace, "devworkspace", dw.Name, "id", dwID) - if err := r.createBackupJob(&dw, ctx, backUpConfig, logger); err != nil { + if err := r.createBackupJob(&dw, ctx, dwOperatorConfig, logger); err != nil { log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID) continue } @@ -309,13 +308,18 @@ func ptrInt32(i int32) *int32 { return &i } func ptrBool(b bool) *bool { return &b } // createBackupJob creates a Kubernetes Job to back up the workspace's PVC data. -func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error { +func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) error { log := logger.WithName("createBackupJob") dwID := workspace.Status.DevWorkspaceId + backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob - // Find a PVC with the name "claim-devworkspace" + // Find a PVC with the name "claim-devworkspace" or based on the name from the operator config + pvcName := "claim-devworkspace" + if dwOperatorConfig.Config.Workspace.PVCName != "" { + pvcName = dwOperatorConfig.Config.Workspace.PVCName + } pvc := &corev1.PersistentVolumeClaim{} - err := r.Get(ctx, client.ObjectKey{Name: "claim-devworkspace", Namespace: workspace.Namespace}, pvc) + err := r.Get(ctx, client.ObjectKey{Name: pvcName, Namespace: workspace.Namespace}, pvc) if err != nil { log.Error(err, "Failed to get PVC for DevWorkspace", "id", dwID) return err From 51b3eec18fd9ceda7fde05518a3956943069d5f9 Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Mon, 3 Nov 2025 10:32:14 +0100 Subject: [PATCH 05/12] feat: Allow using registry auth for private registies The backup job can now push to registries which requires auth token. The token is provided as a secret in operator namespace and added to the operator config. Signed-off-by: Ales Raszka --- .../devworkspaceoperatorconfig_types.go | 5 + .../backupcronjob/backupcronjob_controller.go | 110 +++++++++++++++++- .../backupcronjob_controller_test.go | 55 ++++++++- ...evfile.io_devworkspaceoperatorconfigs.yaml | 5 + ...kspace-operator.clusterserviceversion.yaml | 7 +- deploy/deployment/kubernetes/combined.yaml | 12 +- ...workspace-controller-role.ClusterRole.yaml | 7 +- ...r.devfile.io.CustomResourceDefinition.yaml | 5 + deploy/deployment/openshift/combined.yaml | 12 +- ...workspace-controller-role.ClusterRole.yaml | 7 +- ...r.devfile.io.CustomResourceDefinition.yaml | 5 + deploy/templates/components/rbac/role.yaml | 7 +- ...evfile.io_devworkspaceoperatorconfigs.yaml | 5 + main.go | 7 +- pkg/config/sync.go | 4 + pkg/constants/metadata.go | 2 + 16 files changed, 236 insertions(+), 19 deletions(-) diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index c85d4e211..2a64f6807 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -86,6 +86,11 @@ type BackupCronJobConfig struct { // in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} // +kubebuilder:validation:Optional Registry string `json:"registry,omitempty"` + + // RegistryAuthSecret is the name of a Kubernetes secret of + // type kubernetes.io/dockerconfigjson + // +kubebuilder:validation:Optional + RegistryAuthSecret string `json:"registryAuthSecret,omitempty"` } type RoutingConfig struct { diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 7b1baf22f..3340b5cbd 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -48,8 +48,9 @@ const ( // BackupCronJobReconciler reconciles `BackupCronJob` configuration for the purpose of backing up workspace PVCs. type BackupCronJobReconciler struct { client.Client - Log logr.Logger - Scheme *runtime.Scheme + NonCachingClient client.Client + Log logr.Logger + Scheme *runtime.Scheme cron *cron.Cron } @@ -94,6 +95,12 @@ func shouldReconcileOnUpdate(e event.UpdateEvent, log logr.Logger) bool { if oldBackup.Schedule != newBackup.Schedule { return true } + if oldBackup.Registry != newBackup.Registry { + return true + } + if oldBackup.RegistryAuthSecret != newBackup.RegistryAuthSecret { + return true + } return false } @@ -139,6 +146,7 @@ func (r *BackupCronJobReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list // +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;create;update;patch;delete // +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;update;patch;watch @@ -235,8 +243,14 @@ func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) { func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) error { log := logger.WithName("executeBackupSync") log.Info("Executing backup sync for all DevWorkspaces") + + registyAuthSecret, err := r.getRegistryAuthSecret(ctx, dwOperatorConfig, logger) + if err != nil { + log.Error(err, "Failed to get registry auth secret for backup job") + return err + } devWorkspaces := &dw.DevWorkspaceList{} - err := r.List(ctx, devWorkspaces) + err = r.List(ctx, devWorkspaces) if err != nil { log.Error(err, "Failed to list DevWorkspaces") return err @@ -253,7 +267,7 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera dwID := dw.Status.DevWorkspaceId log.Info("Found DevWorkspace", "namespace", dw.Namespace, "devworkspace", dw.Name, "id", dwID) - if err := r.createBackupJob(&dw, ctx, dwOperatorConfig, logger); err != nil { + if err := r.createBackupJob(&dw, ctx, dwOperatorConfig, registyAuthSecret, logger); err != nil { log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID) continue } @@ -274,6 +288,24 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera return nil } +func (r *BackupCronJobReconciler) getRegistryAuthSecret(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) (*corev1.Secret, error) { + log := logger.WithName("getRegistryAuthSecret") + registryAuthSecret := &corev1.Secret{} + if dwOperatorConfig.Config.Workspace.BackupCronJob.RegistryAuthSecret != "" { + err := r.NonCachingClient.Get(ctx, client.ObjectKey{ + Name: dwOperatorConfig.Config.Workspace.BackupCronJob.RegistryAuthSecret, + Namespace: dwOperatorConfig.Namespace, + }, registryAuthSecret) + if err != nil { + log.Error(err, "Failed to get registry auth secret for backup job", "secretName", dwOperatorConfig.Config.Workspace.BackupCronJob.RegistryAuthSecret) + return nil, err + } + log.Info("Successfully retrieved registry auth secret for backup job", "secretName", dwOperatorConfig.Config.Workspace.BackupCronJob.RegistryAuthSecret) + return registryAuthSecret, nil + } + return nil, nil +} + // wasStoppedSinceLastBackup checks if the DevWorkspace was stopped since the last backup time. func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWorkspace, lastBackupTime *metav1.Time, logger logr.Logger) bool { log := logger.WithName("wasStoppedSinceLastBackup") @@ -308,7 +340,13 @@ func ptrInt32(i int32) *int32 { return &i } func ptrBool(b bool) *bool { return &b } // createBackupJob creates a Kubernetes Job to back up the workspace's PVC data. -func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) error { +func (r *BackupCronJobReconciler) createBackupJob( + workspace *dw.DevWorkspace, + ctx context.Context, + dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, + registyAuthSecret *corev1.Secret, + logger logr.Logger, +) error { log := logger.WithName("createBackupJob") dwID := workspace.Status.DevWorkspaceId backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob @@ -358,7 +396,6 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct {Name: "BUILDAH_PUSH_OPTIONS", Value: "--tls-verify=false"}, }, Image: images.GetProjectBackupImage(), - // Image: "localhost:5001/workspace-backup-image:v3", Args: []string{ "/workspace-recovery.sh", "--backup", @@ -402,6 +439,30 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct }, }, } + if registyAuthSecret != nil { + secret, err := r.copySecret(workspace, ctx, registyAuthSecret, logger) + if err != nil { + return err + } + job.Spec.Template.Spec.Volumes = append(job.Spec.Template.Spec.Volumes, corev1.Volume{ + Name: "registry-auth-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secret.Name, + }, + }, + }) + job.Spec.Template.Spec.Containers[0].VolumeMounts = append(job.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "registry-auth-secret", + MountPath: "/home/user/.docker", + ReadOnly: true, + }) + job.Spec.Template.Spec.Containers[0].Env = append(job.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "REGISTRY_AUTH_FILE", + Value: "/home/user/.docker/.dockerconfigjson", + }) + + } if err := controllerutil.SetControllerReference(workspace, job, r.Scheme); err != nil { return err } @@ -413,3 +474,40 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct log.Info("Created backup Job for DevWorkspace", "jobName", job.Name, "devworkspace", workspace.Name) return nil } + +func (r *BackupCronJobReconciler) copySecret(workspace *dw.DevWorkspace, ctx context.Context, sourceSecret *corev1.Secret, logger logr.Logger) (namespaceSecret *corev1.Secret, err error) { + log := logger.WithName("copySecret") + existingNamespaceSecret := &corev1.Secret{} + err = r.NonCachingClient.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackuptAuthSecretName, + Namespace: workspace.Namespace}, existingNamespaceSecret) + if client.IgnoreNotFound(err) != nil { + log.Error(err, "Failed to check for existing registry auth secret in workspace namespace", "namespace", workspace.Namespace) + return nil, err + } + if err == nil { + log.Info("Deleting existing registry auth secret in workspace namespace", "namespace", workspace.Namespace) + err = r.Delete(ctx, existingNamespaceSecret) + if err != nil { + return nil, err + } + log.Info("Successfully deleted existing registry auth secret in workspace namespace", "namespace", workspace.Namespace) + } + namespaceSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DevWorkspaceBackuptAuthSecretName, + Namespace: workspace.Namespace, + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: workspace.Status.DevWorkspaceId, + constants.DevWorkspaceWatchSecretLabel: "true", + }, + }, + Data: sourceSecret.Data, + Type: sourceSecret.Type, + } + if err := controllerutil.SetControllerReference(workspace, namespaceSecret, r.Scheme); err != nil { + return nil, err + } + err = r.Create(ctx, namespaceSecret) + return namespaceSecret, err +} diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index b245f2cd6..ee22d21fd 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -60,10 +60,11 @@ var _ = Describe("BackupCronJobReconciler", func() { log = zap.New(zap.UseDevMode(true)).WithName("BackupCronJobReconcilerTest") reconciler = BackupCronJobReconciler{ - Client: fakeClient, - Log: log, - Scheme: scheme, - cron: cron.New(), + Client: fakeClient, + NonCachingClient: fakeClient, + Log: log, + Scheme: scheme, + cron: cron.New(), } nameNamespace = types.NamespacedName{ @@ -298,6 +299,40 @@ var _ = Describe("BackupCronJobReconciler", func() { Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) Expect(jobList.Items).To(HaveLen(0)) }) + + It("creates a Job for a DevWorkspace stopped with no previsou backup and auth registry", func() { + enabled := true + schedule := "* * * * *" + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace}, + Config: &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + Enable: &enabled, + Schedule: schedule, + Registry: "my-registry:5000", + RegistryAuthSecret: "my-secret", + }, + }, + }, + } + dw := createDevWorkspace("dw-recent", "ns-a", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.Phase = dwv2.DevWorkspaceStatusStopped + dw.Status.DevWorkspaceId = "id-recent" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} + Expect(fakeClient.Create(ctx, pvc)).To(Succeed()) + + authSecret := createAuthSecret("my-secret", nameNamespace.Namespace, map[string][]byte{}) + Expect(fakeClient.Create(ctx, authSecret)).To(Succeed()) + + Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed()) + + jobList := &batchv1.JobList{} + Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) + Expect(jobList.Items).To(HaveLen(1)) + }) }) }) @@ -334,6 +369,18 @@ func createDevWorkspace(name, namespace string, started bool, lastTransitionTime return dw } +func createAuthSecret(name, namespace string, data map[string][]byte) *corev1.Secret { + { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: data, + } + } +} + var _ = Describe("DevWorkspaceOperatorConfig UpdateFunc Tests", func() { var configPredicate predicate.Funcs diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index d592b7897..b2e60a891 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -194,6 +194,11 @@ spec: A registry where backup images are stored. Images are stored in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} type: string + registryAuthSecret: + description: |- + RegistryAuthSecret is the name of a Kubernetes secret of + type kubernetes.io/dockerconfigjson + type: string schedule: default: 0 2 * * * description: |- diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index a2b00d28c..2339bbe0e 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -102,7 +102,6 @@ spec: resources: - configmaps - pods - - secrets - serviceaccounts - services verbs: @@ -128,6 +127,12 @@ spec: - pods/exec verbs: - create + - apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resourceNames: diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 72c26350e..fc5f5e4a6 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -196,6 +196,11 @@ spec: A registry where backup images are stored. Images are stored in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} type: string + registryAuthSecret: + description: |- + RegistryAuthSecret is the name of a Kubernetes secret of + type kubernetes.io/dockerconfigjson + type: string schedule: default: 0 2 * * * description: |- @@ -25903,7 +25908,6 @@ rules: resources: - configmaps - pods - - secrets - serviceaccounts - services verbs: @@ -25929,6 +25933,12 @@ rules: - pods/exec verbs: - create +- apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resourceNames: diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index 311097a15..158eba08e 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -22,7 +22,6 @@ rules: resources: - configmaps - pods - - secrets - serviceaccounts - services verbs: @@ -48,6 +47,12 @@ rules: - pods/exec verbs: - create +- apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resourceNames: 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 3e1a1b1f5..199ffee70 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -196,6 +196,11 @@ spec: A registry where backup images are stored. Images are stored in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} type: string + registryAuthSecret: + description: |- + RegistryAuthSecret is the name of a Kubernetes secret of + type kubernetes.io/dockerconfigjson + type: string schedule: default: 0 2 * * * description: |- diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index ae3d589ed..f639037c8 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -196,6 +196,11 @@ spec: A registry where backup images are stored. Images are stored in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} type: string + registryAuthSecret: + description: |- + RegistryAuthSecret is the name of a Kubernetes secret of + type kubernetes.io/dockerconfigjson + type: string schedule: default: 0 2 * * * description: |- @@ -25903,7 +25908,6 @@ rules: resources: - configmaps - pods - - secrets - serviceaccounts - services verbs: @@ -25929,6 +25933,12 @@ rules: - pods/exec verbs: - create +- apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resourceNames: diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index 311097a15..158eba08e 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -22,7 +22,6 @@ rules: resources: - configmaps - pods - - secrets - serviceaccounts - services verbs: @@ -48,6 +47,12 @@ rules: - pods/exec verbs: - create +- apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resourceNames: 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 3e1a1b1f5..199ffee70 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -196,6 +196,11 @@ spec: A registry where backup images are stored. Images are stored in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} type: string + registryAuthSecret: + description: |- + RegistryAuthSecret is the name of a Kubernetes secret of + type kubernetes.io/dockerconfigjson + type: string schedule: default: 0 2 * * * description: |- diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index 6d9cc6886..332abafe5 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -20,7 +20,6 @@ rules: resources: - configmaps - pods - - secrets - serviceaccounts - services verbs: @@ -46,6 +45,12 @@ rules: - pods/exec verbs: - create +- apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resourceNames: diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 1e4a7de84..0f26fd363 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -194,6 +194,11 @@ spec: A registry where backup images are stored. Images are stored in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} type: string + registryAuthSecret: + description: |- + RegistryAuthSecret is the name of a Kubernetes secret of + type kubernetes.io/dockerconfigjson + type: string schedule: default: 0 2 * * * description: |- diff --git a/main.go b/main.go index 4f41c5e5a..754c2258b 100644 --- a/main.go +++ b/main.go @@ -190,9 +190,10 @@ func main() { os.Exit(1) } if err = (&backupCronJobController.BackupCronJobReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("BackupCronJob"), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + NonCachingClient: nonCachingClient, + Log: ctrl.Log.WithName("controllers").WithName("BackupCronJob"), + Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "BackupCronJob") os.Exit(1) diff --git a/pkg/config/sync.go b/pkg/config/sync.go index a7844fa35..f18203fc6 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -444,6 +444,9 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { if from.Workspace.BackupCronJob.Registry != "" { to.Workspace.BackupCronJob.Registry = from.Workspace.BackupCronJob.Registry } + if from.Workspace.BackupCronJob.RegistryAuthSecret != "" { + to.Workspace.BackupCronJob.RegistryAuthSecret = from.Workspace.BackupCronJob.RegistryAuthSecret + } } if from.Workspace.PostStartTimeout != "" { @@ -706,6 +709,7 @@ func GetCurrentConfigString(currConfig *controller.OperatorConfiguration) string if workspace.BackupCronJob.Schedule != defaultConfig.Workspace.BackupCronJob.Schedule { config = append(config, fmt.Sprintf("workspace.backupCronJob.cronJobScript=%s", workspace.BackupCronJob.Schedule)) } + } if workspace.HostUsers != nil { config = append(config, fmt.Sprintf("workspace.hostUsers=%t", *workspace.HostUsers)) diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 3f8f1d33e..03f2f23fa 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -178,4 +178,6 @@ const ( // DevWorkspaceBackupJobLabel is the label key to identify backup jobs created for DevWorkspaces DevWorkspaceBackupJobLabel = "controller.devfile.io/backup-job" + + DevWorkspaceBackuptAuthSecretName = "devworkspace-backup-registry-auth" ) From 9b0ed962be2e869f05ed311cfd2dca77fd0ef27f Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Mon, 3 Nov 2025 10:38:49 +0100 Subject: [PATCH 06/12] fix: Use correct repository for backup job Signed-off-by: Ales Raszka --- Makefile | 2 +- .../devworkspace-operator.clusterserviceversion.yaml | 4 ++-- deploy/deployment/kubernetes/combined.yaml | 2 +- .../objects/devworkspace-controller-manager.Deployment.yaml | 2 +- deploy/deployment/openshift/combined.yaml | 2 +- .../objects/devworkspace-controller-manager.Deployment.yaml | 2 +- deploy/templates/components/csv/clusterserviceversion.yaml | 2 +- make-release.sh | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 89a6a8f97..d7053812b 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,7 @@ export DWO_IMG ?= quay.io/devfile/devworkspace-controller:next export DWO_BUNDLE_IMG ?= quay.io/devfile/devworkspace-operator-bundle:next export DWO_INDEX_IMG ?= quay.io/devfile/devworkspace-operator-index:next export PROJECT_CLONE_IMG ?= quay.io/devfile/project-clone:next -export PROJECT_BACKUP_IMG ?= quay.io/devfile/project-clone:next +export PROJECT_BACKUP_IMG ?= quay.io/devfile/project-backup:next export PULL_POLICY ?= Always export DEFAULT_ROUTING ?= basic export KUBECONFIG ?= ${HOME}/.kube/config diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index 2339bbe0e..fcf4ee6f1 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -345,7 +345,7 @@ spec: - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next - name: RELATED_IMAGE_project_backup - value: quay.io/devfile/project-clone:next + value: quay.io/devfile/project-backup:next - name: WATCH_NAMESPACE valueFrom: fieldRef: @@ -479,7 +479,7 @@ spec: name: devworkspace_webhook_server - image: quay.io/devfile/project-clone:next name: project_clone - - image: quay.io/devfile/project-clone:next + - image: quay.io/devfile/project-backup:next name: project_backup - image: registry.access.redhat.com/ubi9/ubi-micro:9.5-1733126338 name: pvc_cleanup_job diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index fc5f5e4a6..748670445 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -26269,7 +26269,7 @@ spec: - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next - name: RELATED_IMAGE_project_backup - value: quay.io/devfile/project-clone:next + value: quay.io/devfile/project-backup:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml index cb31391bf..6be98e9be 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-manager.Deployment.yaml @@ -31,7 +31,7 @@ spec: - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next - name: RELATED_IMAGE_project_backup - value: quay.io/devfile/project-clone:next + value: quay.io/devfile/project-backup:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index f639037c8..bacbc8aaa 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -26271,7 +26271,7 @@ spec: - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next - name: RELATED_IMAGE_project_backup - value: quay.io/devfile/project-clone:next + value: quay.io/devfile/project-backup:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml index 95da7453d..101a59fa3 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-manager.Deployment.yaml @@ -31,7 +31,7 @@ spec: - name: RELATED_IMAGE_project_clone value: quay.io/devfile/project-clone:next - name: RELATED_IMAGE_project_backup - value: quay.io/devfile/project-clone:next + value: quay.io/devfile/project-backup:next - name: WATCH_NAMESPACE value: "" - name: POD_NAME diff --git a/deploy/templates/components/csv/clusterserviceversion.yaml b/deploy/templates/components/csv/clusterserviceversion.yaml index 12e771e86..50bede51e 100644 --- a/deploy/templates/components/csv/clusterserviceversion.yaml +++ b/deploy/templates/components/csv/clusterserviceversion.yaml @@ -104,7 +104,7 @@ spec: name: devworkspace_webhook_server - image: quay.io/devfile/project-clone:next name: project_clone - - image: quay.io/devfile/project-clone:next + - image: quay.io/devfile/project-backup:next name: project_backup - image: registry.access.redhat.com/ubi9/ubi-micro:9.5-1733126338 name: pvc_cleanup_job diff --git a/make-release.sh b/make-release.sh index 912419092..1076b8acd 100755 --- a/make-release.sh +++ b/make-release.sh @@ -143,7 +143,7 @@ update_images() { sed -i build/scripts/generate_deployment.sh -r \ -e "s|quay.io/devfile/devworkspace-controller:[0-9a-zA-Z._-]+|${DWO_QUAY_IMG}|g" \ -e "s|quay.io/devfile/project-clone:[0-9a-zA-Z._-]+|${PROJECT_CLONE_QUAY_IMG}|g" - -e "s|quay.io/devfile/project-backup:[0-9a-zA-Z._-]+|${PROJECT_PROJECT_BACKUP_QUAY_IMGCLONE_QUAY_IMG}|g" + -e "s|quay.io/devfile/project-backup:[0-9a-zA-Z._-]+|${PROJECT_BACKUP_QUAY_IMG}|g" local DEFAULT_DWO_IMG="$DWO_QUAY_IMG" local PROJECT_CLONE_IMG="$PROJECT_CLONE_QUAY_IMG" From c282a597ba0bfe9bcd31b103285eb8be1f84496c Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Mon, 3 Nov 2025 13:43:37 +0100 Subject: [PATCH 07/12] Dynamic PVC selection base on workspace A backup job now determines the name of pvc based on used storage type. It distinguish between different storage types (common and per-workspace) and mount the volume dynamically. Signed-off-by: Ales Raszka --- .../backupcronjob/backupcronjob_controller.go | 45 ++++++++++++++++--- project-backup/workspace-recovery.sh | 8 ++-- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 3340b5cbd..1ad374f65 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -24,10 +24,13 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/internal/images" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/conditions" "github.com/devfile/devworkspace-operator/pkg/config" + wkspConfig "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/infrastructure" + "github.com/devfile/devworkspace-operator/pkg/provision/storage" "github.com/go-logr/logr" "github.com/robfig/cron/v3" batchv1 "k8s.io/api/batch/v1" @@ -352,12 +355,18 @@ func (r *BackupCronJobReconciler) createBackupJob( backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob // Find a PVC with the name "claim-devworkspace" or based on the name from the operator config - pvcName := "claim-devworkspace" - if dwOperatorConfig.Config.Workspace.PVCName != "" { - pvcName = dwOperatorConfig.Config.Workspace.PVCName + pvcName, workspacePath, err := r.getWorkspacePVCName(workspace, dwOperatorConfig, ctx, logger) + if err != nil { + log.Error(err, "Failed to get workspace PVC name", "devworkspace", workspace.Name) + return err + } + if pvcName == "" { + log.Error(err, "No PVC found for DevWorkspace", "id", dwID) + return err } + pvc := &corev1.PersistentVolumeClaim{} - err := r.Get(ctx, client.ObjectKey{Name: pvcName, Namespace: workspace.Namespace}, pvc) + err = r.Get(ctx, client.ObjectKey{Name: pvcName, Namespace: workspace.Namespace}, pvc) if err != nil { log.Error(err, "Failed to get PVC for DevWorkspace", "id", dwID) return err @@ -388,7 +397,7 @@ func (r *BackupCronJobReconciler) createBackupJob( {Name: "WORKSPACE_ID", Value: dwID}, { Name: "BACKUP_SOURCE_PATH", - Value: "/workspace/" + dwID + "/" + constants.DefaultProjectsSourcesRoot, + Value: "/workspace/" + workspacePath, }, {Name: "STORAGE_DRIVER", Value: "overlay"}, {Name: "BUILDAH_ISOLATION", Value: "chroot"}, @@ -475,6 +484,32 @@ func (r *BackupCronJobReconciler) createBackupJob( return nil } +// getWorkspacePVCName determines the PVC name and workspace path based on the storage provisioner used. +func (r *BackupCronJobReconciler) getWorkspacePVCName(workspace *dw.DevWorkspace, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, ctx context.Context, logger logr.Logger) (string, string, error) { + config, err := wkspConfig.ResolveConfigForWorkspace(workspace, r.Client) + + workspaceWithConfig := &common.DevWorkspaceWithConfig{} + workspaceWithConfig.DevWorkspace = workspace + workspaceWithConfig.Config = config + + storageProvisioner, err := storage.GetProvisioner(workspaceWithConfig) + if err != nil { + return "", "", err + } + if _, ok := storageProvisioner.(*storage.PerWorkspaceStorageProvisioner); ok { + pvcName := common.PerWorkspacePVCName(workspace.Status.DevWorkspaceId) + return pvcName, constants.DefaultProjectsSourcesRoot, nil + + } else if _, ok := storageProvisioner.(*storage.CommonStorageProvisioner); ok { + pvcName := "claim-devworkspace" + if dwOperatorConfig.Config.Workspace.PVCName != "" { + pvcName = dwOperatorConfig.Config.Workspace.PVCName + } + return pvcName, workspace.Status.DevWorkspaceId + "/" + constants.DefaultProjectsSourcesRoot, nil + } + return "", "", nil +} + func (r *BackupCronJobReconciler) copySecret(workspace *dw.DevWorkspace, ctx context.Context, sourceSecret *corev1.Secret, logger logr.Logger) (namespaceSecret *corev1.Secret, err error) { log := logger.WithName("copySecret") existingNamespaceSecret := &corev1.Secret{} diff --git a/project-backup/workspace-recovery.sh b/project-backup/workspace-recovery.sh index 266cbe9f8..7fa9771b5 100644 --- a/project-backup/workspace-recovery.sh +++ b/project-backup/workspace-recovery.sh @@ -6,7 +6,6 @@ set -x : "${DEVWORKSPACE_BACKUP_REGISTRY:?Missing DEVWORKSPACE_BACKUP_REGISTRY}" : "${DEVWORKSPACE_NAMESPACE:?Missing DEVWORKSPACE_NAMESPACE}" : "${DEVWORKSPACE_NAME:?Missing DEVWORKSPACE_NAME}" -: "${PROJECTS_ROOT:?Missing PROJECTS_ROOT}" : "${BACKUP_SOURCE_PATH:?Missing BACKUP_SOURCE_PATH}" BACKUP_IMAGE="${DEVWORKSPACE_BACKUP_REGISTRY}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME}:latest" @@ -16,6 +15,9 @@ backup() { local new_image new_image=$(buildah from scratch) + echo "Backing up workspace from path: $BACKUP_SOURCE_PATH" + ls -la "$BACKUP_SOURCE_PATH" + buildah copy "$new_image" "$BACKUP_SOURCE_PATH" / buildah config --label DEVWORKSPACE="$DEVWORKSPACE_NAME" "$new_image" buildah config --label NAMESPACE="$DEVWORKSPACE_NAMESPACE" "$new_image" @@ -29,8 +31,8 @@ restore() { local container_name="workspace-restore" podman create --name "$container_name" "$BACKUP_IMAGE" - rm -rf "${PROJECTS_ROOT:?}"/* - podman cp "$container_name":/. "$PROJECTS_ROOT" + rm -rf "${BACKUP_SOURCE_PATH:?}"/* + podman cp "$container_name":/. "$BACKUP_SOURCE_PATH" podman rm "$container_name" } From aa1053d66ec04996ccf3a4d006703bb9e22a46ae Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Wed, 5 Nov 2025 09:33:42 +0100 Subject: [PATCH 08/12] Remove unnecessary capabilities from backup Job It turns out the capabilities from the prototype are not needed. Signed-off-by: Ales Raszka --- controllers/backupcronjob/backupcronjob_controller.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 1ad374f65..023cfc732 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -420,10 +420,7 @@ func (r *BackupCronJobReconciler) createBackupJob( }, }, SecurityContext: &corev1.SecurityContext{ - RunAsUser: ptrInt64(0), - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{"SYS_ADMIN", "SYS_CHROOT"}, - }, + RunAsUser: ptrInt64(0), AllowPrivilegeEscalation: ptrBool(false), }, }, From b99674a979412e5042653d45708cd381713a407b Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Wed, 5 Nov 2025 12:35:26 +0100 Subject: [PATCH 09/12] Create SA for backup jobs A new SA is created for the backup jobs to limit the permission to just what is necessary. Signed-off-by: Ales Raszka --- .../backupcronjob/backupcronjob_controller.go | 12 +++- controllers/backupcronjob/rbac.go | 56 +++++++++++++++++++ ...kspace-operator.clusterserviceversion.yaml | 14 ++--- deploy/deployment/kubernetes/combined.yaml | 14 ++--- ...workspace-controller-role.ClusterRole.yaml | 14 ++--- deploy/deployment/openshift/combined.yaml | 14 ++--- ...workspace-controller-role.ClusterRole.yaml | 14 ++--- deploy/templates/components/rbac/role.yaml | 14 ++--- 8 files changed, 108 insertions(+), 44 deletions(-) create mode 100644 controllers/backupcronjob/rbac.go diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 023cfc732..8e613e889 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -149,8 +149,9 @@ func (r *BackupCronJobReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=serviceaccounts;,verbs=get;list;create;update;patch;delete // +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;create;update;patch;delete // +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;update;patch;watch // +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list @@ -270,6 +271,12 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera dwID := dw.Status.DevWorkspaceId log.Info("Found DevWorkspace", "namespace", dw.Namespace, "devworkspace", dw.Name, "id", dwID) + err = r.ensureJobRunnerRBAC(ctx, &dw) + if err != nil { + log.Error(err, "Failed to ensure Job runner RBAC for DevWorkspace", "id", dwID) + continue + } + if err := r.createBackupJob(&dw, ctx, dwOperatorConfig, registyAuthSecret, logger); err != nil { log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID) continue @@ -384,7 +391,8 @@ func (r *BackupCronJobReconciler) createBackupJob( Spec: batchv1.JobSpec{ Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, + ServiceAccountName: JobRunnerSAName, + RestartPolicy: corev1.RestartPolicyNever, SecurityContext: &corev1.PodSecurityContext{ FSGroup: ptrInt64(0), }, diff --git a/controllers/backupcronjob/rbac.go b/controllers/backupcronjob/rbac.go new file mode 100644 index 000000000..5e5d8f4cd --- /dev/null +++ b/controllers/backupcronjob/rbac.go @@ -0,0 +1,56 @@ +// +// Copyright (c) 2019-2025 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" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/constants" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +const ( + JobRunnerSAName = "devworkspace-job-runner" +) + +func (r *BackupCronJobReconciler) ensureJobRunnerRBAC(ctx context.Context, workspace *dw.DevWorkspace) error { + const ( + roleName = "devworkspace-job-runner-role" + rbName = "devworkspace-job-runner-rolebinding" + ) + + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: JobRunnerSAName, Namespace: workspace.Namespace, Labels: map[string]string{ + constants.DevWorkspaceIDLabel: workspace.Status.DevWorkspaceId, + constants.DevWorkspaceWatchSecretLabel: "true", + }}, + } + + // Create or update ServiceAccount + if err := controllerutil.SetControllerReference(workspace, sa, r.Scheme); err != nil { + return err + } + if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, sa, func() error { return nil }); err != nil { + return fmt.Errorf("ensuring ServiceAccount: %w", err) + } + + return nil + +} diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index fcf4ee6f1..83eee8288 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -102,7 +102,6 @@ spec: resources: - configmaps - pods - - serviceaccounts - services verbs: - '*' @@ -127,12 +126,6 @@ spec: - pods/exec verbs: - create - - apiGroups: - - "" - resources: - - secrets - verbs: - - '*' - apiGroups: - "" resourceNames: @@ -144,6 +137,13 @@ spec: - delete - get - patch + - apiGroups: + - "" + resources: + - secrets + - serviceaccounts + verbs: + - '*' - apiGroups: - admissionregistration.k8s.io resources: diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 748670445..a12d3352e 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -25908,7 +25908,6 @@ rules: resources: - configmaps - pods - - serviceaccounts - services verbs: - '*' @@ -25933,12 +25932,6 @@ rules: - pods/exec verbs: - create -- apiGroups: - - "" - resources: - - secrets - verbs: - - '*' - apiGroups: - "" resourceNames: @@ -25950,6 +25943,13 @@ rules: - delete - get - patch +- apiGroups: + - "" + resources: + - secrets + - serviceaccounts + verbs: + - '*' - apiGroups: - admissionregistration.k8s.io resources: diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index 158eba08e..63ea08778 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -22,7 +22,6 @@ rules: resources: - configmaps - pods - - serviceaccounts - services verbs: - '*' @@ -47,12 +46,6 @@ rules: - pods/exec verbs: - create -- apiGroups: - - "" - resources: - - secrets - verbs: - - '*' - apiGroups: - "" resourceNames: @@ -64,6 +57,13 @@ rules: - delete - get - patch +- apiGroups: + - "" + resources: + - secrets + - serviceaccounts + verbs: + - '*' - apiGroups: - admissionregistration.k8s.io resources: diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index bacbc8aaa..66cd24dbc 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -25908,7 +25908,6 @@ rules: resources: - configmaps - pods - - serviceaccounts - services verbs: - '*' @@ -25933,12 +25932,6 @@ rules: - pods/exec verbs: - create -- apiGroups: - - "" - resources: - - secrets - verbs: - - '*' - apiGroups: - "" resourceNames: @@ -25950,6 +25943,13 @@ rules: - delete - get - patch +- apiGroups: + - "" + resources: + - secrets + - serviceaccounts + verbs: + - '*' - apiGroups: - admissionregistration.k8s.io resources: diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index 158eba08e..63ea08778 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -22,7 +22,6 @@ rules: resources: - configmaps - pods - - serviceaccounts - services verbs: - '*' @@ -47,12 +46,6 @@ rules: - pods/exec verbs: - create -- apiGroups: - - "" - resources: - - secrets - verbs: - - '*' - apiGroups: - "" resourceNames: @@ -64,6 +57,13 @@ rules: - delete - get - patch +- apiGroups: + - "" + resources: + - secrets + - serviceaccounts + verbs: + - '*' - apiGroups: - admissionregistration.k8s.io resources: diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index 332abafe5..029909bd4 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -20,7 +20,6 @@ rules: resources: - configmaps - pods - - serviceaccounts - services verbs: - '*' @@ -45,12 +44,6 @@ rules: - pods/exec verbs: - create -- apiGroups: - - "" - resources: - - secrets - verbs: - - '*' - apiGroups: - "" resourceNames: @@ -62,6 +55,13 @@ rules: - delete - get - patch +- apiGroups: + - "" + resources: + - secrets + - serviceaccounts + verbs: + - '*' - apiGroups: - admissionregistration.k8s.io resources: From 9c416840b22a1868349a47059f2c192368a668e2 Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Wed, 5 Nov 2025 12:45:35 +0100 Subject: [PATCH 10/12] Replace custom pointer fn with k8s ptr package Signed-off-by: Ales Raszka --- .../backupcronjob/backupcronjob_controller.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 8e613e889..37593ba5d 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -21,6 +21,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "k8s.io/utils/ptr" + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/internal/images" @@ -345,10 +347,6 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWor return false } -func ptrInt64(i int64) *int64 { return &i } -func ptrInt32(i int32) *int32 { return &i } -func ptrBool(b bool) *bool { return &b } - // createBackupJob creates a Kubernetes Job to back up the workspace's PVC data. func (r *BackupCronJobReconciler) createBackupJob( workspace *dw.DevWorkspace, @@ -394,7 +392,7 @@ func (r *BackupCronJobReconciler) createBackupJob( ServiceAccountName: JobRunnerSAName, RestartPolicy: corev1.RestartPolicyNever, SecurityContext: &corev1.PodSecurityContext{ - FSGroup: ptrInt64(0), + FSGroup: ptr.To[int64](0), }, Containers: []corev1.Container{ { @@ -428,8 +426,8 @@ func (r *BackupCronJobReconciler) createBackupJob( }, }, SecurityContext: &corev1.SecurityContext{ - RunAsUser: ptrInt64(0), - AllowPrivilegeEscalation: ptrBool(false), + RunAsUser: ptr.To[int64](0), + AllowPrivilegeEscalation: ptr.To[bool](false), }, }, }, From b0f56b3b734a47bccc0cd79bcb8518192b55a8fb Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Tue, 11 Nov 2025 10:03:30 +0100 Subject: [PATCH 11/12] Address code review comments - Make registry field required - Replace custom bool comparison with library - Minor tweeks Signed-off-by: Ales Raszka --- .../devworkspaceoperatorconfig_types.go | 2 +- .../backupcronjob/backupcronjob_controller.go | 21 +++---------------- ...evfile.io_devworkspaceoperatorconfigs.yaml | 2 ++ deploy/deployment/kubernetes/combined.yaml | 2 ++ ...r.devfile.io.CustomResourceDefinition.yaml | 2 ++ deploy/deployment/openshift/combined.yaml | 2 ++ ...r.devfile.io.CustomResourceDefinition.yaml | 2 ++ ...evfile.io_devworkspaceoperatorconfigs.yaml | 2 ++ pkg/constants/metadata.go | 2 +- 9 files changed, 17 insertions(+), 20 deletions(-) diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 2a64f6807..2351f1807 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -84,7 +84,7 @@ type BackupCronJobConfig struct { Schedule string `json:"schedule,omitempty"` // A registry where backup images are stored. Images are stored // in {registry}/backup-${DEVWORKSPACE_NAMESPACE}-${DEVWORKSPACE_NAME} - // +kubebuilder:validation:Optional + // +kubebuilder:validation:Required Registry string `json:"registry,omitempty"` // RegistryAuthSecret is the name of a Kubernetes secret of diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 37593ba5d..3218a1b26 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -46,10 +46,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ) -const ( - defaultBackupImage = "registry.access.redhat.com/ubi8/ubi-minimal:latest" -) - // BackupCronJobReconciler reconciles `BackupCronJob` configuration for the purpose of backing up workspace PVCs. type BackupCronJobReconciler struct { client.Client @@ -76,24 +72,13 @@ func shouldReconcileOnUpdate(e event.UpdateEvent, log logr.Logger) bool { oldBackup := oldConfig.Config.Workspace.BackupCronJob newBackup := newConfig.Config.Workspace.BackupCronJob - 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 - } - } - if oldBackup == nil && newBackup == nil { return false } if (oldBackup == nil && newBackup != nil) || (oldBackup != nil && newBackup == nil) { return true } - if differentBool(oldBackup.Enable, newBackup.Enable) { + if !ptr.Equal(oldBackup.Enable, newBackup.Enable) { return true } @@ -517,7 +502,7 @@ func (r *BackupCronJobReconciler) copySecret(workspace *dw.DevWorkspace, ctx con log := logger.WithName("copySecret") existingNamespaceSecret := &corev1.Secret{} err = r.NonCachingClient.Get(ctx, client.ObjectKey{ - Name: constants.DevWorkspaceBackuptAuthSecretName, + Name: constants.DevWorkspaceBackupAuthSecretName, Namespace: workspace.Namespace}, existingNamespaceSecret) if client.IgnoreNotFound(err) != nil { log.Error(err, "Failed to check for existing registry auth secret in workspace namespace", "namespace", workspace.Namespace) @@ -533,7 +518,7 @@ func (r *BackupCronJobReconciler) copySecret(workspace *dw.DevWorkspace, ctx con } namespaceSecret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: constants.DevWorkspaceBackuptAuthSecretName, + Name: constants.DevWorkspaceBackupAuthSecretName, Namespace: workspace.Namespace, Labels: map[string]string{ constants.DevWorkspaceIDLabel: workspace.Status.DevWorkspaceId, diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index b2e60a891..c660a6580 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -205,6 +205,8 @@ spec: Schedule specifies the cron schedule for the backup cron job. For example, "0 2 * * *" runs daily at 2 AM. type: string + required: + - registry type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces. diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index a12d3352e..94334c379 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -207,6 +207,8 @@ spec: Schedule specifies the cron schedule for the backup cron job. For example, "0 2 * * *" runs daily at 2 AM. type: string + required: + - registry type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options 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 199ffee70..4012eacf3 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -207,6 +207,8 @@ spec: Schedule specifies the cron schedule for the backup cron job. For example, "0 2 * * *" runs daily at 2 AM. type: string + required: + - registry type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 66cd24dbc..207d1545e 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -207,6 +207,8 @@ spec: Schedule specifies the cron schedule for the backup cron job. For example, "0 2 * * *" runs daily at 2 AM. type: string + required: + - registry type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options 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 199ffee70..4012eacf3 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -207,6 +207,8 @@ spec: Schedule specifies the cron schedule for the backup cron job. For example, "0 2 * * *" runs daily at 2 AM. type: string + required: + - registry type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 0f26fd363..2613a0326 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -205,6 +205,8 @@ spec: Schedule specifies the cron schedule for the backup cron job. For example, "0 2 * * *" runs daily at 2 AM. type: string + required: + - registry type: object cleanupCronJob: description: CleanupCronJobConfig defines configuration options diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 03f2f23fa..b27cef445 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -179,5 +179,5 @@ const ( // DevWorkspaceBackupJobLabel is the label key to identify backup jobs created for DevWorkspaces DevWorkspaceBackupJobLabel = "controller.devfile.io/backup-job" - DevWorkspaceBackuptAuthSecretName = "devworkspace-backup-registry-auth" + DevWorkspaceBackupAuthSecretName = "devworkspace-backup-registry-auth" ) From 4be0f988012e2135ceecff2f94c03dbc8ee591b4 Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Tue, 11 Nov 2025 11:00:28 +0100 Subject: [PATCH 12/12] Unify logging in the backup controller Use single logger across the controller and only add context if needed. Signed-off-by: Ales Raszka --- .../backupcronjob/backupcronjob_controller.go | 43 ++++++++----------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 3218a1b26..2db335b37 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -179,8 +179,7 @@ func (r *BackupCronJobReconciler) isBackupEnabled(config *controllerv1alpha1.Dev } // startCron starts the cron scheduler with the backup job according to the provided configuration. -func (r *BackupCronJobReconciler) startCron(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) { - log := logger.WithName("backup cron") +func (r *BackupCronJobReconciler) startCron(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, log logr.Logger) { log.Info("Starting backup cron scheduler") // remove existing cronjob tasks @@ -195,13 +194,11 @@ func (r *BackupCronJobReconciler) startCron(ctx context.Context, dwOperatorConfi backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob log.Info("Adding cronjob task", "schedule", backUpConfig.Schedule) _, err := r.cron.AddFunc(backUpConfig.Schedule, func() { - taskLog := logger.WithName("cronTask") - - taskLog.Info("Starting DevWorkspace backup job") - if err := r.executeBackupSync(ctx, dwOperatorConfig, logger); err != nil { - taskLog.Error(err, "Failed to execute backup job for DevWorkspaces") + log.Info("Starting DevWorkspace backup job") + if err := r.executeBackupSync(ctx, dwOperatorConfig, log); err != nil { + log.Error(err, "Failed to execute backup job for DevWorkspaces") } - taskLog.Info("DevWorkspace backup job finished") + log.Info("DevWorkspace backup job finished") }) if err != nil { log.Error(err, "Failed to add cronjob function") @@ -213,8 +210,7 @@ func (r *BackupCronJobReconciler) startCron(ctx context.Context, dwOperatorConfi } // stopCron stops the cron scheduler and removes all existing cronjob tasks. -func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) { - log := logger.WithName("backup cron") +func (r *BackupCronJobReconciler) stopCron(log logr.Logger) { log.Info("Stopping cron scheduler") // remove existing cronjob tasks @@ -231,11 +227,10 @@ func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) { // executeBackupSync executes the backup job for all DevWorkspaces in the cluster that // have been stopped in the last N minutes. -func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) error { - log := logger.WithName("executeBackupSync") +func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, log logr.Logger) error { log.Info("Executing backup sync for all DevWorkspaces") - registyAuthSecret, err := r.getRegistryAuthSecret(ctx, dwOperatorConfig, logger) + registyAuthSecret, err := r.getRegistryAuthSecret(ctx, dwOperatorConfig, log) if err != nil { log.Error(err, "Failed to get registry auth secret for backup job") return err @@ -251,7 +246,7 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera lastBackupTime = dwOperatorConfig.Status.LastBackupTime } for _, dw := range devWorkspaces.Items { - if !r.wasStoppedSinceLastBackup(&dw, lastBackupTime, logger) { + if !r.wasStoppedSinceLastBackup(&dw, lastBackupTime, log) { log.Info("Skipping backup for DevWorkspace that wasn't stopped recently", "namespace", dw.Namespace, "name", dw.Name) continue } @@ -264,7 +259,7 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera continue } - if err := r.createBackupJob(&dw, ctx, dwOperatorConfig, registyAuthSecret, logger); err != nil { + if err := r.createBackupJob(&dw, ctx, dwOperatorConfig, registyAuthSecret, log); err != nil { log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID) continue } @@ -285,8 +280,7 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera return nil } -func (r *BackupCronJobReconciler) getRegistryAuthSecret(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) (*corev1.Secret, error) { - log := logger.WithName("getRegistryAuthSecret") +func (r *BackupCronJobReconciler) getRegistryAuthSecret(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, log logr.Logger) (*corev1.Secret, error) { registryAuthSecret := &corev1.Secret{} if dwOperatorConfig.Config.Workspace.BackupCronJob.RegistryAuthSecret != "" { err := r.NonCachingClient.Get(ctx, client.ObjectKey{ @@ -304,8 +298,7 @@ func (r *BackupCronJobReconciler) getRegistryAuthSecret(ctx context.Context, dwO } // wasStoppedSinceLastBackup checks if the DevWorkspace was stopped since the last backup time. -func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWorkspace, lastBackupTime *metav1.Time, logger logr.Logger) bool { - log := logger.WithName("wasStoppedSinceLastBackup") +func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWorkspace, lastBackupTime *metav1.Time, log logr.Logger) bool { if workspace.Status.Phase != dw.DevWorkspaceStatusStopped { return false } @@ -338,14 +331,13 @@ func (r *BackupCronJobReconciler) createBackupJob( ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, registyAuthSecret *corev1.Secret, - logger logr.Logger, + log logr.Logger, ) error { - log := logger.WithName("createBackupJob") dwID := workspace.Status.DevWorkspaceId backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob // Find a PVC with the name "claim-devworkspace" or based on the name from the operator config - pvcName, workspacePath, err := r.getWorkspacePVCName(workspace, dwOperatorConfig, ctx, logger) + pvcName, workspacePath, err := r.getWorkspacePVCName(workspace, dwOperatorConfig, ctx, log) if err != nil { log.Error(err, "Failed to get workspace PVC name", "devworkspace", workspace.Name) return err @@ -437,7 +429,7 @@ func (r *BackupCronJobReconciler) createBackupJob( }, } if registyAuthSecret != nil { - secret, err := r.copySecret(workspace, ctx, registyAuthSecret, logger) + secret, err := r.copySecret(workspace, ctx, registyAuthSecret, log) if err != nil { return err } @@ -473,7 +465,7 @@ func (r *BackupCronJobReconciler) createBackupJob( } // getWorkspacePVCName determines the PVC name and workspace path based on the storage provisioner used. -func (r *BackupCronJobReconciler) getWorkspacePVCName(workspace *dw.DevWorkspace, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, ctx context.Context, logger logr.Logger) (string, string, error) { +func (r *BackupCronJobReconciler) getWorkspacePVCName(workspace *dw.DevWorkspace, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, ctx context.Context, log logr.Logger) (string, string, error) { config, err := wkspConfig.ResolveConfigForWorkspace(workspace, r.Client) workspaceWithConfig := &common.DevWorkspaceWithConfig{} @@ -498,8 +490,7 @@ func (r *BackupCronJobReconciler) getWorkspacePVCName(workspace *dw.DevWorkspace return "", "", nil } -func (r *BackupCronJobReconciler) copySecret(workspace *dw.DevWorkspace, ctx context.Context, sourceSecret *corev1.Secret, logger logr.Logger) (namespaceSecret *corev1.Secret, err error) { - log := logger.WithName("copySecret") +func (r *BackupCronJobReconciler) copySecret(workspace *dw.DevWorkspace, ctx context.Context, sourceSecret *corev1.Secret, log logr.Logger) (namespaceSecret *corev1.Secret, err error) { existingNamespaceSecret := &corev1.Secret{} err = r.NonCachingClient.Get(ctx, client.ObjectKey{ Name: constants.DevWorkspaceBackupAuthSecretName,