Skip to content

Commit 80e7264

Browse files
authored
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
1 parent 4ea11cb commit 80e7264

File tree

4 files changed

+123
-1
lines changed

4 files changed

+123
-1
lines changed

controllers/core/pod_controller.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node"
2828
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager"
2929
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource"
30+
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils"
3031
"github.com/google/uuid"
3132

3233
"github.com/go-logr/logr"
@@ -112,6 +113,10 @@ func (r *PodReconciler) Reconcile(request custom.Request) (ctrl.Result, error) {
112113
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)
113114
return PodRequeueRequest, nil
114115
} else if !node.IsManaged() {
116+
if utils.PodHasENIRequest(pod) {
117+
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)
118+
return PodRequeueRequest, nil
119+
}
115120
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)
116121
return ctrl.Result{}, nil
117122
} else if !node.IsReady() {

controllers/core/pod_controller_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package controllers
1616
import (
1717
"errors"
1818
"testing"
19+
"time"
1920

2021
"github.com/aws/amazon-vpc-resource-controller-k8s/controllers/custom"
2122
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) {
188189

189190
result, err := mock.PodReconciler.Reconcile(mockReq)
190191
assert.NoError(t, err)
191-
assert.Equal(t, result, controllerruntime.Result{})
192+
assert.Equal(t, controllerruntime.Result{Requeue: true, RequeueAfter: time.Second}, result)
192193
}
193194

194195
// TestPodReconciler_Reconcile_NoNodeAssigned tests that the request for a Pod with no Node assigned

pkg/utils/helper.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ import (
2121

2222
vpcresourcesv1beta1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1"
2323
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/vpc"
24+
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
2425

2526
"github.com/aws/aws-sdk-go/aws/arn"
2627

2728
"github.com/go-logr/logr"
2829
corev1 "k8s.io/api/core/v1"
30+
v1 "k8s.io/api/core/v1"
2931
"k8s.io/apimachinery/pkg/api/meta"
3032
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3133
"k8s.io/apimachinery/pkg/labels"
@@ -232,3 +234,17 @@ func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, s
232234
sourceArn := fmt.Sprintf("arn:%s:eks:%s:%s:cluster/%s", parsedArn.Partition, region, parsedArn.AccountID, clusterName)
233235
return parsedArn.AccountID, parsedArn.Partition, sourceArn, nil
234236
}
237+
238+
// PodHasENIRequest will return true if first container of pod spec has request for eni indicating
239+
// it needs trunk interface from vpc-rc
240+
func PodHasENIRequest(pod *v1.Pod) bool {
241+
if pod == nil {
242+
return false
243+
}
244+
for _, container := range pod.Spec.Containers {
245+
if _, hasEniRequest := container.Resources.Requests[config.ResourceNamePodENI]; hasEniRequest {
246+
return true
247+
}
248+
}
249+
return false
250+
}

pkg/utils/helper_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ import (
1818

1919
"github.com/samber/lo"
2020
"github.com/stretchr/testify/assert"
21+
v1 "k8s.io/api/core/v1"
22+
"k8s.io/apimachinery/pkg/api/resource"
2123
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2224

2325
vpcresourcesv1beta1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1"
26+
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
2427
)
2528

2629
// TestRemoveDuplicatedSg tests if RemoveDuplicatedSg func works as expected.
@@ -579,3 +582,100 @@ func TestGetSourceAcctAndArn_NoRegion(t *testing.T) {
579582
assert.Equal(t, "", part, "correct partiton should be retrieved")
580583

581584
}
585+
586+
func TestPodHasENIRequest(t *testing.T) {
587+
tests := []struct {
588+
name string
589+
pod *v1.Pod
590+
expected bool
591+
}{
592+
{
593+
name: "Pod with ENI request in first container",
594+
pod: &v1.Pod{
595+
Spec: v1.PodSpec{
596+
Containers: []v1.Container{
597+
{
598+
Resources: v1.ResourceRequirements{
599+
Requests: v1.ResourceList{
600+
config.ResourceNamePodENI: resource.MustParse("1"),
601+
},
602+
},
603+
},
604+
},
605+
},
606+
},
607+
expected: true,
608+
},
609+
{
610+
name: "Pod with multiple containers, no ENI request",
611+
pod: &v1.Pod{
612+
Spec: v1.PodSpec{
613+
Containers: []v1.Container{
614+
{
615+
Resources: v1.ResourceRequirements{
616+
Requests: v1.ResourceList{
617+
v1.ResourceCPU: resource.MustParse("100m"),
618+
},
619+
},
620+
},
621+
{
622+
Resources: v1.ResourceRequirements{
623+
Requests: v1.ResourceList{
624+
v1.ResourceMemory: resource.MustParse("128Mi"),
625+
},
626+
},
627+
},
628+
{
629+
Resources: v1.ResourceRequirements{
630+
Requests: v1.ResourceList{
631+
v1.ResourceStorage: resource.MustParse("1Gi"),
632+
},
633+
},
634+
},
635+
},
636+
},
637+
},
638+
expected: false,
639+
},
640+
{
641+
name: "Pod without ENI request",
642+
pod: &v1.Pod{
643+
Spec: v1.PodSpec{
644+
Containers: []v1.Container{
645+
{
646+
Resources: v1.ResourceRequirements{
647+
Requests: v1.ResourceList{
648+
v1.ResourceCPU: resource.MustParse("100m"),
649+
},
650+
},
651+
},
652+
},
653+
},
654+
},
655+
expected: false,
656+
},
657+
{
658+
name: "Pod with empty containers",
659+
pod: &v1.Pod{
660+
Spec: v1.PodSpec{
661+
Containers: []v1.Container{},
662+
},
663+
},
664+
expected: false,
665+
},
666+
{
667+
name: "Nil pod",
668+
pod: nil,
669+
expected: false,
670+
},
671+
}
672+
673+
for _, tt := range tests {
674+
t.Run(tt.name, func(t *testing.T) {
675+
result := PodHasENIRequest(tt.pod)
676+
if result != tt.expected {
677+
t.Errorf("PodHasENIRequest() = %v, want %v", result, tt.expected)
678+
}
679+
})
680+
}
681+
}

0 commit comments

Comments
 (0)