Add support to configure branch ENI cooldown period via configmap (#342)

* Add support to configure branch ENI cooldown period via configmap

* support configurable branch ENI cooldown period

* moving error check out from CM update

* Fix logs and remove mutex lock in Get function

* Update to go1.21.5

---------

Co-authored-by: Hao Zhou <[email protected]>

Author: 2 people

.github/workflows/presubmit.yaml

@@ -45,7 +45,7 @@ jobs:
         uses: actions/checkout@v3
       - uses: actions/setup-go@v4
         with:
-          go-version: '1.21.4'
+          go-version: '1.21.5'
           cache-dependency-path: "**/go.sum"
       - name: Install `govulncheck`
         run: go install golang.org/x/vuln/cmd/govulncheck@latest

Makefile

@@ -12,7 +12,7 @@ MAKEFILE_PATH = $(dir $(realpath -s $(firstword $(MAKEFILE_LIST))))
 VERSION ?= $(GIT_VERSION)
 IMAGE ?= $(REPO):$(VERSION)
 BASE_IMAGE ?= public.ecr.aws/eks-distro-build-tooling/eks-distro-minimal-base-nonroot:latest.2
-BUILD_IMAGE ?= public.ecr.aws/bitnami/golang:1.21.4
+BUILD_IMAGE ?= public.ecr.aws/bitnami/golang:1.21.5
 GOARCH ?= amd64
 PLATFORM ?= linux/amd64
 

controllers/core/configmap_controller.go

@@ -22,9 +22,12 @@ import (
 	rcHealthz "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/healthz"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager"
+	cooldown "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown"
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -73,6 +76,24 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 	}
 
+	// Check if branch ENI cooldown period is updated
+	curCoolDownPeriod := cooldown.GetCoolDown().GetCoolDownPeriod()
+	if newCoolDownPeriod, err := cooldown.GetVpcCniConfigMapCoolDownPeriodOrDefault(r.K8sAPI, r.Log); err == nil {
+		if curCoolDownPeriod != newCoolDownPeriod {
+			r.Log.Info("Branch ENI cool down period has been updated", "newCoolDownPeriod", newCoolDownPeriod, "OldCoolDownPeriod", curCoolDownPeriod)
+			cooldown.GetCoolDown().SetCoolDownPeriod(newCoolDownPeriod)
+			utils.SendBroadcastNodeEvent(
+				r.K8sAPI,
+				utils.BranchENICoolDownUpdateReason,
+				fmt.Sprintf("Branch ENI cool down period has been updated to %s", cooldown.GetCoolDown().GetCoolDownPeriod()),
+				v1.EventTypeNormal,
+				r.Log,
+			)
+		}
+	} else {
+		r.Log.Error(err, "failed to retrieve branch ENI cool down period from amazon-vpc-cni configmap, will retain the current cooldown period", "cool down period", curCoolDownPeriod)
+	}
+
 	// Check if the Windows IPAM flag has changed
 	newWinIPAMEnabledCond := r.Condition.IsWindowsIPAMEnabled()
 

controllers/core/configmap_controller_test.go

@@ -23,9 +23,11 @@ import (
 	mock_node "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node"
 	mock_manager "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
+	cooldown "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -112,6 +114,9 @@ func Test_Reconcile_ConfigMap_Updated(t *testing.T) {
 	mock.MockNodeManager.EXPECT().GetNode(mockNodeName).Return(mock.MockNode, true)
 	mock.MockNodeManager.EXPECT().UpdateNode(mockNodeName).Return(nil)
 
+	mock.MockK8sAPI.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(createCoolDownMockCM("30"), nil).AnyTimes()
+
+	cooldown.InitCoolDownPeriod(mock.MockK8sAPI, zap.New(zap.UseDevMode(true)).WithName("cooldown"))
 	res, err := mock.ConfigMapReconciler.Reconcile(context.TODO(), mockConfigMapReq)
 	assert.NoError(t, err)
 	assert.Equal(t, res, reconcile.Result{})
@@ -125,6 +130,9 @@ func Test_Reconcile_ConfigMap_PD_Disabled_If_IPAM_Disabled(t *testing.T) {
 	mock := NewConfigMapMock(ctrl, mockConfigMapPD)
 	mock.MockCondition.EXPECT().IsWindowsIPAMEnabled().Return(false)
 	mock.MockCondition.EXPECT().IsWindowsPrefixDelegationEnabled().Return(false)
+	mock.MockK8sAPI.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(createCoolDownMockCM("30"), nil).AnyTimes()
+
+	cooldown.InitCoolDownPeriod(mock.MockK8sAPI, zap.New(zap.UseDevMode(true)).WithName("cooldown"))
 
 	res, err := mock.ConfigMapReconciler.Reconcile(context.TODO(), mockConfigMapReq)
 	assert.NoError(t, err)
@@ -142,6 +150,9 @@ func Test_Reconcile_ConfigMap_NoData(t *testing.T) {
 
 	mock.MockCondition.EXPECT().IsWindowsIPAMEnabled().Return(false)
 	mock.MockCondition.EXPECT().IsWindowsPrefixDelegationEnabled().Return(false)
+	mock.MockK8sAPI.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(createCoolDownMockCM("30"), nil).AnyTimes()
+
+	cooldown.InitCoolDownPeriod(mock.MockK8sAPI, zap.New(zap.UseDevMode(true)).WithName("cooldown"))
 	res, err := mock.ConfigMapReconciler.Reconcile(context.TODO(), mockConfigMapReq)
 	assert.NoError(t, err)
 	assert.Equal(t, res, reconcile.Result{})
@@ -154,7 +165,9 @@ func Test_Reconcile_ConfigMap_Deleted(t *testing.T) {
 	mock := NewConfigMapMock(ctrl)
 	mock.MockCondition.EXPECT().IsWindowsIPAMEnabled().Return(false)
 	mock.MockCondition.EXPECT().IsWindowsPrefixDelegationEnabled().Return(false)
+	mock.MockK8sAPI.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(createCoolDownMockCM("30"), nil).AnyTimes()
 
+	cooldown.InitCoolDownPeriod(mock.MockK8sAPI, zap.New(zap.UseDevMode(true)).WithName("cooldown"))
 	res, err := mock.ConfigMapReconciler.Reconcile(context.TODO(), mockConfigMapReq)
 	assert.NoError(t, err)
 	assert.Equal(t, res, reconcile.Result{})
@@ -170,9 +183,23 @@ func Test_Reconcile_UpdateNode_Error(t *testing.T) {
 	mock.MockK8sAPI.EXPECT().ListNodes().Return(nodeList, nil)
 	mock.MockNodeManager.EXPECT().GetNode(mockNodeName).Return(mock.MockNode, true)
 	mock.MockNodeManager.EXPECT().UpdateNode(mockNodeName).Return(errMock)
+	mock.MockK8sAPI.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(createCoolDownMockCM("30"), nil).AnyTimes()
 
+	cooldown.InitCoolDownPeriod(mock.MockK8sAPI, zap.New(zap.UseDevMode(true)).WithName("cooldown"))
 	res, err := mock.ConfigMapReconciler.Reconcile(context.TODO(), mockConfigMapReq)
 	assert.Error(t, err)
 	assert.Equal(t, res, reconcile.Result{})
 
 }
+
+func createCoolDownMockCM(cooldownTime string) *v1.ConfigMap {
+	return &v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      config.VpcCniConfigMapName,
+			Namespace: config.KubeSystemNamespace,
+		},
+		Data: map[string]string{
+			config.BranchENICooldownPeriodKey: cooldownTime,
+		},
+	}
+}

main.go

@@ -34,6 +34,7 @@ import (
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s/pod"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager"
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/version"
@@ -290,6 +291,9 @@ func main() {
 	controllerConditions := condition.NewControllerConditions(
 		ctrl.Log.WithName("controller conditions"), k8sApi, enableWindowsPrefixDelegation)
 
+	// initialize the branch ENI cool down period
+	cooldown.InitCoolDownPeriod(k8sApi, ctrl.Log)
+
 	// when Windows PD feature flag is OFF, do not initialize resource for prefix IPs
 	var supportedResources []string
 	if enableWindowsPrefixDelegation {

mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go

@@ -80,6 +80,7 @@ const (
 	KubeSystemNamespace            = "kube-system"
 	VpcCNIDaemonSetName            = "aws-node"
 	OldVPCControllerDeploymentName = "vpc-resource-controller"
+	BranchENICooldownPeriodKey     = "branch-eni-cooldown"
 )
 
 type ResourceType string

pkg/config/type.go

@@ -0,0 +1,87 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//     http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package cooldown
+
+import (
+	"fmt"
+	"strconv"
+	"sync"
+	"time"
+
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s"
+	"github.com/go-logr/logr"
+)
+
+// Global variable for CoolDownPeriod allows packages to Get and Set the coolDown period
+var coolDown *cooldown
+
+type cooldown struct {
+	mu sync.RWMutex
+	// CoolDownPeriod is the period to wait before deleting the branch ENI for propagation of ip tables rule for deleted pod
+	coolDownPeriod time.Duration
+}
+
+type CoolDown interface {
+	GetCoolDownPeriod() time.Duration
+	SetCoolDownPeriod(time.Duration)
+}
+
+const (
+	DefaultCoolDownPeriod = time.Second * 60
+	MinimalCoolDownPeriod = time.Second * 30
+)
+
+// Initialize coolDown period by setting the value in configmap or to default
+func InitCoolDownPeriod(k8sApi k8s.K8sWrapper, log logr.Logger) {
+	coolDown = &cooldown{}
+	coolDownPeriod, err := GetVpcCniConfigMapCoolDownPeriodOrDefault(k8sApi, log)
+	if err != nil {
+		log.Info("setting coolDown period to default", "cool down period", coolDownPeriod)
+	}
+	coolDown.SetCoolDownPeriod(coolDownPeriod)
+}
+
+func GetCoolDown() CoolDown {
+	return coolDown
+}
+
+func GetVpcCniConfigMapCoolDownPeriodOrDefault(k8sApi k8s.K8sWrapper, log logr.Logger) (time.Duration, error) {
+	vpcCniConfigMap, err := k8sApi.GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace)
+	if err == nil && vpcCniConfigMap.Data != nil {
+		if val, ok := vpcCniConfigMap.Data[config.BranchENICooldownPeriodKey]; ok {
+			coolDownPeriodInt, err := strconv.Atoi(val)
+			if err != nil {
+				log.Error(err, "failed to parse branch ENI coolDown period", "cool down period", val)
+			} else {
+				return time.Second * time.Duration(coolDownPeriodInt), nil
+			}
+		}
+	}
+	// If configmap not found, or configmap data not found, or error in parsing coolDown period, return default coolDown period and error
+	return DefaultCoolDownPeriod, fmt.Errorf("failed to get cool down period:%v", err)
+}
+
+func (c *cooldown) GetCoolDownPeriod() time.Duration {
+	if c.coolDownPeriod < 30*time.Second {
+		return MinimalCoolDownPeriod
+	}
+	return c.coolDownPeriod
+}
+
+func (c *cooldown) SetCoolDownPeriod(newCoolDownPeriod time.Duration) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.coolDownPeriod = newCoolDownPeriod
+}