fix(reflection): terminated pod reflection logic

Author: aleoli

pkg/virtualKubelet/reflection/workload/pod.go

@@ -305,6 +305,11 @@ func (fpr *FallbackPodReflector) Handle(ctx context.Context, key types.Namespace
 		return nil
 	}
 
+	// The local pod already failed (terminal), hence no change shall be performed.
+	if local.Status.Phase == corev1.PodFailed {
+		return nil
+	}
+
 	// Otherwise, mark the pod as rejected (either Pending or Failed based on its previous status).
 	phase := corev1.PodPending
 	reason := forge.PodOffloadingBackOffReason

pkg/virtualKubelet/reflection/workload/pod_test.go

@@ -180,7 +180,7 @@ var _ = Describe("Pod Reflection Tests", func() {
 						corev1.PodFailed, forge.PodOffloadingAbortedReason),
 					)
 					When("phase is running", WhenBody(corev1.PodStatus{Phase: corev1.PodRunning}, corev1.PodFailed, forge.PodOffloadingAbortedReason))
-					When("phase is failed", WhenBody(corev1.PodStatus{Phase: corev1.PodFailed}, corev1.PodFailed, forge.PodOffloadingAbortedReason))
+					When("phase is failed", WhenBody(corev1.PodStatus{Phase: corev1.PodFailed}, corev1.PodFailed, ""))
 				})
 
 				When("it is terminating", func() {

pkg/virtualKubelet/reflection/workload/podns.go

@@ -199,6 +199,13 @@ func (npr *NamespacedPodReflector) Handle(ctx context.Context, name string) erro
 		return nil
 	}
 
+	// Skip reflection for pods in a terminal phase (Succeeded or Failed).
+	// Avoid recreating ShadowPods and changing local pod status when the workload already completed.
+	if local.Status.Phase == corev1.PodSucceeded || local.Status.Phase == corev1.PodFailed {
+		klog.V(4).Infof("Skipping reflection of local pod %q as it is in terminal phase %q", npr.LocalRef(name), local.Status.Phase)
+		return nil
+	}
+
 	// Ensure the local pod has the appropriate labels to mark it as offloaded.
 	if err := npr.HandleLabels(ctx, local); err != nil {
 		return err

pkg/virtualKubelet/reflection/workload/podns_test.go

@@ -275,6 +275,47 @@ var _ = Describe("Namespaced Pod Reflection Tests", func() {
 
 				When("the remote object already exists, but is not managed by the reflection", WhenBodyRemoteNotManagedByReflection())
 
+				When("the local object is in terminal phase Succeeded", func() {
+					BeforeEach(func() {
+						// Mark local as Succeeded and persist.
+						local.Status.Phase = corev1.PodSucceeded
+						UpdatePod(client, &local)
+					})
+
+					It("should succeed", func() { Expect(err).ToNot(HaveOccurred()) })
+					It("should not create a remote shadowpod", func() {
+						Expect(GetShadowPodError(liqoClient, RemoteNamespace, PodName)).To(BeNotFound())
+					})
+				})
+
+				When("the local object is in terminal phase Failed without OffloadingAborted reason and a ShadowPod exists", func() {
+					BeforeEach(func() {
+						// Prepare local Failed with a non-aborted reason and ensure it is stored.
+						local.Status.Phase = corev1.PodFailed
+						local.Status.Reason = "SomeOtherReason"
+						UpdatePod(client, &local)
+
+						// Create a managed ShadowPod.
+						shadow.SetLabels(forge.ReflectionLabels())
+						CreateShadowPod(liqoClient, &shadow)
+
+						// Ensure that any attempt to mutate the ShadowPod would fail the test (should not happen).
+						if c, ok := liqoClient.(*liqoclientfake.Clientset); ok {
+							c.PrependReactor("update", "shadowpods", func(action testing.Action) (bool, runtime.Object, error) {
+								return true, nil, fmt.Errorf("should not call update on shadowpods")
+							})
+							c.PrependReactor("delete", "shadowpods", func(action testing.Action) (bool, runtime.Object, error) {
+								return true, nil, fmt.Errorf("should not call delete on shadowpods")
+							})
+						}
+					})
+
+					It("should succeed", func() { Expect(err).ToNot(HaveOccurred()) })
+					It("should not delete the existing remote shadowpod", func() {
+						Expect(GetShadowPodError(liqoClient, RemoteNamespace, PodName)).ToNot(HaveOccurred())
+					})
+				})
+
 				When("the local object has already the appropriate offloading label", func() {
 					BeforeEach(func() {
 						shouldDenyPodPatches = true