Add name generation to services

pkg/api/rest/types.go

@@ -68,3 +68,28 @@ func (podStrategy) Validate(obj runtime.Object) errors.ValidationErrorList {
 	pod := obj.(*api.Pod)
 	return validation.ValidatePod(pod)
 }
+
+// svcStrategy implements behavior for Services
+// TODO: move to a service specific package.
+type svcStrategy struct {
+	runtime.ObjectTyper
+	api.NameGenerator
+}
+
+// Services is the default logic that applies when creating and updating Service
+// objects.
+var Services RESTCreateStrategy = svcStrategy{api.Scheme, api.SimpleNameGenerator}
+
+// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation.
+func (svcStrategy) ResetBeforeCreate(obj runtime.Object) {
+	service := obj.(*api.Service)
+	// TODO: Get rid of ProxyPort.
+	service.Spec.ProxyPort = 0
+	service.Status = api.ServiceStatus{}
+}
+
+// Validate validates a new pod.
+func (svcStrategy) Validate(obj runtime.Object) errors.ValidationErrorList {
+	service := obj.(*api.Service)
+	return validation.ValidateService(service)
+}

pkg/registry/service/rest.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider"
@@ -81,35 +82,29 @@ func reloadIPsFromStorage(ipa *ipAllocator, registry Registry) {
 
 func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) {
 	service := obj.(*api.Service)
-	if !api.ValidNamespace(ctx, &service.ObjectMeta) {
-		return nil, errors.NewConflict("service", service.Namespace, fmt.Errorf("Service.Namespace does not match the provided context"))
-	}
-	if errs := validation.ValidateService(service); len(errs) > 0 {
-		return nil, errors.NewInvalid("service", service.Name, errs)
-	}
 
-	api.FillObjectMetaSystemFields(ctx, &service.ObjectMeta)
+	if err := rest.BeforeCreate(rest.Services, ctx, obj); err != nil {
+		return nil, err
+	}
 
-	if service.Spec.PortalIP == "" {
+	if len(service.Spec.PortalIP) == 0 {
 		// Allocate next available.
-		if ip, err := rs.portalMgr.AllocateNext(); err != nil {
+		ip, err := rs.portalMgr.AllocateNext()
+		if err != nil {
 			return nil, err
-		} else {
-			service.Spec.PortalIP = ip.String()
 		}
+		service.Spec.PortalIP = ip.String()
 	} else {
 		// Try to respect the requested IP.
 		if err := rs.portalMgr.Allocate(net.ParseIP(service.Spec.PortalIP)); err != nil {
 			el := errors.ValidationErrorList{errors.NewFieldInvalid("spec.portalIP", service.Spec.PortalIP, err.Error())}
-			return nil, errors.NewInvalid("service", service.Name, el)
+			return nil, errors.NewInvalid("Service", service.Name, el)
 		}
 	}
 
 	return apiserver.MakeAsync(func() (runtime.Object, error) {
-		// TODO: Consider moving this to a rectification loop, so that we make/remove external load balancers
+		// TODO: Move this to post-creation rectification loop, so that we make/remove external load balancers
 		// correctly no matter what http operations happen.
-		// TODO: Get rid of ProxyPort.
-		service.Spec.ProxyPort = 0
 		if service.Spec.CreateExternalLoadBalancer {
 			if rs.cloud == nil {
 				return nil, fmt.Errorf("requested an external service, but no cloud provider supplied.")
@@ -155,8 +150,8 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE
 				service.Spec.PublicIPs = []string{ip.String()}
 			}
 		}
-		err := rs.registry.CreateService(ctx, service)
-		if err != nil {
+
+		if err := rs.registry.CreateService(ctx, service); err != nil {
 			return nil, err
 		}
 		return rs.registry.GetService(ctx, service.Name)

pkg/registry/service/rest_test.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	cloud "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@@ -621,6 +622,7 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) {
 	}
 }
 
+// TODO: remove, covered by TestCreate
 func TestCreateServiceWithConflictingNamespace(t *testing.T) {
 	storage := REST{}
 	service := &api.Service{
@@ -634,7 +636,7 @@ func TestCreateServiceWithConflictingNamespace(t *testing.T) {
 	}
 	if err == nil {
 		t.Errorf("Expected an error, but we didn't get one")
-	} else if strings.Index(err.Error(), "Service.Namespace does not match the provided context") == -1 {
+	} else if strings.Contains(err.Error(), "Service.Namespace does not match the provided context") {
 		t.Errorf("Expected 'Service.Namespace does not match the provided context' error, got '%s'", err.Error())
 	}
 }
@@ -656,3 +658,26 @@ func TestUpdateServiceWithConflictingNamespace(t *testing.T) {
 		t.Errorf("Expected 'Service.Namespace does not match the provided context' error, got '%s'", err.Error())
 	}
 }
+
+func TestCreate(t *testing.T) {
+	registry := registrytest.NewServiceRegistry()
+	fakeCloud := &cloud.FakeCloud{}
+	machines := []string{"foo", "bar", "baz"}
+	rest := NewREST(registry, fakeCloud, registrytest.NewMinionRegistry(machines, api.NodeResources{}), makeIPNet(t))
+	rest.portalMgr.randomAttempts = 0
+
+	test := resttest.New(t, rest)
+	test.TestCreate(
+		// valid
+		&api.Service{
+			Spec: api.ServiceSpec{
+				Selector: map[string]string{"bar": "baz"},
+				Port:     6502,
+			},
+		},
+		// invalid
+		&api.Service{
+			Spec: api.ServiceSpec{},
+		},
+	)
+}