Skip to content

Commit 728f107

Browse files
committed
Move restore container condition to controller level
The condition whether an workspace should be restored from workspace was in the restore module itself. This make a reading a code more difficult. Now the condition is checked in the controller itself and restore container is only added when enabled. This commit also fixes few minor changes based on the code review comments: - Licence header - Attribute validation - Add a test for disabled workspace recovery - Typos Signed-off-by: Ales Raszka <araszka@redhat.com>
1 parent 214b762 commit 728f107

6 files changed

Lines changed: 82 additions & 40 deletions

File tree

controllers/workspace/devworkspace_controller.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -354,25 +354,23 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
354354
if err := projects.ValidateAllProjects(&workspace.Spec.Template); err != nil {
355355
return r.failWorkspace(workspace, fmt.Sprintf("Invalid devfile: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
356356
}
357-
// Add init container to restore workspace from backup if requested
358-
restoreOptions := restore.Options{
359-
Env: env.GetErnvinmentVariablesForProjectRestore(workspace),
360-
}
361-
if config.Workspace.ImagePullPolicy != "" {
362-
restoreOptions.PullPolicy = corev1.PullPolicy(config.Workspace.ImagePullPolicy)
357+
if restore.IsWorkspaceRestoreRequested(&workspace.Spec.Template) {
358+
// Add init container to restore workspace from backup if requested
359+
restoreOptions := restore.Options{
360+
Env: env.GetEnvironmentVariablesForProjectRestore(workspace),
361+
}
362+
if config.Workspace.ImagePullPolicy != "" {
363+
restoreOptions.PullPolicy = corev1.PullPolicy(config.Workspace.ImagePullPolicy)
364+
} else {
365+
restoreOptions.PullPolicy = corev1.PullIfNotPresent
366+
}
367+
if workspaceRestore, err := restore.GetWorkspaceRestoreInitContainer(ctx, workspace, clusterAPI.Client, restoreOptions, reqLogger); err != nil {
368+
return r.failWorkspace(workspace, fmt.Sprintf("Failed to set up workspace-restore init container: %s", err), metrics.ReasonInfrastructureFailure, reqLogger, &reconcileStatus), nil
369+
} else if workspaceRestore != nil {
370+
devfilePodAdditions.InitContainers = append([]corev1.Container{*workspaceRestore}, devfilePodAdditions.InitContainers...)
371+
}
363372
} else {
364-
restoreOptions.PullPolicy = corev1.PullIfNotPresent
365-
}
366-
var workspaceRestoreCreated bool
367-
if workspaceRestore, err := restore.GetWorkspaceRestoreInitContainer(ctx, workspace, clusterAPI.Client, restoreOptions, reqLogger); err != nil {
368-
return r.failWorkspace(workspace, fmt.Sprintf("Failed to set up workspace-restore init container: %s", err), metrics.ReasonInfrastructureFailure, reqLogger, &reconcileStatus), nil
369-
} else if workspaceRestore != nil {
370-
devfilePodAdditions.InitContainers = append([]corev1.Container{*workspaceRestore}, devfilePodAdditions.InitContainers...)
371-
workspaceRestoreCreated = true
372-
}
373-
374-
// Add init container to clone projects only if restore container wasn't created
375-
if !workspaceRestoreCreated {
373+
// Add init container to clone projects only if restore container wasn't created
376374
projectCloneOptions := projects.Options{
377375
Image: workspace.Config.Workspace.ProjectCloneConfig.Image,
378376
Env: env.GetEnvironmentVariablesForProjectClone(workspace),

controllers/workspace/devworkspace_controller_test.go

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,7 +1094,6 @@ var _ = Describe("DevWorkspace Controller", func() {
10941094
cloneInitContainer = container
10951095
}
10961096
}
1097-
// Expect(initContainers).To(BeEmpty(), "Init containers should be present in deployment")
10981097
Expect(cloneInitContainer.Name).To(BeEmpty(), "Project clone init container should be omitted when restoring from backup")
10991098
Expect(restoreInitContainer).ToNot(BeNil(), "Workspace restore init container should not be nil")
11001099
Expect(restoreInitContainer.Name).To(Equal(restore.WorkspaceRestoreContainerName), "Workspace restore init container should be present in deployment")
@@ -1157,7 +1156,6 @@ var _ = Describe("DevWorkspace Controller", func() {
11571156
cloneInitContainer = container
11581157
}
11591158
}
1160-
// Expect(initContainers).To(BeEmpty(), "Init containers should be present in deployment")
11611159
Expect(cloneInitContainer.Name).To(BeEmpty(), "Project clone init container should be omitted when restoring from backup")
11621160
Expect(restoreInitContainer).ToNot(BeNil(), "Workspace restore init container should not be nil")
11631161
Expect(restoreInitContainer.Name).To(Equal(restore.WorkspaceRestoreContainerName), "Workspace restore init container should be present in deployment")
@@ -1173,6 +1171,57 @@ var _ = Describe("DevWorkspace Controller", func() {
11731171
}), "Restore init container should have workspace storage volume mounted at correct path")
11741172

11751173
})
1174+
It("Doesn't restore workspace from backup if restore is disabled", func() {
1175+
config.SetGlobalConfigForTesting(&controllerv1alpha1.OperatorConfiguration{
1176+
Workspace: &controllerv1alpha1.WorkspaceConfig{
1177+
BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{
1178+
Enable: ptr.To[bool](true),
1179+
Registry: &controllerv1alpha1.RegistryConfig{
1180+
Path: "localhost:5000",
1181+
},
1182+
},
1183+
},
1184+
})
1185+
defer config.SetGlobalConfigForTesting(nil)
1186+
By("Reading DevWorkspace with restore configuration from testdata file")
1187+
createDevWorkspace(devWorkspaceName, "restore-workspace-disabled.yaml")
1188+
devworkspace := getExistingDevWorkspace(devWorkspaceName)
1189+
workspaceID := devworkspace.Status.DevWorkspaceId
1190+
1191+
By("Waiting for DevWorkspaceRouting to be created")
1192+
dwr := &controllerv1alpha1.DevWorkspaceRouting{}
1193+
dwrName := common.DevWorkspaceRoutingName(workspaceID)
1194+
Eventually(func() error {
1195+
return k8sClient.Get(ctx, namespacedName(dwrName, testNamespace), dwr)
1196+
}, timeout, interval).Should(Succeed(), "DevWorkspaceRouting should be created")
1197+
1198+
By("Manually making Routing ready to continue")
1199+
markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID))
1200+
1201+
By("Setting the deployment to have 1 ready replica")
1202+
markDeploymentReady(common.DeploymentName(devworkspace.Status.DevWorkspaceId))
1203+
1204+
deployment := &appsv1.Deployment{}
1205+
err := k8sClient.Get(ctx, namespacedName(devworkspace.Status.DevWorkspaceId, devworkspace.Namespace), deployment)
1206+
Expect(err).ToNot(HaveOccurred(), "Failed to get DevWorkspace deployment")
1207+
1208+
initContainers := deployment.Spec.Template.Spec.InitContainers
1209+
Expect(len(initContainers)).To(BeNumerically(">", 0), "No initContainers found in deployment")
1210+
1211+
var restoreInitContainer corev1.Container
1212+
var cloneInitContainer corev1.Container
1213+
for _, container := range initContainers {
1214+
if container.Name == restore.WorkspaceRestoreContainerName {
1215+
restoreInitContainer = container
1216+
}
1217+
if container.Name == projects.ProjectClonerContainerName {
1218+
cloneInitContainer = container
1219+
}
1220+
}
1221+
Expect(restoreInitContainer.Name).To(BeEmpty(), "Workspace restore init container should be omitted when restore is disabled")
1222+
Expect(cloneInitContainer).ToNot(BeNil(), "Project clone init container should not be nil")
1223+
1224+
})
11761225

