Fix bug report include option not working as expected (istio#45842)

* fix bug report include option * add releasenotes
WillardHu · Jul 6, 2023 · c53885c · c53885c
1 parent 0df3f88
commit c53885c
Show file tree

Hide file tree

Showing 3 changed files with 159 additions and 7 deletions.
diff --git a/releasenotes/notes/45842.yaml b/releasenotes/notes/45842.yaml
@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: istioctl
+issue:
+- 45839
+releaseNotes:
+- |
+  **Fixed** an issue where specifying multiple include conditions by `--include` in bug report didn't't work as expected.
diff --git a/tools/bug-report/pkg/cluster/cluster.go b/tools/bug-report/pkg/cluster/cluster.go
@@ -87,12 +87,12 @@ func shouldSkipPod(pod *corev1.Pod, config *config2.BugReportConfig) bool {
 	for _, ild := range config.Include {
 		if len(ild.Namespaces) > 0 {
 			if !isIncludeOrExcludeEntriesMatched(ild.Namespaces, pod.Namespace) {
-				return true
+				continue
 			}
 		}
 		if len(ild.Pods) > 0 {
 			if !isIncludeOrExcludeEntriesMatched(ild.Pods, pod.Name) {
-				return true
+				continue
 			}
 		}
 
@@ -104,7 +104,7 @@ func shouldSkipPod(pod *corev1.Pod, config *config2.BugReportConfig) bool {
 				}
 			}
 			if !isContainerMatch {
-				return true
+				continue
 			}
 		}
 
@@ -119,7 +119,7 @@ func shouldSkipPod(pod *corev1.Pod, config *config2.BugReportConfig) bool {
 				}
 			}
 			if !isLabelsMatch {
-				return true
+				continue
 			}
 		}
 
@@ -134,12 +134,14 @@ func shouldSkipPod(pod *corev1.Pod, config *config2.BugReportConfig) bool {
 				}
 			}
 			if !isAnnotationMatch {
-				return true
+				continue
 			}
 		}
+		// If we reach here, it means that all include entries are matched.
+		return false
 	}
-
-	return false
+	// If we reach here, it means that no include entries are matched.
+	return true
 }
 
 func shouldSkipDeployment(deployment string, config *config2.BugReportConfig) bool {

diff --git a/tools/bug-report/pkg/cluster/cluster_test.go b/tools/bug-report/pkg/cluster/cluster_test.go
@@ -344,6 +344,148 @@ func TestShouldSkipPod(t *testing.T) {
 			},
 			true,
 		},
+		{
+			"tested difference namespace skip exclude",
+			&v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "in-test1",
+					Namespace: "test",
+					Labels: map[string]string{
+						"l3": "lv3",
+						"l4": "lv4",
+					},
+					Annotations: map[string]string{
+						"a3": "av3",
+					},
+				},
+			},
+			&config2.BugReportConfig{
+				Exclude: []*config2.SelectionSpec{
+					{
+						Namespaces: []string{
+							"fake",
+						},
+					},
+					{
+						Namespaces: []string{
+							"test",
+						},
+					},
+				},
+			},
+			true,
+		},
+		{
+			"tested include difference namespace not skip",
+			&v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "in-test1",
+					Namespace: "test",
+					Labels: map[string]string{
+						"l3": "lv3",
+						"l4": "lv4",
+					},
+					Annotations: map[string]string{
+						"a3": "av3",
+					},
+				},
+			},
+			&config2.BugReportConfig{
+				Include: []*config2.SelectionSpec{
+					{
+						Namespaces: []string{
+							"fake",
+						},
+					},
+					{
+						Namespaces: []string{
+							"test",
+						},
+					},
+				},
+			},
+			false,
+		},
+		{
+			"tested include difference namespace not skip",
+			&v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "in-test1",
+					Namespace: "test",
+					Labels: map[string]string{
+						"l3": "lv3",
+						"l4": "lv4",
+					},
+					Annotations: map[string]string{
+						"a3": "av3",
+					},
+				},
+			},
+			&config2.BugReportConfig{
+				Include: []*config2.SelectionSpec{
+					{
+						Namespaces: []string{
+							"fake",
+						},
+					},
+					{
+						Namespaces: []string{
+							"test",
+						},
+					},
+				},
+			},
+			false,
+		},
+		{
+			"tested include difference namespace/pod... not skip",
+			&v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "in-test1",
+					Namespace: "test",
+					Labels: map[string]string{
+						"l3": "lv3",
+						"l4": "lv4",
+					},
+					Annotations: map[string]string{
+						"a3": "av3",
+					},
+				},
+			},
+			&config2.BugReportConfig{
+				Include: []*config2.SelectionSpec{
+					{
+						Namespaces: []string{
+							"fake",
+						},
+						Pods: []string{
+							"in-test1",
+						},
+						Labels: map[string]string{
+							"l3": "lv3",
+						},
+						Annotations: map[string]string{
+							"a3": "av3",
+						},
+					},
+					{
+						Namespaces: []string{
+							"test",
+						},
+						Pods: []string{
+							"in-test1",
+						},
+						Labels: map[string]string{
+							"l3": "lv3",
+						},
+						Annotations: map[string]string{
+							"a3": "av3",
+						},
+					},
+				},
+			},
+			false,
+		},
 	}
 
 	for _, c := range cases {