From 8f83564a393568f431cedc506c3a9976dd9b53b1 Mon Sep 17 00:00:00 2001 From: Hao Zhou Date: Wed, 17 May 2023 20:11:02 +0000 Subject: [PATCH] refactoring eniconfig func to only take node as parameter --- Makefile | 2 +- pkg/eniconfig/eniconfig.go | 20 +++++++++----------- pkg/ipamd/introspect.go | 9 ++++++++- pkg/ipamd/ipamd.go | 10 +++++++++- pkg/ipamd/ipamd_test.go | 2 +- pkg/k8sapi/k8sutils.go | 14 ++++++++++++++ pkg/k8sapi/k8sutils_test.go | 37 +++++++++++++++++++++++++++++++++++++ 7 files changed, 79 insertions(+), 15 deletions(-) create mode 100644 pkg/k8sapi/k8sutils_test.go diff --git a/Makefile b/Makefile index a22a96b9e62..960a35c7a3c 100644 --- a/Makefile +++ b/Makefile @@ -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)" \ diff --git a/pkg/eniconfig/eniconfig.go b/pkg/eniconfig/eniconfig.go index fb7dc951db0..1bfde1cf37f 100644 --- a/pkg/eniconfig/eniconfig.go +++ b/pkg/eniconfig/eniconfig.go @@ -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" ) @@ -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) @@ -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] diff --git a/pkg/ipamd/introspect.go b/pkg/ipamd/introspect.go index 402b835d626..3939e736b18 100644 --- a/pkg/ipamd/introspect.go +++ b/pkg/ipamd/introspect.go @@ -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" ) @@ -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) diff --git a/pkg/ipamd/ipamd.go b/pkg/ipamd/ipamd.go index 7087611d4ad..91289818fec 100644 --- a/pkg/ipamd/ipamd.go +++ b/pkg/ipamd/ipamd.go @@ -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" @@ -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) diff --git a/pkg/ipamd/ipamd_test.go b/pkg/ipamd/ipamd_test.go index aa5b9af7d3a..906f7a391b3 100644 --- a/pkg/ipamd/ipamd_test.go +++ b/pkg/ipamd/ipamd_test.go @@ -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) } diff --git a/pkg/k8sapi/k8sutils.go b/pkg/k8sapi/k8sutils.go index 3454cdb5c01..35b6d2092bf 100644 --- a/pkg/k8sapi/k8sutils.go +++ b/pkg/k8sapi/k8sutils.go @@ -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" @@ -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 +} diff --git a/pkg/k8sapi/k8sutils_test.go b/pkg/k8sapi/k8sutils_test.go new file mode 100644 index 00000000000..92bd7d9f64e --- /dev/null +++ b/pkg/k8sapi/k8sutils_test.go @@ -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) +}