From 8dfc0e105a26016c025b5a28086e61054bba58bb Mon Sep 17 00:00:00 2001 From: Peter Strzyzewski Date: Wed, 5 Dec 2018 16:58:13 -0800 Subject: [PATCH 1/7] Support for node labels to set eniConfig --- pkg/eniconfig/eniconfig.go | 23 ++++++++--- pkg/eniconfig/eniconfig_test.go | 70 ++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/pkg/eniconfig/eniconfig.go b/pkg/eniconfig/eniconfig.go index 135b0fd9f5..8deacfebb7 100644 --- a/pkg/eniconfig/eniconfig.go +++ b/pkg/eniconfig/eniconfig.go @@ -36,6 +36,7 @@ import ( const ( eniConfigAnnotationDef = "k8s.amazonaws.com/eniConfig" + eniConfigLabelDef = "k8s.amazonaws.com/eniConfig" eniConfigDefault = "default" ) @@ -104,7 +105,7 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { case *corev1.Node: - log.Infof("Handle corev1.Node: %s, %v", o.GetName(), o.GetAnnotations()) + log.Infof("Handle corev1.Node: %s, %v, %v", o.GetName(), o.GetAnnotations(), o.GetLabels()) if h.controller.myNodeName == o.GetName() { annotation := o.GetAnnotations() @@ -115,10 +116,22 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { h.controller.myENI = val log.Infof(" Setting myENI to: %s", val) } else { - h.controller.eniLock.Lock() - defer h.controller.eniLock.Unlock() - h.controller.myENI = eniConfigDefault - log.Infof(" Setting myENI to: %s", eniConfigDefault) + + label := o.GetLabels() + + val, ok := label[eniConfigLabelDef] + if ok { + h.controller.eniLock.Lock() + defer h.controller.eniLock.Unlock() + h.controller.myENI = val + log.Infof(" Setting myENI to: %s", val) + } else { + + h.controller.eniLock.Lock() + defer h.controller.eniLock.Unlock() + h.controller.myENI = eniConfigDefault + log.Infof(" Setting myENI to: %s", eniConfigDefault) + } } } } diff --git a/pkg/eniconfig/eniconfig_test.go b/pkg/eniconfig/eniconfig_test.go index a87810953a..c53c8a2a52 100644 --- a/pkg/eniconfig/eniconfig_test.go +++ b/pkg/eniconfig/eniconfig_test.go @@ -68,6 +68,33 @@ func updateNodeAnnotation(hdlr sdk.Handler, nodeName string, configName string, hdlr.Handle(nil, event) } +func updateNodeLabel(hdlr sdk.Handler, nodeName string, configName string, toDelete bool) { + + node := corev1.Node{ + TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + }, + } + accessor, err := meta.Accessor(&node) + + if err != nil { + fmt.Printf("Failed to call meta.Access %v", err) + } + + event := sdk.Event{ + Object: &node, + Deleted: toDelete, + } + eniLabels := make(map[string]string) + + if !toDelete { + eniLabels[eniConfigLabelDef] = configName + } + accessor.SetLabels(eniLabels) + hdlr.Handle(nil, event) +} + func TestENIConfig(t *testing.T) { testENIConfigController := NewENIConfigController() @@ -105,7 +132,7 @@ func TestENIConfig(t *testing.T) { } func TestNodeENIConfig(t *testing.T) { - myNodeName := "testMyNode" + myNodeName := "testMyNodeWithAnnotation" myENIConfig := "testMyENIConfig" os.Setenv("MY_NODE_NAME", myNodeName) testENIConfigController := NewENIConfigController() @@ -144,3 +171,44 @@ func TestNodeENIConfig(t *testing.T) { assert.Equal(t, defaultCfg, *outputCfg) } + +func TestNodeENIConfigLabel(t *testing.T) { + myNodeName := "testMyNodeWithLabel" + myENIConfig := "testMyENIConfig" + os.Setenv("MY_NODE_NAME", myNodeName) + testENIConfigController := NewENIConfigController() + + testHandler := NewHandler(testENIConfigController) + updateNodeLabel(testHandler, myNodeName, myENIConfig, false) + + // If there is no ENI config + _, err := testENIConfigController.MyENIConfig() + assert.Error(t, err) + + // Add eniconfig for myENIConfig + group1Cfg := v1alpha1.ENIConfigSpec{ + SecurityGroups: []string{"sg21-id", "sg22-id"}, + Subnet: "subnet21"} + updateENIConfig(testHandler, myENIConfig, group1Cfg, false) + outputCfg, err := testENIConfigController.MyENIConfig() + assert.NoError(t, err) + assert.Equal(t, group1Cfg, *outputCfg) + + // Add default config + defaultSGs := []string{"sg1-id", "sg2-id"} + defaultSubnet := "subnet1" + defaultCfg := v1alpha1.ENIConfigSpec{ + SecurityGroups: defaultSGs, + Subnet: defaultSubnet} + updateENIConfig(testHandler, eniConfigDefault, defaultCfg, false) + outputCfg, err = testENIConfigController.MyENIConfig() + assert.NoError(t, err) + assert.Equal(t, group1Cfg, *outputCfg) + + // Delete node's myENIConfig annotation, then the value should fallback to default + updateNodeLabel(testHandler, myNodeName, myENIConfig, true) + outputCfg, err = testENIConfigController.MyENIConfig() + assert.NoError(t, err) + assert.Equal(t, defaultCfg, *outputCfg) + +} From 9e039cd37737b3c427ecb9c23262f0608aacc85e Mon Sep 17 00:00:00 2001 From: Peter Strzyzewski Date: Thu, 3 Jan 2019 14:44:21 -0800 Subject: [PATCH 2/7] Annotations and labels can be configured with env variable --- pkg/eniconfig/eniconfig.go | 53 ++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/pkg/eniconfig/eniconfig.go b/pkg/eniconfig/eniconfig.go index 8deacfebb7..3c8f8c50c2 100644 --- a/pkg/eniconfig/eniconfig.go +++ b/pkg/eniconfig/eniconfig.go @@ -35,9 +35,19 @@ import ( ) const ( - eniConfigAnnotationDef = "k8s.amazonaws.com/eniConfig" - eniConfigLabelDef = "k8s.amazonaws.com/eniConfig" - eniConfigDefault = "default" + defaultEniConfigAnnotationDef = "k8s.amazonaws.com/eniConfig" + defaultEniConfigLabelDef = "k8s.amazonaws.com/eniConfig" + eniConfigDefault = "default" + + // when "ENI_CONFIG_LABEL_DEF is defined, ENIConfigController will use that label key to + // search if is setting value for eniConfigLabelDef + // Example: + // Node has set label k8s.amazonaws.com/eniConfigCustom=customConfig + // We can get that value in controller by setting environmental variable ENI_CONFIG_LABEL_DEF + // ENI_CONFIG_LABEL_DEF=k8s.amazonaws.com/eniConfigOverride + // This will set eniConfigLabelDef to eniConfigOverride + envEniConfigAnnotationDef = "ENI_CONFIG_ANNOTATION_DEF" + envEniConfigLabelDef = "ENI_CONFIG_LABEL_DEF" ) type ENIConfig interface { @@ -106,10 +116,12 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { case *corev1.Node: log.Infof("Handle corev1.Node: %s, %v, %v", o.GetName(), o.GetAnnotations(), o.GetLabels()) + // Get annotations if not found get labels if not found fallback use default if h.controller.myNodeName == o.GetName() { annotation := o.GetAnnotations() + annotationDef := getEniConfigAnnotationDef() - val, ok := annotation[eniConfigAnnotationDef] + val, ok := annotation[annotationDef] if ok { h.controller.eniLock.Lock() defer h.controller.eniLock.Unlock() @@ -118,8 +130,9 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { } else { label := o.GetLabels() + labelDef := getEniConfigLabelDef() - val, ok := label[eniConfigLabelDef] + val, ok := label[labelDef] if ok { h.controller.eniLock.Lock() defer h.controller.eniLock.Unlock() @@ -195,3 +208,33 @@ func (eniCfg *ENIConfigController) MyENIConfig() (*v1alpha1.ENIConfigSpec, error } return nil, ErrNoENIConfig } + +// getEniConfigAnnotationDef returns eniConfigAnnotation +func getEniConfigAnnotationDef() string { + inputStr, found := os.LookupEnv(envEniConfigAnnotationDef) + + if !found { + return defaultEniConfigAnnotationDef + } + if len(inputStr) > 0 { + log.Debugf("Using ENI_CONFIG_ANNOTATION_DEF %v", inputStr) + return inputStr + } + + return defaultEniConfigAnnotationDef +} + +// getEniConfigLabelDef returns eniConfigLabel name +func getEniConfigLabelDef() string { + inputStr, found := os.LookupEnv(envEniConfigLabelDef) + + if !found { + return defaultEniConfigLabelDef + } + if len(inputStr) > 0 { + log.Debugf("Using ENI_CONFIG_LABEL_DEF %v", inputStr) + return inputStr + } + + return defaultEniConfigLabelDef +} From 49e7e95c44f6d0d209eef4158b436000db7d4df2 Mon Sep 17 00:00:00 2001 From: Peter Strzyzewski Date: Thu, 3 Jan 2019 14:47:20 -0800 Subject: [PATCH 3/7] tests for annotations and labels config --- pkg/eniconfig/eniconfig_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pkg/eniconfig/eniconfig_test.go b/pkg/eniconfig/eniconfig_test.go index c53c8a2a52..f5afdc0e55 100644 --- a/pkg/eniconfig/eniconfig_test.go +++ b/pkg/eniconfig/eniconfig_test.go @@ -60,6 +60,7 @@ func updateNodeAnnotation(hdlr sdk.Handler, nodeName string, configName string, Deleted: toDelete, } eniAnnotations := make(map[string]string) + eniConfigAnnotationDef := getEniConfigAnnotationDef() if !toDelete { eniAnnotations[eniConfigAnnotationDef] = configName @@ -87,6 +88,7 @@ func updateNodeLabel(hdlr sdk.Handler, nodeName string, configName string, toDel Deleted: toDelete, } eniLabels := make(map[string]string) + eniConfigLabelDef := getEniConfigLabelDef() if !toDelete { eniLabels[eniConfigLabelDef] = configName @@ -212,3 +214,27 @@ func TestNodeENIConfigLabel(t *testing.T) { assert.Equal(t, defaultCfg, *outputCfg) } + +func TestGetEniConfigAnnotationDefDefault(t *testing.T) { + os.Unsetenv(envEniConfigAnnotationDef) + eniConfigAnnotationDef := getEniConfigAnnotationDef() + assert.Equal(t, eniConfigAnnotationDef, defaultEniConfigAnnotationDef) +} + +func TestGetEniConfigAnnotationlDefCustom(t *testing.T) { + os.Setenv(envEniConfigAnnotationDef, "k8s.amazonaws.com/eniConfigCustom") + eniConfigAnnotationDef := getEniConfigAnnotationDef() + assert.Equal(t, eniConfigAnnotationDef, "k8s.amazonaws.com/eniConfigCustom") +} + +func TestGetEniConfigLabelDefDefault(t *testing.T) { + os.Unsetenv(envEniConfigLabelDef) + eniConfigLabelDef := getEniConfigLabelDef() + assert.Equal(t, eniConfigLabelDef, defaultEniConfigLabelDef) +} + +func TestGetEniConfigLabelDefCustom(t *testing.T) { + os.Setenv(envEniConfigLabelDef, "k8s.amazonaws.com/eniConfigCustom") + eniConfigLabelDef := getEniConfigLabelDef() + assert.Equal(t, eniConfigLabelDef, "k8s.amazonaws.com/eniConfigCustom") +} From de5048f4180c6fd2e14fdbc8fdf0ded1d0665a73 Mon Sep 17 00:00:00 2001 From: Peter Strzyzewski Date: Thu, 3 Jan 2019 15:17:47 -0800 Subject: [PATCH 4/7] Custom annotation and label key environmental variables --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index 970b60daab..be5a56f914 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,17 @@ Type: Boolean Default: `false` Specifies that your pods may use subnets and security groups that are independent of your worker node's VPC configuration\. By default, pods share the same subnet and security groups as the worker node's primary interface\. Setting this variable to `true` causes `ipamD` to use the security groups and VPC subnet in a worker node's `ENIConfig` for elastic network interface allocation\. You must create an `ENIConfig` custom resource definition for each subnet that your pods will reside in, and then annotate each worker node to use a specific `ENIConfig` \(multiple worker nodes can be annotated with the same `ENIConfig`\)\. Worker nodes can only be annotated with a single `ENIConfig` at a time, and the subnet in the `ENIConfig` must belong to the same Availability Zone that the worker node resides in\. For more information, see [https://github.com/aws/amazon-vpc-cni-k8s/pull/165](https://github.com/aws/amazon-vpc-cni-k8s/pull/165)\. +`ENI_CONFIG_ANNOTATION_DEF` +Type: String +Default: k8s.amazonaws.com/eniConfig +Specifies node annotation key name. This should be used when AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true. Custom annotation value will be used to set eniConfig name. + +`ENI_CONFIG_LABEL_DEF` +Type: String +Default: k8s.amazonaws.com/eniConfig +Specifies node label key name. This should be used when AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true. Custom annotation value will be used to set eniConfig name. +For example if you set ENI_CONFIG_LABEL_DEF=failure-domain.beta.kubernetes.io/zone, the node that is deployed in `us-east-1a` would have it set to that zone and CNI would use eniConfig named `us-east-1a`. + `AWS_VPC_K8S_CNI_EXTERNALSNAT` Type: Boolean Default: `false` From b7d7acb2485b2e7091177830ad31e34ab69baafe Mon Sep 17 00:00:00 2001 From: Peter Strzyzewski Date: Wed, 9 Jan 2019 22:09:37 -0800 Subject: [PATCH 5/7] initializing eniConfigLabels and eniConfigAnnotations only once --- pkg/eniconfig/eniconfig.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/eniconfig/eniconfig.go b/pkg/eniconfig/eniconfig.go index 3c8f8c50c2..c0f7a395a9 100644 --- a/pkg/eniconfig/eniconfig.go +++ b/pkg/eniconfig/eniconfig.go @@ -59,10 +59,12 @@ var ErrNoENIConfig = errors.New("eniconfig: eniconfig is not available") // ENIConfigController defines global context for ENIConfig controller type ENIConfigController struct { - eni map[string]*v1alpha1.ENIConfigSpec - myENI string - eniLock sync.RWMutex - myNodeName string + eni map[string]*v1alpha1.ENIConfigSpec + myENI string + eniLock sync.RWMutex + myNodeName string + eniConfigAnnotationDef string + eniConfigLabelDef string } // ENIConfigInfo returns locally cached ENIConfigs @@ -77,6 +79,8 @@ func NewENIConfigController() *ENIConfigController { myNodeName: os.Getenv("MY_NODE_NAME"), eni: make(map[string]*v1alpha1.ENIConfigSpec), myENI: eniConfigDefault, + eniConfigAnnotationDef: getEniConfigAnnotationDef(), + eniConfigLabelDef: getEniConfigLabelDef(), } } @@ -119,9 +123,8 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { // Get annotations if not found get labels if not found fallback use default if h.controller.myNodeName == o.GetName() { annotation := o.GetAnnotations() - annotationDef := getEniConfigAnnotationDef() - val, ok := annotation[annotationDef] + val, ok := annotation[h.controller.eniConfigAnnotationDef] if ok { h.controller.eniLock.Lock() defer h.controller.eniLock.Unlock() @@ -130,9 +133,8 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { } else { label := o.GetLabels() - labelDef := getEniConfigLabelDef() - val, ok := label[labelDef] + val, ok := label[h.controller.eniConfigLabelDef] if ok { h.controller.eniLock.Lock() defer h.controller.eniLock.Unlock() From a41b24b3e7d47c1e9f40fe06750f41438d7efab7 Mon Sep 17 00:00:00 2001 From: Peter Strzyzewski Date: Thu, 10 Jan 2019 08:46:55 -0800 Subject: [PATCH 6/7] Better examples --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index be5a56f914..1278baac4f 100644 --- a/README.md +++ b/README.md @@ -96,13 +96,14 @@ Specifies that your pods may use subnets and security groups that are independen `ENI_CONFIG_ANNOTATION_DEF` Type: String Default: k8s.amazonaws.com/eniConfig -Specifies node annotation key name. This should be used when AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true. Custom annotation value will be used to set eniConfig name. +Specifies node annotation key name. This should be used when AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true. Custom annotation value will be used to set eniConfig name. See ENI_CONFIG_LABEL_DEF for examples. `ENI_CONFIG_LABEL_DEF` Type: String Default: k8s.amazonaws.com/eniConfig -Specifies node label key name. This should be used when AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true. Custom annotation value will be used to set eniConfig name. -For example if you set ENI_CONFIG_LABEL_DEF=failure-domain.beta.kubernetes.io/zone, the node that is deployed in `us-east-1a` would have it set to that zone and CNI would use eniConfig named `us-east-1a`. +Specifies node label key name. This should be used when AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true. Custom label value will be used to set eniConfig name. Note that annotations will take precedence over labels. To use labels, ensure default annotation k8s.amazonaws.com/eniConfig is not set on node and ENI_CONFIG_ANNOTATION_DEF is not used. +For example, you can use custom node label key _example.com/eniConfig_ by setting ENI_CONFIG_LABEL_DEF=example.com/eniConfig. Then you can set that label on node with value of your custom eniConfig name like `eniConfig-us-east-1a`. +In other example if your node has label _failure-domain.beta.kubernetes.io/zone_ and its value is set to availability zone `us-east-1a`, you can set ENI_CONFIG_LABEL_DEF=failure-domain.beta.kubernetes.io/zone. In such case eniConfig would be set to availability zone name `us-east-1a`, and you could use it to differentiate configs between zones. `AWS_VPC_K8S_CNI_EXTERNALSNAT` Type: Boolean From f29e71e4a749857490cc6bfd6e7d4a049eed2d7a Mon Sep 17 00:00:00 2001 From: Peter Strzyzewski Date: Thu, 10 Jan 2019 16:19:40 -0800 Subject: [PATCH 7/7] Cleaner logic implementation --- pkg/eniconfig/eniconfig.go | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/pkg/eniconfig/eniconfig.go b/pkg/eniconfig/eniconfig.go index c0f7a395a9..b229652e03 100644 --- a/pkg/eniconfig/eniconfig.go +++ b/pkg/eniconfig/eniconfig.go @@ -122,32 +122,19 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { log.Infof("Handle corev1.Node: %s, %v, %v", o.GetName(), o.GetAnnotations(), o.GetLabels()) // Get annotations if not found get labels if not found fallback use default if h.controller.myNodeName == o.GetName() { - annotation := o.GetAnnotations() - - val, ok := annotation[h.controller.eniConfigAnnotationDef] - if ok { - h.controller.eniLock.Lock() - defer h.controller.eniLock.Unlock() - h.controller.myENI = val - log.Infof(" Setting myENI to: %s", val) - } else { - - label := o.GetLabels() - - val, ok := label[h.controller.eniConfigLabelDef] - if ok { - h.controller.eniLock.Lock() - defer h.controller.eniLock.Unlock() - h.controller.myENI = val - log.Infof(" Setting myENI to: %s", val) - } else { - - h.controller.eniLock.Lock() - defer h.controller.eniLock.Unlock() - h.controller.myENI = eniConfigDefault - log.Infof(" Setting myENI to: %s", eniConfigDefault) + + val, ok := o.GetAnnotations()[h.controller.eniConfigAnnotationDef] + if !ok { + val, ok = o.GetLabels()[h.controller.eniConfigLabelDef] + if !ok { + val = eniConfigDefault } } + + h.controller.eniLock.Lock() + defer h.controller.eniLock.Unlock() + h.controller.myENI = val + log.Infof(" Setting myENI to: %s", val) } } return nil