Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this defaulted to 30s since if first time get fails what will be the value?

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