Only set cookies on paths that enable session affinity

[zrdaley]

What this PR does / why we need it:

Session affinity is set on the a backend, not on the location path. This causes problems when multiple ingress definitions are created for the same host and not all enable session affinity.

Following this PR, session affinity cookies will only be set on paths with ingresses that have session affinity enabled.

Which issue this PR fixes: #3168

[diazjf]

LGTM 👍

[k8s-ci-robot]

@diazjf: Bad category. Please see https://api.thecatapi.com/api/categories/list

In response to this:

/meowvie cookies

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/test-infra repository.

[k8s-ci-robot]

@diazjf: Bad category. Please see https://api.thecatapi.com/api/categories/list

In response to this:

/meowvie psychedelic

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/test-infra repository.

[ElvinEfendi]

@zrdaley can you rebase with latest master so that this gets tested with your other merged session affinity change?

[zrdaley]

I already did @ElvinEfendi

[ElvinEfendi]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ElvinEfendi, zrdaley

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:

• OWNERS [ElvinEfendi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[unicast]

@zrdaley what is expected behaviour if host is not specified and there are multiple Ingresses like that? Currently I'm not getting any cookie and I'm wondering whether it's a bug or a feature?

[zrdaley]

@unicast The current implementation expects a host, if it's not specified it won't set a cookie:
https://github.com/kubernetes/ingress-nginx/blob/master/rootfs/etc/nginx/lua/balancer/sticky.lua#L74-L78

[unicast]

@zrdaley thanks for coming back so soon! This looks like a bug to me because host is an optional directive and all other features are supported there. I've created a bug with more details in there #3959, does it makes sense?

[lfundaro]

Hi @zrdaley ,

The current implementation expects a host, if it's not specified it won't set a cookie

Why is suddenly host a required parameter on the ingress definition. It's been always an optional parameter and by making it required it breaks backwards compatibility. 😕

[nusx]

If the host is specified and it's a wildcard, no session cookie will be set either.

This breaks our current setup.

We have:

• an ingress definition with session affinity enabled, a wildcard hostname and multiple paths routing to individual backends.
• another ingress definition without session affinity, with the same wildcard hostname for serving static content from a single service.

Having to specify each hostname leads to a multiplication of a large number of paths on both ingress definitions. The second ingress (serving static content) can no longer use wildcards - nginx will prefer the more specific hostnames and will no longer route traffic to the webserver.