diff --git a/pkg/resourceinterpreter/defaultinterpreter/aggregatestatus.go b/pkg/resourceinterpreter/defaultinterpreter/aggregatestatus.go index 38c6b6f3f..803f9e83c 100644 --- a/pkg/resourceinterpreter/defaultinterpreter/aggregatestatus.go +++ b/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 { - runningFlag = false - } + podPhases.Insert(string(temp.Phase)) 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 { - newStatus.Phase = corev1.PodRunning - } else { + // Check final phase in order: PodFailed-->PodPending-->PodRunning-->PodSucceeded + // Ignore to check PodUnknown as it has been deprecated since year 2015. + // 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) { diff --git a/pkg/resourceinterpreter/defaultinterpreter/aggregatestatus_test.go b/pkg/resourceinterpreter/defaultinterpreter/aggregatestatus_test.go index 6e0a76448..26db6aa5c 100644 --- a/pkg/resourceinterpreter/defaultinterpreter/aggregatestatus_test.go +++ b/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 {