Merge pull request #397 from brendandburns/service

Add initial validation of Service objects

cmd/integration/integration.go

@@ -145,13 +145,19 @@ func runAtomicPutTest(c *client.Client) {
 			Labels: map[string]string{
 				"name": "atomicService",
 			},
+			// This is here because validation requires it.
+			Selector: map[string]string{
+				"foo": "bar",
+			},
 		},
 	).Do().Into(&svc)
 	if err != nil {
 		glog.Fatalf("Failed creating atomicService: %v", err)
 	}
 	glog.Info("Created atomicService")
-	testLabels := labels.Set{}
+	testLabels := labels.Set{
+		"foo": "bar",
+	}
 	for i := 0; i < 5; i++ {
 		// a: z, b: y, etc...
 		testLabels[string([]byte{byte('a' + i)})] = string([]byte{byte('z' - i)})

pkg/api/validation.go

@@ -246,3 +246,14 @@ func ValidateManifest(manifest *ContainerManifest) []error {
 	allErrs.Append(validateContainers(manifest.Containers, allVolumes)...)
 	return []error(allErrs)
 }
+
+func ValidateService(service *Service) []error {
+	allErrs := errorList{}
+	if service.ID == "" {
+		allErrs.Append(makeInvalidError("Service.ID", service.ID))
+	}
+	if len(service.Selector) == 0 {
+		allErrs.Append(makeInvalidError("Service.Selector", service.Selector))
+	}
+	return []error(allErrs)
+}

pkg/api/validation_test.go

@@ -262,3 +262,36 @@ func TestValidateManifest(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateService(t *testing.T) {
+	errs := ValidateService(&Service{
+		JSONBase: JSONBase{ID: "foo"},
+		Selector: map[string]string{
+			"foo": "bar",
+		},
+	})
+	if len(errs) != 0 {
+		t.Errorf("Unexpected non-zero error list: %#v", errs)
+	}
+
+	errs = ValidateService(&Service{
+		Selector: map[string]string{
+			"foo": "bar",
+		},
+	})
+	if len(errs) != 1 {
+		t.Errorf("Unexpected error list: %#v", errs)
+	}
+
+	errs = ValidateService(&Service{
+		JSONBase: JSONBase{ID: "foo"},
+	})
+	if len(errs) != 1 {
+		t.Errorf("Unexpected error list: %#v", errs)
+	}
+
+	errs = ValidateService(&Service{})
+	if len(errs) != 2 {
+		t.Errorf("Unexpected error list: %#v", errs)
+	}
+}

pkg/registry/service_registry.go

@@ -112,8 +112,9 @@ func (sr *ServiceRegistryStorage) Extract(body []byte) (interface{}, error) {
 
 func (sr *ServiceRegistryStorage) Create(obj interface{}) (<-chan interface{}, error) {
 	srv := obj.(api.Service)
-	if srv.ID == "" {
+	errs := api.ValidateService(&srv)
-		return nil, fmt.Errorf("ID should not be empty: %#v", srv)
+	if len(errs) > 0 {
+		return nil, fmt.Errorf("Validation errors: %v", errs)
 	}
 	return apiserver.MakeAsync(func() (interface{}, error) {
 		// TODO: Consider moving this to a rectification loop, so that we make/remove external load balancers

pkg/registry/service_registry_test.go

@@ -33,6 +33,7 @@ func TestServiceRegistry(t *testing.T) {
 
 	svc := api.Service{
 		JSONBase: api.JSONBase{ID: "foo"},
+		Selector: map[string]string{"bar": "baz"},
 	}
 	c, _ := storage.Create(svc)
 	<-c
@@ -56,6 +57,7 @@ func TestServiceRegistryExternalService(t *testing.T) {
 
 	svc := api.Service{
 		JSONBase:                   api.JSONBase{ID: "foo"},
+		Selector:                   map[string]string{"bar": "baz"},
 		CreateExternalLoadBalancer: true,
 	}
 	c, _ := storage.Create(svc)
@@ -82,6 +84,7 @@ func TestServiceRegistryExternalServiceError(t *testing.T) {
 
 	svc := api.Service{
 		JSONBase:                   api.JSONBase{ID: "foo"},
+		Selector:                   map[string]string{"bar": "baz"},
 		CreateExternalLoadBalancer: true,
 	}
 	c, _ := storage.Create(svc)
@@ -106,6 +109,7 @@ func TestServiceRegistryDelete(t *testing.T) {
 
 	svc := api.Service{
 		JSONBase: api.JSONBase{ID: "foo"},
+		Selector: map[string]string{"bar": "baz"},
 	}
 	memory.CreateService(svc)
 
@@ -131,6 +135,7 @@ func TestServiceRegistryDeleteExternal(t *testing.T) {
 
 	svc := api.Service{
 		JSONBase:                   api.JSONBase{ID: "foo"},
+		Selector:                   map[string]string{"bar": "baz"},
 		CreateExternalLoadBalancer: true,
 	}
 	memory.CreateService(svc)