service: respond to healthchecks on LBips #26728
Merged
Conversation
This file contains hidden or 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
asauber
reviewed
Jul 14, 2023
joamaki
requested changes
Jul 17, 2023
There was a problem hiding this comment.
Few nits, but other than that implementation looks good to me.
I would however like to see a test for this. It should be fairly straightforward to add to
pkg/service/service_test.go. TestHealthCheckNodePort is likely a good starting point.
Remember to restore option.Config.EnableHealthCheckLoadBalancerIP after the test case.
@joamaki I agree that a test would be nice, but its not straightforward for me when it come to mocking |
asauber
reviewed
Jul 17, 2023
|
@joamaki friendly reminder :) |
thorn3r
approved these changes
Jul 25, 2023
joamaki
approved these changes
Jul 25, 2023
|
Great that the PR is approved, could anymody issue a |
|
/test |
This commit introduces the 'EnableHealthCheckLoadBalancerIP', which exposes the healthCheckNodePort on the LoadBalancerIP. The healthcheck server doesn't bind the healthCheckNodePort on the LoadBalancerIP (LBip). Instead, Google Cloud Platform's Load Balancer performs the health checks using the LB IP as the destination IP rather than the node IP. To facilitate this change, 'EnableHealthCheckLoadBalancerIP' adds service specifically for the healthcheck port. The creation and removal of this additional service are coupled to the primary service that requires health checking. Fixes: #26727 Signed-off-by: Nico Berlee <nico.berlee@on2it.net>
|
/test |
asauber
approved these changes
Aug 1, 2023
brb
added a commit
that referenced
this pull request
Jun 3, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - #26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
brb
added a commit
that referenced
this pull request
Jun 3, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - #26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
brb
added a commit
that referenced
this pull request
Jun 3, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - #26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
brb
added a commit
that referenced
this pull request
Jun 13, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - #26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
brb
added a commit
that referenced
this pull request
Jun 16, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - #26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
brb
added a commit
that referenced
this pull request
Jun 16, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - #26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
brb
added a commit
that referenced
this pull request
Jun 16, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - #26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
harsimran-pabla
pushed a commit
to harsimran-pabla/cilium
that referenced
this pull request
Jun 17, 2025
Create a LoadBalancer service for HC only if EnableHealthCheckLoadBalancerIP is set. More ctx about the latter - cilium#26728. Signed-off-by: Martynas Pumputis <martynas@isovalent.com>
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.
This Pull Request introduces the 'EnableHealthCheckLoadBalancerIP' flag, which exposes the healthCheckNodePort on the LoadBalancerIP (LB IP). This addition aims to resolve an issue where clusters hosted on GCP in KubeProxyReplacement mode, using a loadbalancer-service with
externalTrafficPolicy: Local, were triggering failures in GCP health checks. This, in turn, was causing ingress traffic to be distributed among all Kubernetes nodes, which is not compatible with the Local traffic policy.Specifically, GCP's Load Balancer performs the health checks using the LB IP as the destination IP rather than the node IP. While the cloud-provider-gcp sets up these healthchecks based on the services' spec.healthCheckNodePort which is accessible by Cilium on the nodeIP, it doesn't acknowledge the GCP LB healthcheck packets because Cilium's HealthServer binds to local IPs and doesn't have forwarding rules for the LB IPs.
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXXline if the commit addresses a particularGitHub issue.
Fixes: <commit-id>tag, thenplease add the commit author[s] as reviewer[s] to this issue.
Fixes: #26727