give the kubernetes service client ip session affinity

[mikedanese]

No description provided.

[k8s-github-robot]

Labelling this PR as size/M

[k8s-bot]

GCE e2e build/test passed for commit 127eaf26fa164f264688576d3a120536ffe45248.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[ncdc]

FYI @kubernetes/rh-cluster-infra @kubernetes/rh-platform-management @kubernetes/rh-networking

[k8s-bot]

GCE e2e build/test passed for commit e21ebbc.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[pweil-]

@mikedanese - what's the motivation for this? I don't object to the change, just curious

[mikedanese]

@pweil- we talked about this in the sig HA meeting with @smarterclayton and @lavalamp. The idea is that if we start to run controllers like scheduler and controller-manager onto the cluster in HA configurations, they should talk to the same API server if possible. Because each apiserver serves from a cache (unless the linearize read flag is passed explicitly) and each apiserver cache could be a different degree behind etcd. This could cause bugs in naive clients that don't handle this. We can avoid these bugs by adding clientIP session affinity.

[pweil-]

👍 thanks

[lavalamp]

LGTM. Makes it a bit safer to run multi-apiserver without turning on consistent reads.

[k8s-github-robot]

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

[k8s-bot]

GCE e2e build/test passed for commit e21ebbc.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

Automatic merge from submit-queue

[jsravn]

Is there a way to disable this? We ran into an issue where a single apiserver had an outage, in a multi master setup. Because of this issue change, some of the clients were unable to reconnect because they kept retrying the same apiserver. We enable quorum reads (because we had transient problems w/ it disabled), so don't care about isolating connections. All our client connections to the apiserver are long lived anyways.

[jsravn]

I suppose we'll just apply our own service to override it. But I think this change is wrong in multi master setups (sort of defeats the point of having multi master if your clients can't reconnect to the other apiservers...).

[mikedanese]

What we should fix is the latency of a bad endpoint being remove from the master service. This change is orthogonal. Right now they are not removed. Is having 1/3 of all requests fail for all clients worse then having all requests fail for 1/3 of clients? The controller-manager will be able to continue working in case 1 but not in case 2.

Regardless we should just fix the kubernetes service to remove down endpoints.

[jsravn]

Considering client-go auto retries failed connections, I think it's better to have requests fail 1/3rd of the time rather than always for 1/3rd of clients. In the latter case, there is no way to recover on the client.

[tatsuhiro-t]

I completely agree with @jsravn
We have been suffered from this issue, and a failure of a just single master makes some workers unusable which is really not HA. I think this fix should be reverted until we "fix the kubernetes service to remove down endpoints", or just give us the knob to disable this.

[tatsuhiro-t]

I just found #51698
I hope it fixes the fixing down endpoints, and is going to be available in v1.8 release.

[liggitt]

That is targeted for 1.9. It’s a significant change to add at the tail end of the release.

[tatsuhiro-t]

Thank you for reply. We'll wait for v1.9. For another 3 or 4 month, we have to workaround the endpoints issue. For some reason, we cannot use LB, so probably 1) use --api-server-count=1 and let masters fight, and some more frequent iptables updates, or 2) build our own hyperkube without this change, whichever less painful.

[redbaron]

Just been hit by this :( Worstest thing is that there is no way to switch back to the old way of doing things without recompiling.

Per comments above, this change intent is to help scheduler & controller manager in HA setup, but effect of this change is for every user of 'kubernetes.svc' in the cluster :( That is not to mention, that in every HA setup I've heard of, controller managers run on same nodes as apiservers, so they can use $(NODE_IP) to connect to, practically without loosing any resilience (if local apiserver is down, lock would be released and another controller manager would take over), in fact it is even more reliable as it doesn't depend on kube-proxy being healthy.

[jsravn]

As a workaround you can apply your own service file for the apiserver:

apiVersion: v1
kind: Service
metadata:
  labels:
    component: apiserver
    provider: kubernetes
  name: kubernetes
  namespace: default
spec:
  clusterIP: {{ apiserver_serviceip }}
  ports:
  - name: https
    port: 443
    protocol: TCP
    targetPort: 443
  sessionAffinity: None
  type: ClusterIP

[redbaron]

That is ugly. If service resource is managed by something, going and interfering with it even if it works today is bad. There should be no easy way to paint yourself into a corner. Only thing which saves numerous kubernetes deployment from hitting deadlock is that apiserver-count is 1 by default.

[jsravn]

It's ugly sure, but it works in all versions of k8s until it gets fixed. I don't agree w/ this PR (it flat out broke HA resiliency), but I'm not sure how much luck you'll have reverting it - seems like the plan is to wait on apiserver endpoint updates.

[tatsuhiro-t]

At the moment, our workaround is running our own reconciler controller which watches master Nodes resources, and then synchronizes default/kubernetes Endpoints with them.
Hopefully, we can get rid of it when k8s v1.9 with lease reconciler is released.

[jsravn]

What happens if the reconciler controller itself can't connect to apiserver due to session affinity?

[tatsuhiro-t]

reconciler controller runs on master node, and it can connect to apiserver on the same node without using service ip, so there is no session affinity involved.