Allow an empty service selector

pkg/api/types.go

@@ -593,8 +593,10 @@ type ServiceSpec struct {
 	// Optional: Supports "TCP" and "UDP".  Defaults to "TCP".
 	Protocol Protocol `json:"protocol,omitempty"`
 
-	// This service will route traffic to pods having labels matching this selector.
-	Selector map[string]string `json:"selector,omitempty"`
+	// This service will route traffic to pods having labels matching this selector. If empty or not present,
+	// the service is assumed to have endpoints set by an external process and Kubernetes will not modify
+	// those endpoints.
+	Selector map[string]string `json:"selector"`
 
 	// PortalIP is usually assigned by the master.  If specified by the user
 	// we will try to respect it or else fail the request.  This field can

pkg/api/v1beta1/conversion_test.go

@@ -192,3 +192,35 @@ func TestMinionListConversionToOld(t *testing.T) {
 		t.Errorf("Expected: %#v, got %#v", e, a)
 	}
 }
+
+func TestServiceEmptySelector(t *testing.T) {
+	// Nil map should be preserved
+	svc := &v1beta1.Service{Selector: nil}
+	data, err := newer.Scheme.EncodeToVersion(svc, "v1beta1")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	obj, err := newer.Scheme.Decode(data)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	selector := obj.(*newer.Service).Spec.Selector
+	if selector != nil {
+		t.Errorf("unexpected selector: %#v", obj)
+	}
+
+	// Empty map should be preserved
+	svc2 := &v1beta1.Service{Selector: map[string]string{}}
+	data, err = newer.Scheme.EncodeToVersion(svc2, "v1beta1")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	obj, err = newer.Scheme.Decode(data)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	selector = obj.(*newer.Service).Spec.Selector
+	if selector == nil || len(selector) != 0 {
+		t.Errorf("unexpected selector: %#v", obj)
+	}
+}

pkg/api/v1beta1/types.go

