konnectivity-agent - improve readiness probe

[mihivagyok]

What would you like to be added:

Readiness probe returns OK but it does not consider the actual state/readiness:
https://github.com/kubernetes-sigs/apiserver-network-proxy/blob/master/cmd/agent/app/server.go#L107

This could shall be improved by reporting based on the fact that the agent is connected or not to the server.

• "Ready" - agent is connected successfully to the server
• "Not Ready" - agent fails to connect to the server

Why is this needed:

We see cases when agents are not able to connect to the konnectivity server due network issues (e.g. firewall, ACL, etc), but agents are still reporting "ready" status and it gives the false idea that everything is fine with the agent. With this enhancement, we could see easier problems like that.

[jkh52]

This was discussed at #485 and in a community meeting about a month ago.

Some requirements questions:

If, for example, konnectivity-server is used with flag --proxy-strategies=destHost (note that "default" strategy is omitted) then this agent readiness looks very meaningful (for the associated node running the agent). But for other configurations where proxy strategies includes default (which is more common, and the default), this proposal around agent readiness seems less meaningful - a user should care more about konnectivity-server readiness (which already exposes agent presence signal).

It's also important to consider the multiple control planes case. Should the proposed agent readiness be true only when the agent is fully connected to all konnectivity-server (i.e. "cs.ClientsCount() >= cs.serverCount") as designed, or only "at least 1"?

• @ipochi , @cheftako

[mihivagyok]

Hi @jkh52 !

Thank you very much for your feedback.
I think any improvement is useful at this time. So I would vote that the konnectivity-agent shall report notReady if not connected at all.

Can you elaborate a bit how konnectivity-server exposes the agent presence signal?

Thank you!
Adam

[jkh52]

Can you elaborate a bit how konnectivity-server exposes the agent presence signal?

Yes - see konnectivity-server readiness logic

[jkh52]

Despite the ambiguity around the open questions, overall I am +1 on this idea, and the simpler logic (at least 1 agent:server connection) has the pros: 1. simpler 2. similar to konnectivity-server readiness 3. still meets the basic given use case (signal that a user may need to debug firewall rules, etc).

@cheftako WDYT overall, and if we proceed do you think this proposal requires a new agent flag (and keep old behavior as default)?

[ipochi]

@jkh52 @mihivagyok @cheftako

What do you think about this ?
#485

[mihivagyok]

Hi @ipochi !

I would be happy to try it out. I let you know about my experiences.

Thank you!
Adam

[jkh52]

/assign @ipochi

[ipochi]

The open PR #485 Is good to be merged, if someone other than me can test it out and evaluate whether its meets the requirements discussed above.

[mihivagyok]

@ipochi @jkh52
Thank you very much! Today I was able to test it and it works well for my scenarios.

If you want, I could share some details tomorrow about the test.
Thank you!
Adam

[ipochi]

Closing as this is addressed via feature in #485

[ipochi]

/close

[k8s-ci-robot]

@ipochi: Closing this issue.

In response to this:

/close

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.