Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pods will requeue for reconcile if nodes are not managed and requested eni #463

Merged
merged 4 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions controllers/core/pod_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
yash97 marked this conversation as resolved.
Show resolved Hide resolved
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() {
Expand Down
3 changes: 2 additions & 1 deletion controllers/core/pod_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions pkg/utils/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -232,3 +234,17 @@ func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, e
sourceArn := fmt.Sprintf("arn:%s:eks:%s:%s:cluster/%s", parsedArn.Partition, region, parsedArn.AccountID, clusterName)
return parsedArn.AccountID, 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 {
yash97 marked this conversation as resolved.
Show resolved Hide resolved
if pod == nil {
yash97 marked this conversation as resolved.
Show resolved Hide resolved
return false
}
for _, container := range pod.Spec.Containers {
if _, hasEniRequest := container.Resources.Requests[config.ResourceNamePodENI]; hasEniRequest {
return true
}
}
return false
}
100 changes: 100 additions & 0 deletions pkg/utils/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -574,3 +577,100 @@ func TestGetSourceAcctAndArn_NoRegion(t *testing.T) {
assert.Equal(t, "", acct, "correct account ID should be retrieved")
assert.Equal(t, "", arn, "correct cluster arn 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)
}
})
}
}
Loading