Merge pull request #2146 from chaunceyjiang/issue_2173

sync pod failded and succeeded status

pkg/resourceinterpreter/defaultinterpreter/aggregatestatus.go

@@ -10,6 +10,7 @@ import (
 	networkingv1 "k8s.io/api/networking/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
 
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
@@ -280,10 +281,12 @@ func aggregatePodStatus(object *unstructured.Unstructured, aggregatedStatusItems
 
 	newStatus := &corev1.PodStatus{}
 	newStatus.ContainerStatuses = make([]corev1.ContainerStatus, 0)
-	runningFlag := true
+	podPhases := sets.NewString()
 	for _, item := range aggregatedStatusItems {
 		if item.Status == nil {
-			runningFlag = false
+			// maybe pod's status hasn't been collected yet, assume it's in pending state.
+			// Otherwise, it may affect the final aggregated state.
+			podPhases.Insert(string(corev1.PodPending))
 			continue
 		}
 
@@ -292,9 +295,7 @@ func aggregatePodStatus(object *unstructured.Unstructured, aggregatedStatusItems
 			return nil, err
 		}
 
-		if temp.Phase != corev1.PodRunning {
+		podPhases.Insert(string(temp.Phase))
-			runningFlag = false
-		}
 
 		for _, containerStatus := range temp.ContainerStatuses {
 			tempStatus := corev1.ContainerStatus{
@@ -307,10 +308,20 @@ func aggregatePodStatus(object *unstructured.Unstructured, aggregatedStatusItems
 			pod.Name, item.ClusterName, temp.Phase)
 	}
 
-	if runningFlag {
+	// Check final phase in order: PodFailed-->PodPending-->PodRunning-->PodSucceeded
-		newStatus.Phase = corev1.PodRunning
+	// Ignore to check PodUnknown as it has been deprecated since year 2015.
-	} else {
+	// More details please refer to: https://github.com/karmada-io/karmada/issues/2137
+	switch {
+	case podPhases.Has(string(corev1.PodFailed)):
+		newStatus.Phase = corev1.PodFailed
+	case podPhases.Has(string(corev1.PodPending)):
 		newStatus.Phase = corev1.PodPending
+	case podPhases.Has(string(corev1.PodRunning)):
+		newStatus.Phase = corev1.PodRunning
+	case podPhases.Has(string(corev1.PodSucceeded)):
+		newStatus.Phase = corev1.PodSucceeded
+	default:
+		klog.Errorf("SHOULD-NEVER-HAPPEN, maybe Pod added a new state that Karmada don't know about.")
 	}
 
 	if reflect.DeepEqual(pod.Status, *newStatus) {

pkg/resourceinterpreter/defaultinterpreter/aggregatestatus_test.go

@@ -510,6 +510,72 @@ func TestAggregatePodStatus(t *testing.T) {
 	}}
 	newObjFailed, _ := helper.ToUnstructured(newPodFailed)
 
+	containerStatusesRunning := []corev1.ContainerStatus{
+		{
+			Ready: true,
+			State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}},
+		},
+	}
+	newContainerStatusesFail := []corev1.ContainerStatus{
+		{
+			Ready: false,
+			State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 125}},
+		},
+	}
+	newContainerStatusesSucceeded := []corev1.ContainerStatus{
+		{
+			Ready: false,
+			State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 0}},
+		},
+	}
+	statusMapRunning := map[string]interface{}{
+		"containerStatuses": []corev1.ContainerStatus{containerStatusesRunning[0]},
+		"phase":             corev1.PodRunning,
+	}
+	statusMapFailed := map[string]interface{}{
+		"containerStatuses": []corev1.ContainerStatus{newContainerStatusesFail[0]},
+		"phase":             corev1.PodFailed,
+	}
+	statusMapSucceeded := map[string]interface{}{
+		"containerStatuses": []corev1.ContainerStatus{newContainerStatusesSucceeded[0]},
+		"phase":             corev1.PodSucceeded,
+	}
+
+	rawRunning, _ := helper.BuildStatusRawExtension(statusMapRunning)
+
+	// test failed
+	rawFail, _ := helper.BuildStatusRawExtension(statusMapFailed)
+	aggregatedStatusItemsFail := []workv1alpha2.AggregatedStatusItem{
+		{ClusterName: "member1", Status: rawRunning, Applied: true},
+		{ClusterName: "member2", Status: rawFail, Applied: true},
+	}
+	aggregatedStatusItemsPending := []workv1alpha2.AggregatedStatusItem{
+		{ClusterName: "member1", Status: rawRunning, Applied: true},
+		{ClusterName: "member2", Status: nil, Applied: true},
+	}
+
+	failObj, _ := helper.ToUnstructured(&corev1.Pod{Status: corev1.PodStatus{
+		ContainerStatuses: []corev1.ContainerStatus{containerStatusesRunning[0], newContainerStatusesFail[0]},
+		Phase:             corev1.PodFailed,
+	}})
+
+	// test succeeded
+	rawSucceeded, _ := helper.BuildStatusRawExtension(statusMapSucceeded)
+	aggregatedStatusItemsSucceeded := []workv1alpha2.AggregatedStatusItem{
+		{ClusterName: "member1", Status: rawRunning, Applied: true},
+		{ClusterName: "member2", Status: rawSucceeded, Applied: true},
+	}
+
+	succeededObj, _ := helper.ToUnstructured(&corev1.Pod{Status: corev1.PodStatus{
+		ContainerStatuses: []corev1.ContainerStatus{containerStatusesRunning[0], newContainerStatusesSucceeded[0]},
+		Phase:             corev1.PodRunning,
+	}})
+
+	pendingObj, _ := helper.ToUnstructured(&corev1.Pod{Status: corev1.PodStatus{
+		ContainerStatuses: []corev1.ContainerStatus{containerStatusesRunning[0]},
+		Phase:             corev1.PodPending,
+	}})
+
 	tests := []struct {
 		name                  string
 		curObj                *unstructured.Unstructured
@@ -534,6 +600,25 @@ func TestAggregatePodStatus(t *testing.T) {
 			aggregatedStatusItems: aggregatedStatusItems2,
 			expectedObj:           newObjFailed,
 		},
+		// More details please refer to: https://github.com/karmada-io/karmada/issues/2137
+		{
+			name:                  "failed pod status",
+			curObj:                curObj,
+			aggregatedStatusItems: aggregatedStatusItemsFail, //  Running + Failed => Failed
+			expectedObj:           failObj,
+		},
+		{
+			name:                  "succeeded pod status",
+			curObj:                curObj,
+			aggregatedStatusItems: aggregatedStatusItemsSucceeded, //  Running + Succeeded => Running
+			expectedObj:           succeededObj,
+		},
+		{
+			name:                  "pending pod status",
+			curObj:                curObj,
+			aggregatedStatusItems: aggregatedStatusItemsPending, //  Running + nil => Pending
+			expectedObj:           pendingObj,
+		},
 	}
 
 	for _, tt := range tests {