Merge pull request #4356 from spidernet-io/update/log

Reduce excessive WARN logs for forbidden resource access

pkg/podownercache/pod_owner_cache.go

@@ -5,7 +5,6 @@ package podownercache
 
 import (
 	"context"
-	"fmt"
 	"github.com/spidernet-io/spiderpool/pkg/lock"
 	"github.com/spidernet-io/spiderpool/pkg/logutils"
 	"go.uber.org/zap"
@@ -83,21 +82,17 @@ func (s *PodOwnerCache) onPodAdd(obj interface{}) {
 			}
 			owner, err := s.getFinalOwner(pod)
 			if err != nil {
-				if errors.IsForbidden(err) {
-					logger.Sugar().Debugf("forbidden to get owner of pod %s/%s", pod.Namespace, pod.Name)
-					return
-				}
-				logger.Warn("", zap.Error(err))
+				logger.Warn("failed to get final owner", zap.Error(err))
 				return
 			}
 			s.cacheLock.Lock()
 			defer s.cacheLock.Unlock()
 			key := types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name}
-			s.pods[key] = Pod{
-				NamespacedName: key,
-				OwnerInfo:      *owner,
-				IPs:            ips,
+			item := Pod{NamespacedName: key, IPs: ips}
+			if owner != nil {
+				item.OwnerInfo = *owner
 			}
+			s.pods[key] = item
 			for _, ip := range ips {
 				s.ipToPod[ip] = key
 			}
@@ -134,7 +129,8 @@ func (s *PodOwnerCache) getFinalOwner(obj metav1.Object) (*OwnerInfo, error) {
 			break
 		}
 
-		ownerRef := ownerRefs[0] // Assuming the first owner reference
+		// Assuming the first owner reference
+		ownerRef := ownerRefs[0]
 		finalOwner = &OwnerInfo{
 			APIVersion: ownerRef.APIVersion,
 			Kind:       ownerRef.Kind,
@@ -152,7 +148,11 @@ func (s *PodOwnerCache) getFinalOwner(obj metav1.Object) (*OwnerInfo, error) {
 			Name:      ownerRef.Name,
 		}, ownerObj)
 		if err != nil {
-			return nil, fmt.Errorf("error fetching owner: %v", err)
+			if errors.IsForbidden(err) {
+				logger.Sugar().Debugf("forbidden to get owner of pod %s/%s", obj.GetNamespace(), obj.GetName())
+				return nil, nil
+			}
+			return nil, err
 		}
 
 		// Set obj to the current owner to continue the loop

pkg/podownercache/pod_owner_cache_test.go

@@ -6,16 +6,22 @@ package podownercache
 import (
 	"context"
 	"fmt"
+	"testing"
+	"time"
+
 	appv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	k8sfakecli "sigs.k8s.io/controller-runtime/pkg/client/fake"
-	"testing"
-	"time"
+
+	"github.com/spidernet-io/spiderpool/pkg/logutils"
 )
 
 // Label(K00002)
@@ -24,7 +30,6 @@ func TestPodOwnerCache(t *testing.T) {
 	fakeCli := fake.NewSimpleClientset()
 	factory := informers.NewSharedInformerFactory(fakeCli, 0*time.Second)
 	informer := factory.Core().V1().Pods().Informer()
-	//indexer := informer.GetIndexer()
 
 	pod := &corev1.Pod{
 		ObjectMeta: v1.ObjectMeta{
@@ -47,10 +52,19 @@ func TestPodOwnerCache(t *testing.T) {
 		},
 	}
 
-	//err := indexer.Add()
-	//if err != nil {
-	//	t.Fatal(err)
-	//}
+	noOwnerPod := &corev1.Pod{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "test-pod-2",
+			Namespace: "test-ns",
+		},
+		Status: corev1.PodStatus{
+			PodIPs: []corev1.PodIP{
+				{
+					IP: "10.6.1.21",
+				},
+			},
+		},
+	}
 
 	stopCh := make(chan struct{})
 	defer close(stopCh)
@@ -79,6 +93,10 @@ func TestPodOwnerCache(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	_, err = fakeCli.CoreV1().Pods("test-ns").Create(context.Background(), noOwnerPod, v1.CreateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	time.Sleep(time.Second * 6)
 
@@ -90,6 +108,11 @@ func TestPodOwnerCache(t *testing.T) {
 		t.Fatal(fmt.Println("res is not equal to test-ns and test-deployment"))
 	}
 
+	res = cache.GetPodByIP("10.6.1.21")
+	if res == nil {
+		t.Fatal("res is nil")
+	}
+
 	// case update
 	_, err = fakeCli.CoreV1().Pods("test-ns").Update(context.Background(), pod, v1.UpdateOptions{})
 	if err != nil {
@@ -155,3 +178,48 @@ func getMockObjs() []client.Object {
 		},
 	}
 }
+
+// MockForbiddenClient is a custom mock implementation of the client.Reader interface
+type MockForbiddenClient struct{}
+
+func (m *MockForbiddenClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+	return errors.NewForbidden(schema.GroupResource{Group: "apps", Resource: "replicasets"}, "test-rs", nil)
+}
+
+func (m *MockForbiddenClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
+	return nil
+}
+
+// TestGetFinalOwnerForbidden tests the getFinalOwner method when the Get call returns a forbidden error.
+func TestGetFinalOwnerForbidden(t *testing.T) {
+	logger = logutils.Logger.Named("PodOwnerCache")
+
+	mockClient := &MockForbiddenClient{}
+	cache := &PodOwnerCache{
+		ctx:       context.Background(),
+		apiReader: mockClient,
+	}
+
+	// Create a mock pod object
+	pod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-pod",
+			Namespace: "test-ns",
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					APIVersion: "apps/v1",
+					Kind:       "ReplicaSet",
+					Name:       "test-rs",
+				},
+			},
+		},
+	}
+
+	ownerInfo, err := cache.getFinalOwner(pod)
+	if err != nil {
+		t.Fatalf("expected no error, got %v", err)
+	}
+	if ownerInfo != nil {
+		t.Fatalf("expected ownerInfo to be nil, got %v", ownerInfo)
+	}
+}