-
Notifications
You must be signed in to change notification settings - Fork 621
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
[octavia-ingress-controller] Update listener timeouts #2468
Conversation
This commit makes sure that listener timeouts are updated when their values are changed on the Ingress resource annotations.
This extends #2435 a bit. |
@pierreprinetti and @jichenjc, you might want to take a look. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jichenjc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -367,7 +367,11 @@ func (os *OpenStack) EnsureListener(name string, lbID string, secretRefs []strin | |||
} else { | |||
if len(listenerAllowedCIDRs) > 0 && !reflect.DeepEqual(listener.AllowedCIDRs, listenerAllowedCIDRs) { | |||
_, err := listeners.Update(os.Octavia, listener.ID, listeners.UpdateOpts{ | |||
AllowedCIDRs: &listenerAllowedCIDRs, | |||
AllowedCIDRs: &listenerAllowedCIDRs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the timeouts be updated when there is a change in the timeouts and not when allowedCIDRs has changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaysaMacedo: Whoops, I missed this, obviously valid comment. It's fixed in #2518.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
PR kubernetes#2468 used an incorrect condition when updating the listener timeout values and only updates them when listener's allowedCIDRs are updated. This commit fixes this by making sure timeout values are checked too.
PR kubernetes#2468 used an incorrect condition when updating the listener timeout values and only updates them when listener's allowedCIDRs are updated. This commit fixes this by making sure timeout values are checked too.
PR #2468 used an incorrect condition when updating the listener timeout values and only updates them when listener's allowedCIDRs are updated. This commit fixes this by making sure timeout values are checked too.
What this PR does / why we need it:
This commit makes sure that listener timeouts are updated when their values are changed on the Ingress resource annotations.
cc @sakshi-1505
Which issue this PR fixes(if applicable):
fixes #2243
Special notes for reviewers:
Release note: