From 0a87f0332b21a1f13782035b6dcedd24c11e438e Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 28 Jan 2015 21:36:36 -0500 Subject: [PATCH] Add name generation to services --- pkg/api/rest/types.go | 25 +++++++++++++++++++++++++ pkg/registry/service/rest.go | 29 ++++++++++++----------------- pkg/registry/service/rest_test.go | 27 ++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/pkg/api/rest/types.go b/pkg/api/rest/types.go index 8a08bc5dde7..9ac9bba9aba 100644 --- a/pkg/api/rest/types.go +++ b/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) +} diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 718b34b798e..e7af0cd605f 100644 --- a/pkg/registry/service/rest.go +++ b/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) + + if err := rest.BeforeCreate(rest.Services, ctx, obj); err != nil { + return nil, err } - api.FillObjectMetaSystemFields(ctx, &service.ObjectMeta) - - 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) diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index eec35f117be..4c4d1beb5f3 100644 --- a/pkg/registry/service/rest_test.go +++ b/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{}, + }, + ) +}