* address code review

* handle no when outcome * fix Go lint G115
replicatedhq · Dec 2, 2024 · 51c678e · 51c678e
1 parent e363669
commit 51c678e
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -297,7 +297,8 @@
                     }
                   },
                   "restartCount": {
-                    "type": "integer"
+                    "type": "integer",
+                    "format": "int32"
                   },
                   "strict": {
                     "oneOf": [{"type": "string"},{"type": "boolean"}]