diff --git a/pkg/api/annotation_key_constants.go b/pkg/api/annotation_key_constants.go index bd7f21d64f4..0d41438191f 100644 --- a/pkg/api/annotation_key_constants.go +++ b/pkg/api/annotation_key_constants.go @@ -89,20 +89,4 @@ const ( // // Not all cloud providers support this annotation, though AWS & GCE do. AnnotationLoadBalancerSourceRangesKey = "service.beta.kubernetes.io/load-balancer-source-ranges" - - // AnnotationValueExternalTrafficLocal Value of annotation to specify local endpoints behavior. - AnnotationValueExternalTrafficLocal = "OnlyLocal" - // AnnotationValueExternalTrafficGlobal Value of annotation to specify global (legacy) behavior. - AnnotationValueExternalTrafficGlobal = "Global" - - // TODO: The beta annotations have been deprecated, remove them when we release k8s 1.8. - - // BetaAnnotationHealthCheckNodePort Annotation specifying the healthcheck nodePort for the service. - // If not specified, annotation is created by the service api backend with the allocated nodePort. - // Will use user-specified nodePort value if specified by the client. - BetaAnnotationHealthCheckNodePort = "service.beta.kubernetes.io/healthcheck-nodeport" - - // BetaAnnotationExternalTraffic An annotation that denotes if this Service desires to route - // external traffic to local endpoints only. This preserves Source IP and avoids a second hop. - BetaAnnotationExternalTraffic = "service.beta.kubernetes.io/external-traffic" ) diff --git a/pkg/api/service/BUILD b/pkg/api/service/BUILD index a514e373811..2fa3a6e54f3 100644 --- a/pkg/api/service/BUILD +++ b/pkg/api/service/BUILD @@ -15,7 +15,6 @@ go_library( deps = [ "//pkg/api:go_default_library", "//pkg/util/net/sets:go_default_library", - "//vendor/github.com/golang/glog:go_default_library", ], ) @@ -27,8 +26,6 @@ go_test( deps = [ "//pkg/api:go_default_library", "//pkg/util/net/sets:go_default_library", - "//vendor/github.com/davecgh/go-spew/spew:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", ], ) diff --git a/pkg/api/service/util.go b/pkg/api/service/util.go index 50041f77e2e..242aab77f1f 100644 --- a/pkg/api/service/util.go +++ b/pkg/api/service/util.go @@ -18,13 +18,10 @@ package service import ( "fmt" - "strconv" "strings" "k8s.io/kubernetes/pkg/api" netsets "k8s.io/kubernetes/pkg/util/net/sets" - - "github.com/golang/glog" ) const ( @@ -77,72 +74,13 @@ func RequestsOnlyLocalTraffic(service *api.Service) bool { return false } - // First check the beta annotation and then the first class field. This is so that - // existing Services continue to work till the user decides to transition to the - // first class field. - if l, ok := service.Annotations[api.BetaAnnotationExternalTraffic]; ok { - switch l { - case api.AnnotationValueExternalTrafficLocal: - return true - case api.AnnotationValueExternalTrafficGlobal: - return false - default: - glog.Errorf("Invalid value for annotation %v: %v", api.BetaAnnotationExternalTraffic, l) - return false - } - } return service.Spec.ExternalTrafficPolicy == api.ServiceExternalTrafficPolicyTypeLocal } -// NeedsHealthCheck Check if service needs health check. +// NeedsHealthCheck checks 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 for service, if one exists -func GetServiceHealthCheckNodePort(service *api.Service) int32 { - // First check the beta annotation and then the first class field. This is so that - // existing Services continue to work till the user decides to transition to the - // first class field. - if l, ok := service.Annotations[api.BetaAnnotationHealthCheckNodePort]; ok { - p, err := strconv.Atoi(l) - if err != nil { - glog.Errorf("Failed to parse annotation %v: %v", api.BetaAnnotationHealthCheckNodePort, err) - return 0 - } - return int32(p) - } - return service.Spec.HealthCheckNodePort -} - -// ClearExternalTrafficPolicy resets the ExternalTrafficPolicy field. -func ClearExternalTrafficPolicy(service *api.Service) { - // First check the beta annotation and then the first class field. This is so that - // existing Services continue to work till the user decides to transition to the - // first class field. - if _, ok := service.Annotations[api.BetaAnnotationExternalTraffic]; ok { - delete(service.Annotations, api.BetaAnnotationExternalTraffic) - return - } - service.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyType("") -} - -// SetServiceHealthCheckNodePort sets the given health check node port on service. -// It does not check whether this service needs healthCheckNodePort. -func SetServiceHealthCheckNodePort(service *api.Service, hcNodePort int32) { - // First check the beta annotation and then the first class field. This is so that - // existing Services continue to work till the user decides to transition to the - // first class field. - if _, ok := service.Annotations[api.BetaAnnotationExternalTraffic]; ok { - if hcNodePort == 0 { - delete(service.Annotations, api.BetaAnnotationHealthCheckNodePort) - } else { - service.Annotations[api.BetaAnnotationHealthCheckNodePort] = fmt.Sprintf("%d", hcNodePort) - } - return - } - service.Spec.HealthCheckNodePort = hcNodePort -} diff --git a/pkg/api/service/util_test.go b/pkg/api/service/util_test.go index 6c87173ed27..c3eb0d1fb9b 100644 --- a/pkg/api/service/util_test.go +++ b/pkg/api/service/util_test.go @@ -17,16 +17,11 @@ limitations under the License. package service import ( + "strings" "testing" - "fmt" - "strings" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/kubernetes/pkg/api" netsets "k8s.io/kubernetes/pkg/util/net/sets" - - "github.com/davecgh/go-spew/spew" ) func TestGetLoadBalancerSourceRanges(t *testing.T) { @@ -218,200 +213,4 @@ func TestNeedsHealthCheck(t *testing.T) { ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeLocal, }, }) - - checkNeedsHealthCheck(false, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: "invalid", - }, - }, - }) - checkNeedsHealthCheck(false, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficGlobal, - }, - }, - }) - checkNeedsHealthCheck(true, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - }, - }, - }) -} - -func TestGetServiceHealthCheckNodePort(t *testing.T) { - checkGetServiceHealthCheckNodePort := func(healthCheckNodePort int32, service *api.Service) { - res := GetServiceHealthCheckNodePort(service) - if res != healthCheckNodePort { - t.Errorf("Expected health check node port = %v, got %v", - healthCheckNodePort, res) - } - } - - checkGetServiceHealthCheckNodePort(0, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeClusterIP, - }, - }) - checkGetServiceHealthCheckNodePort(0, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeNodePort, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, - }, - }) - checkGetServiceHealthCheckNodePort(0, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, - }, - }) - checkGetServiceHealthCheckNodePort(34567, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeLocal, - HealthCheckNodePort: int32(34567), - }, - }) - checkGetServiceHealthCheckNodePort(34567, &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - api.BetaAnnotationHealthCheckNodePort: "34567", - }, - }, - }) -} - -func TestClearExternalTrafficPolicy(t *testing.T) { - testCases := []struct { - inputService *api.Service - }{ - // First class fields cases. - { - &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeClusterIP, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, - }, - }, - }, - // Beta annotations cases. - { - &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeClusterIP, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - }, - }, - }, - }, - } - - for i, tc := range testCases { - ClearExternalTrafficPolicy(tc.inputService) - if _, ok := tc.inputService.Annotations[api.BetaAnnotationExternalTraffic]; ok || - tc.inputService.Spec.ExternalTrafficPolicy != "" { - t.Errorf("%v: failed to clear ExternalTrafficPolicy", i) - spew.Dump(tc) - } - } -} - -func TestSetServiceHealthCheckNodePort(t *testing.T) { - testCases := []struct { - inputService *api.Service - hcNodePort int32 - beta bool - }{ - // First class fields cases. - { - &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeClusterIP, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, - }, - }, - 30012, - false, - }, - { - &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeClusterIP, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, - }, - }, - 0, - false, - }, - // Beta annotations cases. - { - &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeClusterIP, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficGlobal, - }, - }, - }, - 30012, - true, - }, - { - &api.Service{ - Spec: api.ServiceSpec{ - Type: api.ServiceTypeClusterIP, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficGlobal, - }, - }, - }, - 0, - true, - }, - } - - for i, tc := range testCases { - SetServiceHealthCheckNodePort(tc.inputService, tc.hcNodePort) - if !tc.beta { - if tc.inputService.Spec.HealthCheckNodePort != tc.hcNodePort { - t.Errorf("%v: got HealthCheckNodePort %v, want %v", i, tc.inputService.Spec.HealthCheckNodePort, tc.hcNodePort) - } - } else { - l, ok := tc.inputService.Annotations[api.BetaAnnotationHealthCheckNodePort] - if tc.hcNodePort == 0 { - if ok { - t.Errorf("%v: HealthCheckNodePort set, want it to be cleared", i) - } - } else { - if !ok { - t.Errorf("%v: HealthCheckNodePort unset, want %v", i, tc.hcNodePort) - } else if l != fmt.Sprintf("%v", tc.hcNodePort) { - t.Errorf("%v: got HealthCheckNodePort %v, want %v", i, l, tc.hcNodePort) - } - } - } - } } diff --git a/pkg/api/v1/defaults.go b/pkg/api/v1/defaults.go index e4a1600dbf4..70c1d709efb 100644 --- a/pkg/api/v1/defaults.go +++ b/pkg/api/v1/defaults.go @@ -115,10 +115,7 @@ func SetDefaults_Service(obj *v1.Service) { } // Defaults ExternalTrafficPolicy field for NodePort / LoadBalancer service // to Global for consistency. - if _, ok := obj.Annotations[v1.BetaAnnotationExternalTraffic]; ok { - // Don't default this field if beta annotation exists. - return - } else if (obj.Spec.Type == v1.ServiceTypeNodePort || + if (obj.Spec.Type == v1.ServiceTypeNodePort || obj.Spec.Type == v1.ServiceTypeLoadBalancer) && obj.Spec.ExternalTrafficPolicy == "" { obj.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeCluster diff --git a/pkg/api/v1/defaults_test.go b/pkg/api/v1/defaults_test.go index 3b8eba645c2..0e39bd55d9f 100644 --- a/pkg/api/v1/defaults_test.go +++ b/pkg/api/v1/defaults_test.go @@ -896,18 +896,6 @@ func TestSetDefaulServiceExternalTraffic(t *testing.T) { if out.Spec.ExternalTrafficPolicy != v1.ServiceExternalTrafficPolicyTypeCluster { t.Errorf("Expected ExternalTrafficPolicy to be %v, got %v", v1.ServiceExternalTrafficPolicyTypeCluster, out.Spec.ExternalTrafficPolicy) } - - in = &v1.Service{ - Spec: v1.ServiceSpec{Type: v1.ServiceTypeLoadBalancer}, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{v1.BetaAnnotationExternalTraffic: v1.AnnotationValueExternalTrafficLocal}, - }, - } - obj = roundTrip(t, runtime.Object(in)) - out = obj.(*v1.Service) - if out.Spec.ExternalTrafficPolicy != "" { - t.Errorf("Expected ExternalTrafficPolicy to be empty, got %v", out.Spec.ExternalTrafficPolicy) - } } func TestSetDefaultNamespace(t *testing.T) { diff --git a/pkg/api/v1/service/BUILD b/pkg/api/v1/service/BUILD index 51ffdcac445..3788b24ae57 100644 --- a/pkg/api/v1/service/BUILD +++ b/pkg/api/v1/service/BUILD @@ -14,7 +14,6 @@ go_library( tags = ["automanaged"], deps = [ "//pkg/util/net/sets:go_default_library", - "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", ], ) @@ -26,9 +25,7 @@ go_test( tags = ["automanaged"], deps = [ "//pkg/util/net/sets:go_default_library", - "//vendor/github.com/davecgh/go-spew/spew:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", ], ) diff --git a/pkg/api/v1/service/util.go b/pkg/api/v1/service/util.go index 03d18492b35..4cb453cf3eb 100644 --- a/pkg/api/v1/service/util.go +++ b/pkg/api/v1/service/util.go @@ -18,13 +18,10 @@ package service import ( "fmt" - "strconv" "strings" "k8s.io/api/core/v1" netsets "k8s.io/kubernetes/pkg/util/net/sets" - - "github.com/golang/glog" ) const ( @@ -76,25 +73,10 @@ func RequestsOnlyLocalTraffic(service *v1.Service) bool { service.Spec.Type != v1.ServiceTypeNodePort { return false } - - // First check the beta annotation and then the first class field. This is so that - // existing Services continue to work till the user decides to transition to the - // first class field. - if l, ok := service.Annotations[v1.BetaAnnotationExternalTraffic]; ok { - switch l { - case v1.AnnotationValueExternalTrafficLocal: - return true - case v1.AnnotationValueExternalTrafficGlobal: - return false - default: - glog.Errorf("Invalid value for annotation %v: %v", v1.BetaAnnotationExternalTraffic, l) - return false - } - } return service.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal } -// NeedsHealthCheck Check if service needs health check. +// NeedsHealthCheck checks if service needs health check. func NeedsHealthCheck(service *v1.Service) bool { if service.Spec.Type != v1.ServiceTypeLoadBalancer { return false @@ -102,56 +84,12 @@ func NeedsHealthCheck(service *v1.Service) bool { return RequestsOnlyLocalTraffic(service) } -// GetServiceHealthCheckNodePort Return health check node port for service, if one exists -func GetServiceHealthCheckNodePort(service *v1.Service) int32 { - // First check the beta annotation and then the first class field. This is so that - // existing Services continue to work till the user decides to transition to the - // first class field. - if l, ok := service.Annotations[v1.BetaAnnotationHealthCheckNodePort]; ok { - p, err := strconv.Atoi(l) - if err != nil { - glog.Errorf("Failed to parse annotation %v: %v", v1.BetaAnnotationHealthCheckNodePort, err) - return 0 - } - return int32(p) - } - return service.Spec.HealthCheckNodePort -} - -// ClearExternalTrafficPolicy resets the ExternalTrafficPolicy field. -func ClearExternalTrafficPolicy(service *v1.Service) { - // First check the beta annotation and then the first class field. This is so existing - // Services continue to work till the user decides to transition to the first class field. - if _, ok := service.Annotations[v1.BetaAnnotationExternalTraffic]; ok { - delete(service.Annotations, v1.BetaAnnotationExternalTraffic) - return - } - service.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyType("") -} - -// SetServiceHealthCheckNodePort sets the given health check node port on service. -// It does not check whether this service needs healthCheckNodePort. -func SetServiceHealthCheckNodePort(service *v1.Service, hcNodePort int32) { - // First check the beta annotation and then the first class field. This is so that - // existing Services continue to work till the user decides to transition to the - // first class field. - if _, ok := service.Annotations[v1.BetaAnnotationExternalTraffic]; ok { - if hcNodePort == 0 { - delete(service.Annotations, v1.BetaAnnotationHealthCheckNodePort) - } else { - service.Annotations[v1.BetaAnnotationHealthCheckNodePort] = fmt.Sprintf("%d", hcNodePort) - } - return - } - service.Spec.HealthCheckNodePort = hcNodePort -} - -// GetServiceHealthCheckPathPort Return the path and nodePort programmed into the Cloud LB Health Check +// GetServiceHealthCheckPathPort returns 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) + port := service.Spec.HealthCheckNodePort if port == 0 { return "", 0 } diff --git a/pkg/api/v1/service/util_test.go b/pkg/api/v1/service/util_test.go index e4ad9daeca7..df9504cbe6c 100644 --- a/pkg/api/v1/service/util_test.go +++ b/pkg/api/v1/service/util_test.go @@ -17,16 +17,11 @@ limitations under the License. package service import ( + "strings" "testing" - "fmt" - "strings" - "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" netsets "k8s.io/kubernetes/pkg/util/net/sets" - - "github.com/davecgh/go-spew/spew" ) func TestGetLoadBalancerSourceRanges(t *testing.T) { @@ -218,200 +213,4 @@ func TestNeedsHealthCheck(t *testing.T) { ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal, }, }) - - checkNeedsHealthCheck(false, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.BetaAnnotationExternalTraffic: "invalid", - }, - }, - }) - checkNeedsHealthCheck(false, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.BetaAnnotationExternalTraffic: v1.AnnotationValueExternalTrafficGlobal, - }, - }, - }) - checkNeedsHealthCheck(true, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.BetaAnnotationExternalTraffic: v1.AnnotationValueExternalTrafficLocal, - }, - }, - }) -} - -func TestGetServiceHealthCheckNodePort(t *testing.T) { - checkGetServiceHealthCheckNodePort := func(healthCheckNodePort int32, service *v1.Service) { - res := GetServiceHealthCheckNodePort(service) - if res != healthCheckNodePort { - t.Errorf("Expected health check node port = %v, got %v", - healthCheckNodePort, res) - } - } - - checkGetServiceHealthCheckNodePort(0, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - }, - }) - checkGetServiceHealthCheckNodePort(0, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeNodePort, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, - }, - }) - checkGetServiceHealthCheckNodePort(0, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, - }, - }) - checkGetServiceHealthCheckNodePort(34567, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal, - HealthCheckNodePort: int32(34567), - }, - }) - checkGetServiceHealthCheckNodePort(34567, &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeLoadBalancer, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.BetaAnnotationExternalTraffic: v1.AnnotationValueExternalTrafficLocal, - v1.BetaAnnotationHealthCheckNodePort: "34567", - }, - }, - }) -} - -func TestClearExternalTrafficPolicy(t *testing.T) { - testCases := []struct { - inputService *v1.Service - }{ - // First class fields cases. - { - &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, - }, - }, - }, - // Beta annotations cases. - { - &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.BetaAnnotationExternalTraffic: v1.AnnotationValueExternalTrafficLocal, - }, - }, - }, - }, - } - - for i, tc := range testCases { - ClearExternalTrafficPolicy(tc.inputService) - if _, ok := tc.inputService.Annotations[v1.BetaAnnotationExternalTraffic]; ok || - tc.inputService.Spec.ExternalTrafficPolicy != "" { - t.Errorf("%v: failed to clear ExternalTrafficPolicy", i) - spew.Dump(tc) - } - } -} - -func TestSetServiceHealthCheckNodePort(t *testing.T) { - testCases := []struct { - inputService *v1.Service - hcNodePort int32 - beta bool - }{ - // First class fields cases. - { - &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, - }, - }, - 30012, - false, - }, - { - &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, - }, - }, - 0, - false, - }, - // Beta annotations cases. - { - &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.BetaAnnotationExternalTraffic: v1.AnnotationValueExternalTrafficGlobal, - }, - }, - }, - 30012, - true, - }, - { - &v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - }, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.BetaAnnotationExternalTraffic: v1.AnnotationValueExternalTrafficGlobal, - }, - }, - }, - 0, - true, - }, - } - - for i, tc := range testCases { - SetServiceHealthCheckNodePort(tc.inputService, tc.hcNodePort) - if !tc.beta { - if tc.inputService.Spec.HealthCheckNodePort != tc.hcNodePort { - t.Errorf("%v: got HealthCheckNodePort %v, want %v", i, tc.inputService.Spec.HealthCheckNodePort, tc.hcNodePort) - } - } else { - l, ok := tc.inputService.Annotations[v1.BetaAnnotationHealthCheckNodePort] - if tc.hcNodePort == 0 { - if ok { - t.Errorf("%v: HealthCheckNodePort set, want it to be cleared", i) - } - } else { - if !ok { - t.Errorf("%v: HealthCheckNodePort unset, want %v", i, tc.hcNodePort) - } else if l != fmt.Sprintf("%v", tc.hcNodePort) { - t.Errorf("%v: got HealthCheckNodePort %v, want %v", i, l, tc.hcNodePort) - } - } - } - } } diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index b04fd7c09cb..b160c0110c1 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -24,7 +24,6 @@ import ( "path/filepath" "reflect" "regexp" - "strconv" "strings" "github.com/golang/glog" @@ -2970,7 +2969,6 @@ func ValidateService(service *api.Service) field.ErrorList { } allErrs = append(allErrs, validateServiceExternalTrafficFieldsValue(service)...) - allErrs = append(allErrs, validateServiceExternalTrafficAPIVersion(service)...) return allErrs } @@ -3018,25 +3016,6 @@ func validateServicePort(sp *api.ServicePort, requireName, isHeadlessService boo func validateServiceExternalTrafficFieldsValue(service *api.Service) field.ErrorList { allErrs := field.ErrorList{} - // Check beta annotations. - if l, ok := service.Annotations[api.BetaAnnotationExternalTraffic]; ok { - if l != api.AnnotationValueExternalTrafficLocal && - l != api.AnnotationValueExternalTrafficGlobal { - allErrs = append(allErrs, field.Invalid(field.NewPath("metadata", "annotations").Key(api.BetaAnnotationExternalTraffic), l, - fmt.Sprintf("ExternalTraffic must be %v or %v", api.AnnotationValueExternalTrafficLocal, api.AnnotationValueExternalTrafficGlobal))) - } - } - if l, ok := service.Annotations[api.BetaAnnotationHealthCheckNodePort]; ok { - p, err := strconv.Atoi(l) - if err != nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("metadata", "annotations").Key(api.BetaAnnotationHealthCheckNodePort), l, - "HealthCheckNodePort must be a valid port number")) - } else if p <= 0 { - allErrs = append(allErrs, field.Invalid(field.NewPath("metadata", "annotations").Key(api.BetaAnnotationHealthCheckNodePort), l, - "HealthCheckNodePort must be greater than 0")) - } - } - // Check first class fields. if service.Spec.ExternalTrafficPolicy != "" && service.Spec.ExternalTrafficPolicy != api.ServiceExternalTrafficPolicyTypeCluster && @@ -3052,54 +3031,6 @@ func validateServiceExternalTrafficFieldsValue(service *api.Service) field.Error return allErrs } -// serviceExternalTrafficStatus stores flags indicating whether ExternalTraffic -// related beta annotations and GA fields are set on service. -type serviceExternalTrafficStatus struct { - betaExternalTrafficIsSet bool - betaHealthCheckIsSet bool - gaExternalTrafficIsSet bool - gaHealthCheckIsSet bool -} - -func (s *serviceExternalTrafficStatus) useBetaExternalTrafficWithGA() bool { - return s.betaExternalTrafficIsSet && (s.gaExternalTrafficIsSet || s.gaHealthCheckIsSet) -} - -func (s *serviceExternalTrafficStatus) useBetaHealthCheckWithGA() bool { - return s.betaHealthCheckIsSet && (s.gaExternalTrafficIsSet || s.gaHealthCheckIsSet) -} - -func getServiceExternalTrafficStatus(service *api.Service) *serviceExternalTrafficStatus { - s := serviceExternalTrafficStatus{} - _, s.betaExternalTrafficIsSet = service.Annotations[api.BetaAnnotationExternalTraffic] - _, s.betaHealthCheckIsSet = service.Annotations[api.BetaAnnotationHealthCheckNodePort] - s.gaExternalTrafficIsSet = service.Spec.ExternalTrafficPolicy != "" - s.gaHealthCheckIsSet = service.Spec.HealthCheckNodePort != 0 - return &s -} - -// validateServiceExternalTrafficAPIVersion checks if user mixes ExternalTraffic -// API versions. -func validateServiceExternalTrafficAPIVersion(service *api.Service) field.ErrorList { - allErrs := field.ErrorList{} - - status := getServiceExternalTrafficStatus(service) - - if status.useBetaExternalTrafficWithGA() { - fieldPath := field.NewPath("metadata", "annotations").Key(api.BetaAnnotationExternalTraffic) - msg := fmt.Sprintf("please replace the beta annotation with 'ExternalTrafficPolicy' field") - allErrs = append(allErrs, field.Invalid(fieldPath, api.BetaAnnotationExternalTraffic, msg)) - } - - if status.useBetaHealthCheckWithGA() { - fieldPath := field.NewPath("metadata", "annotations").Key(api.BetaAnnotationHealthCheckNodePort) - msg := fmt.Sprintf("please replace the beta annotation with 'HealthCheckNodePort' field") - allErrs = append(allErrs, field.Invalid(fieldPath, api.BetaAnnotationHealthCheckNodePort, msg)) - } - - return allErrs -} - // ValidateServiceExternalTrafficFieldsCombination validates if ExternalTrafficPolicy, // HealthCheckNodePort and Type combination are legal. For update, it should be called // after clearing externalTraffic related fields for the ease of transitioning between diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 6adca43ec87..a849df8445f 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -6526,48 +6526,6 @@ func TestValidateService(t *testing.T) { numErrs: 1, }, // ESIPP section begins. - { - name: "LoadBalancer allows onlyLocal beta annotations", - tweakSvc: func(s *api.Service) { - s.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - }, - numErrs: 0, - }, - { - name: "invalid externalTraffic beta annotation", - tweakSvc: func(s *api.Service) { - s.Spec.Type = api.ServiceTypeLoadBalancer - s.Annotations[api.BetaAnnotationExternalTraffic] = "invalid" - }, - numErrs: 1, - }, - { - name: "nagative healthCheckNodePort beta annotation", - tweakSvc: func(s *api.Service) { - s.Spec.Type = api.ServiceTypeLoadBalancer - s.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - s.Annotations[api.BetaAnnotationHealthCheckNodePort] = "-1" - }, - numErrs: 1, - }, - { - name: "invalid healthCheckNodePort beta annotation", - tweakSvc: func(s *api.Service) { - s.Spec.Type = api.ServiceTypeLoadBalancer - s.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - s.Annotations[api.BetaAnnotationHealthCheckNodePort] = "whatisthis" - }, - numErrs: 1, - }, - { - name: "valid healthCheckNodePort beta annotation", - tweakSvc: func(s *api.Service) { - s.Spec.Type = api.ServiceTypeLoadBalancer - s.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - s.Annotations[api.BetaAnnotationHealthCheckNodePort] = "31100" - }, - numErrs: 0, - }, { name: "invalid externalTraffic field", tweakSvc: func(s *api.Service) { @@ -6594,33 +6552,6 @@ func TestValidateService(t *testing.T) { }, numErrs: 0, }, - { - name: "disallows use ExternalTraffic beta annotation with first class field", - tweakSvc: func(s *api.Service) { - s.Spec.Type = api.ServiceTypeLoadBalancer - s.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - s.Spec.HealthCheckNodePort = 3001 - }, - numErrs: 1, - }, - { - name: "disallows duplicated ExternalTraffic beta annotation with first class field", - tweakSvc: func(s *api.Service) { - s.Spec.Type = api.ServiceTypeLoadBalancer - s.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - s.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal - }, - numErrs: 1, - }, - { - name: "disallows use HealthCheckNodePort beta annotation with first class field", - tweakSvc: func(s *api.Service) { - s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal - s.Annotations[api.BetaAnnotationHealthCheckNodePort] = "3001" - }, - numErrs: 1, - }, // ESIPP section ends. } @@ -8081,50 +8012,6 @@ func TestValidateServiceUpdate(t *testing.T) { }, numErrs: 1, }, - { - name: "Service allows removing onlyLocal beta annotations", - tweakSvc: func(oldSvc, newSvc *api.Service) { - oldSvc.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - oldSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] = "3001" - }, - numErrs: 0, - }, - { - name: "Service allows modifying onlyLocal beta annotations", - tweakSvc: func(oldSvc, newSvc *api.Service) { - oldSvc.Spec.Type = api.ServiceTypeLoadBalancer - oldSvc.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - oldSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] = "3001" - newSvc.Spec.Type = api.ServiceTypeLoadBalancer - newSvc.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficGlobal - newSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] = oldSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] - }, - numErrs: 0, - }, - { - name: "Service disallows promoting one of the onlyLocal pair to GA", - tweakSvc: func(oldSvc, newSvc *api.Service) { - oldSvc.Spec.Type = api.ServiceTypeLoadBalancer - oldSvc.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - oldSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] = "3001" - newSvc.Spec.Type = api.ServiceTypeLoadBalancer - newSvc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal - newSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] = oldSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] - }, - numErrs: 1, - }, - { - name: "Service allows changing both onlyLocal annotations from beta to GA", - tweakSvc: func(oldSvc, newSvc *api.Service) { - oldSvc.Spec.Type = api.ServiceTypeLoadBalancer - oldSvc.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal - oldSvc.Annotations[api.BetaAnnotationHealthCheckNodePort] = "3001" - newSvc.Spec.Type = api.ServiceTypeLoadBalancer - newSvc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal - newSvc.Spec.HealthCheckNodePort = 3001 - }, - numErrs: 0, - }, { name: "`None` ClusterIP cannot be changed", tweakSvc: func(oldSvc, newSvc *api.Service) { diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index dda2d9f6de7..aa340e96b91 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -211,7 +211,7 @@ func newServiceInfo(svcPortName proxy.ServicePortName, port *api.ServicePort, se copy(info.externalIPs, service.Spec.ExternalIPs) if apiservice.NeedsHealthCheck(service) { - p := apiservice.GetServiceHealthCheckNodePort(service) + p := service.Spec.HealthCheckNodePort if p == 0 { glog.Errorf("Service %q has no healthcheck nodeport", svcPortName.NamespacedName.String()) } else { diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 768f0db8ff2..35f288c53b3 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -867,7 +867,7 @@ func TestOnlyLocalLoadBalancing(t *testing.T) { svc.Status.LoadBalancer.Ingress = []api.LoadBalancerIngress{{ IP: svcLBIP, }} - svc.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal + svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal }), ) @@ -958,7 +958,7 @@ func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTable Protocol: api.ProtocolTCP, NodePort: int32(svcNodePort), }} - svc.Annotations[api.BetaAnnotationExternalTraffic] = api.AnnotationValueExternalTrafficLocal + svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal }), ) @@ -1069,10 +1069,6 @@ func TestBuildServiceMapAddRemove(t *testing.T) { } }), makeTestService("somewhere", "only-local-load-balancer", func(svc *api.Service) { - svc.ObjectMeta.Annotations = map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - api.BetaAnnotationHealthCheckNodePort: "345", - } svc.Spec.Type = api.ServiceTypeLoadBalancer svc.Spec.ClusterIP = "172.16.55.12" svc.Spec.LoadBalancerIP = "5.6.7.8" @@ -1083,6 +1079,8 @@ func TestBuildServiceMapAddRemove(t *testing.T) { {IP: "10.1.2.3"}, }, } + svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal + svc.Spec.HealthCheckNodePort = 345 }), } @@ -1214,10 +1212,6 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { svc.Spec.Ports = addTestPort(svc.Spec.Ports, "somethingelse", "TCP", 1235, 5321, 0) }) servicev2 := makeTestService("somewhere", "some-service", func(svc *api.Service) { - svc.ObjectMeta.Annotations = map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - api.BetaAnnotationHealthCheckNodePort: "345", - } svc.Spec.Type = api.ServiceTypeLoadBalancer svc.Spec.ClusterIP = "172.16.55.4" svc.Spec.LoadBalancerIP = "5.6.7.8" @@ -1228,6 +1222,8 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { {IP: "10.1.2.3"}, }, } + svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal + svc.Spec.HealthCheckNodePort = 345 }) fp.OnServiceAdd(servicev1) diff --git a/pkg/registry/core/service/rest.go b/pkg/registry/core/service/rest.go index e35dea95fea..46ac0100e37 100644 --- a/pkg/registry/core/service/rest.go +++ b/pkg/registry/core/service/rest.go @@ -183,7 +183,7 @@ func (rs *REST) Delete(ctx genericapirequest.Context, id string) (runtime.Object if utilfeature.DefaultFeatureGate.Enabled(features.ExternalTrafficLocalOnly) && apiservice.NeedsHealthCheck(service) { - nodePort := apiservice.GetServiceHealthCheckNodePort(service) + nodePort := service.Spec.HealthCheckNodePort if nodePort > 0 { err := rs.serviceNodePorts.Release(int(nodePort)) if err != nil { @@ -238,7 +238,7 @@ func externalTrafficPolicyUpdate(oldService, service *api.Service) { } if neededExternalTraffic && !needsExternalTraffic { // Clear ExternalTrafficPolicy to prevent confusion from ineffective field. - apiservice.ClearExternalTrafficPolicy(service) + service.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyType("") } } @@ -246,10 +246,10 @@ func externalTrafficPolicyUpdate(oldService, service *api.Service) { // and adjusts HealthCheckNodePort during service update if needed. func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (bool, error) { neededHealthCheckNodePort := apiservice.NeedsHealthCheck(oldService) - oldHealthCheckNodePort := apiservice.GetServiceHealthCheckNodePort(oldService) + oldHealthCheckNodePort := oldService.Spec.HealthCheckNodePort needsHealthCheckNodePort := apiservice.NeedsHealthCheck(service) - newHealthCheckNodePort := apiservice.GetServiceHealthCheckNodePort(service) + newHealthCheckNodePort := service.Spec.HealthCheckNodePort switch { // Case 1: Transition from don't need HealthCheckNodePort to needs HealthCheckNodePort. @@ -272,19 +272,14 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (boo } glog.Infof("Freed health check nodePort: %d", oldHealthCheckNodePort) // Clear the HealthCheckNodePort field. - apiservice.SetServiceHealthCheckNodePort(service, 0) + service.Spec.HealthCheckNodePort = 0 // Case 3: Remain in needs HealthCheckNodePort. // Reject changing the value of the HealthCheckNodePort field. case neededHealthCheckNodePort && needsHealthCheckNodePort: if oldHealthCheckNodePort != newHealthCheckNodePort { glog.Warningf("Attempt to change value of health check node port DENIED") - var fldPath *field.Path - if _, ok := service.Annotations[api.BetaAnnotationHealthCheckNodePort]; ok { - fldPath = field.NewPath("metadata", "annotations").Key(api.BetaAnnotationHealthCheckNodePort) - } else { - fldPath = field.NewPath("spec", "healthCheckNodePort") - } + fldPath := field.NewPath("spec", "healthCheckNodePort") el := field.ErrorList{field.Invalid(fldPath, newHealthCheckNodePort, "cannot change healthCheckNodePort on loadBalancer service with externalTraffic=Local during update")} return false, errors.NewInvalid(api.Kind("Service"), service.Name, el) @@ -480,7 +475,7 @@ func findRequestedNodePort(port int, servicePorts []api.ServicePort) int { // allocateHealthCheckNodePort allocates health check node port to service. func (rs *REST) allocateHealthCheckNodePort(service *api.Service) error { - healthCheckNodePort := apiservice.GetServiceHealthCheckNodePort(service) + healthCheckNodePort := service.Spec.HealthCheckNodePort if healthCheckNodePort != 0 { // If the request has a health check nodePort in mind, attempt to reserve it. err := rs.serviceNodePorts.Allocate(int(healthCheckNodePort)) @@ -495,7 +490,7 @@ func (rs *REST) allocateHealthCheckNodePort(service *api.Service) error { if err != nil { return fmt.Errorf("failed to allocate a HealthCheck NodePort %v: %v", healthCheckNodePort, err) } - apiservice.SetServiceHealthCheckNodePort(service, int32(healthCheckNodePort)) + service.Spec.HealthCheckNodePort = int32(healthCheckNodePort) glog.Infof("Reserved allocated nodePort: %d", healthCheckNodePort) } return nil diff --git a/pkg/registry/core/service/rest_test.go b/pkg/registry/core/service/rest_test.go index 36cdf1be00f..a3fba863597 100644 --- a/pkg/registry/core/service/rest_test.go +++ b/pkg/registry/core/service/rest_test.go @@ -19,7 +19,6 @@ package service import ( "testing" - "fmt" "net" "reflect" "strings" @@ -994,47 +993,7 @@ func TestServiceRegistryExternalTrafficHealthCheckNodePortAllocation(t *testing. if !service.NeedsHealthCheck(created_service) { t.Errorf("Expecting health check needed, returned health check not needed instead") } - port := service.GetServiceHealthCheckNodePort(created_service) - if port == 0 { - t.Errorf("Failed to allocate health check node port and set the HealthCheckNodePort") - } else { - // Release the node port at the end of the test case. - storage.serviceNodePorts.Release(int(port)) - } -} - -// Validate allocation of a nodePort when ExternalTraffic beta annotation is set to OnlyLocal -// and type is LoadBalancer. -func TestServiceRegistryExternalTrafficHealthCheckNodePortAllocationBeta(t *testing.T) { - ctx := genericapirequest.NewDefaultContext() - storage, _ := NewTestREST(t, nil) - svc := &api.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "external-lb-esipp", - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - }, - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"bar": "baz"}, - SessionAffinity: api.ServiceAffinityNone, - Type: api.ServiceTypeLoadBalancer, - Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, - TargetPort: intstr.FromInt(6502), - }}, - }, - } - created_svc, err := storage.Create(ctx, svc, false) - if created_svc == nil || err != nil { - t.Errorf("Unexpected failure creating service %v", err) - } - created_service := created_svc.(*api.Service) - if !service.NeedsHealthCheck(created_service) { - t.Errorf("Expecting health check needed, returned health check not needed instead") - } - port := service.GetServiceHealthCheckNodePort(created_service) + port := created_service.Spec.HealthCheckNodePort if port == 0 { t.Errorf("Failed to allocate health check node port and set the HealthCheckNodePort") } else { @@ -1072,54 +1031,7 @@ func TestServiceRegistryExternalTrafficHealthCheckNodePortUserAllocation(t *test if !service.NeedsHealthCheck(created_service) { t.Errorf("Expecting health check needed, returned health check not needed instead") } - port := service.GetServiceHealthCheckNodePort(created_service) - if port == 0 { - t.Errorf("Failed to allocate health check node port and set the HealthCheckNodePort") - } - if port != randomNodePort { - t.Errorf("Failed to allocate requested nodePort expected %d, got %d", randomNodePort, port) - } - - if port != 0 { - // Release the node port at the end of the test case. - storage.serviceNodePorts.Release(int(port)) - } -} - -// Validate using the user specified nodePort when ExternalTraffic beta annotation is set to OnlyLocal -// and type is LoadBalancer. -func TestServiceRegistryExternalTrafficHealthCheckNodePortUserAllocationBeta(t *testing.T) { - randomNodePort := generateRandomNodePort() - ctx := genericapirequest.NewDefaultContext() - storage, _ := NewTestREST(t, nil) - svc := &api.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "external-lb-esipp", - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - api.BetaAnnotationHealthCheckNodePort: fmt.Sprintf("%v", randomNodePort), - }, - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"bar": "baz"}, - SessionAffinity: api.ServiceAffinityNone, - Type: api.ServiceTypeLoadBalancer, - Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, - TargetPort: intstr.FromInt(6502), - }}, - }, - } - created_svc, err := storage.Create(ctx, svc, false) - if created_svc == nil || err != nil { - t.Fatalf("Unexpected failure creating service :%v", err) - } - created_service := created_svc.(*api.Service) - if !service.NeedsHealthCheck(created_service) { - t.Errorf("Expecting health check needed, returned health check not needed instead") - } - port := service.GetServiceHealthCheckNodePort(created_service) + port := created_service.Spec.HealthCheckNodePort if port == 0 { t.Errorf("Failed to allocate health check node port and set the HealthCheckNodePort") } @@ -1159,36 +1071,6 @@ func TestServiceRegistryExternalTrafficHealthCheckNodePortNegative(t *testing.T) t.Errorf("Unexpected creation of service with invalid HealthCheckNodePort specified") } -// Validate that the service creation fails when the requested port number in beta annotation is -1. -func TestServiceRegistryExternalTrafficHealthCheckNodePortNegativeBeta(t *testing.T) { - ctx := genericapirequest.NewDefaultContext() - storage, _ := NewTestREST(t, nil) - svc := &api.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "external-lb-esipp", - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficLocal, - api.BetaAnnotationHealthCheckNodePort: "-1", - }, - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"bar": "baz"}, - SessionAffinity: api.ServiceAffinityNone, - Type: api.ServiceTypeLoadBalancer, - Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, - TargetPort: intstr.FromInt(6502), - }}, - }, - } - created_svc, err := storage.Create(ctx, svc, false) - if created_svc == nil || err != nil { - return - } - t.Errorf("Unexpected creation of service with invalid HealthCheckNodePort specified") -} - // Validate that the health check nodePort is not allocated when ExternalTrafficPolicy is set to Global. func TestServiceRegistryExternalTrafficGlobal(t *testing.T) { ctx := genericapirequest.NewDefaultContext() @@ -1216,7 +1098,7 @@ func TestServiceRegistryExternalTrafficGlobal(t *testing.T) { t.Errorf("Expecting health check not needed, returned health check needed instead") } // Make sure the service does not have the health check node port allocated - port := service.GetServiceHealthCheckNodePort(created_service) + port := created_service.Spec.HealthCheckNodePort if port != 0 { // Release the node port at the end of the test case. storage.serviceNodePorts.Release(int(port)) @@ -1224,80 +1106,6 @@ func TestServiceRegistryExternalTrafficGlobal(t *testing.T) { } } -// Validate that the health check nodePort is not allocated when ExternalTraffic beta annotation is set to Global. -func TestServiceRegistryExternalTrafficGlobalBeta(t *testing.T) { - ctx := genericapirequest.NewDefaultContext() - storage, _ := NewTestREST(t, nil) - svc := &api.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "external-lb-esipp", - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficGlobal, - }, - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"bar": "baz"}, - SessionAffinity: api.ServiceAffinityNone, - Type: api.ServiceTypeLoadBalancer, - Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, - TargetPort: intstr.FromInt(6502), - }}, - }, - } - created_svc, err := storage.Create(ctx, svc, false) - if created_svc == nil || err != nil { - t.Errorf("Unexpected failure creating service %v", err) - } - created_service := created_svc.(*api.Service) - if service.NeedsHealthCheck(created_service) { - t.Errorf("Expecting health check not needed, returned health check needed instead") - } - // Make sure the service does not have the health check node port allocated - port := service.GetServiceHealthCheckNodePort(created_service) - if port != 0 { - // Release the node port at the end of the test case. - storage.serviceNodePorts.Release(int(port)) - t.Errorf("Unexpected allocation of health check node port: %v", port) - } -} - -// Validate that the health check nodePort is not allocated when service type is ClusterIP -func TestServiceRegistryExternalTrafficAnnotationClusterIP(t *testing.T) { - ctx := genericapirequest.NewDefaultContext() - storage, _ := NewTestREST(t, nil) - svc := &api.Service{ - ObjectMeta: metav1.ObjectMeta{Name: "external-lb-esipp", - Annotations: map[string]string{ - api.BetaAnnotationExternalTraffic: api.AnnotationValueExternalTrafficGlobal, - }, - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"bar": "baz"}, - SessionAffinity: api.ServiceAffinityNone, - Type: api.ServiceTypeClusterIP, - Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, - TargetPort: intstr.FromInt(6502), - }}, - }, - } - created_svc, err := storage.Create(ctx, svc, false) - if created_svc == nil || err != nil { - t.Errorf("Unexpected failure creating service %v", err) - } - created_service := created_svc.(*api.Service) - // Make sure that ClusterIP services do not have the health check node port allocated - port := service.GetServiceHealthCheckNodePort(created_service) - if port != 0 { - // Release the node port at the end of the test case. - storage.serviceNodePorts.Release(int(port)) - t.Errorf("Unexpected allocation of health check node port annotation %s", api.BetaAnnotationHealthCheckNodePort) - } -} - func TestInitClusterIP(t *testing.T) { storage, _ := NewTestREST(t, nil) diff --git a/staging/src/k8s.io/api/core/v1/annotation_key_constants.go b/staging/src/k8s.io/api/core/v1/annotation_key_constants.go index 6af9c6c2b1a..6c5deb65fd3 100644 --- a/staging/src/k8s.io/api/core/v1/annotation_key_constants.go +++ b/staging/src/k8s.io/api/core/v1/annotation_key_constants.go @@ -89,20 +89,4 @@ const ( // // Not all cloud providers support this annotation, though AWS & GCE do. AnnotationLoadBalancerSourceRangesKey = "service.beta.kubernetes.io/load-balancer-source-ranges" - - // AnnotationValueExternalTrafficLocal Value of annotation to specify local endpoints behavior. - AnnotationValueExternalTrafficLocal = "OnlyLocal" - // AnnotationValueExternalTrafficGlobal Value of annotation to specify global (legacy) behavior. - AnnotationValueExternalTrafficGlobal = "Global" - - // TODO: The beta annotations have been deprecated, remove them when we release k8s 1.8. - - // BetaAnnotationHealthCheckNodePort Annotation specifying the healthcheck nodePort for the service. - // If not specified, annotation is created by the service api backend with the allocated nodePort. - // Will use user-specified nodePort value if specified by the client. - BetaAnnotationHealthCheckNodePort = "service.beta.kubernetes.io/healthcheck-nodeport" - - // BetaAnnotationExternalTraffic An annotation that denotes if this Service desires to route - // external traffic to local endpoints only. This preserves Source IP and avoids a second hop. - BetaAnnotationExternalTraffic = "service.beta.kubernetes.io/external-traffic" ) diff --git a/test/e2e/framework/BUILD b/test/e2e/framework/BUILD index 1a75bf66b48..77bfdb534fb 100644 --- a/test/e2e/framework/BUILD +++ b/test/e2e/framework/BUILD @@ -48,7 +48,6 @@ go_library( "//pkg/api/v1/helper:go_default_library", "//pkg/api/v1/node:go_default_library", "//pkg/api/v1/pod:go_default_library", - "//pkg/api/v1/service:go_default_library", "//pkg/apis/batch:go_default_library", "//pkg/apis/componentconfig:go_default_library", "//pkg/apis/extensions:go_default_library", diff --git a/test/e2e/framework/firewall_util.go b/test/e2e/framework/firewall_util.go index 3fea73df8b8..d253fd87806 100644 --- a/test/e2e/framework/firewall_util.go +++ b/test/e2e/framework/firewall_util.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" - apiservice "k8s.io/kubernetes/pkg/api/v1/service" "k8s.io/kubernetes/pkg/cloudprovider" gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" @@ -87,7 +86,7 @@ func ConstructHealthCheckFirewallForLBService(clusterID string, svc *v1.Service, fw.SourceRanges = gcecloud.LoadBalancerSrcRanges() healthCheckPort := gcecloud.GetNodesHealthCheckPort() if !isNodesHealthCheck { - healthCheckPort = apiservice.GetServiceHealthCheckNodePort(svc) + healthCheckPort = svc.Spec.HealthCheckNodePort } fw.Allowed = []*compute.FirewallAllowed{ { diff --git a/test/e2e/network/BUILD b/test/e2e/network/BUILD index c82d8ae7b43..ad3e787ca06 100644 --- a/test/e2e/network/BUILD +++ b/test/e2e/network/BUILD @@ -32,7 +32,6 @@ go_library( deps = [ "//pkg/api:go_default_library", "//pkg/api/testapi:go_default_library", - "//pkg/api/v1/service:go_default_library", "//pkg/apis/networking:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/cloudprovider:go_default_library", diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index 6de1f54c945..eee7dc663a1 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/api/v1/service" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/controller/endpoint" @@ -1454,7 +1453,7 @@ var _ = SIGDescribe("ESIPP [Slow]", func() { svc := jig.CreateOnlyLocalLoadBalancerService(namespace, serviceName, loadBalancerCreateTimeout, true, nil) serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(svc)) - healthCheckNodePort := int(service.GetServiceHealthCheckNodePort(svc)) + healthCheckNodePort := int(svc.Spec.HealthCheckNodePort) if healthCheckNodePort == 0 { framework.Failf("Service HealthCheck NodePort was not allocated") } @@ -1531,7 +1530,7 @@ var _ = SIGDescribe("ESIPP [Slow]", func() { Expect(cs.Core().Services(svc.Namespace).Delete(svc.Name, nil)).NotTo(HaveOccurred()) }() - healthCheckNodePort := int(service.GetServiceHealthCheckNodePort(svc)) + healthCheckNodePort := int(svc.Spec.HealthCheckNodePort) if healthCheckNodePort == 0 { framework.Failf("Service HealthCheck NodePort was not allocated") } @@ -1636,13 +1635,13 @@ var _ = SIGDescribe("ESIPP [Slow]", func() { }() // save the health check node port because it disappears when ESIPP is turned off. - healthCheckNodePort := int(service.GetServiceHealthCheckNodePort(svc)) + healthCheckNodePort := int(svc.Spec.HealthCheckNodePort) By("turning ESIPP off") svc = jig.UpdateServiceOrFail(svc.Namespace, svc.Name, func(svc *v1.Service) { svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeCluster }) - if service.GetServiceHealthCheckNodePort(svc) > 0 { + if svc.Spec.HealthCheckNodePort > 0 { framework.Failf("Service HealthCheck NodePort still present") }