From 8311bfa9481b2391e50320e6a80cccdd51d58865 Mon Sep 17 00:00:00 2001 From: Linghao Su Date: Wed, 16 Oct 2024 03:05:34 +0800 Subject: [PATCH] fix(controller/ui): fix pod with sidecar state (#19843) * fix(controller): change pod status calculate with sidecar Signed-off-by: linghaoSu * fix(controller): add restartable sidecar count in total container display Signed-off-by: linghaoSu * fix(controller): update info test case conditions Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: Linghao Su * fix(controller): add more test case to cover more conditions Signed-off-by: linghaoSu * fix(ui): check is condition exist before for of Signed-off-by: linghaoSu --------- Signed-off-by: linghaoSu Signed-off-by: Linghao Su Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: austin5219 <3936059+austin5219@users.noreply.github.com> --- controller/cache/info.go | 57 +- controller/cache/info_test.go | 546 +++++++++++++++++++ ui/src/app/applications/components/utils.tsx | 46 +- 3 files changed, 640 insertions(+), 9 deletions(-) diff --git a/controller/cache/info.go b/controller/cache/info.go index 98129eddfb83b..7260487af859f 100644 --- a/controller/cache/info.go +++ b/controller/cache/info.go @@ -296,6 +296,32 @@ func populateIstioServiceEntryInfo(un *unstructured.Unstructured, res *ResourceI } } +func isPodInitializedConditionTrue(status *v1.PodStatus) bool { + for _, condition := range status.Conditions { + if condition.Type != v1.PodInitialized { + continue + } + + return condition.Status == v1.ConditionTrue + } + return false +} + +func isRestartableInitContainer(initContainer *v1.Container) bool { + if initContainer == nil { + return false + } + if initContainer.RestartPolicy == nil { + return false + } + + return *initContainer.RestartPolicy == v1.ContainerRestartPolicyAlways +} + +func isPodPhaseTerminal(phase v1.PodPhase) bool { + return phase == v1.PodFailed || phase == v1.PodSucceeded +} + func populatePodInfo(un *unstructured.Unstructured, res *ResourceInfo) { pod := v1.Pod{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(un.Object, &pod) @@ -306,7 +332,8 @@ func populatePodInfo(un *unstructured.Unstructured, res *ResourceInfo) { totalContainers := len(pod.Spec.Containers) readyContainers := 0 - reason := string(pod.Status.Phase) + podPhase := pod.Status.Phase + reason := string(podPhase) if pod.Status.Reason != "" { reason = pod.Status.Reason } @@ -324,6 +351,21 @@ func populatePodInfo(un *unstructured.Unstructured, res *ResourceInfo) { res.Images = append(res.Images, image) } + // If the Pod carries {type:PodScheduled, reason:SchedulingGated}, set reason to 'SchedulingGated'. + for _, condition := range pod.Status.Conditions { + if condition.Type == v1.PodScheduled && condition.Reason == v1.PodReasonSchedulingGated { + reason = v1.PodReasonSchedulingGated + } + } + + initContainers := make(map[string]*v1.Container) + for i := range pod.Spec.InitContainers { + initContainers[pod.Spec.InitContainers[i].Name] = &pod.Spec.InitContainers[i] + if isRestartableInitContainer(&pod.Spec.InitContainers[i]) { + totalContainers++ + } + } + initializing := false for i := range pod.Status.InitContainerStatuses { container := pod.Status.InitContainerStatuses[i] @@ -331,6 +373,12 @@ func populatePodInfo(un *unstructured.Unstructured, res *ResourceInfo) { switch { case container.State.Terminated != nil && container.State.Terminated.ExitCode == 0: continue + case isRestartableInitContainer(initContainers[container.Name]) && + container.Started != nil && *container.Started: + if container.Ready { + readyContainers++ + } + continue case container.State.Terminated != nil: // initialization is failed if len(container.State.Terminated.Reason) == 0 { @@ -352,8 +400,7 @@ func populatePodInfo(un *unstructured.Unstructured, res *ResourceInfo) { } break } - if !initializing { - restarts = 0 + if !initializing || isPodInitializedConditionTrue(&pod.Status) { hasRunning := false for i := len(pod.Status.ContainerStatuses) - 1; i >= 0; i-- { container := pod.Status.ContainerStatuses[i] @@ -388,7 +435,9 @@ func populatePodInfo(un *unstructured.Unstructured, res *ResourceInfo) { // and https://github.com/kubernetes/kubernetes/issues/90358#issuecomment-617859364 if pod.DeletionTimestamp != nil && pod.Status.Reason == "NodeLost" { reason = "Unknown" - } else if pod.DeletionTimestamp != nil { + // If the pod is being deleted and the pod phase is not succeeded or failed, set the reason to "Terminating". + // See https://github.com/kubernetes/kubectl/issues/1595#issuecomment-2080001023 + } else if pod.DeletionTimestamp != nil && !isPodPhaseTerminal(podPhase) { reason = "Terminating" } diff --git a/controller/cache/info_test.go b/controller/cache/info_test.go index 38c0a75bc7fee..08cfbeffb8064 100644 --- a/controller/cache/info_test.go +++ b/controller/cache/info_test.go @@ -308,6 +308,552 @@ func TestGetPodInfo(t *testing.T) { assert.Equal(t, &v1alpha1.ResourceNetworkingInfo{Labels: map[string]string{"app": "guestbook"}}, info.NetworkingInfo) } +func TestGetPodWithInitialContainerInfo(t *testing.T) { + pod := strToUnstructured(` + apiVersion: "v1" + kind: "Pod" + metadata: + labels: + app: "app-with-initial-container" + name: "app-with-initial-container-5f46976fdb-vd6rv" + namespace: "default" + ownerReferences: + - apiVersion: "apps/v1" + kind: "ReplicaSet" + name: "app-with-initial-container-5f46976fdb" + spec: + containers: + - image: "alpine:latest" + imagePullPolicy: "Always" + name: "app-with-initial-container" + initContainers: + - image: "alpine:latest" + imagePullPolicy: "Always" + name: "app-with-initial-container-logshipper" + nodeName: "minikube" + status: + containerStatuses: + - image: "alpine:latest" + name: "app-with-initial-container" + ready: true + restartCount: 0 + started: true + state: + running: + startedAt: "2024-10-08T08:44:25Z" + initContainerStatuses: + - image: "alpine:latest" + name: "app-with-initial-container-logshipper" + ready: true + restartCount: 0 + started: false + state: + terminated: + exitCode: 0 + reason: "Completed" + phase: "Running" +`) + + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Running"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "1/1"}, + }, info.Info) +} + +func TestGetPodInfoWithSidecar(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + labels: + app: app-with-sidecar + name: app-with-sidecar-6664cc788c-lqlrp + namespace: default + ownerReferences: + - apiVersion: apps/v1 + kind: ReplicaSet + name: app-with-sidecar-6664cc788c + spec: + containers: + - image: 'docker.m.daocloud.io/library/alpine:latest' + imagePullPolicy: Always + name: app-with-sidecar + initContainers: + - image: 'docker.m.daocloud.io/library/alpine:latest' + imagePullPolicy: Always + name: logshipper + restartPolicy: Always + nodeName: minikube + status: + containerStatuses: + - image: 'docker.m.daocloud.io/library/alpine:latest' + name: app-with-sidecar + ready: true + restartCount: 0 + started: true + state: + running: + startedAt: '2024-10-08T08:39:43Z' + initContainerStatuses: + - image: 'docker.m.daocloud.io/library/alpine:latest' + name: logshipper + ready: true + restartCount: 0 + started: true + state: + running: + startedAt: '2024-10-08T08:39:40Z' + phase: Running +`) + + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Running"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "2/2"}, + }, info.Info) +} + +func TestGetPodInfoWithInitialContainer(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + generateName: myapp-long-exist-56b7d8794d- + labels: + app: myapp-long-exist + name: myapp-long-exist-56b7d8794d-pbgrd + namespace: linghao + ownerReferences: + - apiVersion: apps/v1 + kind: ReplicaSet + name: myapp-long-exist-56b7d8794d + spec: + containers: + - image: alpine:latest + imagePullPolicy: Always + name: myapp-long-exist + initContainers: + - image: alpine:latest + imagePullPolicy: Always + name: myapp-long-exist-logshipper + nodeName: minikube + status: + containerStatuses: + - image: alpine:latest + name: myapp-long-exist + ready: false + restartCount: 0 + started: false + state: + waiting: + reason: PodInitializing + initContainerStatuses: + - image: alpine:latest + name: myapp-long-exist-logshipper + ready: false + restartCount: 0 + started: true + state: + running: + startedAt: '2024-10-09T08:03:45Z' + phase: Pending + startTime: '2024-10-09T08:02:39Z' +`) + + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Init:0/1"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/1"}, + }, info.Info) +} + +// Test pod has 2 restartable init containers, the first one running but not started. +func TestGetPodInfoWithRestartableInitContainer(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test1 + spec: + initContainers: + - name: restartable-init-1 + restartPolicy: Always + - name: restartable-init-2 + restartPolicy: Always + containers: + - name: container + nodeName: minikube + status: + phase: Pending + initContainerStatuses: + - name: restartable-init-1 + ready: false + restartCount: 3 + state: + running: {} + started: false + lastTerminationState: + terminated: + finishedAt: "2023-10-01T00:00:00Z" # Replace with actual time + - name: restartable-init-2 + ready: false + state: + waiting: {} + started: false + containerStatuses: + - ready: false + restartCount: 0 + state: + waiting: {} + conditions: + - type: ContainersReady + status: "False" + - type: Initialized + status: "False" +`) + + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Init:0/2"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/3"}, + {Name: "Restart Count", Value: "3"}, + }, info.Info) +} + +// Test pod has 2 restartable init containers, the first one started and the second one running but not started. +func TestGetPodInfoWithPartiallyStartedInitContainers(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test1 + spec: + initContainers: + - name: restartable-init-1 + restartPolicy: Always + - name: restartable-init-2 + restartPolicy: Always + containers: + - name: container + nodeName: minikube + status: + phase: Pending + initContainerStatuses: + - name: restartable-init-1 + ready: false + restartCount: 3 + state: + running: {} + started: true + lastTerminationState: + terminated: + finishedAt: "2023-10-01T00:00:00Z" # Replace with actual time + - name: restartable-init-2 + ready: false + state: + running: {} + started: false + containerStatuses: + - ready: false + restartCount: 0 + state: + waiting: {} + conditions: + - type: ContainersReady + status: "False" + - type: Initialized + status: "False" +`) + + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Init:1/2"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/3"}, + {Name: "Restart Count", Value: "3"}, + }, info.Info) +} + +// Test pod has 2 restartable init containers started and 1 container running +func TestGetPodInfoWithStartedInitContainers(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test2 + spec: + initContainers: + - name: restartable-init-1 + restartPolicy: Always + - name: restartable-init-2 + restartPolicy: Always + containers: + - name: container + nodeName: minikube + status: + phase: Running + initContainerStatuses: + - name: restartable-init-1 + ready: false + restartCount: 3 + state: + running: {} + started: true + lastTerminationState: + terminated: + finishedAt: "2023-10-01T00:00:00Z" # Replace with actual time + - name: restartable-init-2 + ready: false + state: + running: {} + started: true + containerStatuses: + - ready: true + restartCount: 4 + state: + running: {} + lastTerminationState: + terminated: + finishedAt: "2023-10-01T00:00:00Z" # Replace with actual time + conditions: + - type: ContainersReady + status: "False" + - type: Initialized + status: "True" +`) + + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Running"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "1/3"}, + {Name: "Restart Count", Value: "7"}, + }, info.Info) +} + +// Test pod has 1 init container restarting and 1 container not running +func TestGetPodInfoWithNormalInitContainer(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test7 + spec: + initContainers: + - name: init-container + containers: + - name: main-container + nodeName: minikube + status: + phase: podPhase + initContainerStatuses: + - ready: false + restartCount: 3 + state: + running: {} + lastTerminationState: + terminated: + finishedAt: "2023-10-01T00:00:00Z" # Replace with the actual time + containerStatuses: + - ready: false + restartCount: 0 + state: + waiting: {} +`) + + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Init:0/1"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/1"}, + {Name: "Restart Count", Value: "3"}, + }, info.Info) +} + +// Test pod condition succeed +func TestPodConditionSucceeded(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test8 + spec: + nodeName: minikube + containers: + - name: container + status: + phase: Succeeded + containerStatuses: + - ready: false + restartCount: 0 + state: + terminated: + reason: Completed + exitCode: 0 +`) + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Completed"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/1"}, + }, info.Info) +} + +// Test pod condition failed +func TestPodConditionFailed(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test9 + spec: + nodeName: minikube + containers: + - name: container + status: + phase: Failed + containerStatuses: + - ready: false + restartCount: 0 + state: + terminated: + reason: Error + exitCode: 1 +`) + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Error"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/1"}, + }, info.Info) +} + +// Test pod condition succeed with deletion +func TestPodConditionSucceededWithDeletion(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test10 + deletionTimestamp: "2023-10-01T00:00:00Z" + spec: + nodeName: minikube + containers: + - name: container + status: + phase: Succeeded + containerStatuses: + - ready: false + restartCount: 0 + state: + terminated: + reason: Completed + exitCode: 0 +`) + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Completed"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/1"}, + }, info.Info) +} + +// Test pod condition running with deletion +func TestPodConditionRunningWithDeletion(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test11 + deletionTimestamp: "2023-10-01T00:00:00Z" + spec: + nodeName: minikube + containers: + - name: container + status: + phase: Running + containerStatuses: + - ready: false + restartCount: 0 + state: + running: {} +`) + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Terminating"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/1"}, + }, info.Info) +} + +// Test pod condition pending with deletion +func TestPodConditionPendingWithDeletion(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test12 + deletionTimestamp: "2023-10-01T00:00:00Z" + spec: + nodeName: minikube + containers: + - name: container + status: + phase: Pending +`) + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "Terminating"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/1"}, + }, info.Info) +} + +// Test PodScheduled condition with reason SchedulingGated +func TestPodScheduledWithSchedulingGated(t *testing.T) { + pod := strToUnstructured(` + apiVersion: v1 + kind: Pod + metadata: + name: test13 + spec: + nodeName: minikube + containers: + - name: container1 + - name: container2 + status: + phase: podPhase + conditions: + - type: PodScheduled + status: "False" + reason: SchedulingGated +`) + info := &ResourceInfo{} + populateNodeInfo(pod, info, []string{}) + assert.Equal(t, []v1alpha1.InfoItem{ + {Name: "Status Reason", Value: "SchedulingGated"}, + {Name: "Node", Value: "minikube"}, + {Name: "Containers", Value: "0/2"}, + }, info.Info) +} + func TestGetNodeInfo(t *testing.T) { node := strToUnstructured(` apiVersion: v1 diff --git a/ui/src/app/applications/components/utils.tsx b/ui/src/app/applications/components/utils.tsx index a1386921bbb8b..14b31b5edeafd 100644 --- a/ui/src/app/applications/components/utils.tsx +++ b/ui/src/app/applications/components/utils.tsx @@ -987,23 +987,59 @@ export const OperationState = ({app, quiet}: {app: appModels.Application; quiet? ); }; +function isPodInitializedConditionTrue(status: any): boolean { + if (!status?.conditions) { + return false; + } + + for (const condition of status.conditions) { + if (condition.type !== 'Initialized') { + continue; + } + return condition.status === 'True'; + } + + return false; +} + +// isPodPhaseTerminal returns true if the pod's phase is terminal. +function isPodPhaseTerminal(phase: appModels.PodPhase): boolean { + return phase === appModels.PodPhase.PodFailed || phase === appModels.PodPhase.PodSucceeded; +} + export function getPodStateReason(pod: appModels.State): {message: string; reason: string; netContainerStatuses: any[]} { - let reason = pod.status.phase; + const podPhase = pod.status.phase; + let reason = podPhase; let message = ''; if (pod.status.reason) { reason = pod.status.reason; } - let initializing = false; - let netContainerStatuses = pod.status.initContainerStatuses || []; netContainerStatuses = netContainerStatuses.concat(pod.status.containerStatuses || []); + for (const condition of pod.status.conditions || []) { + if (condition.type === 'PodScheduled' && condition.reason === 'SchedulingGated') { + reason = 'SchedulingGated'; + } + } + + const initContainers: Record = {}; + + for (const container of pod.spec.initContainers ?? []) { + initContainers[container.name] = container; + } + + let initializing = false; for (const container of (pod.status.initContainerStatuses || []).slice().reverse()) { if (container.state.terminated && container.state.terminated.exitCode === 0) { continue; } + if (container.started && initContainers[container.name].restartPolicy === 'Always') { + continue; + } + if (container.state.terminated) { if (container.state.terminated.reason) { reason = `Init:ExitCode:${container.state.terminated.exitCode}`; @@ -1021,7 +1057,7 @@ export function getPodStateReason(pod: appModels.State): {message: string; reaso break; } - if (!initializing) { + if (!initializing || isPodInitializedConditionTrue(pod.status)) { let hasRunning = false; for (const container of pod.status.containerStatuses || []) { if (container.state.waiting && container.state.waiting.reason) { @@ -1053,7 +1089,7 @@ export function getPodStateReason(pod: appModels.State): {message: string; reaso if ((pod as any).metadata.deletionTimestamp && pod.status.reason === 'NodeLost') { reason = 'Unknown'; message = ''; - } else if ((pod as any).metadata.deletionTimestamp) { + } else if ((pod as any).metadata.deletionTimestamp && !isPodPhaseTerminal(podPhase)) { reason = 'Terminating'; message = ''; }