From 80e7264c1ac05d119810ba77f8de83dda91c8faf Mon Sep 17 00:00:00 2001 From: Yash Thakkar Date: Thu, 12 Sep 2024 12:05:56 -0700 Subject: [PATCH] pods will requeue for reconcile if nodes are not managed and requested 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 --- controllers/core/pod_controller.go | 5 ++ controllers/core/pod_controller_test.go | 3 +- pkg/utils/helper.go | 16 ++++ pkg/utils/helper_test.go | 100 ++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/controllers/core/pod_controller.go b/controllers/core/pod_controller.go index 825b2870..fc8ab5c5 100644 --- 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 index 12fed741..02883f4d 100644 --- 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 index 14fda665..6b16d045 100644 --- 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 index aee3659b..2a603b75 100644 --- 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) + } + }) + } +}