-
Notifications
You must be signed in to change notification settings - Fork 8.4k
Add persistent-drainable
option for affinity-mode
ingress annotation to support draining sticky server sessions
#13480
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
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gulachek The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This issue is currently awaiting triage. If Ingress contributors determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Welcome @gulachek! |
Hi @gulachek. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for kubernetes-ingress-nginx ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This currently behaves exactly like `persistent` after this commit.
This wires up `getEndpointsFromSlices` to have different selection modes for querying Endpoint objects. This is necessary so that `persistent-drainable` can eventually select Endpoints that are `Serving` instead of only `Ready`. This added a unit test to demonstrate the selection behavior, and the wiring up in the controller remains to be done. The Endpoint type was also updated to have an `IsDraining` field when the Endpoint is not Ready.
I believe that this is the only place that needs to check for `Serving` Endpoint objects to implement this feature. There are 6 total invocations of `getEndpointsFromSlices` in the repo, all in `controller.go`. 2 of them are relating to `L4Service`. I'm assuming L4 is referring to the OSI model and these seem to relate to TCP and UDP, which have no knowledge of HTTP cookies, which `persistent-drainable` relies on, so these can't be required. (They appear to be for controller level config instead of ingress rules). A third is in `getDefaultUpstream`. My understanding is this is for a fallback service should all of the primary services in the ingress rules be down. This seems like a fallback service handler and not useful for `persistent-drainable`. A fourth one is used in `getBackendServers` when building the `custom-default-backend-` upstream when no Endpoints are available for the service. Again, this doesn't seem intended for the primary application that needs session affinity, so exluding the draining Endpoints in the default backend seems desirable. The last two are used in `serviceEndpoints`, which itself is called in two places, both in `createUpstreams`. One of them is for the default backend service, which discussed above, should not be included. The other one is for our main ingress rules and **is** the one that we want to update.
What this PR does / why we need it:
Legacy web applications that have sticky session affinity are likely to store important application information in memory on the web server associated with the session. This stateful nature can increase application performance by reducing the amount of contextual information necessary to be supplied by the browser or loaded from a persistent database to handle a request. This performance comes with a maintenance cost, where the state on the web server needs to be carefully handled so that users are not disrupted during web server maintenance.
In a traditional model, where web server application management is hands on, sys admins can mark web servers in a "maintenance" or "draining" state to coordinate offloading user sessions from the draining web servers to other servers that are available. This may take several additional requests to the draining web server, but eventually the number of sessions on the web server will approach zero. This allows the admin to safely install OS updates, etc, on the server without disrupting users.
When these applications are ported to Kubernetes, there is a challenge. Kubernetes may dynamically mark a pod for deletion. While pods are given a
preStop
hook to gracefully terminate, if the pods' service is exposed via aningress-nginx
controller, even withaffinity-mode: persistent
, they will immediately stop receiving additional requests that may be necessary for gracefully migrating user sessions to other pods. This is becauseingress-nginx
removes Endpoint objects from the set of available upstreams when their Endpoint condition is no longerReady
, or in other words when it isTerminating
.This PR addresses this problem by adding an
affinity-mode: persistent-drainable
option. I'll paste my updated documentation for the annotation here:This issue has been discussed since 2018 in nginx/kubernetes-ingress#5962, which should provide further motivation for this feature. You can see that many of those involved in the discussion have resorted to using jcmoraisjr/haproxy-ingress which has a drain-support feature.
While
haproxy-ingress
might be a viable alternative, I would like to useingress-nginx
for my organization. There is a strong community of support, and hopefully as you will see through reviewing the PR, it is already almost all the way there to supporting this drain support feature, which is critical for the success of my organization's web application.Types of changes
Which issue/s this PR fixes
I didn't find one specifically on this repo, but nginx/kubernetes-ingress#5962 looks like the OP was originally trying to use this ingress controller, which implicitly is an issue on this repo. 😄
How Has This Been Tested?
I ran the automated go, lua, and e2e tests to make sure I didn't break anything, and I added additional go and lua unit tests for the new feature.
To test that this worked end to end, I created a very basic PHP deployment that echo's back the pod's hostname. You can download it from affinity-test.tgz.
You can get it set up with:
make dev-env # start the ingress-nginx test cluster tar xfvz affinity-test.tgz kubectl create namespace affinity-test kubectl apply -k affinity-test
You can then validate that it's running by running
curl localhost
curl localhost # Hostname: php-hostname-echo-747df85784-jwd79
You can tune the
affinity-mode
in theaffinity-test/deployment.yaml
file to what you want. I mostly tested betweenpersistent
andpersistent-drainable
.Below are the general test cases I ran.
1.
persistent-drainable
sticks sessions while drainingpersistent-drainable
is appliedphp-hostname-echo-747df85784-dvpc8
)kubectl delete pod <your-pod> -n affinity-test
2.
persistent-drainable
avoids draining pods for new sessionspersistent-drainable
is appliedcurl localhost
in your shellkubectl delete pod <pod name> -n affinity-test
curl localhost
(from a new shell) many times while the pod is deleting. Confirm it's never the one you deletedNote
It's possible that you can run
curl
before the ingress controller picks up the update, so the first one or two requests may still get routed to the old pod.3.
persistent
avoids draining pods for new sessionspesrsistent
is applied4.
persistent
does not stick sessions while drainingpersistent
is appliedphp-hostname-echo-747df85784-dvpc8
)kubectl delete pod <your-pod> -n affinity-test
5.
persistent-drainable
hits thedefault-backend
after deleting a deploymentpersistent-drainable
is appliedkubectl delete deployment php-hostname-echo -n affinity-test
curl localhost
hits the default backend (the http response will tell you)Note
While the pods are going down, new upstreams do get traffic sent to the
Serving
upstreams. This seems acceptable. It seems like a tragic state that the application is in when all of the pods are terminating, and it seems somewhat up in the air with what to do with new traffic. Since the pods are stillServing
, it seems ok to continue sending new traffic there until inevitably there are no more pods to serve the new requests.6.
persistent
hits thedefault-backend
immediately after deleting a deploymentpersistent
is appliedkubectl delete deployment php-hostname-echo -n affinity-test
curl localhost
hits the default backend (the http response will tell you)7.
persistent-drainable
warns when all upstreams for a service are drainingMake sure
persistent-drainable
is appliedDelete the deployment with
kubectl delete deployment php-hostname-echo -n affinity-test
Check the logs for the
ingress-nginx-controller
podConfirm you see an entry resembling the following
Checklist: