diff --git a/pkg/api/service/annotations.go b/pkg/api/service/annotations.go index bd13cc51664..4b45549983b 100644 --- a/pkg/api/service/annotations.go +++ b/pkg/api/service/annotations.go @@ -33,29 +33,44 @@ const ( // Not all cloud providers support this annotation, though AWS & GCE do. AnnotationLoadBalancerSourceRangesKey = "service.beta.kubernetes.io/load-balancer-source-ranges" - // AnnotationExternalTraffic 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. - AnnotationExternalTraffic = "service.alpha.kubernetes.io/external-traffic" // AnnotationValueExternalTrafficLocal Value of annotation to specify local endpoints behaviour AnnotationValueExternalTrafficLocal = "OnlyLocal" // AnnotationValueExternalTrafficGlobal Value of annotation to specify global (legacy) behaviour AnnotationValueExternalTrafficGlobal = "Global" - // AnnotationHealthCheckNodePort Annotation specifying the healthcheck nodePort for the service + + // TODO: The alpha annotations have been deprecated, remove them when we move this feature to GA. + + // AlphaAnnotationHealthCheckNodePort 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 - AnnotationHealthCheckNodePort = "service.alpha.kubernetes.io/healthcheck-nodeport" + AlphaAnnotationHealthCheckNodePort = "service.alpha.kubernetes.io/healthcheck-nodeport" + + // AlphaAnnotationExternalTraffic 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. + AlphaAnnotationExternalTraffic = "service.alpha.kubernetes.io/external-traffic" + + // BetaAnnotationHealthCheckNodePort is the beta version of AlphaAnnotationHealthCheckNodePort. + BetaAnnotationHealthCheckNodePort = "service.beta.kubernetes.io/healthcheck-nodeport" + + // 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 { - if l, ok := service.Annotations[AnnotationExternalTraffic]; ok { - if l == AnnotationValueExternalTrafficLocal { - return true - } else if l == AnnotationValueExternalTrafficGlobal { - return false - } else { - glog.Errorf("Invalid value for annotation %v", AnnotationExternalTraffic) - 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 { + return true + } else if l == AnnotationValueExternalTrafficGlobal { + return false + } else { + glog.Errorf("Invalid value for annotation %v: %v", annotation, l) + } } } return false @@ -63,12 +78,19 @@ func NeedsHealthCheck(service *api.Service) bool { // GetServiceHealthCheckNodePort Return health check node port annotation for service, if one exists func GetServiceHealthCheckNodePort(service *api.Service) int32 { - if NeedsHealthCheck(service) { - if l, ok := service.Annotations[AnnotationHealthCheckNodePort]; ok { + 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", AnnotationHealthCheckNodePort, err) - return 0 + glog.Errorf("Failed to parse annotation %v: %v", annotation, err) + continue } return int32(p) } diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index 30eb35a2794..0de20d2b1d3 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -736,9 +736,7 @@ func (gce *GCECloud) EnsureLoadBalancer(clusterName string, apiService *api.Serv // This logic exists to detect a transition for a pre-existing service and turn on // the tpNeedsUpdate flag to delete/recreate fwdrule/tpool adding the health check // to the target pool. - glog.V(2).Infof("Annotation %s=%s added to new or pre-existing service", - apiservice.AnnotationExternalTraffic, - apiservice.AnnotationValueExternalTrafficLocal) + glog.V(2).Infof("Annotation external-traffic=OnlyLocal added to new or pre-existing service") tpNeedsUpdate = true } hcToCreate, err = gce.ensureHttpHealthCheck(loadBalancerName, path, healthCheckNodePort) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index c9a886004b1..4f08a302b87 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -460,7 +460,7 @@ func (proxier *Proxier) OnServiceUpdate(allServices []api.Service) { p := apiservice.GetServiceHealthCheckNodePort(service) if p == 0 { glog.Errorf("Service does not contain necessary annotation %v", - apiservice.AnnotationHealthCheckNodePort) + apiservice.BetaAnnotationHealthCheckNodePort) } else { glog.V(4).Infof("Adding health check for %+v, port %v", serviceName.NamespacedName, p) info.healthCheckNodePort = int(p) diff --git a/pkg/registry/core/service/rest.go b/pkg/registry/core/service/rest.go index 37886a97336..10ef74d8f62 100644 --- a/pkg/registry/core/service/rest.go +++ b/pkg/registry/core/service/rest.go @@ -161,10 +161,10 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err if shouldCheckOrAssignHealthCheckNodePort(service) { var healthCheckNodePort int var err error - if l, ok := service.Annotations[apiservice.AnnotationHealthCheckNodePort]; ok { + if l, ok := service.Annotations[apiservice.BetaAnnotationHealthCheckNodePort]; ok { healthCheckNodePort, err = strconv.Atoi(l) if err != nil || healthCheckNodePort <= 0 { - return nil, errors.NewInternalError(fmt.Errorf("Failed to parse annotation %v: %v", apiservice.AnnotationHealthCheckNodePort, err)) + return nil, errors.NewInternalError(fmt.Errorf("Failed to parse annotation %v: %v", apiservice.BetaAnnotationHealthCheckNodePort, err)) } } if healthCheckNodePort > 0 { @@ -183,7 +183,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err return nil, errors.NewInternalError(fmt.Errorf("failed to allocate a nodePort: %v", err)) } // Insert the newly allocated health check port as an annotation (plan of record for Alpha) - service.Annotations[apiservice.AnnotationHealthCheckNodePort] = fmt.Sprintf("%d", healthCheckNodePort) + service.Annotations[apiservice.BetaAnnotationHealthCheckNodePort] = fmt.Sprintf("%d", healthCheckNodePort) } } @@ -313,7 +313,7 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (boo errmsg := fmt.Sprintf("Failed to allocate requested HealthCheck nodePort %v:%v", requestedHealthCheckNodePort, err) el := field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"), - apiservice.AnnotationHealthCheckNodePort, errmsg)} + apiservice.BetaAnnotationHealthCheckNodePort, errmsg)} return false, errors.NewInvalid(api.Kind("Service"), service.Name, el) } glog.Infof("Reserved user requested nodePort: %d", requestedHealthCheckNodePort) @@ -327,7 +327,7 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (boo return false, errors.NewInternalError(fmt.Errorf("failed to allocate a nodePort: %v", err)) } // Insert the newly allocated health check port as an annotation (plan of record for Alpha) - service.Annotations[apiservice.AnnotationHealthCheckNodePort] = fmt.Sprintf("%d", healthCheckNodePort) + service.Annotations[apiservice.BetaAnnotationHealthCheckNodePort] = fmt.Sprintf("%d", healthCheckNodePort) glog.Infof("Reserved health check nodePort: %d", healthCheckNodePort) } @@ -338,29 +338,33 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (boo glog.Warningf("Error releasing service health check %s node port %d: %v", service.Name, oldHealthCheckNodePort, err) return false, errors.NewInternalError(fmt.Errorf("failed to free health check nodePort: %v", err)) } else { - delete(service.Annotations, apiservice.AnnotationHealthCheckNodePort) + delete(service.Annotations, apiservice.BetaAnnotationHealthCheckNodePort) + delete(service.Annotations, apiservice.AlphaAnnotationHealthCheckNodePort) glog.Infof("Freed health check nodePort: %d", oldHealthCheckNodePort) } case !oldServiceHasHealthCheckNodePort && !assignHealthCheckNodePort: - if _, ok := service.Annotations[apiservice.AnnotationHealthCheckNodePort]; ok { + if _, ok := service.Annotations[apiservice.BetaAnnotationHealthCheckNodePort]; ok { glog.Warningf("Attempt to insert health check node port annotation DENIED") el := field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"), - apiservice.AnnotationHealthCheckNodePort, "Cannot insert healthcheck nodePort annotation")} + apiservice.BetaAnnotationHealthCheckNodePort, "Cannot insert healthcheck nodePort annotation")} return false, errors.NewInvalid(api.Kind("Service"), service.Name, el) } case oldServiceHasHealthCheckNodePort && assignHealthCheckNodePort: - if _, ok := service.Annotations[apiservice.AnnotationHealthCheckNodePort]; !ok { - glog.Warningf("Attempt to delete health check node port annotation DENIED") - el := field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"), - apiservice.AnnotationHealthCheckNodePort, "Cannot delete healthcheck nodePort annotation")} - return false, errors.NewInvalid(api.Kind("Service"), service.Name, el) + for _, annotation := range []string{ + apiservice.AlphaAnnotationHealthCheckNodePort, apiservice.BetaAnnotationHealthCheckNodePort} { + if _, ok := service.Annotations[annotation]; !ok { + glog.Warningf("Attempt to delete health check node port annotation DENIED") + el := field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"), + annotation, "Cannot delete healthcheck nodePort annotation")} + return false, errors.NewInvalid(api.Kind("Service"), service.Name, el) + } } if oldHealthCheckNodePort != requestedHealthCheckNodePort { glog.Warningf("Attempt to change value of health check node port annotation DENIED") el := field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"), - apiservice.AnnotationHealthCheckNodePort, "Cannot change healthcheck nodePort during update")} + apiservice.BetaAnnotationHealthCheckNodePort, "Cannot change healthcheck nodePort during update")} return false, errors.NewInvalid(api.Kind("Service"), service.Name, el) } } diff --git a/pkg/registry/core/service/rest_test.go b/pkg/registry/core/service/rest_test.go index 806414ea796..b479a039933 100644 --- a/pkg/registry/core/service/rest_test.go +++ b/pkg/registry/core/service/rest_test.go @@ -955,7 +955,7 @@ func TestServiceRegistryExternalTrafficAnnotationHealthCheckNodePortAllocation(t svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "external-lb-esipp", Annotations: map[string]string{ - service.AnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal, + service.BetaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal, }, }, Spec: api.ServiceSpec{ @@ -975,25 +975,25 @@ func TestServiceRegistryExternalTrafficAnnotationHealthCheckNodePortAllocation(t } created_service := created_svc.(*api.Service) if !service.NeedsHealthCheck(created_service) { - t.Errorf("Unexpected missing annotation %s", service.AnnotationExternalTraffic) + t.Errorf("Unexpected missing annotation %s", service.BetaAnnotationExternalTraffic) } port := service.GetServiceHealthCheckNodePort(created_service) if port == 0 { - t.Errorf("Failed to allocate and create the health check node port annotation %s", service.AnnotationHealthCheckNodePort) + t.Errorf("Failed to allocate and create the health check node port annotation %s", service.BetaAnnotationHealthCheckNodePort) } } // Validate using the user specified nodePort when the externalTraffic=OnlyLocal annotation is set // and type is LoadBalancer -func TestServiceRegistryExternalTrafficAnnotationHealthCheckNodePortUserAllocation(t *testing.T) { +func TestServiceRegistryExternalTrafficBetaAnnotationHealthCheckNodePortUserAllocation(t *testing.T) { ctx := api.NewDefaultContext() storage, _ := NewTestREST(t, nil) svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "external-lb-esipp", Annotations: map[string]string{ - service.AnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal, - service.AnnotationHealthCheckNodePort: "30200", + service.BetaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal, + service.BetaAnnotationHealthCheckNodePort: "30200", }, }, Spec: api.ServiceSpec{ @@ -1013,11 +1013,11 @@ func TestServiceRegistryExternalTrafficAnnotationHealthCheckNodePortUserAllocati } created_service := created_svc.(*api.Service) if !service.NeedsHealthCheck(created_service) { - t.Errorf("Unexpected missing annotation %s", service.AnnotationExternalTraffic) + t.Errorf("Unexpected missing annotation %s", service.BetaAnnotationExternalTraffic) } port := service.GetServiceHealthCheckNodePort(created_service) if port == 0 { - t.Errorf("Failed to allocate and create the health check node port annotation %s", service.AnnotationHealthCheckNodePort) + t.Errorf("Failed to allocate and create the health check node port annotation %s", service.BetaAnnotationHealthCheckNodePort) } if port != 30200 { t.Errorf("Failed to allocate requested nodePort expected 30200, got %d", port) @@ -1031,8 +1031,8 @@ func TestServiceRegistryExternalTrafficAnnotationNegative(t *testing.T) { svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "external-lb-esipp", Annotations: map[string]string{ - service.AnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal, - service.AnnotationHealthCheckNodePort: "-1", + service.BetaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal, + service.BetaAnnotationHealthCheckNodePort: "-1", }, }, Spec: api.ServiceSpec{ @@ -1060,7 +1060,7 @@ func TestServiceRegistryExternalTrafficAnnotationGlobal(t *testing.T) { svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "external-lb-esipp", Annotations: map[string]string{ - service.AnnotationExternalTraffic: service.AnnotationValueExternalTrafficGlobal, + service.BetaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficGlobal, }, }, Spec: api.ServiceSpec{ @@ -1081,12 +1081,12 @@ func TestServiceRegistryExternalTrafficAnnotationGlobal(t *testing.T) { created_service := created_svc.(*api.Service) // Make sure the service does not have the annotation if service.NeedsHealthCheck(created_service) { - t.Errorf("Unexpected value for annotation %s", service.AnnotationExternalTraffic) + t.Errorf("Unexpected value for annotation %s", service.BetaAnnotationExternalTraffic) } // Make sure the service does not have the health check node port allocated port := service.GetServiceHealthCheckNodePort(created_service) if port != 0 { - t.Errorf("Unexpected allocation of health check node port annotation %s", service.AnnotationHealthCheckNodePort) + t.Errorf("Unexpected allocation of health check node port annotation %s", service.BetaAnnotationHealthCheckNodePort) } } @@ -1097,7 +1097,7 @@ func TestServiceRegistryExternalTrafficAnnotationClusterIP(t *testing.T) { svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "external-lb-esipp", Annotations: map[string]string{ - service.AnnotationExternalTraffic: service.AnnotationValueExternalTrafficGlobal, + service.BetaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficGlobal, }, }, Spec: api.ServiceSpec{ @@ -1119,6 +1119,6 @@ func TestServiceRegistryExternalTrafficAnnotationClusterIP(t *testing.T) { // Make sure that ClusterIP services do not have the health check node port allocated port := service.GetServiceHealthCheckNodePort(created_service) if port != 0 { - t.Errorf("Unexpected allocation of health check node port annotation %s", service.AnnotationHealthCheckNodePort) + t.Errorf("Unexpected allocation of health check node port annotation %s", service.BetaAnnotationHealthCheckNodePort) } }