Skip to content

Commit

Permalink
Add support to configure branch ENI cooldown period via configmap (#342)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
sushrk and haouc authored Dec 7, 2023
1 parent f21a4f9 commit 90024bb
Show file tree
Hide file tree
Showing 14 changed files with 381 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/presubmit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
21 changes: 21 additions & 0 deletions controllers/core/configmap_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()

Expand Down
27 changes: 27 additions & 0 deletions controllers/core/configmap_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{})
Expand All @@ -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)
Expand All @@ -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{})
Expand All @@ -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{})
Expand All @@ -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,
},
}
}
4 changes: 4 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/config/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const (
KubeSystemNamespace = "kube-system"
VpcCNIDaemonSetName = "aws-node"
OldVPCControllerDeploymentName = "vpc-resource-controller"
BranchENICooldownPeriodKey = "branch-eni-cooldown"
)

type ResourceType string
Expand Down
87 changes: 87 additions & 0 deletions pkg/provider/branch/cooldown/cooldown.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit 90024bb

Please sign in to comment.