11771226
})
11781227

pkg/constants/attributes.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,15 +152,15 @@ const (
152152
// from the DevWorkspace
153153
BootstrapDevWorkspaceAttribute = "controller.devfile.io/bootstrap-devworkspace"
154154

155-
// WorkspaceRestoreAttribute defines whether workspace restore should be performed when creating a DevWorkspace.
155+
// WorkspaceRestoreAttribute defines whether workspace restore should be performed for a DevWorkspace.
156156
// If this attribute is present, the restore process will be performed during workspace
157157
// initialization before the workspace containers start.
158158

159159
// The backup source is automatically determined from the cluster configuration or can be overridden
160160
// by specifying the WorkspaceRestoreSourceImageAttribute.
161161
WorkspaceRestoreAttribute = "controller.devfile.io/restore-workspace"
162162

163-
// WorkspaceRestoreSourceImageAttribute defines the backup image source to restore from when creating a DevWorkspace.
163+
// WorkspaceRestoreSourceImageAttribute defines the backup image source to restore from for a DevWorkspace.
164164
// The value should be a container image reference containing a workspace backup created by the backup functionality.
165165
// The restore will be performed during workspace initialization before the workspace containers start.
166166
// For example:

pkg/library/env/workspaceenv.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func AddCommonEnvironmentVariables(podAdditions *v1alpha1.PodAdditions, clusterD
4949
return nil
5050
}
5151

52-
func GetErnvinmentVariablesForProjectRestore(workspace *common.DevWorkspaceWithConfig) []corev1.EnvVar {
52+
func GetEnvironmentVariablesForProjectRestore(workspace *common.DevWorkspaceWithConfig) []corev1.EnvVar {
5353
var restoreEnv []corev1.EnvVar
5454
restoreEnv = append(restoreEnv, commonEnvironmentVariables(workspace)...)
5555
restoreEnv = append(restoreEnv, corev1.EnvVar{

pkg/library/restore/restore.go

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright (c) 2019-2025 Red Hat, Inc.
2+
// Copyright (c) 2019-2026 Red Hat, Inc.
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
55
// You may obtain a copy of the License at
@@ -23,7 +23,6 @@ import (
2323
dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
2424
"github.com/devfile/devworkspace-operator/pkg/common"
2525
devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants"
26-
"github.com/devfile/devworkspace-operator/pkg/library/storage"
2726
"github.com/go-logr/logr"
2827
corev1 "k8s.io/api/core/v1"
2928
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -43,6 +42,15 @@ type Options struct {
4342
Env []corev1.EnvVar
4443
}
4544

45+
func IsWorkspaceRestoreRequested(workspace *dw.DevWorkspaceTemplateSpec) bool {
46+
if !workspace.Attributes.Exists(constants.WorkspaceRestoreAttribute) {
47+
return false
48+
}
49+
enableRecovery := workspace.Attributes.GetBoolean(constants.WorkspaceRestoreAttribute, nil)
50+
return enableRecovery
51+
52+
}
53+
4654
// GetWorkspaceRestoreInitContainer creates an init container that restores workspace data from a backup image.
4755
// The restore container uses the existing workspace-recovery.sh script to extract backup content.
4856
func GetWorkspaceRestoreInitContainer(
@@ -53,21 +61,9 @@ func GetWorkspaceRestoreInitContainer(
5361
log logr.Logger,
5462
) (*corev1.Container, error) {
5563
wokrspaceTempplate := &workspace.Spec.Template
56-
// Check if restore is requested via workspace attribute
57-
if !wokrspaceTempplate.Attributes.Exists(constants.WorkspaceRestoreAttribute) {
58-
return nil, nil
59-
}
60-
61-
// Get workspace PVC information for mounting into the restore container
62-
pvcName, _, err := storage.GetWorkspacePVCInfo(ctx, workspace.DevWorkspace, workspace.Config, k8sClient, log)
63-
if err != nil {
64-
return nil, fmt.Errorf("failed to resolve workspace PVC info for restore: %w", err)
65-
}
66-
if pvcName == "" {
67-
return nil, fmt.Errorf("no PVC found for workspace %s during restore", workspace.Name)
68-
}
6964

7065
// Determine the source image for restore
66+
var err error
7167
var restoreSourceImage string
7268
if wokrspaceTempplate.Attributes.Exists(constants.WorkspaceRestoreSourceImageAttribute) {
7369
// User choose custom image specified in the attribute
@@ -115,7 +111,6 @@ func GetWorkspaceRestoreInitContainer(
115111
},
116112
},
117113
ImagePullPolicy: options.PullPolicy,
118-
// },
119114
}, nil
120115
}
121116

pkg/library/storage/storage.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright (c) 2019-2025 Red Hat, Inc.
2+
// Copyright (c) 2019-2026 Red Hat, Inc.
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
55
// You may obtain a copy of the License at

0 commit comments

Comments
 (0)