Skip to content

Commit

Permalink
refactoring eniconfig func to only take node as parameter (#2387)
Browse files Browse the repository at this point in the history
  • Loading branch information
haouc authored May 17, 2023
1 parent 3865743 commit 2eef830
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 15 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ build-aws-vpc-cni: ## Build the VPC CNI container using the host's Go toolcha
go build $(VENDOR_OVERRIDE_FLAG) $(BUILD_FLAGS) -o aws-vpc-cni ./cmd/aws-vpc-cni

# Build VPC CNI plugin & agent container image.
docker: setup-ec2-sdk-override ## Build VPC CNI plugin & agent container image.
docker: setup-ec2-sdk-override ## Build VPC CNI plugin & agent container image.
docker build $(DOCKER_BUILD_FLAGS_CNI) \
-f scripts/dockerfiles/Dockerfile.release \
-t "$(IMAGE_NAME)" \
Expand Down
20 changes: 9 additions & 11 deletions pkg/eniconfig/eniconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1"
"github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi"
"github.com/aws/amazon-vpc-cni-k8s/pkg/utils/logger"
)

Expand Down Expand Up @@ -69,9 +70,14 @@ type ENIConfigInfo struct {

// MyENIConfig returns the ENIConfig applicable to the particular node
func MyENIConfig(ctx context.Context, k8sClient client.Client) (*v1alpha1.ENIConfigSpec, error) {
eniConfigName, err := GetNodeSpecificENIConfigName(ctx, k8sClient)
node, err := k8sapi.GetNode(ctx, k8sClient)
if err != nil {
log.Debugf("Error while retrieving Node name")
log.Debugf("Error while retrieving Node")
}

eniConfigName, err := GetNodeSpecificENIConfigName(node)
if err != nil {
log.Debugf("Error while retrieving Node ENIConfig name")
}

log.Infof("Found ENI Config Name: %s", eniConfigName)
Expand Down Expand Up @@ -116,17 +122,9 @@ func getEniConfigLabelDef() string {
return defaultEniConfigLabelDef
}

func GetNodeSpecificENIConfigName(ctx context.Context, k8sClient client.Client) (string, error) {
func GetNodeSpecificENIConfigName(node corev1.Node) (string, error) {
var eniConfigName string

log.Infof("Get Node Info for: %s", os.Getenv("MY_NODE_NAME"))
var node corev1.Node
err := k8sClient.Get(ctx, types.NamespacedName{Name: os.Getenv("MY_NODE_NAME")}, &node)
if err != nil {
log.Errorf("error retrieving node: %s", err)
return eniConfigName, err
}

//Derive ENIConfig Name from either externally managed label, Node Annotations or Labels
labels := node.GetLabels()
eniConfigName, ok := labels[externalEniConfigLabel]
Expand Down
9 changes: 8 additions & 1 deletion pkg/ipamd/introspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"time"

"github.com/aws/amazon-vpc-cni-k8s/pkg/eniconfig"
"github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi"
"github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils"
"github.com/aws/amazon-vpc-cni-k8s/pkg/utils/retry"
)
Expand Down Expand Up @@ -144,7 +145,13 @@ func eniV1RequestHandler(ipam *IPAMContext) func(http.ResponseWriter, *http.Requ
func eniConfigRequestHandler(ipam *IPAMContext) func(http.ResponseWriter, *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
myENIConfig, err := eniconfig.GetNodeSpecificENIConfigName(ctx, ipam.cachedK8SClient)
node, err := k8sapi.GetNode(ctx, ipam.cachedK8SClient)
if err != nil {
log.Errorf("Failed to get host node: %v", err)
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
return
}
myENIConfig, err := eniconfig.GetNodeSpecificENIConfigName(node)
if err != nil {
log.Errorf("Failed to get ENI config: %v", err)
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
Expand Down
10 changes: 9 additions & 1 deletion pkg/ipamd/ipamd.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils"
"github.com/aws/amazon-vpc-cni-k8s/pkg/eniconfig"
"github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd/datastore"
"github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi"
"github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils"
"github.com/aws/amazon-vpc-cni-k8s/pkg/utils/eventrecorder"
"github.com/aws/amazon-vpc-cni-k8s/pkg/utils/logger"
Expand Down Expand Up @@ -566,7 +567,14 @@ func (c *IPAMContext) nodeInit() error {
vpcV4CIDRs = c.updateCIDRsRulesOnChange(vpcV4CIDRs)
}, 30*time.Second)

eniConfigName, err := eniconfig.GetNodeSpecificENIConfigName(ctx, c.cachedK8SClient)
node, err := k8sapi.GetNode(ctx, c.cachedK8SClient)
if err != nil {
log.Errorf("Failed to host node", err)
podENIErrInc("nodeInit")
return err
}

eniConfigName, err := eniconfig.GetNodeSpecificENIConfigName(node)
if err == nil && c.useCustomNetworking && eniConfigName != "default" {
// Signal to VPC Resource Controller that the node is using custom networking
err := c.SetNodeLabel(ctx, vpcENIConfigLabel, eniConfigName)
Expand Down
2 changes: 1 addition & 1 deletion pkg/ipamd/ipamd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func TestNodeInit(t *testing.T) {

// Add IPs
m.awsutils.EXPECT().AllocIPAddresses(gomock.Any(), gomock.Any())

os.Setenv("MY_NODE_NAME", myNodeName)
err := mockContext.nodeInit()
assert.NoError(t, err)
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/k8sapi/k8sutils.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package k8sapi

import (
"context"
"fmt"
"os"
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/types"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"

Expand Down Expand Up @@ -141,3 +144,14 @@ func getRestConfig() (*rest.Config, error) {
}
return restCfg, nil
}

func GetNode(ctx context.Context, k8sClient client.Client) (corev1.Node, error) {
log.Infof("Get Node Info for: %s", os.Getenv("MY_NODE_NAME"))
var node corev1.Node
err := k8sClient.Get(ctx, types.NamespacedName{Name: os.Getenv("MY_NODE_NAME")}, &node)
if err != nil {
log.Errorf("error retrieving node: %s", err)
return node, err
}
return node, nil
}
37 changes: 37 additions & 0 deletions pkg/k8sapi/k8sutils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package k8sapi

import (
"context"
"os"
"testing"

eniconfigscheme "github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestGetNode(t *testing.T) {
ctx := context.Background()
k8sSchema := runtime.NewScheme()
clientgoscheme.AddToScheme(k8sSchema)
eniconfigscheme.AddToScheme(k8sSchema)

fakeNode := &corev1.Node{
ObjectMeta: v1.ObjectMeta{
Name: "testNode",
},
}
k8sClient := fake.NewFakeClientWithScheme(k8sSchema, fakeNode)
os.Setenv("MY_NODE_NAME", "testNode")
node, err := GetNode(ctx, k8sClient)
assert.NoError(t, err)
assert.Equal(t, node.Name, "testNode")

os.Setenv("MY_NODE_NAME", "dummyNode")
_, err = GetNode(ctx, k8sClient)
assert.Error(t, err)
}

0 comments on commit 2eef830

Please sign in to comment.