@@ -465,9 +465,11 @@ type Service struct {
 	// This service's labels.
 	Labels map[string]string `json:"labels,omitempty" description:"map of string keys and values that can be used to organize and categorize services"`
 
-	// This service will route traffic to pods having labels matching this selector.
-	Selector                   map[string]string `json:"selector,omitempty" description:"label keys and values that must match in order to receive traffic for this service"`
-	CreateExternalLoadBalancer bool              `json:"createExternalLoadBalancer,omitempty" description:"set up a cloud-provider-specific load balancer on an external IP"`
+	// This service will route traffic to pods having labels matching this selector. If null, no endpoints will be automatically created. If empty, all pods will be selected.
+	Selector map[string]string `json:"selector" description:"label keys and values that must match in order to receive traffic for this service; if empty, all pods are selected, if not specified, endpoints must be manually specified"`
+	// An external load balancer should be set up via the cloud-provider
+	CreateExternalLoadBalancer bool `json:"createExternalLoadBalancer,omitempty" description:"set up a cloud-provider-specific load balancer on an external IP"`
+
 	// PublicIPs are used by external load balancers.
 	PublicIPs []string `json:"publicIPs,omitempty" description:"externally visible IPs from which to select the address for the external load balancer"`
 

pkg/api/v1beta2/conversion_test.go

@@ -16,4 +16,41 @@ limitations under the License.
 
 package v1beta2_test
 
-import ()
+import (
+	"testing"
+
+	newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2"
+)
+
+func TestServiceEmptySelector(t *testing.T) {
+	// Nil map should be preserved
+	svc := &v1beta2.Service{Selector: nil}
+	data, err := newer.Scheme.EncodeToVersion(svc, "v1beta2")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	obj, err := newer.Scheme.Decode(data)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	selector := obj.(*newer.Service).Spec.Selector
+	if selector != nil {
+		t.Errorf("unexpected selector: %#v", obj)
+	}
+
+	// Empty map should be preserved
+	svc2 := &v1beta2.Service{Selector: map[string]string{}}
+	data, err = newer.Scheme.EncodeToVersion(svc2, "v1beta2")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	obj, err = newer.Scheme.Decode(data)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	selector = obj.(*newer.Service).Spec.Selector
+	if selector == nil || len(selector) != 0 {
+		t.Errorf("unexpected selector: %#v", obj)
+	}
+}

pkg/api/v1beta2/types.go

@@ -430,9 +430,11 @@ type Service struct {
 	// This service's labels.
 	Labels map[string]string `json:"labels,omitempty" description:"map of string keys and values that can be used to organize and categorize services"`
 
-	// This service will route traffic to pods having labels matching this selector.
-	Selector                   map[string]string `json:"selector,omitempty" description:"label keys and values that must match in order to receive traffic for this service"`
-	CreateExternalLoadBalancer bool              `json:"createExternalLoadBalancer,omitempty" description:"set up a cloud-provider-specific load balancer on an external IP"`
+	// This service will route traffic to pods having labels matching this selector. If null, no endpoints will be automatically created. If empty, all pods will be selected.
+	Selector map[string]string `json:"selector" description:"label keys and values that must match in order to receive traffic for this service; if empty, all pods are selected, if not specified, endpoints must be manually specified"`
+	// An external load balancer should be set up via the cloud-provider
+	CreateExternalLoadBalancer bool `json:"createExternalLoadBalancer,omitempty" description:"set up a cloud-provider-specific load balancer on an external IP"`
+
 	// PublicIPs are used by external load balancers.
 	PublicIPs []string `json:"publicIPs,omitempty" description:"externally visible IPs from which to select the address for the external load balancer"`
 

pkg/api/v1beta3/types.go

@@ -601,8 +601,8 @@ type ServiceSpec struct {
 	// Optional: Supports "TCP" and "UDP".  Defaults to "TCP".
 	Protocol Protocol `json:"protocol,omitempty"`
 
-	// This service will route traffic to pods having labels matching this selector.
-	Selector map[string]string `json:"selector,omitempty"`
+	// This service will route traffic to pods having labels matching this selector. If null, no endpoints will be automatically created. If empty, all pods will be selected.
+	Selector map[string]string `json:"selector"`
 
 	// PortalIP is usually assigned by the master.  If specified by the user
 	// we will try to respect it or else fail the request.  This field can

pkg/api/validation/validation.go

@@ -438,9 +438,12 @@ func ValidateService(service *api.Service, lister ServiceLister, ctx api.Context
 	} else if !supportedPortProtocols.Has(strings.ToUpper(string(service.Spec.Protocol))) {
 		allErrs = append(allErrs, errs.NewFieldNotSupported("spec.protocol", service.Spec.Protocol))
 	}
-	if labels.Set(service.Spec.Selector).AsSelector().Empty() {
-		allErrs = append(allErrs, errs.NewFieldRequired("spec.selector", service.Spec.Selector))
+
+	if service.Spec.Selector != nil {
+		allErrs = append(allErrs, validateLabels(service.Spec.Selector, "spec.selector")...)
 	}
+	allErrs = append(allErrs, validateLabels(service.Labels, "labels")...)
+
 	if service.Spec.CreateExternalLoadBalancer {
 		services, err := lister.ListServices(ctx)
 		if err != nil {
@@ -456,8 +459,6 @@ func ValidateService(service *api.Service, lister ServiceLister, ctx api.Context
 			}
 		}
 	}
-	allErrs = append(allErrs, validateLabels(service.Labels, "labels")...)
-	allErrs = append(allErrs, validateLabels(service.Spec.Selector, "selector")...)
 	return allErrs
 }
 

pkg/api/validation/validation_test.go

@@ -710,8 +710,8 @@ func TestValidateService(t *testing.T) {
 					Port: 8675,
 				},
 			},
-			// Should fail because the selector is missing.
-			numErrs: 1,
+			// Should be ok because the selector is missing.
+			numErrs: 0,
 		},
 		{
 			name: "valid 1",
@@ -824,12 +824,25 @@ func TestValidateService(t *testing.T) {
 						"NoUppercaseOrSpecialCharsLike=Equals": "bar",
 					},
 				},
+				Spec: api.ServiceSpec{
+					Port: 8675,
+				},
+			},
+			numErrs: 1,
+		},
+		{
+			name: "invalid selector",
+			svc: api.Service{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "abc123",
+					Namespace: api.NamespaceDefault,
+				},
 				Spec: api.ServiceSpec{
 					Port:     8675,
 					Selector: map[string]string{"foo": "bar", "NoUppercaseOrSpecialCharsLike=Equals": "bar"},
 				},
 			},
