From cd8852e20553d5dd733ca13ab89a2287eb7a315a Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Tue, 13 Jan 2026 15:40:34 +0100 Subject: [PATCH 1/2] Extract GetWorkspacePVCInfo to separate module The function will be used across both backup and restore features and should be accessed multiple places. Signed-off-by: Ales Raszka --- .../backupcronjob/backupcronjob_controller.go | 32 +-------- pkg/library/storage/storage.go | 65 +++++++++++++++++++ 2 files changed, 67 insertions(+), 30 deletions(-) create mode 100644 pkg/library/storage/storage.go diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 7012e13f0..3917954b1 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -26,13 +26,11 @@ 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/devfile/devworkspace-operator/pkg/library/storage" "github.com/go-logr/logr" "github.com/robfig/cron/v3" batchv1 "k8s.io/api/batch/v1" @@ -353,7 +351,7 @@ func (r *BackupCronJobReconciler) createBackupJob( } // Find a PVC with used by the workspace - pvcName, workspacePath, err := r.getWorkspacePVCName(ctx, workspace, dwOperatorConfig, log) + pvcName, workspacePath, err := storage.GetWorkspacePVCInfo(ctx, workspace, dwOperatorConfig.Config, r.Client, log) if err != nil { log.Error(err, "Failed to get workspace PVC name", "devworkspace", workspace.Name) return err @@ -482,32 +480,6 @@ func (r *BackupCronJobReconciler) createBackupJob( return nil } -// getWorkspacePVCName determines the PVC name and workspace path based on the storage provisioner used. -func (r *BackupCronJobReconciler) getWorkspacePVCName(ctx context.Context, workspace *dw.DevWorkspace, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, log 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 := constants.DefaultWorkspacePVCName - if dwOperatorConfig.Config.Workspace.PVCName != "" { - pvcName = dwOperatorConfig.Config.Workspace.PVCName - } - return pvcName, workspace.Status.DevWorkspaceId + constants.DefaultProjectsSourcesRoot, nil - } - return "", "", nil -} - func (r *BackupCronJobReconciler) handleRegistryAuthSecret(ctx context.Context, workspace *dw.DevWorkspace, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, log logr.Logger, ) (*corev1.Secret, error) { diff --git a/pkg/library/storage/storage.go b/pkg/library/storage/storage.go new file mode 100644 index 000000000..38467b489 --- /dev/null +++ b/pkg/library/storage/storage.go @@ -0,0 +1,65 @@ +// +// 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 storage + +import ( + "context" + + 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/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/provision/storage" + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// GetWorkspacePVCInfo determines the PVC name and workspace path based on the storage provisioner used. +// This function can be used by both backup and restore operations to ensure consistent PVC resolution logic. +// +// Returns: +// - pvcName: The name of the PVC that stores workspace data +// - workspacePath: The path within the PVC where workspace data is stored +// - error: Any error that occurred during PVC resolution +func GetWorkspacePVCInfo( + ctx context.Context, + workspace *dw.DevWorkspace, + config *controllerv1alpha1.OperatorConfiguration, + k8sClient client.Client, + log logr.Logger, +) (pvcName string, workspacePath string, err error) { + 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 := constants.DefaultWorkspacePVCName + if config.Workspace != nil && config.Workspace.PVCName != "" { + pvcName = config.Workspace.PVCName + } + return pvcName, workspace.Status.DevWorkspaceId + constants.DefaultProjectsSourcesRoot, nil + } + return "", "", nil +} From 63b1f4f15191cd1d6889c1b55aa31a20e1b52098 Mon Sep 17 00:00:00 2001 From: Ales Raszka Date: Wed, 14 Jan 2026 14:58:41 +0100 Subject: [PATCH 2/2] Skip backup if the workspace is missing volumes Workspace with no storage failed in the backup process. This commit fixes it by checking if the volumes are present and skip the job if no pvc is available. fixes: 1555 Signed-off-by: Ales Raszka --- .../backupcronjob/backupcronjob_controller.go | 5 +- .../backupcronjob_controller_test.go | 55 +++++++++++++++++++ pkg/library/storage/storage.go | 4 ++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 3917954b1..e0445defa 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -357,8 +357,9 @@ func (r *BackupCronJobReconciler) createBackupJob( return err } if pvcName == "" { - log.Error(err, "No PVC found for DevWorkspace", "id", dwID) - return err + // No PVC to back up + log.Info("No workspace PVC found, skipping backup", "devworkspace", workspace.Name) + return nil } pvc := &corev1.PersistentVolumeClaim{} diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index cb2522def..f223017e7 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -522,6 +522,39 @@ var _ = Describe("BackupCronJobReconciler", func() { Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed()) Expect(jobList.Items).To(HaveLen(1)) }) + It("It doesn't create a Job for a DevWorkspace with no PVC", 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: &controllerv1alpha1.RegistryConfig{ + Path: "fake-registry", + }, + OrasConfig: &controllerv1alpha1.OrasConfig{ + ExtraArgs: "--extra-arg1", + }, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, dwoc)).To(Succeed()) + dw := createDevWorkspace("dw-recent", "ns-a", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Spec.Template.Components = []dwv2.Component{} // No volume component, so no PVC + dw.Status.Phase = dwv2.DevWorkspaceStatusStopped + dw.Status.DevWorkspaceId = "id-recent" + Expect(fakeClient.Create(ctx, dw)).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(0)) + }) }) Context("ensureJobRunnerRBAC", func() { It("creates ServiceAccount for Job runner", func() { @@ -907,6 +940,28 @@ func createDevWorkspace(name, namespace string, started bool, lastTransitionTime }, Spec: dwv2.DevWorkspaceSpec{ Started: started, + Template: dwv2.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: dwv2.DevWorkspaceTemplateSpecContent{ + Components: []dwv2.Component{ + { + Name: "test-container", + ComponentUnion: dwv2.ComponentUnion{ + Container: &dwv2.ContainerComponent{ + Container: dwv2.Container{ + Image: "test-image:latest", + }, + }, + Volume: &dwv2.VolumeComponent{ + Volume: dwv2.Volume{ + Ephemeral: pointer.BoolPtr(true), + Size: "1Mi", + }, + }, + }, + }, + }, + }, + }, }, Status: dwv2.DevWorkspaceStatus{ Conditions: []dwv2.DevWorkspaceCondition{}, diff --git a/pkg/library/storage/storage.go b/pkg/library/storage/storage.go index 38467b489..f32aecb91 100644 --- a/pkg/library/storage/storage.go +++ b/pkg/library/storage/storage.go @@ -49,6 +49,10 @@ func GetWorkspacePVCInfo( if err != nil { return "", "", err } + if !storageProvisioner.NeedsStorage(&workspace.Spec.Template) { + // No storage provisioned for this workspace + return "", "", nil + } if _, ok := storageProvisioner.(*storage.PerWorkspaceStorageProvisioner); ok { pvcName := common.PerWorkspacePVCName(workspace.Status.DevWorkspaceId)