Add antreaProxy.disableServiceHealthCheckServer config #6939
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hongliangl
reviewed
Jan 18, 2025
| @@ -411,6 +411,11 @@ antreaProxy: | |||
| # can reply to clients directly, bypassing the ingress Node. | |||
| # A Service's load balancer mode can be overridden by annotating it with `service.antrea.io/load-balancer-mode`. | |||
| defaultLoadBalancerMode: {{ .defaultLoadBalancerMode | quote }} | |||
| # Disables the health check server run by Antrea Proxy, which provides health information about | |||
There was a problem hiding this comment.
Suggested change
| # Disables the health check server run by Antrea Proxy, which provides health information about | |
| # Disable the health check server run by Antrea Proxy, which provides health information about |
There was a problem hiding this comment.
@hongliangl we don't seem to have adopted a unified style for this. As it is, it is consistent with the configuration parameter that comes just before (defaultLoadBalancerMode):
Determines how external traffic is processed when it's load balanced across Nodes by default.
The use of the third-person singular makes sense to me, because it is supposed to mean: disableServiceHealthCheckServer disables the health check server run by Antrea Proxy
| @@ -411,6 +411,11 @@ antreaProxy: | |||
| # can reply to clients directly, bypassing the ingress Node. | |||
| # A Service's load balancer mode can be overridden by annotating it with `service.antrea.io/load-balancer-mode`. | |||
| defaultLoadBalancerMode: {{ .defaultLoadBalancerMode | quote }} | |||
| # Disables the health check server run by Antrea Proxy, which provides health information about | |||
| # Services of type LoadBalancer with externalTrafficPolicy set to Local, when proxyAll is | |||
| # enabled. This avoids race conditions between kube-proxy and Antrea proxy, with both trying to | |||
There was a problem hiding this comment.
Suggested change
| # enabled. This avoids race conditions between kube-proxy and Antrea proxy, with both trying to | |
| # enabled. This avoids race conditions between kube-proxy and Antrea Proxy, with both trying to |
| @@ -153,6 +153,12 @@ antreaProxy: | |||
| # -- Determines how external traffic is processed when it's load balanced | |||
| # across Nodes by default. It must be one of "nat" or "dsr". | |||
| defaultLoadBalancerMode: "nat" | |||
| # -- Disables the health check server run by Antrea Proxy, which provides health | |||
There was a problem hiding this comment.
Suggested change
| # -- Disables the health check server run by Antrea Proxy, which provides health | |
| # -- Disable the health check server run by Antrea Proxy, which provides health |
| @@ -238,6 +238,11 @@ type AntreaProxyConfig struct { | |||
| // can reply to clients directly, bypassing the ingress Node. | |||
| // A Service's load balancer mode can be overridden by annotating it with `service.antrea.io/load-balancer-mode`. | |||
| DefaultLoadBalancerMode string `yaml:"defaultLoadBalancerMode,omitempty"` | |||
| // Disables the health check server run by Antrea Proxy, which provides health information about Services of | |||
There was a problem hiding this comment.
Suggested change
| // Disables the health check server run by Antrea Proxy, which provides health information about Services of | |
| // Disable the health check server run by Antrea Proxy, which provides health information about Services of |
antoninbas
force-pushed
the
add-antreaProxy.disableServiceHealthCheckServer-config
branch
from
863a108 to
a20d029
Compare
tnqn
reviewed
Jan 22, 2025
docs/antrea-proxy.md
Outdated
|
|
||
| These log messages will keep repeating periodically, as kube-proxy handles | ||
| Service updates. While the messages are harmless, they can create a lot of | ||
| necessary noise. You may want to set `antreaProxy.disableServiceHealthCheckServer: true` |
There was a problem hiding this comment.
Suggested change
| necessary noise. You may want to set `antreaProxy.disableServiceHealthCheckServer: true` | |
| unnecessary noise. You may want to set `antreaProxy.disableServiceHealthCheckServer: true` |
| @@ -1385,7 +1387,11 @@ func newProxier( | |||
| for i, address := range nodePortAddresses { | |||
| nodePortAddressesString[i] = address.String() | |||
| } | |||
| serviceHealthServer = healthcheck.NewServiceHealthServer(hostname, nil, nodePortAddressesString) | |||
| if serviceHealthServerDisabled { | |||
There was a problem hiding this comment.
serviceHealthServer is the only consumer of nodePortAddressesString, should the check of serviceHealthServerDisabled be moved to L1385?
Running both kube-proxy and Antrea Proxy with `proxyAll` can trigger some error logs for Services of type LoadBalancer with `externalTrafficPolicy` set to `Local`. For such Services, the proxy is in charge of running a health check server on each Node, in order to report the number of local Endpoints which implement the Service. Because both kube-proxy and Antrea Proxy (with `proxyAll`) will try to run the health check servers on the same ports, one of them will fail to bind to the desired address. In practice, we typically observe that Antrea Proxy tries to bind to the address first (and succeeds), while kube-proxy fails and logs an error message, but there is no guarantee that it will be the case. To avoid kube-proxy error logs, users can now set `antreaProxy.disableServiceHealthCheckServer` to true in the Antrea Agent's config, which will instruct Antrea Proxy to stop running health check servers for LoadBalancer Services. This is not a perfect solution as ideally the component responsible for the proxy implementation (Antrea Proxy in this case) should also be responsible for providing health check information. Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
antoninbas
force-pushed
the
add-antreaProxy.disableServiceHealthCheckServer-config
branch
from
a20d029 to
0702c7e
Compare
|
/test-all |
antoninbas
deleted the
add-antreaProxy.disableServiceHealthCheckServer-config
branch
antoninbas
added
the
action/release-note
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Running both kube-proxy and Antrea Proxy with
proxyAllcan trigger some error logs for Services of type LoadBalancer withexternalTrafficPolicyset toLocal. For such Services, the proxy is in charge of running a health check server on each Node, in order to report the number of local Endpoints which implement the Service. Because both kube-proxy and Antrea Proxy (withproxyAll) will try to run the health check servers on the same ports, one of them will fail to bind to the desired address. In practice, we typically observe that Antrea Proxy tries to bind to the address first (and succeeds), while kube-proxy fails and logs an error message, but there is no guarantee that it will be the case.To avoid kube-proxy error logs, users can now set
antreaProxy.disableServiceHealthCheckServerto true in the Antrea Agent's config, which will instruct Antrea Proxy to stop running health check servers for LoadBalancer Services. This is not a perfect solution as ideally the component responsible for the proxy implementation (Antrea Proxy in this case) should also be responsible for providing health check information.