[Feature] close downstream if upstream is closed

[xuzhenglun]

HI:

I'm using Caddy as a layer4 load balancer for Kubelet to Kube-APIServer. And I found Kubelet could be hang in some time.

After days of digging, I think I found the reason:
In the scenario of Kubernetes, the Client uses Watch&List to describe resources changing events. This means that kube-apiserver returns all resources and pushes events only if resources changed. however, after kube-apiserver restarted, the upstream of Caddy is closed, but the downstream is still connected. At the same time, the client is still waiting for events to come which is impossible. And the worst thing is the client will never find out this connection should be reset.

Kubernetes fix this issue in 1.20 in kubernetes/client-go#374. but in some case, we can't upgrade to the latest version so quickly.

Here is my proposal:
Maybe we can add an option to the LoadBalancer struct, such as DisconnectDownstreamPolicy. And the value could be Never、OnAnyUpsteamClosed or OnAllUpstreamClosed. the default value is Never which keeps the same behavior as current.

How does this sound to you? And If you are busy, I'm ok to submit a Pull&Request and wait for you to review it. the poc change is working for me. xuzhenglun@f013a43

[mholt]

That's an interesting point, and worth considering. I like the idea. Not a huge fan of "modes" (which this kind of reminds me of) but I'm not sure a better way to do it. I think it'd be fine. I'd be open to reviewing a PR 👍

[RussellLuo]

Not a huge fan of "modes" (which this kind of reminds me of) but I'm not sure a better way to do it.

I think we should close the downstream connection if all upstream connections are closed. Only one question left is that whether we should provide an option like OnAnyUpsteamClosed, which depends on the use case (or idea) behind the design of "multiple upstream connections per downstream connection".

[thehackercat]

Any progress on this issue?

[mholt]

@thehackercat Well, there was a comment 13 hours ago (7 hours before yours). Does that count?

@RussellLuo I think that makes sense. I'm not really sure what people use multiple upstreams for tbh so maybe we can shelve that particular feature until it's requested?

[RussellLuo]

Sounds reasonable, I'll get it done today (the last day before holiday 😃).

[xuzhenglun]

Sounds reasonable, I'll get it done today (the last day before holiday 😃).

In the past months, I'm busy with my job. Sorry for it.

I read the commend above, It seems that we decide to close the downstream connection when upstream is closed without an option config?

Have you been working on it? If not, I can open a PR from xuzhenglun@f013a43

[RussellLuo]

Have you been working on it? If not, I can open a PR from ...

Oh, I have just opened a PR. Sorry, I didn't notice your comments just now 😢

[xuzhenglun]

comments

It doesn't matter. I can see my change is stale and conflicts with the master code.