Skip to content
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

Delayed IP release of NSM connection IPs #524

Merged
merged 3 commits into from
Apr 30, 2024
Merged

Conversation

zolug
Copy link
Collaborator

@zolug zolug commented Apr 26, 2024

Description

It's really hard to asses when IP addresses allocated to NSM connections are no longer needed,
without risking early re-allocation that could result in duplicated IP issues across the NSM interfaces.

For example NSM Close() might fail, but even if it succeeds there's no guarantee that the NSM interfaces associated
with the connection are actually removed. (After nsmgr restart the new instance will not return an error for a Close() request the connection is not known to it.)

As a short-term fix, IP release is delayed by a configurable value. The main goal is to avoid freeing up IPs of NSM connections subject to NSM heal even if the related Close/Request calls might timeout. The default delay value is set to exceed the current timeout value of NSM requests (15s).

The delay can be configured via the Operator environment variable PROXY_IP_RELEASE_DELAY which in turn configures a respective env variable for the proxy containers.

Issue link

#521

Checklist

  • Purpose
    • Bug fix
    • New functionality
    • Documentation
    • Refactoring
    • CI
  • Test
    • Unit test
    • E2E Test
    • Tested manually
  • Introduce a breaking change
    • Yes (description required)
    • No

zolug added 2 commits April 26, 2024 17:33
Short term solution to avoid duplicated IPs by only
releasing them with a configurable delay.
Operator environment variable PROXY_IP_RELEASE_DELAY is set to 20
seconds.
@@ -73,6 +73,9 @@ func (i *Proxy) getEnvVars(allEnv []corev1.EnvVar) []corev1.EnvVar {
if mtu := common.GetConduitMTU(); mtu != "" {
operatorEnv = append(operatorEnv, corev1.EnvVar{Name: "NSM_MTU", Value: mtu})
}
if ipReleaseDelay := common.GetProxyIPReleaseDelay(); ipReleaseDelay != "" {
Copy link
Member

Choose a reason for hiding this comment

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

Otherwise the option could stay in the template

ctxRelease, cancelRelease := context.WithTimeout(ctx, 4*time.Second)
defer cancelRelease()
for _, subnet := range p.Subnets {
child := &ipamAPI.Child{
Copy link
Member

Choose a reason for hiding this comment

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

Repetition with l468

Co-authored-by: Lionel Jouin <[email protected]>
@LionelJouin LionelJouin merged commit db3066c into master Apr 30, 2024
13 checks passed
@zolug zolug deleted the delayed-ip-release branch May 14, 2024 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants