Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refine NeedsHealthCheck logic for ESIPP #44578

Merged
merged 2 commits into from
Apr 18, 2017
Merged

Refine NeedsHealthCheck logic for ESIPP #44578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb53dbb
Move healthcheck util functions to util.go
MrHohn Apr 17, 2017
ae93b0d
Refine NeedsHealthCheck logic
MrHohn Apr 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 0 additions & 49 deletions pkg/api/service/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down Expand Up @@ -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
}
58 changes: 58 additions & 0 deletions pkg/api/service/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -66,3 +69,58 @@ func GetLoadBalancerSourceRanges(service *api.Service) (netsets.IPNet, error) {
}
return ipnets, nil
}

// 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} {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we drop the alpha annotations now? I guess it doesn't hurt to leave them...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we should drop the alpha. The plan is to drop them when we add corresponding api fields, in that way we could re-use some existing logic :)

if l, ok := service.Annotations[annotation]; ok {
switch l {
case AnnotationValueExternalTrafficLocal:
return true
case AnnotationValueExternalTrafficGlobal:
return false
default:
glog.Errorf("Invalid value for annotation %v: %v", annotation, l)
}
}
}
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) {
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
}
61 changes: 0 additions & 61 deletions pkg/api/v1/service/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down Expand Up @@ -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
}
70 changes: 70 additions & 0 deletions pkg/api/v1/service/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -66,3 +69,70 @@ func GetLoadBalancerSourceRanges(service *v1.Service) (netsets.IPNet, error) {
}
return ipnets, nil
}

// 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 {
switch l {
case AnnotationValueExternalTrafficLocal:
return true
case AnnotationValueExternalTrafficGlobal:
return false
default:
glog.Errorf("Invalid value for annotation %v: %v", annotation, l)
}
}
}
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) {
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
}
3 changes: 1 addition & 2 deletions pkg/proxy/iptables/proxier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
7 changes: 1 addition & 6 deletions pkg/registry/core/service/rest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down