pods will requeue for reconcile if nodes are not managed and requeste…

…d eni (#463) * pod will requeue for reconcile if nodes are not managed and requested eni * log statement change * looping through all container for eni requests * adding ut for utils function
aws · Sep 12, 2024 · 80e7264 · 80e7264
1 parent 4ea11cb
commit 80e7264
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 1 deletion.
diff --git a/controllers/core/pod_controller.go b/controllers/core/pod_controller.go
@@ -27,6 +27,7 @@ import (
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource"
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils"
 	"github.com/google/uuid"
 
 	"github.com/go-logr/logr"
@@ -112,6 +113,10 @@ func (r *PodReconciler) Reconcile(request custom.Request) (ctrl.Result, error) {
 			logger.V(1).Info("pod's node is not yet initialized by the manager, will retry", "Requested", request.NamespacedName.String(), "Cached pod name", pod.ObjectMeta.Name, "Cached pod namespace", pod.ObjectMeta.Namespace)
 			return PodRequeueRequest, nil
 		} else if !node.IsManaged() {
+			if utils.PodHasENIRequest(pod) {
+				r.Log.Info("pod's node is not managed, but has eni request, will retry", "Requested", request.NamespacedName.String(), "Cached pod name", pod.ObjectMeta.Name, "Cached pod namespace", pod.ObjectMeta.Namespace)
+				return PodRequeueRequest, nil
+			}
 			logger.V(1).Info("pod's node is not managed, skipping pod event", "Requested", request.NamespacedName.String(), "Cached pod name", pod.ObjectMeta.Name, "Cached pod namespace", pod.ObjectMeta.Namespace)
 			return ctrl.Result{}, nil
 		} else if !node.IsReady() {

diff --git a/controllers/core/pod_controller_test.go b/controllers/core/pod_controller_test.go
@@ -16,6 +16,7 @@ package controllers
 import (
 	"errors"
 	"testing"
+	"time"
 
 	"github.com/aws/amazon-vpc-resource-controller-k8s/controllers/custom"
 	mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition"
@@ -188,7 +189,7 @@ func TestPodReconciler_Reconcile_NonManaged(t *testing.T) {
 
 	result, err := mock.PodReconciler.Reconcile(mockReq)
 	assert.NoError(t, err)
-	assert.Equal(t, result, controllerruntime.Result{})
+	assert.Equal(t, controllerruntime.Result{Requeue: true, RequeueAfter: time.Second}, result)
 }
 
 // TestPodReconciler_Reconcile_NoNodeAssigned tests that the request for a Pod with no Node assigned

diff --git a/pkg/utils/helper.go b/pkg/utils/helper.go
@@ -21,11 +21,13 @@ import (
 
 	vpcresourcesv1beta1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/vpc"
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
 
 	"github.com/aws/aws-sdk-go/aws/arn"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -232,3 +234,17 @@ func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, s
 	sourceArn := fmt.Sprintf("arn:%s:eks:%s:%s:cluster/%s", parsedArn.Partition, region, parsedArn.AccountID, clusterName)
 	return parsedArn.AccountID, parsedArn.Partition, sourceArn, nil
 }
+
+// PodHasENIRequest will return true if first container of pod spec has request for eni indicating
+// it needs trunk interface from vpc-rc
+func PodHasENIRequest(pod *v1.Pod) bool {
+	if pod == nil {
+		return false
+	}
+	for _, container := range pod.Spec.Containers {
+		if _, hasEniRequest := container.Resources.Requests[config.ResourceNamePodENI]; hasEniRequest {
+			return true
+		}
+	}
+	return false
+}
diff --git a/pkg/utils/helper_test.go b/pkg/utils/helper_test.go
@@ -18,9 +18,12 @@ import (
 
 	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	vpcresourcesv1beta1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1"
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
 )
 
 // TestRemoveDuplicatedSg tests if RemoveDuplicatedSg func works as expected.
@@ -579,3 +582,100 @@ func TestGetSourceAcctAndArn_NoRegion(t *testing.T) {
 	assert.Equal(t, "", part, "correct partiton should be retrieved")
 
 }
+
+func TestPodHasENIRequest(t *testing.T) {
+	tests := []struct {
+		name     string
+		pod      *v1.Pod
+		expected bool
+	}{
+		{
+			name: "Pod with ENI request in first container",
+			pod: &v1.Pod{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						{
+							Resources: v1.ResourceRequirements{
+								Requests: v1.ResourceList{
+									config.ResourceNamePodENI: resource.MustParse("1"),
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "Pod with multiple containers, no ENI request",
+			pod: &v1.Pod{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						{
+							Resources: v1.ResourceRequirements{
+								Requests: v1.ResourceList{
+									v1.ResourceCPU: resource.MustParse("100m"),
+								},
+							},
+						},
+						{
+							Resources: v1.ResourceRequirements{
+								Requests: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("128Mi"),
+								},
+							},
+						},
+						{
+							Resources: v1.ResourceRequirements{
+								Requests: v1.ResourceList{
+									v1.ResourceStorage: resource.MustParse("1Gi"),
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "Pod without ENI request",
+			pod: &v1.Pod{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						{
+							Resources: v1.ResourceRequirements{
+								Requests: v1.ResourceList{
+									v1.ResourceCPU: resource.MustParse("100m"),
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "Pod with empty containers",
+			pod: &v1.Pod{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{},
+				},
+			},
+			expected: false,
+		},
+		{
+			name:     "Nil pod",
+			pod:      nil,
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := PodHasENIRequest(tt.pod)
+			if result != tt.expected {
+				t.Errorf("PodHasENIRequest() = %v, want %v", result, tt.expected)
+			}
+		})
+	}
+}