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

Conversation

sushrk
Copy link
Contributor

@sushrk sushrk commented Dec 4, 2023

Issue #, if available:
#306

Description of changes:
Adding support to configure the cool down period via amazon-vpc-cni configmap using flag branch-eni-cooldown, in seconds. The minimum value for the cool period is 30s, if user updates configmap to a lower value, this will be overridden and set to 30s.

Testing updated in comments.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@haouc
Copy link
Contributor

haouc commented Dec 5, 2023

@sushrk made some changes in the new commit. please let me know if you have comments on them. I will ask for reviews from the team.

@haouc
Copy link
Contributor

haouc commented Dec 5, 2023

We allow the period configurable but force the lowest value as 30 seconds. Whenever the value is changed, the controller broadcast a node event to all nodes which ensure users the planned value being applied successfully.
Testing some behaviors
Setting cooldown period to 120 seconds

Events:
  Type    Reason                          Age   From                     Message
  ----    ------                          ----  ----                     -------
  Normal  ControllerVersionNotice         11m   vpc-resource-controller  The node is managed by VPC resource controller version v1.4.2-9-gf21a4f9
  Normal  NodeTrunkInitiated              11m   vpc-resource-controller  The node has trunk interface initialized successfully
  Normal  BranchENICoolDownPeriodUpdated  41s   vpc-resource-controller  Branch ENI cool down period has been updated to 2m0s

Then change the value to 29 seconds which should be forced to 30 seconds although CM still has 29 seconds

apiVersion: v1
data:
  branch-eni-cooldown: "29"
  enable-network-policy-controller: "false"
  enable-windows-ipam: "false"
kind: ConfigMap
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"v1","data":{"enable-network-policy-controller":"false","enable-windows-ipam":"false"},"kind":"ConfigMap","metadata":{"annotations":{},"labels":{"app.kubernetes.io/instance":"aws-vpc-cni","app.kubernetes.io/name":"aws-node","app.kubernetes.io/version":"v1.15.4","k8s-app":"aws-node"},"name":"amazon-vpc-cni","namespace":"kube-system"}}
  creationTimestamp: "2023-12-02T19:56:21Z"
  labels:
    app.kubernetes.io/instance: aws-vpc-cni
    app.kubernetes.io/name: aws-node
    app.kubernetes.io/version: v1.15.4
    k8s-app: aws-node
  name: amazon-vpc-cni
  namespace: kube-system
  resourceVersion: "989462"
  uid: 1c7920ed-5d6b-452d-8228-f8dee3c771c8
Events:
  Type    Reason                          Age    From                     Message
  ----    ------                          ----   ----                     -------
  Normal  ControllerVersionNotice         15m    vpc-resource-controller  The node is managed by VPC resource controller version v1.4.2-9-gf21a4f9
  Normal  NodeTrunkInitiated              15m    vpc-resource-controller  The node has trunk interface initialized successfully
  Normal  BranchENICoolDownPeriodUpdated  4m12s  vpc-resource-controller  Branch ENI cool down period has been updated to 2m0s
  Normal  BranchENICoolDownPeriodUpdated  37s    vpc-resource-controller  Branch ENI cool down period has been updated to 30s

@haouc haouc requested a review from achevuru December 6, 2023 06:43
achevuru
achevuru previously approved these changes Dec 6, 2023
@sushrk sushrk marked this pull request as ready for review December 7, 2023 00:48
@sushrk sushrk requested a review from a team as a code owner December 7, 2023 00:48
@sushrk sushrk merged commit 90024bb into aws:master Dec 7, 2023
@@ -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?

sushrk added a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Dec 8, 2023
…s#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]>
haouc added a commit that referenced this pull request Dec 8, 2023
* adding govulnscheck to action (#330)

* fix typo in document (#331)

* Upgrade vpc-cni to v1.15.0

* Bump github.com/onsi/gomega from 1.28.0 to 1.30.0 (#338)

Bumps [github.com/onsi/gomega](https://github.com/onsi/gomega) from 1.28.0 to 1.30.0.
- [Release notes](https://github.com/onsi/gomega/releases)
- [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md)
- [Commits](onsi/gomega@v1.28.0...v1.30.0)

---
updated-dependencies:
- dependency-name: github.com/onsi/gomega
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Run go mod tidy

* Upgrade golang/x/time to 0.5.0

* Bump github.com/prometheus/common from 0.44.0 to 0.45.0 (#337)

Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.44.0 to 0.45.0.
- [Release notes](https://github.com/prometheus/common/releases)
- [Commits](prometheus/common@v0.44.0...v0.45.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/common
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* update supported EC2 instances (#340)

* 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]>

* fix the build issue

* Update cooldown period in test (#344)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Hao Zhou <[email protected]>
Co-authored-by: yochien <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Hao Zhou <[email protected]>
@sushrk sushrk deleted the sgpp-dns-fix branch March 18, 2024 17:03
yash97 pushed a commit that referenced this pull request Aug 28, 2024
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants