Skip to content

Commit 313f47b

Browse files
committed
fix(kubeovn): retry migration setup until target pod exists
Signed-off-by: Yan Bondarenko <[email protected]>
1 parent 9317fed commit 313f47b

2 files changed

Lines changed: 40 additions & 13 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
Additional retry for the target Pod creation race, separate from Kube-OVN #6834.
2+
Keep until an equivalent guard is verified in the selected upstream version.
3+
4+
diff --git a/pkg/controller/kubevirt.go b/pkg/controller/kubevirt.go
5+
--- a/pkg/controller/kubevirt.go
6+
+++ b/pkg/controller/kubevirt.go
7+
@@ -210,9 +210,10 @@
8+
klog.Infof("successfully set migrate options for lsp %s from %s to %s", portName, sourceNode, targetPod.Spec.NodeName)
9+
}
10+
} else {
11+
- klog.Warningf("target pod not yet created for migration job UID %s in phase %s, waiting for pod creation",
12+
+ // Pod creation does not change the migration phase, so retry even when
13+
+ // the target pod has not reached the informer cache yet.
14+
+ return fmt.Errorf("target pod not yet created for migration job UID %s in phase %s, waiting for pod creation",
15+
vmiMigration.UID, vmiMigration.Status.Phase)
16+
- return nil
17+
}
18+
case kubevirtv1.MigrationSucceeded:
19+
for _, portName := range portNames {

‎packages/system/kubeovn/images/kubeovn/tests/kubevirt_migration_test.go.txt‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,19 @@ import (
2020

2121
func TestVMIMigrationSchedulingRetry(t *testing.T) {
2222
for _, tc := range []struct {
23-
name string
24-
sourceNode string
25-
targetNode string
26-
wantRetry bool
27-
wantSetup bool
23+
name string
24+
sourceNode string
25+
targetNode string
26+
wantRetry bool
27+
wantSetup bool
28+
missingTargetPod bool
2829
}{
29-
{"target not scheduled", "source", "", true, true},
30-
{"source not known", "", "target", true, true},
31-
{"both nodes unknown", "", "", true, true},
32-
{"already scheduled", "source", "target", false, true},
33-
{"same node", "source", "source", false, false},
30+
{"target pod not created", "source", "target", true, true, true},
31+
{"target not scheduled", "source", "", true, true, false},
32+
{"source not known", "", "target", true, true, false},
33+
{"both nodes unknown", "", "", true, true, false},
34+
{"already scheduled", "source", "target", false, true, false},
35+
{"same node", "source", "source", false, false, false},
3436
} {
3537
t.Run(tc.name, func(t *testing.T) {
3638
mockCtrl := gomock.NewController(t)
@@ -57,7 +59,9 @@ func TestVMIMigrationSchedulingRetry(t *testing.T) {
5759
Spec: corev1.PodSpec{NodeName: tc.targetNode},
5860
}
5961
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
60-
require.NoError(t, indexer.Add(pod))
62+
if !tc.missingTargetPod {
63+
require.NoError(t, indexer.Add(pod))
64+
}
6165
virtClient.EXPECT().VirtualMachineInstanceMigration("default").Return(migrationClient).AnyTimes()
6266
virtClient.EXPECT().VirtualMachineInstance("default").Return(vmiClient).AnyTimes()
6367
migrationClient.EXPECT().Get(gomock.Any(), "migration", metav1.GetOptions{}).Return(migration, nil).AnyTimes()
@@ -76,11 +80,15 @@ func TestVMIMigrationSchedulingRetry(t *testing.T) {
7680
queue.Add(key)
7781
require.True(t, processNextWorkItem("migration", queue, controller.handleAddOrUpdateVMIMigration, getWorkItemKey))
7882
if tc.wantRetry {
79-
require.Equal(t, 1, queue.NumRequeues(key), "an unscheduled target must not drop the migration event")
83+
require.Equal(t, 1, queue.NumRequeues(key), "a missing or unscheduled target must not drop the migration event")
8084
vmi.Status.NodeName = "source"
8185
scheduledPod := pod.DeepCopy()
8286
scheduledPod.Spec.NodeName = "target"
83-
require.NoError(t, indexer.Update(scheduledPod))
87+
if tc.missingTargetPod {
88+
require.NoError(t, indexer.Add(scheduledPod))
89+
} else {
90+
require.NoError(t, indexer.Update(scheduledPod))
91+
}
8492
// The VMIM stays in Scheduling: only the rate-limited retry can finish setup.
8593
require.Eventually(t, func() bool { return queue.Len() == 1 }, time.Second, time.Millisecond)
8694
require.True(t, processNextWorkItem("migration", queue, controller.handleAddOrUpdateVMIMigration, getWorkItemKey))

0 commit comments

Comments
 (0)