-			numErrs: 2,
+			numErrs: 1,
 		},
 	}
 

pkg/master/publish.go

@@ -32,6 +32,7 @@ func (m *Master) serviceWriterLoop(stop chan struct{}) {
 		// stop polling and start watching.
 		// TODO: add endpoints of all replicas, not just the elected master.
 		if m.readWriteServer != "" {
+			// TODO: the public port should be part of the argument here, port will not always be 443
 			if err := m.createMasterServiceIfNeeded("kubernetes", 443); err != nil {
 				glog.Errorf("Can't create rw service: %v", err)
 			}
@@ -54,6 +55,7 @@ func (m *Master) roServiceWriterLoop(stop chan struct{}) {
 		// TODO: when it becomes possible to change this stuff,
 		// stop polling and start watching.
 		if m.readOnlyServer != "" {
+			// TODO: the public port should be part of the argument here, port will not always be 80
 			if err := m.createMasterServiceIfNeeded("kubernetes-ro", 80); err != nil {
 				glog.Errorf("Can't create ro service: %v", err)
 			}
@@ -81,14 +83,13 @@ func (m *Master) createMasterServiceIfNeeded(serviceName string, port int) error
 	svc := &api.Service{
 		ObjectMeta: api.ObjectMeta{
 			Name:      serviceName,
-			Namespace: "default",
+			Namespace: api.NamespaceDefault,
+			Labels:    map[string]string{"provider": "kubernetes", "component": "apiserver"},
 		},
 		Spec: api.ServiceSpec{
 			Port: port,
-			// We're going to add the endpoints by hand, so this selector is mainly to
-			// prevent identification of other pods. This selector will be useful when
-			// we start hosting apiserver in a pod.
-			Selector: map[string]string{"provider": "kubernetes", "component": "apiserver"},
+			// maintained by this code, not by the pod selector
+			Selector: nil,
 		},
 	}
 	// Kids, don't do this at home: this is a hack. There's no good way to call the business
@@ -113,10 +114,12 @@ func (m *Master) ensureEndpointsContain(serviceName string, endpoint string) err
 	ctx := api.NewDefaultContext()
 	e, err := m.endpointRegistry.GetEndpoints(ctx, serviceName)
 	if err != nil {
-		e = &api.Endpoints{}
-		// Fill in ID if it didn't exist already
-		e.ObjectMeta.Name = serviceName
-		e.ObjectMeta.Namespace = "default"
+		e = &api.Endpoints{
+			ObjectMeta: api.ObjectMeta{
+				Name:      serviceName,
+				Namespace: api.NamespaceDefault,
+			},
+		}
 	}
 	found := false
 	for i := range e.Endpoints {

pkg/registry/service/rest_test.go

@@ -167,11 +167,11 @@ func TestServiceStorageValidatesUpdate(t *testing.T) {
 				Selector: map[string]string{"bar": "baz"},
 			},
 		},
-		"empty selector": {
+		"invalid selector": {
 			ObjectMeta: api.ObjectMeta{Name: "foo"},
 			Spec: api.ServiceSpec{
 				Port:     6502,
-				Selector: map[string]string{},
+				Selector: map[string]string{"ThisSelectorFailsValidation": "ok"},
 			},
 		},
 	}

pkg/service/endpoints_controller.go

@@ -51,11 +51,11 @@ func (e *EndpointController) SyncServiceEndpoints() error {
 	}
 	var resultErr error
 	for _, service := range services.Items {
-		if service.Name == "kubernetes" || service.Name == "kubernetes-ro" {
-			// This is a temporary hack for supporting the master services
-			// until we actually start running apiserver in a pod.
+		if service.Spec.Selector == nil {
+			// services without a selector receive no endpoints.  The last endpoint will be used.
 			continue
 		}
+
 		glog.Infof("About to update endpoints for service %v", service.Name)
 		pods, err := e.client.Pods(service.Namespace).List(labels.Set(service.Spec.Selector).AsSelector())
 		if err != nil {

pkg/service/endpoints_controller_test.go

@@ -222,6 +222,71 @@ func TestSyncEndpointsError(t *testing.T) {
 	}
 }
 
+func TestSyncEndpointsItemsPreserveNoSelector(t *testing.T) {
+	serviceList := api.ServiceList{
+		Items: []api.Service{
+			{
+				ObjectMeta: api.ObjectMeta{Name: "foo"},
+				Spec:       api.ServiceSpec{},
+			},
+		},
+	}
+	testServer, endpointsHandler := makeTestServer(t,
+		serverResponse{http.StatusOK, newPodList(0)},
+		serverResponse{http.StatusOK, &serviceList},
+		serverResponse{http.StatusOK, &api.Endpoints{
+			ObjectMeta: api.ObjectMeta{
+				Name:            "foo",
+				ResourceVersion: "1",
+			},
+			Endpoints: []string{"6.7.8.9:1000"},
+		}})
+	defer testServer.Close()
+	client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Version()})
+	endpoints := NewEndpointController(client)
+	if err := endpoints.SyncServiceEndpoints(); err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	endpointsHandler.ValidateRequestCount(t, 0)
+}
+
+func TestSyncEndpointsItemsEmptySelectorSelectsAll(t *testing.T) {
+	serviceList := api.ServiceList{
+		Items: []api.Service{
+			{
+				ObjectMeta: api.ObjectMeta{Name: "foo"},
+				Spec: api.ServiceSpec{
+					Selector: map[string]string{},
+				},
+			},
+		},
+	}
+	testServer, endpointsHandler := makeTestServer(t,
+		serverResponse{http.StatusOK, newPodList(1)},
+		serverResponse{http.StatusOK, &serviceList},
+		serverResponse{http.StatusOK, &api.Endpoints{
+			ObjectMeta: api.ObjectMeta{
+				Name:            "foo",
+				ResourceVersion: "1",
+			},
+			Endpoints: []string{},
+		}})
+	defer testServer.Close()
+	client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Version()})
+	endpoints := NewEndpointController(client)
+	if err := endpoints.SyncServiceEndpoints(); err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	data := runtime.EncodeOrDie(testapi.Codec(), &api.Endpoints{
+		ObjectMeta: api.ObjectMeta{
+			Name:            "foo",
+			ResourceVersion: "1",
+		},
+		Endpoints: []string{"1.2.3.4:8080"},
+	})
+	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints/foo", "PUT", &data)
+}
+
 func TestSyncEndpointsItemsPreexisting(t *testing.T) {
 	serviceList := api.ServiceList{
 		Items: []api.Service{

pkg/util/fake_handler.go

@@ -73,6 +73,15 @@ func (f *FakeHandler) ServeHTTP(response http.ResponseWriter, request *http.Requ
 	f.RequestBody = string(bodyReceived)
 }
 
+func (f *FakeHandler) ValidateRequestCount(t TestInterface, count int) {
+	f.lock.Lock()
+	defer f.lock.Unlock()
+	if f.requestCount != count {
+		t.Logf("Expected %d call, but got %d. Only the last call is recorded and checked.", count, f.requestCount)
+	}
+	f.hasBeenChecked = true
+}
+
 // ValidateRequest verifies that FakeHandler received a request with expected path, method, and body.
 func (f *FakeHandler) ValidateRequest(t TestInterface, expectedPath, expectedMethod string, body *string) {
 	f.lock.Lock()