From cb53dbbfae6a1257defcf5a4d1946d44432e672a Mon Sep 17 00:00:00 2001 From: Zihong Zheng Date: Mon, 17 Apr 2017 13:13:59 -0700 Subject: [PATCH 1/2] Move healthcheck util functions to util.go --- pkg/api/service/annotations.go | 49 ------------------------- pkg/api/service/util.go | 45 +++++++++++++++++++++++ pkg/api/v1/service/annotations.go | 61 ------------------------------- pkg/api/v1/service/util.go | 57 +++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 110 deletions(-) diff --git a/pkg/api/service/annotations.go b/pkg/api/service/annotations.go index 42fd68b7593..d6ccaeba32a 100644 --- a/pkg/api/service/annotations.go +++ b/pkg/api/service/annotations.go @@ -16,13 +16,6 @@ limitations under the License. package service -import ( - "strconv" - - "github.com/golang/glog" - "k8s.io/kubernetes/pkg/api" -) - const ( // AnnotationLoadBalancerSourceRangesKey is the key of the annotation on a service to set allowed ingress ranges on their LoadBalancers // @@ -55,45 +48,3 @@ const ( // BetaAnnotationExternalTraffic is the beta version of AlphaAnnotationExternalTraffic. BetaAnnotationExternalTraffic = "service.beta.kubernetes.io/external-traffic" ) - -// NeedsHealthCheck Check service for health check annotations -func NeedsHealthCheck(service *api.Service) bool { - // First check the alpha annotation and then the beta. This is so existing - // Services continue to work till the user decides to transition to beta. - // If they transition to beta, there's no way to go back to alpha without - // rolling back the cluster. - for _, annotation := range []string{AlphaAnnotationExternalTraffic, BetaAnnotationExternalTraffic} { - if l, ok := service.Annotations[annotation]; ok { - if l == AnnotationValueExternalTrafficLocal { - return true - } else if l == AnnotationValueExternalTrafficGlobal { - return false - } else { - glog.Errorf("Invalid value for annotation %v: %v", annotation, l) - } - } - } - return false -} - -// GetServiceHealthCheckNodePort Return health check node port annotation for service, if one exists -func GetServiceHealthCheckNodePort(service *api.Service) int32 { - if !NeedsHealthCheck(service) { - return 0 - } - // First check the alpha annotation and then the beta. This is so existing - // Services continue to work till the user decides to transition to beta. - // If they transition to beta, there's no way to go back to alpha without - // rolling back the cluster. - for _, annotation := range []string{AlphaAnnotationHealthCheckNodePort, BetaAnnotationHealthCheckNodePort} { - if l, ok := service.Annotations[annotation]; ok { - p, err := strconv.Atoi(l) - if err != nil { - glog.Errorf("Failed to parse annotation %v: %v", annotation, err) - continue - } - return int32(p) - } - } - return 0 -} diff --git a/pkg/api/service/util.go b/pkg/api/service/util.go index 6f0e14e2bde..0432379cd3f 100644 --- a/pkg/api/service/util.go +++ b/pkg/api/service/util.go @@ -18,10 +18,13 @@ package service import ( "fmt" + "strconv" "strings" "k8s.io/kubernetes/pkg/api" netsets "k8s.io/kubernetes/pkg/util/net/sets" + + "github.com/golang/glog" ) const ( @@ -66,3 +69,45 @@ func GetLoadBalancerSourceRanges(service *api.Service) (netsets.IPNet, error) { } return ipnets, nil } + +// NeedsHealthCheck Check service for health check annotations +func NeedsHealthCheck(service *api.Service) bool { + // First check the alpha annotation and then the beta. This is so existing + // Services continue to work till the user decides to transition to beta. + // If they transition to beta, there's no way to go back to alpha without + // rolling back the cluster. + for _, annotation := range []string{AlphaAnnotationExternalTraffic, BetaAnnotationExternalTraffic} { + if l, ok := service.Annotations[annotation]; ok { + if l == AnnotationValueExternalTrafficLocal { + return true + } else if l == AnnotationValueExternalTrafficGlobal { + return false + } else { + glog.Errorf("Invalid value for annotation %v: %v", annotation, l) + } + } + } + return false +} + +// GetServiceHealthCheckNodePort Return health check node port annotation for service, if one exists +func GetServiceHealthCheckNodePort(service *api.Service) int32 { + if !NeedsHealthCheck(service) { + return 0 + } + // First check the alpha annotation and then the beta. This is so existing + // Services continue to work till the user decides to transition to beta. + // If they transition to beta, there's no way to go back to alpha without + // rolling back the cluster. + for _, annotation := range []string{AlphaAnnotationHealthCheckNodePort, BetaAnnotationHealthCheckNodePort} { + if l, ok := service.Annotations[annotation]; ok { + p, err := strconv.Atoi(l) + if err != nil { + glog.Errorf("Failed to parse annotation %v: %v", annotation, err) + continue + } + return int32(p) + } + } + return 0 +} diff --git a/pkg/api/v1/service/annotations.go b/pkg/api/v1/service/annotations.go index 141e572124a..d6ccaeba32a 100644 --- a/pkg/api/v1/service/annotations.go +++ b/pkg/api/v1/service/annotations.go @@ -16,13 +16,6 @@ limitations under the License. package service -import ( - "strconv" - - "github.com/golang/glog" - "k8s.io/kubernetes/pkg/api/v1" -) - const ( // AnnotationLoadBalancerSourceRangesKey is the key of the annotation on a service to set allowed ingress ranges on their LoadBalancers // @@ -55,57 +48,3 @@ const ( // BetaAnnotationExternalTraffic is the beta version of AlphaAnnotationExternalTraffic. BetaAnnotationExternalTraffic = "service.beta.kubernetes.io/external-traffic" ) - -// NeedsHealthCheck Check service for health check annotations -func NeedsHealthCheck(service *v1.Service) bool { - // First check the alpha annotation and then the beta. This is so existing - // Services continue to work till the user decides to transition to beta. - // If they transition to beta, there's no way to go back to alpha without - // rolling back the cluster. - for _, annotation := range []string{AlphaAnnotationExternalTraffic, BetaAnnotationExternalTraffic} { - if l, ok := service.Annotations[annotation]; ok { - if l == AnnotationValueExternalTrafficLocal { - return true - } else if l == AnnotationValueExternalTrafficGlobal { - return false - } else { - glog.Errorf("Invalid value for annotation %v: %v", annotation, l) - } - } - } - return false -} - -// GetServiceHealthCheckNodePort Return health check node port annotation for service, if one exists -func GetServiceHealthCheckNodePort(service *v1.Service) int32 { - if !NeedsHealthCheck(service) { - return 0 - } - // First check the alpha annotation and then the beta. This is so existing - // Services continue to work till the user decides to transition to beta. - // If they transition to beta, there's no way to go back to alpha without - // rolling back the cluster. - for _, annotation := range []string{AlphaAnnotationHealthCheckNodePort, BetaAnnotationHealthCheckNodePort} { - if l, ok := service.Annotations[annotation]; ok { - p, err := strconv.Atoi(l) - if err != nil { - glog.Errorf("Failed to parse annotation %v: %v", annotation, err) - continue - } - return int32(p) - } - } - return 0 -} - -// GetServiceHealthCheckPathPort Return the path and nodePort programmed into the Cloud LB Health Check -func GetServiceHealthCheckPathPort(service *v1.Service) (string, int32) { - if !NeedsHealthCheck(service) { - return "", 0 - } - port := GetServiceHealthCheckNodePort(service) - if port == 0 { - return "", 0 - } - return "/healthz", port -} diff --git a/pkg/api/v1/service/util.go b/pkg/api/v1/service/util.go index 3c00a495245..a8c2dbe09a2 100644 --- a/pkg/api/v1/service/util.go +++ b/pkg/api/v1/service/util.go @@ -18,10 +18,13 @@ package service import ( "fmt" + "strconv" "strings" "k8s.io/kubernetes/pkg/api/v1" netsets "k8s.io/kubernetes/pkg/util/net/sets" + + "github.com/golang/glog" ) const ( @@ -66,3 +69,57 @@ func GetLoadBalancerSourceRanges(service *v1.Service) (netsets.IPNet, error) { } return ipnets, nil } + +// NeedsHealthCheck Check service for health check annotations +func NeedsHealthCheck(service *v1.Service) bool { + // First check the alpha annotation and then the beta. This is so existing + // Services continue to work till the user decides to transition to beta. + // If they transition to beta, there's no way to go back to alpha without + // rolling back the cluster. + for _, annotation := range []string{AlphaAnnotationExternalTraffic, BetaAnnotationExternalTraffic} { + if l, ok := service.Annotations[annotation]; ok { + if l == AnnotationValueExternalTrafficLocal { + return true + } else if l == AnnotationValueExternalTrafficGlobal { + return false + } else { + glog.Errorf("Invalid value for annotation %v: %v", annotation, l) + } + } + } + return false +} + +// GetServiceHealthCheckNodePort Return health check node port annotation for service, if one exists +func GetServiceHealthCheckNodePort(service *v1.Service) int32 { + if !NeedsHealthCheck(service) { + return 0 + } + // First check the alpha annotation and then the beta. This is so existing + // Services continue to work till the user decides to transition to beta. + // If they transition to beta, there's no way to go back to alpha without + // rolling back the cluster. + for _, annotation := range []string{AlphaAnnotationHealthCheckNodePort, BetaAnnotationHealthCheckNodePort} { + if l, ok := service.Annotations[annotation]; ok { + p, err := strconv.Atoi(l) + if err != nil { + glog.Errorf("Failed to parse annotation %v: %v", annotation, err) + continue + } + return int32(p) + } + } + return 0 +} + +// GetServiceHealthCheckPathPort Return the path and nodePort programmed into the Cloud LB Health Check +func GetServiceHealthCheckPathPort(service *v1.Service) (string, int32) { + if !NeedsHealthCheck(service) { + return "", 0 + } + port := GetServiceHealthCheckNodePort(service) + if port == 0 { + return "", 0 + } + return "/healthz", port +} From ae93b0da15a8e4c99282ba0c311c5ae74d74f103 Mon Sep 17 00:00:00 2001 From: Zihong Zheng Date: Mon, 17 Apr 2017 14:26:02 -0700 Subject: [PATCH 2/2] Refine NeedsHealthCheck logic --- pkg/api/service/util.go | 23 ++++++++++++++++++----- pkg/api/v1/service/util.go | 23 ++++++++++++++++++----- pkg/proxy/iptables/proxier.go | 3 +-- pkg/registry/core/service/rest.go | 7 +------ 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/pkg/api/service/util.go b/pkg/api/service/util.go index 0432379cd3f..f8561718172 100644 --- a/pkg/api/service/util.go +++ b/pkg/api/service/util.go @@ -70,19 +70,24 @@ func GetLoadBalancerSourceRanges(service *api.Service) (netsets.IPNet, error) { return ipnets, nil } -// NeedsHealthCheck Check service for health check annotations -func NeedsHealthCheck(service *api.Service) bool { +// RequestsOnlyLocalTraffic checks if service requests OnlyLocal traffic. +func RequestsOnlyLocalTraffic(service *api.Service) bool { + if service.Spec.Type != api.ServiceTypeLoadBalancer && + service.Spec.Type != api.ServiceTypeNodePort { + return false + } // First check the alpha annotation and then the beta. This is so existing // Services continue to work till the user decides to transition to beta. // If they transition to beta, there's no way to go back to alpha without // rolling back the cluster. for _, annotation := range []string{AlphaAnnotationExternalTraffic, BetaAnnotationExternalTraffic} { if l, ok := service.Annotations[annotation]; ok { - if l == AnnotationValueExternalTrafficLocal { + switch l { + case AnnotationValueExternalTrafficLocal: return true - } else if l == AnnotationValueExternalTrafficGlobal { + case AnnotationValueExternalTrafficGlobal: return false - } else { + default: glog.Errorf("Invalid value for annotation %v: %v", annotation, l) } } @@ -90,6 +95,14 @@ func NeedsHealthCheck(service *api.Service) bool { return false } +// NeedsHealthCheck Check if service needs health check. +func NeedsHealthCheck(service *api.Service) bool { + if service.Spec.Type != api.ServiceTypeLoadBalancer { + return false + } + return RequestsOnlyLocalTraffic(service) +} + // GetServiceHealthCheckNodePort Return health check node port annotation for service, if one exists func GetServiceHealthCheckNodePort(service *api.Service) int32 { if !NeedsHealthCheck(service) { diff --git a/pkg/api/v1/service/util.go b/pkg/api/v1/service/util.go index a8c2dbe09a2..338e8f8e0fa 100644 --- a/pkg/api/v1/service/util.go +++ b/pkg/api/v1/service/util.go @@ -70,19 +70,24 @@ func GetLoadBalancerSourceRanges(service *v1.Service) (netsets.IPNet, error) { return ipnets, nil } -// NeedsHealthCheck Check service for health check annotations -func NeedsHealthCheck(service *v1.Service) bool { +// RequestsOnlyLocalTraffic checks if service requests OnlyLocal traffic. +func RequestsOnlyLocalTraffic(service *v1.Service) bool { + if service.Spec.Type != v1.ServiceTypeLoadBalancer && + service.Spec.Type != v1.ServiceTypeNodePort { + return false + } // First check the alpha annotation and then the beta. This is so existing // Services continue to work till the user decides to transition to beta. // If they transition to beta, there's no way to go back to alpha without // rolling back the cluster. for _, annotation := range []string{AlphaAnnotationExternalTraffic, BetaAnnotationExternalTraffic} { if l, ok := service.Annotations[annotation]; ok { - if l == AnnotationValueExternalTrafficLocal { + switch l { + case AnnotationValueExternalTrafficLocal: return true - } else if l == AnnotationValueExternalTrafficGlobal { + case AnnotationValueExternalTrafficGlobal: return false - } else { + default: glog.Errorf("Invalid value for annotation %v: %v", annotation, l) } } @@ -90,6 +95,14 @@ func NeedsHealthCheck(service *v1.Service) bool { return false } +// NeedsHealthCheck Check if service needs health check. +func NeedsHealthCheck(service *v1.Service) bool { + if service.Spec.Type != v1.ServiceTypeLoadBalancer { + return false + } + return RequestsOnlyLocalTraffic(service) +} + // GetServiceHealthCheckNodePort Return health check node port annotation for service, if one exists func GetServiceHealthCheckNodePort(service *v1.Service) int32 { if !NeedsHealthCheck(service) { diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 62d8af048fa..9c6cf16acb1 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -163,8 +163,7 @@ func (e *endpointsInfo) String() string { func newServiceInfo(serviceName proxy.ServicePortName, port *api.ServicePort, service *api.Service) *serviceInfo { onlyNodeLocalEndpoints := false if utilfeature.DefaultFeatureGate.Enabled(features.ExternalTrafficLocalOnly) && - (service.Spec.Type == api.ServiceTypeLoadBalancer || service.Spec.Type == api.ServiceTypeNodePort) && - apiservice.NeedsHealthCheck(service) { + apiservice.RequestsOnlyLocalTraffic(service) { onlyNodeLocalEndpoints = true } info := &serviceInfo{ diff --git a/pkg/registry/core/service/rest.go b/pkg/registry/core/service/rest.go index e4bc92afc74..c8cdba60b83 100644 --- a/pkg/registry/core/service/rest.go +++ b/pkg/registry/core/service/rest.go @@ -567,12 +567,7 @@ func shouldAssignNodePorts(service *api.Service) bool { } func shouldCheckOrAssignHealthCheckNodePort(service *api.Service) bool { - if service.Spec.Type == api.ServiceTypeLoadBalancer { - // True if Service-type == LoadBalancer AND annotation AnnotationExternalTraffic present - return (utilfeature.DefaultFeatureGate.Enabled(features.ExternalTrafficLocalOnly) && apiservice.NeedsHealthCheck(service)) - } - glog.V(4).Infof("Service type: %v does not need health check node port", service.Spec.Type) - return false + return (utilfeature.DefaultFeatureGate.Enabled(features.ExternalTrafficLocalOnly) && apiservice.NeedsHealthCheck(service)) } // Loop through the service ports list, find one with the same port number and