From 51c678ec92ec6c3c775afb4de68a15d502d41860 Mon Sep 17 00:00:00 2001 From: Gerard Nguyen Date: Mon, 2 Dec 2024 17:13:33 +1100 Subject: [PATCH] * address code review * handle no when outcome * fix Go lint G115 --- config/crds/troubleshoot.sh_analyzers.yaml | 1 + config/crds/troubleshoot.sh_preflights.yaml | 1 + .../crds/troubleshoot.sh_supportbundles.yaml | 1 + pkg/analyze/cluster_container_statuses.go | 50 ++++++++++--------- .../cluster_container_statuses_test.go | 36 +++++++++++++ .../troubleshoot/v1beta2/analyzer_shared.go | 2 +- schemas/analyzer-troubleshoot-v1beta2.json | 3 +- schemas/preflight-troubleshoot-v1beta2.json | 3 +- .../supportbundle-troubleshoot-v1beta2.json | 3 +- 9 files changed, 73 insertions(+), 27 deletions(-) diff --git a/config/crds/troubleshoot.sh_analyzers.yaml b/config/crds/troubleshoot.sh_analyzers.yaml index 943269260..b68bcd21e 100644 --- a/config/crds/troubleshoot.sh_analyzers.yaml +++ b/config/crds/troubleshoot.sh_analyzers.yaml @@ -188,6 +188,7 @@ spec: type: object type: array restartCount: + format: int32 type: integer strict: type: BoolString diff --git a/config/crds/troubleshoot.sh_preflights.yaml b/config/crds/troubleshoot.sh_preflights.yaml index 370df18f1..f954487d0 100644 --- a/config/crds/troubleshoot.sh_preflights.yaml +++ b/config/crds/troubleshoot.sh_preflights.yaml @@ -188,6 +188,7 @@ spec: type: object type: array restartCount: + format: int32 type: integer strict: type: BoolString diff --git a/config/crds/troubleshoot.sh_supportbundles.yaml b/config/crds/troubleshoot.sh_supportbundles.yaml index cbb00c87d..8f45b0fdf 100644 --- a/config/crds/troubleshoot.sh_supportbundles.yaml +++ b/config/crds/troubleshoot.sh_supportbundles.yaml @@ -219,6 +219,7 @@ spec: type: object type: array restartCount: + format: int32 type: integer strict: type: BoolString diff --git a/pkg/analyze/cluster_container_statuses.go b/pkg/analyze/cluster_container_statuses.go index f2226420e..7f3010b3e 100644 --- a/pkg/analyze/cluster_container_statuses.go +++ b/pkg/analyze/cluster_container_statuses.go @@ -69,8 +69,10 @@ func (a *AnalyzeClusterContainerStatuses) Analyze(getFile getCollectedFileConten } func (a *AnalyzeClusterContainerStatuses) getPodsMatchingFilters(podListFiles map[string][]byte) (podsWithContainers, error) { + var podsMatchedNamespace []corev1.Pod matchedPods := podsWithContainers{} + // filter pods matched namespace selector for fileName, fileContent := range podListFiles { // pod list fileName is the namespace name, e.g. default.json currentNamespace := strings.TrimSuffix(filepath.Base(fileName), ".json") @@ -82,7 +84,6 @@ func (a *AnalyzeClusterContainerStatuses) getPodsMatchingFilters(podListFiles ma } // filter pods by namespace - var podsMatchedNamespace []corev1.Pod var podList corev1.PodList if err := json.Unmarshal(fileContent, &podList); err != nil { var pods []corev1.Pod @@ -94,32 +95,33 @@ func (a *AnalyzeClusterContainerStatuses) getPodsMatchingFilters(podListFiles ma } else { podsMatchedNamespace = append(podsMatchedNamespace, podList.Items...) } + } - // filter pods by container restart count - for _, pod := range podsMatchedNamespace { - for _, containerStatus := range pod.Status.ContainerStatuses { - if containerStatus.RestartCount < int32(a.analyzer.RestartCount) { - continue - } - // check if the pod has already been matched - key := string(pod.UID) - if _, ok := matchedPods[key]; !ok { - matchedPods[key] = struct { - name string - namespace string - containerStatuses []corev1.ContainerStatus - }{ - name: pod.Name, - namespace: pod.Namespace, - containerStatuses: []corev1.ContainerStatus{containerStatus}, - } - continue + // filter pods by container criteria + for _, pod := range podsMatchedNamespace { + for _, containerStatus := range pod.Status.ContainerStatuses { + if containerStatus.RestartCount < a.analyzer.RestartCount { + continue + } + // check if the pod has already been matched + key := string(pod.UID) + if _, ok := matchedPods[key]; !ok { + matchedPods[key] = struct { + name string + namespace string + containerStatuses []corev1.ContainerStatus + }{ + name: pod.Name, + namespace: pod.Namespace, + containerStatuses: []corev1.ContainerStatus{containerStatus}, } - entry := matchedPods[key] - entry.containerStatuses = append(entry.containerStatuses, containerStatus) + continue } + entry := matchedPods[key] + entry.containerStatuses = append(entry.containerStatuses, containerStatus) } } + return matchedPods, nil } @@ -156,10 +158,12 @@ func (a *AnalyzeClusterContainerStatuses) analyzeContainerStatuses(podContainers continue } + // empty when indicates final case, let's return the result if when == "" { - continue + return []*AnalyzeResult{&r}, nil } + // continue matching with when condition reason, isEqualityOp, err := parseWhen(when) if err != nil { return nil, errors.Wrap(err, "failed to parse when") diff --git a/pkg/analyze/cluster_container_statuses_test.go b/pkg/analyze/cluster_container_statuses_test.go index 73dec9473..b880f201b 100644 --- a/pkg/analyze/cluster_container_statuses_test.go +++ b/pkg/analyze/cluster_container_statuses_test.go @@ -45,6 +45,42 @@ func Test_analyzeContainerStatuses(t *testing.T) { "cluster-resources/pods/message-oomkill-pod.json": []byte(messageOOMKillPod), }, }, + { + name: "pass when there is no status detected", + analyzer: troubleshootv1beta2.ClusterContainerStatuses{ + AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{ + CheckName: "oomkilled-container", + }, + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "== OOMKilled", + Message: "Container {{ .ContainerName }} from pod {{ .Namespace }}/{{ .PodName }} has OOMKilled", + }, + }, + { + Pass: &troubleshootv1beta2.SingleOutcome{ + Message: "No OOMKilled container found", + }, + }, + }, + Namespaces: []string{"default"}, + }, + expectResult: []*AnalyzeResult{ + { + IsFail: false, + IsWarn: false, + IsPass: true, + Title: "oomkilled-container", + Message: "No OOMKilled container found", + IconKey: "kubernetes_container_statuses", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/kubernetes.svg?w=16&h=16", + }, + }, + files: map[string][]byte{ + "cluster-resources/pods/default.json": []byte(defaultPods), + }, + }, } for _, test := range tests { diff --git a/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go b/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go index 3e3f877d1..56a31e536 100644 --- a/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go @@ -109,7 +109,7 @@ type ClusterContainerStatuses struct { AnalyzeMeta `json:",inline" yaml:",inline"` Outcomes []*Outcome `json:"outcomes" yaml:"outcomes"` Namespaces []string `json:"namespaces,omitempty" yaml:"namespaces,omitempty"` - RestartCount int `json:"restartCount" yaml:"restartCount"` + RestartCount int32 `json:"restartCount" yaml:"restartCount"` } type ContainerRuntime struct { diff --git a/schemas/analyzer-troubleshoot-v1beta2.json b/schemas/analyzer-troubleshoot-v1beta2.json index b3e32b631..a5f219bb2 100644 --- a/schemas/analyzer-troubleshoot-v1beta2.json +++ b/schemas/analyzer-troubleshoot-v1beta2.json @@ -251,7 +251,8 @@ } }, "restartCount": { - "type": "integer" + "type": "integer", + "format": "int32" }, "strict": { "oneOf": [{"type": "string"},{"type": "boolean"}] diff --git a/schemas/preflight-troubleshoot-v1beta2.json b/schemas/preflight-troubleshoot-v1beta2.json index 961de59f3..ce590d051 100644 --- a/schemas/preflight-troubleshoot-v1beta2.json +++ b/schemas/preflight-troubleshoot-v1beta2.json @@ -251,7 +251,8 @@ } }, "restartCount": { - "type": "integer" + "type": "integer", + "format": "int32" }, "strict": { "oneOf": [{"type": "string"},{"type": "boolean"}] diff --git a/schemas/supportbundle-troubleshoot-v1beta2.json b/schemas/supportbundle-troubleshoot-v1beta2.json index d011ed1b1..a1f416306 100644 --- a/schemas/supportbundle-troubleshoot-v1beta2.json +++ b/schemas/supportbundle-troubleshoot-v1beta2.json @@ -297,7 +297,8 @@ } }, "restartCount": { - "type": "integer" + "type": "integer", + "format": "int32" }, "strict": { "oneOf": [{"type": "string"},{"type": "boolean"}]