Repository: clusternet/clusternet
Pull request #834 (2024-12-04 08:51:12)

pass the finding pod error to caller instead of ignoring
#### What type of PR is this?

Fix/Optimize

#### What this PR does:

 According to submariner [findClusterIPRange](https://github.com/submariner-io/submariner-operator/blob/devel/pkg/discovery/network/generic.go#L81) function, fix ignored finding pod error, which means something wrong.

Related PR: #833

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/clusternet/clusternet/pull/834?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) Report
Attention: Patch coverage is `92.85714%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 6.45%. Comparing base [(`e2ffa5f`)](https://app.codecov.io/gh/clusternet/clusternet/commit/e2ffa5fa54a7e81045fbef497d0dfd68a4bcc7f6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) to head [(`befefc4`)](https://app.codecov.io/gh/clusternet/clusternet/commit/befefc43665d658ef025f625a278517a86e437f4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet).
> Report is 171 commits behind head on main.

| [Files with missing lines](https://app.codecov.io/gh/clusternet/clusternet/pull/834?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) | Patch % | Lines |
|---|---|---|
| [pkg/controllers/clusters/clusterstatus/network.go](https://app.codecov.io/gh/clusternet/clusternet/pull/834?src=pr&el=tree&filepath=pkg%2Fcontrollers%2Fclusters%2Fclusterstatus%2Fnetwork.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-cGtnL2NvbnRyb2xsZXJzL2NsdXN0ZXJzL2NsdXN0ZXJzdGF0dXMvbmV0d29yay5nbw==) | 92.85% | [1 Missing :warning: ](https://app.codecov.io/gh/clusternet/clusternet/pull/834?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) |

> :exclamation:  There is a different number of reports uploaded between BASE (e2ffa5f) and HEAD (befefc4). Click for more details.
> 
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (e2ffa5f) | HEAD (befefc4) |
>|------|------|------|
>||1|0|
></details>

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##             main    #834      +/-   ##
=========================================
- Coverage   13.60%   6.45%   -7.15%     
=========================================
  Files          66     269     +203     
  Lines        7492   27716   +20224     
=========================================
+ Hits         1019    1789     +770     
- Misses       6391   25781   +19390     
- Partials       82     146      +64     
```

| [Flag](https://app.codecov.io/gh/clusternet/clusternet/pull/834/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/clusternet/clusternet/pull/834/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) | `6.45% <92.85%> (?)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/clusternet/clusternet/pull/834?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet).


Patch:
diff --git a/pkg/controllers/clusters/clusterstatus/network.go b/pkg/controllers/clusters/clusterstatus/network.go
index ca6497d..3344511 100644
--- a/pkg/controllers/clusters/clusterstatus/network.go
+++ b/pkg/controllers/clusters/clusterstatus/network.go
@@ -24,7 +24,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	corelisters "k8s.io/client-go/listers/core/v1"
-	"k8s.io/klog/v2"
 )
 
 // findServiceIPRange returns the service ip range for the cluster.
@@ -37,9 +36,9 @@ func findServiceIPRange(podLister corelisters.PodLister) (string, error) {
 	for _, labelValue := range labelValues {
 		for _, labelKey := range labelKeys {
 			labelSelector := labels.SelectorFromSet(labels.Set{labelKey: labelValue})
-			serviceIPRange := findPodCommandParameter(podLister, labelSelector, parameter)
-			if serviceIPRange != "" {
-				return serviceIPRange, nil
+			serviceIPRange, err := findPodCommandParameter(podLister, labelSelector, parameter)
+			if err != nil || serviceIPRange != "" {
+				return serviceIPRange, err
 			}
 		}
 	}
@@ -61,9 +60,9 @@ func findPodIPRange(podLister corelisters.PodLister) (string, error) {
 	for _, labelValue := range labelValues {
 		for _, labelKey := range labelKeys {
 			labelSelector := labels.SelectorFromSet(labels.Set{labelKey: labelValue})
-			podIPRange := findPodCommandParameter(podLister, labelSelector, parameter)
-			if podIPRange != "" {
-				return podIPRange, nil
+			podIPRange, err := findPodCommandParameter(podLister, labelSelector, parameter)
+			if err != nil || podIPRange != "" {
+				return podIPRange, err
 			}
 		}
 	}
@@ -71,9 +70,9 @@ func findPodIPRange(podLister corelisters.PodLister) (string, error) {
 	// Try to find the pod ip range from the kube-proxy
 	labelKey, labelValue := "k8s-app", "kube-proxy"
 	labelSelector := labels.SelectorFromSet(labels.Set{labelKey: labelValue})
-	podIPRange := findPodCommandParameter(podLister, labelSelector, parameter)
-	if podIPRange != "" {
-		return podIPRange, nil
+	podIPRange, err := findPodCommandParameter(podLister, labelSelector, parameter)
+	if err != nil || podIPRange != "" {
+		return podIPRange, err
 	}
 
 	// Try to find the pod ip range from the env.
@@ -85,26 +84,24 @@ func findPodIPRange(podLister corelisters.PodLister) (string, error) {
 }
 
 // findPodCommandParameter returns the pod container command parameter by the given labelSelector.
-func findPodCommandParameter(podLister corelisters.PodLister, labelSelector labels.Selector,
-	parameter string) string {
+func findPodCommandParameter(podLister corelisters.PodLister, labelSelector labels.Selector, parameter string) (string, error) {
 	pods, err := findPods(podLister, labelSelector)
 	if err != nil {
-		klog.Errorf("Failed to find pods by label selector %q: %v", labelSelector, err)
-		return ""
+		return "", err
 	}
 
 	for _, pod := range pods {
 		for _, container := range pod.Spec.Containers {
 			if val := getParameterValue(container.Command, parameter); val != "" {
-				return val
+				return val, nil
 			}
 			if val := getParameterValue(container.Args, parameter); val != "" {
-				return val
+				return val, nil
 			}
 		}
 	}
 
-	return ""
+	return "", nil
 }
 
 // findPods returns the pods filter by the given labelSelector.


Test patch:
diff --git a/pkg/controllers/clusters/clusterstatus/network_test.go b/pkg/controllers/clusters/clusterstatus/network_test.go
index 6670842..36a84c9 100644
--- a/pkg/controllers/clusters/clusterstatus/network_test.go
+++ b/pkg/controllers/clusters/clusterstatus/network_test.go
@@ -101,12 +101,14 @@ func TestFindPodCommandParameter(t *testing.T) {
 
 		podLister, err := buildPodLister(pod)
 		if err != nil {
-			t.Errorf("new pod lister error: %v", err)
+			t.Errorf("test for %s error: %v", test.description, err)
 			continue
 		}
 
-		result := findPodCommandParameter(podLister, labelSelector, parameter)
-		if result != expected {
+		result, err := findPodCommandParameter(podLister, labelSelector, parameter)
+		if err != nil {
+			t.Errorf("test for %s error: %v", test.description, err)
+		} else if result != expected {
 			t.Errorf("test for %s: expected %s, got %s", test.description, expected, result)
 		}
 	}