Skip to content

Commit 41f60d5

Browse files
rohanKanojiaclaude
rohanKanojia
and
claude
authored
fix: add node affinity to backup Jobs to prevent Multi-Attach errors (#1675) (#1677)
On multi-node clusters, backup Jobs for workspaces using ReadWriteOnce PVCs can fail with Multi-Attach errors when scheduled on a different node than the one where the PVC is currently mounted. Pin backup Jobs to the correct node using NodeAffinity, matching the existing pattern in cleanup Jobs. Consolidate duplicated node-lookup logic from cleanup.go and the backup controller into a shared FindNodeForPVC function in pkg/provision/storage. Signed-off-by: Rohan Kumar <rohaan@redhat.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a9cb4de commit 41f60d5

6 files changed

Lines changed: 367 additions & 55 deletions

File tree

‎controllers/backupcronjob/backupcronjob_controller.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/devfile/devworkspace-operator/pkg/constants"
3232
"github.com/devfile/devworkspace-operator/pkg/infrastructure"
3333
"github.com/devfile/devworkspace-operator/pkg/library/storage"
34+
provstorage "github.com/devfile/devworkspace-operator/pkg/provision/storage"
3435
"github.com/devfile/devworkspace-operator/pkg/secrets"
3536
"github.com/go-logr/logr"
3637
"github.com/robfig/cron/v3"
@@ -456,6 +457,18 @@ func (r *BackupCronJobReconciler) createBackupJob(
456457
},
457458
},
458459
}
460+
// Pin backup Job to the node where the PVC is currently mounted to avoid
461+
// Multi-Attach errors with ReadWriteOnce PVCs on multi-node clusters.
462+
targetNode, err := provstorage.FindNodeForPVC(ctx, r.Client, workspace.Namespace, pvc.Name)
463+
if err != nil {
464+
log.Error(err, "Failed to find node with PVC, backup Job will not have node affinity", "pvc", pvc.Name)
465+
} else if targetNode == "" {
466+
log.Info("No target node for backup job, NodeAffinity will not be defined", "pvc", pvc.Name)
467+
}
468+
if targetNode != "" {
469+
job.Spec.Template.Spec.Affinity = provstorage.NodeAffinityForHostname(targetNode)
470+
}
471+
459472
if registryAuthSecret != nil {
460473
job.Spec.Template.Spec.Volumes = append(job.Spec.Template.Spec.Volumes, corev1.Volume{
461474
Name: constants.RegistryAuthVolumeName,

‎controllers/backupcronjob/backupcronjob_controller_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,116 @@ var _ = Describe("BackupCronJobReconciler", func() {
426426
Expect(*jobList.Items[0].Spec.BackoffLimit).To(Equal(int32(2)))
427427
})
428428

429+
It("creates a Job with node affinity when a running pod mounts the PVC", func() {
430+
enabled := true
431+
schedule := "* * * * *"
432+
dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{
433+
ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace},
434+
Config: &controllerv1alpha1.OperatorConfiguration{
435+
Workspace: &controllerv1alpha1.WorkspaceConfig{
436+
BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{
437+
Enable: &enabled,
438+
Schedule: schedule,
439+
Registry: &controllerv1alpha1.RegistryConfig{
440+
Path: "fake-registry",
441+
AuthSecret: "backup-auth",
442+
},
443+
},
444+
},
445+
},
446+
}
447+
Expect(fakeClient.Create(ctx, dwoc)).To(Succeed())
448+
dw := createDevWorkspace("dw-affinity", "ns-affinity", false, metav1.NewTime(time.Now().Add(-10*time.Minute)))
449+
dw.Status.Phase = dwv2.DevWorkspaceStatusStopped
450+
dw.Status.DevWorkspaceId = "id-affinity"
451+
Expect(fakeClient.Create(ctx, dw)).To(Succeed())
452+
453+
pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}}
454+
Expect(fakeClient.Create(ctx, pvc)).To(Succeed())
455+
456+
authSecret := createAuthSecret("backup-auth", nameNamespace.Namespace, map[string][]byte{})
457+
Expect(fakeClient.Create(ctx, authSecret)).To(Succeed())
458+
459+
// Create a running pod that mounts the PVC on a specific node
460+
pod := &corev1.Pod{
461+
ObjectMeta: metav1.ObjectMeta{
462+
Name: "workspace-pod",
463+
Namespace: dw.Namespace,
464+
Labels: map[string]string{constants.DevWorkspaceIDLabel: "other-workspace-id"},
465+
},
466+
Spec: corev1.PodSpec{
467+
NodeName: "worker-node-1",
468+
Volumes: []corev1.Volume{
469+
{
470+
Name: "workspace-data",
471+
VolumeSource: corev1.VolumeSource{
472+
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
473+
ClaimName: "claim-devworkspace",
474+
},
475+
},
476+
},
477+
},
478+
},
479+
Status: corev1.PodStatus{Phase: corev1.PodRunning},
480+
}
481+
Expect(fakeClient.Create(ctx, pod)).To(Succeed())
482+
483+
Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed())
484+
485+
jobList := &batchv1.JobList{}
486+
Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed())
487+
Expect(jobList.Items).To(HaveLen(1))
488+
job := jobList.Items[0]
489+
Expect(job.Spec.Template.Spec.Affinity).ToNot(BeNil())
490+
Expect(job.Spec.Template.Spec.Affinity.NodeAffinity).ToNot(BeNil())
491+
nodeSelector := job.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution
492+
Expect(nodeSelector).ToNot(BeNil())
493+
Expect(nodeSelector.NodeSelectorTerms).To(HaveLen(1))
494+
Expect(nodeSelector.NodeSelectorTerms[0].MatchExpressions).To(HaveLen(1))
495+
expr := nodeSelector.NodeSelectorTerms[0].MatchExpressions[0]
496+
Expect(expr.Key).To(Equal("kubernetes.io/hostname"))
497+
Expect(expr.Operator).To(Equal(corev1.NodeSelectorOpIn))
498+
Expect(expr.Values).To(Equal([]string{"worker-node-1"}))
499+
})
500+
501+
It("creates a Job without node affinity when no running pod mounts the PVC", func() {
502+
enabled := true
503+
schedule := "* * * * *"
504+
dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{
505+
ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace},
506+
Config: &controllerv1alpha1.OperatorConfiguration{
507+
Workspace: &controllerv1alpha1.WorkspaceConfig{
508+
BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{
509+
Enable: &enabled,
510+
Schedule: schedule,
511+
Registry: &controllerv1alpha1.RegistryConfig{
512+
Path: "fake-registry",
513+
AuthSecret: "backup-auth",
514+
},
515+
},
516+
},
517+
},
518+
}
519+
Expect(fakeClient.Create(ctx, dwoc)).To(Succeed())
520+
dw := createDevWorkspace("dw-no-affinity", "ns-no-affinity", false, metav1.NewTime(time.Now().Add(-10*time.Minute)))
521+
dw.Status.Phase = dwv2.DevWorkspaceStatusStopped
522+
dw.Status.DevWorkspaceId = "id-no-affinity"
523+
Expect(fakeClient.Create(ctx, dw)).To(Succeed())
524+
525+
pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}}
526+
Expect(fakeClient.Create(ctx, pvc)).To(Succeed())
527+
528+
authSecret := createAuthSecret("backup-auth", nameNamespace.Namespace, map[string][]byte{})
529+
Expect(fakeClient.Create(ctx, authSecret)).To(Succeed())
530+
531+
Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed())
532+
533+
jobList := &batchv1.JobList{}
534+
Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed())
535+
Expect(jobList.Items).To(HaveLen(1))
536+
Expect(jobList.Items[0].Spec.Template.Spec.Affinity).To(BeNil())
537+
})
538+
429539
It("creates a Job with configured podSecurityContext", func() {
430540
enabled := true
431541
schedule := "* * * * *"

‎pkg/provision/storage/cleanup.go‎

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ import (
2929
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
3030
"k8s.io/apimachinery/pkg/api/resource"
3131
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32-
"k8s.io/apimachinery/pkg/labels"
3332
"k8s.io/apimachinery/pkg/types"
34-
"sigs.k8s.io/controller-runtime/pkg/client"
3533
k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
3634
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3735

@@ -121,7 +119,7 @@ func getSpecCommonPVCCleanupJob(workspace *common.DevWorkspaceWithConfig, cluste
121119
pvcName = workspace.Config.Workspace.PVCName
122120
}
123121

124-
targetNode, err := getTargetNodeName(workspace, clusterAPI)
122+
targetNode, err := FindNodeForPVC(clusterAPI.Ctx, clusterAPI.Client, workspace.Namespace, pvcName)
125123
if err != nil {
126124
clusterAPI.Logger.Error(err, "Error getting target node for cleanup job")
127125
} else if targetNode == "" {
@@ -197,21 +195,7 @@ func getSpecCommonPVCCleanupJob(workspace *common.DevWorkspaceWithConfig, cluste
197195
}
198196

199197
if targetNode != "" {
200-
job.Spec.Template.Spec.Affinity.NodeAffinity = &corev1.NodeAffinity{
201-
RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
202-
NodeSelectorTerms: []corev1.NodeSelectorTerm{
203-
{
204-
MatchExpressions: []corev1.NodeSelectorRequirement{
205-
{
206-
Key: corev1.LabelHostname,
207-
Operator: corev1.NodeSelectorOpIn,
208-
Values: []string{targetNode},
209-
},
210-
},
211-
},
212-
},
213-
},
214-
}
198+
job.Spec.Template.Spec.Affinity = NodeAffinityForHostname(targetNode)
215199
}
216200

217201
podTolerations, nodeSelector, err := nsconfig.GetNamespacePodTolerationsAndNodeSelector(workspace.Namespace, clusterAPI)
@@ -246,40 +230,3 @@ func commonPVCExists(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.C
246230
}
247231
return true, nil
248232
}
249-
250-
// getTargetNodeName returns the node name of the node a running devworkspace pod that already mounts the
251-
// common PVC is running in.
252-
// Returns an empty string if no such pod exists.
253-
func getTargetNodeName(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (string, error) {
254-
255-
labelSelector, err := labels.Parse(constants.DevWorkspaceIDLabel)
256-
if err != nil {
257-
return "", err
258-
}
259-
260-
listOptions := &client.ListOptions{
261-
Namespace: workspace.Namespace,
262-
LabelSelector: labelSelector,
263-
}
264-
265-
found := &corev1.PodList{}
266-
err = clusterAPI.Client.List(clusterAPI.Ctx, found, listOptions)
267-
if err != nil {
268-
return "", err
269-
}
270-
271-
return getNodeNameWithPVC(found, workspace.Config.Workspace.PVCName), nil
272-
}
273-
274-
func getNodeNameWithPVC(list *corev1.PodList, pvcName string) string {
275-
for _, pod := range list.Items {
276-
if pod.Status.Phase == corev1.PodRunning {
277-
for _, volume := range pod.Spec.Volumes {
278-
if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.ClaimName == pvcName {
279-
return pod.Spec.NodeName
280-
}
281-
}
282-
}
283-
}
284-
return ""
285-
}

‎pkg/provision/storage/cleanup_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,68 @@ func TestGetSpecCommonPVCCleanupJobUsesConfigPodSecurityContext(t *testing.T) {
8181
assert.Equal(t, customPodSecurityContext, job.Spec.Template.Spec.SecurityContext)
8282
}
8383

84+
func TestGetSpecCommonPVCCleanupJobHasNodeAffinityWhenPodMountsPVC(t *testing.T) {
85+
infrastructure.InitializeForTesting(infrastructure.Kubernetes)
86+
87+
namespace := "test-ns"
88+
pvcName := "claim-devworkspace"
89+
90+
pod := &corev1.Pod{
91+
ObjectMeta: metav1.ObjectMeta{
92+
Name: "workspace-pod",
93+
Namespace: namespace,
94+
Labels: map[string]string{constants.DevWorkspaceIDLabel: "other-workspace-id"},
95+
},
96+
Spec: corev1.PodSpec{NodeName: "worker-node-1", Volumes: []corev1.Volume{{Name: "data", VolumeSource: corev1.VolumeSource{PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: pvcName}}}}},
97+
Status: corev1.PodStatus{Phase: corev1.PodRunning},
98+
}
99+
100+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(
101+
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}},
102+
pod,
103+
).Build()
104+
105+
workspace := &common.DevWorkspaceWithConfig{
106+
DevWorkspace: &dw.DevWorkspace{
107+
ObjectMeta: metav1.ObjectMeta{
108+
Name: "test-workspace",
109+
Namespace: namespace,
110+
Labels: map[string]string{
111+
constants.DevWorkspaceCreatorLabel: "test-creator",
112+
},
113+
},
114+
Status: dw.DevWorkspaceStatus{
115+
DevWorkspaceId: "test-workspace-id",
116+
},
117+
},
118+
Config: &v1alpha1.OperatorConfiguration{
119+
Workspace: &v1alpha1.WorkspaceConfig{
120+
PVCName: pvcName,
121+
},
122+
},
123+
}
124+
125+
clusterAPI := sync.ClusterAPI{
126+
Client: fakeClient,
127+
Scheme: scheme,
128+
Logger: zap.New(zap.UseDevMode(true)),
129+
Ctx: context.Background(),
130+
}
131+
132+
job, err := getSpecCommonPVCCleanupJob(workspace, clusterAPI)
133+
assert.NoError(t, err)
134+
assert.NotNil(t, job.Spec.Template.Spec.Affinity)
135+
assert.NotNil(t, job.Spec.Template.Spec.Affinity.NodeAffinity)
136+
nodeSelector := job.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution
137+
assert.NotNil(t, nodeSelector)
138+
assert.Len(t, nodeSelector.NodeSelectorTerms, 1)
139+
assert.Len(t, nodeSelector.NodeSelectorTerms[0].MatchExpressions, 1)
140+
expr := nodeSelector.NodeSelectorTerms[0].MatchExpressions[0]
141+
assert.Equal(t, "kubernetes.io/hostname", expr.Key)
142+
assert.Equal(t, corev1.NodeSelectorOpIn, expr.Operator)
143+
assert.Equal(t, []string{"worker-node-1"}, expr.Values)
144+
}
145+
84146
func TestGetSpecCommonPVCCleanupJobWithNilPodSecurityContext(t *testing.T) {
85147
infrastructure.InitializeForTesting(infrastructure.Kubernetes)
86148

‎pkg/provision/storage/shared.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package storage
1717

1818
import (
19+
"context"
1920
"errors"
2021
"fmt"
2122

@@ -27,6 +28,7 @@ import (
2728
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
2829
"k8s.io/apimachinery/pkg/api/resource"
2930
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31+
"k8s.io/apimachinery/pkg/labels"
3032
"k8s.io/apimachinery/pkg/types"
3133
"sigs.k8s.io/controller-runtime/pkg/client"
3234

@@ -266,6 +268,60 @@ func getSharedPVCWorkspaceCount(namespace string, api sync.ClusterAPI) (total in
266268
return total, nil
267269
}
268270

271+
// FindNodeForPVC lists DevWorkspace pods in the given namespace and returns the
272+
// node name where a running pod mounts the specified PVC. Returns an empty
273+
// string (and nil error) when no such pod is found.
274+
func FindNodeForPVC(ctx context.Context, k8sClient client.Client, namespace, pvcName string) (string, error) {
275+
labelSelector, err := labels.Parse(constants.DevWorkspaceIDLabel)
276+
if err != nil {
277+
return "", err
278+
}
279+
podList := &corev1.PodList{}
280+
if err := k8sClient.List(ctx, podList, &client.ListOptions{
281+
Namespace: namespace,
282+
LabelSelector: labelSelector,
283+
}); err != nil {
284+
return "", err
285+
}
286+
return getNodeNameWithPVC(podList, pvcName), nil
287+
}
288+
289+
// NodeAffinityForHostname returns an Affinity that pins a pod to the given node
290+
// using the kubernetes.io/hostname label. Used by both cleanup and backup Jobs
291+
// to avoid Multi-Attach errors with ReadWriteOnce PVCs on multi-node clusters.
292+
func NodeAffinityForHostname(nodeName string) *corev1.Affinity {
293+
return &corev1.Affinity{
294+
NodeAffinity: &corev1.NodeAffinity{
295+
RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
296+
NodeSelectorTerms: []corev1.NodeSelectorTerm{
297+
{
298+
MatchExpressions: []corev1.NodeSelectorRequirement{
299+
{
300+
Key: corev1.LabelHostname,
301+
Operator: corev1.NodeSelectorOpIn,
302+
Values: []string{nodeName},
303+
},
304+
},
305+
},
306+
},
307+
},
308+
},
309+
}
310+
}
311+
312+
func getNodeNameWithPVC(list *corev1.PodList, pvcName string) string {
313+
for _, pod := range list.Items {
314+
if pod.Status.Phase == corev1.PodRunning {
315+
for _, volume := range pod.Spec.Volumes {
316+
if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.ClaimName == pvcName {
317+
return pod.Spec.NodeName
318+
}
319+
}
320+
}
321+
}
322+
return ""
323+
}
324+
269325
func checkPVCTerminating(name, namespace string, api sync.ClusterAPI) (bool, error) {
270326
if name == "" {
271327
// Should not happen

0 commit comments

Comments
 (0)