Add HNS Load Balancer Healthchecks for ExternalTrafficPolicy: Local #99287
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,9 @@ package app | |
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "net" | ||
| goruntime "runtime" | ||
| "strconv" | ||
|
|
||
| // Enable pprof HTTP handlers. | ||
| _ "net/http/pprof" | ||
|
|
@@ -97,8 +99,11 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExi | |
| } | ||
|
|
||
| var healthzServer healthcheck.ProxierHealthUpdater | ||
| var healthzPort int | ||
| if len(config.HealthzBindAddress) > 0 { | ||
| healthzServer = healthcheck.NewProxierHealthServer(config.HealthzBindAddress, 2*config.IPTables.SyncPeriod.Duration, recorder, nodeRef) | ||
| _, port, _ := net.SplitHostPort(config.HealthzBindAddress) | ||
|
There was a problem hiding this comment. this seems is ok since is already validated in https://github.com/kubernetes/kubernetes/blob/a53e2eaeaba064309dceca2dc27f3ac09c6375b0/pkg/proxy/apis/config/validation/validation.go |
||
| healthzPort, _ = strconv.Atoi(port) | ||
| } | ||
|
|
||
| var proxier proxy.Provider | ||
|
|
@@ -120,6 +125,7 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExi | |
| recorder, | ||
| healthzServer, | ||
| config.Winkernel, | ||
| healthzPort, | ||
| ) | ||
| } else { | ||
|
|
||
|
|
@@ -134,6 +140,7 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExi | |
| recorder, | ||
| healthzServer, | ||
| config.Winkernel, | ||
| healthzPort, | ||
| ) | ||
|
|
||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,8 +87,9 @@ type externalIPInfo struct { | |
| } | ||
|
|
||
| type loadBalancerIngressInfo struct { | ||
| ip string | ||
| hnsID string | ||
| healthCheckHnsID string | ||
| } | ||
|
|
||
| type loadBalancerInfo struct { | ||
|
|
@@ -548,6 +549,10 @@ type Proxier struct { | |
| hostMac string | ||
| isDSR bool | ||
| supportedFeatures hcn.SupportedFeatures | ||
| healthzPort int | ||
|
|
||
| forwardHealthCheckVip bool | ||
| rootHnsEndpointName string | ||
| } | ||
|
|
||
| type localPort struct { | ||
|
|
@@ -593,6 +598,7 @@ func NewProxier( | |
| recorder events.EventRecorder, | ||
| healthzServer healthcheck.ProxierHealthUpdater, | ||
| config config.KubeProxyWinkernelConfiguration, | ||
| healthzPort int, | ||
| ) (*Proxier, error) { | ||
| masqueradeValue := 1 << uint(masqueradeBit) | ||
| masqueradeMark := fmt.Sprintf("%#08x/%#08x", masqueradeValue, masqueradeValue) | ||
|
|
@@ -684,24 +690,27 @@ func NewProxier( | |
|
|
||
| isIPv6 := netutils.IsIPv6(nodeIP) | ||
| proxier := &Proxier{ | ||
| endPointsRefCount: make(endPointsReferenceCountMap), | ||
| serviceMap: make(proxy.ServiceMap), | ||
| endpointsMap: make(proxy.EndpointsMap), | ||
| masqueradeAll: masqueradeAll, | ||
| masqueradeMark: masqueradeMark, | ||
| clusterCIDR: clusterCIDR, | ||
| hostname: hostname, | ||
| nodeIP: nodeIP, | ||
| recorder: recorder, | ||
| serviceHealthServer: serviceHealthServer, | ||
| healthzServer: healthzServer, | ||
| hns: hns, | ||
| network: *hnsNetworkInfo, | ||
| sourceVip: sourceVip, | ||
| hostMac: hostMac, | ||
| isDSR: isDSR, | ||
| supportedFeatures: supportedFeatures, | ||
| isIPv6Mode: isIPv6, | ||
| healthzPort: healthzPort, | ||
| rootHnsEndpointName: config.RootHnsEndpointName, | ||
| forwardHealthCheckVip: config.ForwardHealthCheckVip, | ||
| } | ||
|
|
||
| ipFamily := v1.IPv4Protocol | ||
|
|
@@ -730,18 +739,19 @@ func NewDualStackProxier( | |
| recorder events.EventRecorder, | ||
| healthzServer healthcheck.ProxierHealthUpdater, | ||
| config config.KubeProxyWinkernelConfiguration, | ||
| healthzPort int, | ||
| ) (proxy.Provider, error) { | ||
|
|
||
| // Create an ipv4 instance of the single-stack proxier | ||
| ipv4Proxier, err := NewProxier(syncPeriod, minSyncPeriod, masqueradeAll, masqueradeBit, | ||
| clusterCIDR, hostname, nodeIP[0], recorder, healthzServer, config, healthzPort) | ||
|
|
||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to create ipv4 proxier: %v, hostname: %s, clusterCIDR : %s, nodeIP:%v", err, hostname, clusterCIDR, nodeIP[0]) | ||
| } | ||
|
|
||
| ipv6Proxier, err := NewProxier(syncPeriod, minSyncPeriod, masqueradeAll, masqueradeBit, | ||
| clusterCIDR, hostname, nodeIP[1], recorder, healthzServer, config, healthzPort) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to create ipv6 proxier: %v, hostname: %s, clusterCIDR : %s, nodeIP:%v", err, hostname, clusterCIDR, nodeIP[1]) | ||
| } | ||
|
|
@@ -796,6 +806,10 @@ func (svcInfo *serviceInfo) deleteAllHnsLoadBalancerPolicy() { | |
| for _, lbIngressIP := range svcInfo.loadBalancerIngressIPs { | ||
| hns.deleteLoadBalancer(lbIngressIP.hnsID) | ||
| lbIngressIP.hnsID = "" | ||
| if lbIngressIP.healthCheckHnsID != "" { | ||
| hns.deleteLoadBalancer(lbIngressIP.healthCheckHnsID) | ||
| lbIngressIP.healthCheckHnsID = "" | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -988,6 +1002,11 @@ func (proxier *Proxier) syncProxyRules() { | |
| hnsNetworkName := proxier.network.name | ||
| hns := proxier.hns | ||
|
|
||
| var gatewayHnsendpoint *endpointsInfo | ||
| if proxier.forwardHealthCheckVip { | ||
| gatewayHnsendpoint, _ = hns.getEndpointByName(proxier.rootHnsEndpointName) | ||
| } | ||
|
|
||
| prevNetworkID := proxier.network.id | ||
| updatedNetwork, err := hns.getNetworkByName(hnsNetworkName) | ||
| if updatedNetwork == nil || updatedNetwork.id != prevNetworkID || isNetworkNotFoundError(err) { | ||
|
|
@@ -1319,7 +1338,30 @@ func (proxier *Proxier) syncProxyRules() { | |
| } else { | ||
| klog.V(3).InfoS("Skipped creating Hns LoadBalancer for loadBalancer Ingress resources", "lbIngressIP", lbIngressIP) | ||
| } | ||
| lbIngressIP.hnsID = hnsLoadBalancer.hnsID | ||
| klog.V(3).InfoS("Hns LoadBalancer resource created for loadBalancer Ingress resources", "lbIngressIP", lbIngressIP) | ||
|
|
||
| if proxier.forwardHealthCheckVip && gatewayHnsendpoint != nil { | ||
| nodeport := proxier.healthzPort | ||
|
There was a problem hiding this comment. but this is not correct, this is about the healthz of the kube-proxy, not about if the service has local endpoints ... but to be honest I don't fully understand the problem, what is the TL; DR, of why the NewServiceHealthServer doesn't work in windows? that should fix the problem, no? There was a problem hiding this comment. The problem is it's not the node address. The google load balancer works this way: This works right now in kubeproxy's winkernel mode, because the rules are already in place. What doesn't work is the health check for normal service: and for They are dropped because the destination is not among any node IPs. This basically adds hns rules to change request to node IP. There was a problem hiding this comment. yeah, but why it uses the kube-proxy healthz port , it should use the healthzcheck nodeport, no? There was a problem hiding this comment. the part I'm lost is about when it has to use healthz port, but I'm not familiar with the internals of GCE LBs, maybe we should get a reviewer more familiar with this 😅 There was a problem hiding this comment.
kubernetes/cmd/kube-proxy/app/server.go Lines 212 to 220 in 1367cca kubernetes/cmd/kube-proxy/app/server.go Line 226 in 1367cca kubernetes/pkg/cluster/ports/ports.go Line 36 in 1bd0077 this is basically the healthzcheck nodeport. For
I understand it's really not straight-forward.. I'll try to get someone from GKE service team to review as well :) @robscott @bowei can you guys help take a look? There was a problem hiding this comment. cc @freehan can you take a look? There was a problem hiding this comment. Just catching up on this thread. I think this makes sense. Although we don't need to do this in other proxier implementations, it makes sense that we'd need to this lb_vip -> node ip translation here to get health checks working. I'm less familiar with how the primary/default health check needs to work here. Regardless of OS, that seems to be handled exclusively at a higher level, with no extra logic added in either iptables or winkernel proxier implementations: I'm not familiar enough with how this is setup to know what's necessary, but is it possible that this works without any changes for the default health check? The original bug as reported by the PR seems to indicate the problem is only with There was a problem hiding this comment. at higher level, |
||
| if svcInfo.HealthCheckNodePort() != 0 { | ||
| nodeport = svcInfo.HealthCheckNodePort() | ||
| } | ||
| hnsHealthCheckLoadBalancer, err := hns.getLoadBalancer( | ||
| []endpointsInfo{*gatewayHnsendpoint}, | ||
| loadBalancerFlags{isDSR: false, useMUX: svcInfo.preserveDIP, preserveDIP: svcInfo.preserveDIP}, | ||
| sourceVip, | ||
| lbIngressIP.ip, | ||
| Enum(svcInfo.Protocol()), | ||
| uint16(nodeport), | ||
| uint16(nodeport), | ||
| ) | ||
| if err != nil { | ||
| klog.ErrorS(err, "Policy creation failed") | ||
| continue | ||
| } | ||
| lbIngressIP.healthCheckHnsID = hnsHealthCheckLoadBalancer.hnsID | ||
| klog.V(3).InfoS("Hns Health Check LoadBalancer resource created for loadBalancer Ingress resources", "ip", lbIngressIP) | ||
| } | ||
| } | ||
| svcInfo.policyApplied = true | ||
| klog.V(2).InfoS("Policy successfully applied for service", "serviceInfo", svcInfo) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this passed in as a configuration to Windows Kube-proxy similar to the healthzPort?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about this one. This is not configurable in windows startup scripts. Making it configurable here won't be useful I think.