diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go index 075d125af7b3f..29585d7dc7ec5 100644 --- a/pkg/api/rest/resttest/resttest.go +++ b/pkg/api/rest/resttest/resttest.go @@ -24,7 +24,9 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/coreos/go-etcd/etcd" ) type Tester struct { @@ -197,6 +199,44 @@ func (t *Tester) TestCreateRejectsNamespace(valid runtime.Object) { } } +func (t *Tester) TestDeleteInvokesValidation(invalid ...runtime.Object) { + for i, obj := range invalid { + objectMeta, err := api.ObjectMetaFor(obj) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, obj) + } + ctx := api.NewDefaultContext() + _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil) + if !errors.IsInvalid(err) { + t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err) + } + } +} + +func (t *Tester) TestDelete(createFn func() runtime.Object, wasGracefulFn func() bool, invalid ...runtime.Object) { + t.TestDeleteNonExist(createFn) + t.TestDeleteNoGraceful(createFn, wasGracefulFn) + t.TestDeleteInvokesValidation(invalid...) + // TODO: Test delete namespace mismatch rejection + // once #5684 is fixed. +} + +func (t *Tester) TestDeleteNonExist(createFn func() runtime.Object) { + existing := createFn() + objectMeta, err := api.ObjectMetaFor(existing) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, existing) + } + context := api.NewDefaultContext() + + t.withStorageError(&etcd.EtcdError{ErrorCode: tools.EtcdErrorCodeNotFound}, func() { + _, err := t.storage.(rest.GracefulDeleter).Delete(context, objectMeta.Name, nil) + if err == nil || !errors.IsNotFound(err) { + t.Fatalf("Unexpected error: %v", err) + } + }) +} + func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) { t.TestDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn) t.TestDeleteGracefulUsesZeroOnNil(createFn(), 0) diff --git a/pkg/controller/replication_controller.go b/pkg/controller/replication_controller.go index c815602b5f5a7..17312bd84ebcb 100644 --- a/pkg/controller/replication_controller.go +++ b/pkg/controller/replication_controller.go @@ -58,7 +58,7 @@ type RealPodControl struct { } // Time period of main replication controller sync loop -const DefaultSyncPeriod = 10 * time.Second +const DefaultSyncPeriod = 5 * time.Second func (r RealPodControl) createReplica(namespace string, controller api.ReplicationController) { desiredLabels := make(labels.Set) diff --git a/pkg/master/master.go b/pkg/master/master.go index 432cb55ce337f..bad324a364fc3 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -42,7 +42,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/master/ports" - "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/controller" + controlleretcd "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/controller/etcd" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/endpoint" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/etcd" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/event" @@ -384,6 +384,8 @@ func (m *Master) init(c *Config) { podStorage = podStorage.WithPodStatus(podCache) } + controllerStorage := controlleretcd.NewREST(c.EtcdHelper) + // TODO: Factor out the core API registration m.storage = map[string]rest.Storage{ "pods": podStorage, @@ -391,7 +393,7 @@ func (m *Master) init(c *Config) { "pods/binding": bindingStorage, "bindings": bindingStorage, - "replicationControllers": controller.NewStorage(registry, podRegistry), + "replicationControllers": controllerStorage, "services": service.NewStorage(m.serviceRegistry, c.Cloud, m.nodeRegistry, m.portalNet, c.ClusterName), "endpoints": endpoint.NewStorage(m.endpointRegistry), "minions": nodeStorage, diff --git a/pkg/registry/controller/etcd/etcd.go b/pkg/registry/controller/etcd/etcd.go new file mode 100644 index 0000000000000..4bd7ccf2a12e1 --- /dev/null +++ b/pkg/registry/controller/etcd/etcd.go @@ -0,0 +1,76 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package etcd + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/controller" + "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic" + etcdgeneric "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" +) + +// rest implements a RESTStorage for replication controllers against etcd +type REST struct { + *etcdgeneric.Etcd +} + +// controllerPrefix is the location for controllers in etcd, only exposed +// for testing +var controllerPrefix = "/registry/controllers" + +// NewREST returns a RESTStorage object that will work against replication controllers. +func NewREST(h tools.EtcdHelper) *REST { + store := &etcdgeneric.Etcd{ + NewFunc: func() runtime.Object { return &api.ReplicationController{} }, + + // NewListFunc returns an object capable of storing results of an etcd list. + NewListFunc: func() runtime.Object { return &api.ReplicationControllerList{} }, + // Produces a path that etcd understands, to the root of the resource + // by combining the namespace in the context with the given prefix + KeyRootFunc: func(ctx api.Context) string { + return etcdgeneric.NamespaceKeyRootFunc(ctx, controllerPrefix) + }, + // Produces a path that etcd understands, to the resource by combining + // the namespace in the context with the given prefix + KeyFunc: func(ctx api.Context, name string) (string, error) { + return etcdgeneric.NamespaceKeyFunc(ctx, controllerPrefix, name) + }, + // Retrieve the name field of a replication controller + ObjectNameFunc: func(obj runtime.Object) (string, error) { + return obj.(*api.ReplicationController).Name, nil + }, + // Used to match objects based on labels/fields for list and watch + PredicateFunc: func(label labels.Selector, field fields.Selector) generic.Matcher { + return controller.MatchController(label, field) + }, + EndpointName: "replicationControllers", + + // Used to validate controller creation + CreateStrategy: controller.Strategy, + + // Used to validate controller updates + UpdateStrategy: controller.Strategy, + + Helper: h, + } + + return &REST{store} +} diff --git a/pkg/registry/controller/etcd/etcd_test.go b/pkg/registry/controller/etcd/etcd_test.go new file mode 100644 index 0000000000000..54f675e910960 --- /dev/null +++ b/pkg/registry/controller/etcd/etcd_test.go @@ -0,0 +1,623 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package etcd + +import ( + "strconv" + "strings" + "testing" + "time" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + etcdgeneric "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" + "github.com/coreos/go-etcd/etcd" +) + +// newStorage creates a REST storage backed by etcd helpers +func newStorage(t *testing.T) (*REST, *tools.FakeEtcdClient) { + fakeEtcdClient := tools.NewFakeEtcdClient(t) + fakeEtcdClient.TestIndex = true + h := tools.NewEtcdHelper(fakeEtcdClient, latest.Codec) + storage := NewREST(h) + return storage, fakeEtcdClient +} + +// createController is a helper function that returns a controller with the updated resource version. +func createController(storage *REST, rc api.ReplicationController, t *testing.T) (api.ReplicationController, error) { + ctx := api.WithNamespace(api.NewContext(), rc.Namespace) + obj, err := storage.Create(ctx, &rc) + if err != nil { + t.Errorf("Failed to create controller, %v", err) + } + newRc := obj.(*api.ReplicationController) + return *newRc, nil +} + +var validPodTemplate = api.PodTemplate{ + Spec: api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: map[string]string{"a": "b"}, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "test", + Image: "test_image", + ImagePullPolicy: api.PullIfNotPresent, + }, + }, + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + }, + }, +} + +var validControllerSpec = api.ReplicationControllerSpec{ + Selector: validPodTemplate.Spec.Labels, + Template: &validPodTemplate.Spec, +} + +var validController = api.ReplicationController{ + ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "default"}, + Spec: validControllerSpec, +} + +// makeControllerKey constructs etcd paths to controller items enforcing namespace rules. +func makeControllerKey(ctx api.Context, id string) (string, error) { + return etcdgeneric.NamespaceKeyFunc(ctx, controllerPrefix, id) +} + +// makeControllerListKey constructs etcd paths to the root of the resource, +// not a specific controller resource +func makeControllerListKey(ctx api.Context) string { + return etcdgeneric.NamespaceKeyRootFunc(ctx, controllerPrefix) +} + +func TestEtcdCreateController(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + _, err := storage.Create(ctx, &validController) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + key, _ := makeControllerKey(ctx, validController.Name) + resp, err := fakeClient.Get(key, false, false) + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + var ctrl api.ReplicationController + err = latest.Codec.DecodeInto([]byte(resp.Node.Value), &ctrl) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if ctrl.Name != "foo" { + t.Errorf("Unexpected controller: %#v %s", ctrl, resp.Node.Value) + } +} + +func TestEtcdCreateControllerAlreadyExisting(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + key, _ := makeControllerKey(ctx, validController.Name) + fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &validController), 0) + + _, err := storage.Create(ctx, &validController) + if !errors.IsAlreadyExists(err) { + t.Errorf("expected already exists err, got %#v", err) + } +} + +func TestEtcdCreateControllerValidates(t *testing.T) { + ctx := api.NewDefaultContext() + storage, _ := newStorage(t) + emptyName := validController + emptyName.Name = "" + emptySelector := validController + emptySelector.Spec.Selector = map[string]string{} + failureCases := []api.ReplicationController{emptyName, emptySelector} + for _, failureCase := range failureCases { + c, err := storage.Create(ctx, &failureCase) + if c != nil { + t.Errorf("Expected nil channel") + } + if !errors.IsInvalid(err) { + t.Errorf("Expected to get an invalid resource error, got %v", err) + } + } +} + +func TestCreateControllerWithGeneratedName(t *testing.T) { + storage, _ := newStorage(t) + controller := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Namespace: api.NamespaceDefault, + GenerateName: "rc-", + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"a": "b"}, + Template: &validPodTemplate.Spec, + }, + } + + ctx := api.NewDefaultContext() + _, err := storage.Create(ctx, controller) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if controller.Name == "rc-" || !strings.HasPrefix(controller.Name, "rc-") { + t.Errorf("unexpected name: %#v", controller) + } +} + +func TestCreateControllerWithConflictingNamespace(t *testing.T) { + storage, _ := newStorage(t) + controller := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "not-default"}, + } + + ctx := api.NewDefaultContext() + channel, err := storage.Create(ctx, controller) + if channel != nil { + t.Error("Expected a nil channel, but we got a value") + } + errSubString := "namespace" + if err == nil { + t.Errorf("Expected an error, but we didn't get one") + } else if !errors.IsBadRequest(err) || + strings.Index(err.Error(), errSubString) == -1 { + t.Errorf("Expected a Bad Request error with the sub string '%s', got %v", errSubString, err) + } +} + +func TestEtcdGetController(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + key, _ := makeControllerKey(ctx, validController.Name) + fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &validController), 0) + ctrl, err := storage.Get(ctx, validController.Name) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + controller, ok := ctrl.(*api.ReplicationController) + if !ok { + t.Errorf("Expected a controller, got %#v", ctrl) + } + if controller.Name != validController.Name { + t.Errorf("Unexpected controller: %#v", controller) + } +} + +func TestEtcdControllerValidatesUpdate(t *testing.T) { + ctx := api.NewDefaultContext() + storage, _ := newStorage(t) + + updateController, err := createController(storage, validController, t) + if err != nil { + t.Errorf("Failed to create controller, cannot proceed with test.") + } + + updaters := []func(rc api.ReplicationController) (runtime.Object, bool, error){ + func(rc api.ReplicationController) (runtime.Object, bool, error) { + rc.UID = "newUID" + return storage.Update(ctx, &rc) + }, + func(rc api.ReplicationController) (runtime.Object, bool, error) { + rc.Name = "" + return storage.Update(ctx, &rc) + }, + func(rc api.ReplicationController) (runtime.Object, bool, error) { + rc.Spec.Selector = map[string]string{} + return storage.Update(ctx, &rc) + }, + } + for _, u := range updaters { + c, updated, err := u(updateController) + if c != nil || updated { + t.Errorf("Expected nil object and not created") + } + if !errors.IsInvalid(err) && !errors.IsBadRequest(err) { + t.Errorf("Expected invalid or bad request error, got %v of type %T", err, err) + } + } +} + +func TestEtcdControllerValidatesNamespaceOnUpdate(t *testing.T) { + storage, _ := newStorage(t) + ns := "newnamespace" + + // The update should fail if the namespace on the controller is set to something + // other than the namespace on the given context, even if the namespace on the + // controller is valid. + updateController, err := createController(storage, validController, t) + + newNamespaceController := validController + newNamespaceController.Namespace = ns + _, err = createController(storage, newNamespaceController, t) + + c, updated, err := storage.Update(api.WithNamespace(api.NewContext(), ns), &updateController) + if c != nil || updated { + t.Errorf("Expected nil object and not created") + } + // TODO: Be more paranoid about the type of error and make sure it has the substring + // "namespace" in it, once #5684 is fixed. Ideally this would be a NewBadRequest. + if err == nil { + t.Errorf("Expected an error, but we didn't get one") + } +} + +// TestEtcdGetControllerDifferentNamespace ensures same-name controllers in different namespaces do not clash +func TestEtcdGetControllerDifferentNamespace(t *testing.T) { + storage, fakeClient := newStorage(t) + + otherNs := "other" + ctx1 := api.NewDefaultContext() + ctx2 := api.WithNamespace(api.NewContext(), otherNs) + + key1, _ := makeControllerKey(ctx1, validController.Name) + key2, _ := makeControllerKey(ctx2, validController.Name) + + fakeClient.Set(key1, runtime.EncodeOrDie(latest.Codec, &validController), 0) + otherNsController := validController + otherNsController.Namespace = otherNs + fakeClient.Set(key2, runtime.EncodeOrDie(latest.Codec, &otherNsController), 0) + + obj, err := storage.Get(ctx1, validController.Name) + ctrl1, _ := obj.(*api.ReplicationController) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if ctrl1.Name != "foo" { + t.Errorf("Unexpected controller: %#v", ctrl1) + } + if ctrl1.Namespace != "default" { + t.Errorf("Unexpected controller: %#v", ctrl1) + } + + obj, err = storage.Get(ctx2, validController.Name) + ctrl2, _ := obj.(*api.ReplicationController) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if ctrl2.Name != "foo" { + t.Errorf("Unexpected controller: %#v", ctrl2) + } + if ctrl2.Namespace != "other" { + t.Errorf("Unexpected controller: %#v", ctrl2) + } + +} + +func TestEtcdGetControllerNotFound(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + key, _ := makeControllerKey(ctx, validController.Name) + fakeClient.Data[key] = tools.EtcdResponseWithError{ + R: &etcd.Response{ + Node: nil, + }, + E: tools.EtcdErrorNotFound, + } + ctrl, err := storage.Get(ctx, validController.Name) + if ctrl != nil { + t.Errorf("Unexpected non-nil controller: %#v", ctrl) + } + if !errors.IsNotFound(err) { + t.Errorf("Unexpected error returned: %#v", err) + } +} + +func TestEtcdUpdateController(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + key, _ := makeControllerKey(ctx, validController.Name) + + // set a key, then retrieve the current resource version and try updating it + resp, _ := fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &validController), 0) + update := validController + update.ResourceVersion = strconv.FormatUint(resp.Node.ModifiedIndex, 10) + update.Spec.Replicas = validController.Spec.Replicas + 1 + _, created, err := storage.Update(ctx, &update) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if created { + t.Errorf("expected an update but created flag was returned") + } + ctrl, err := storage.Get(ctx, validController.Name) + updatedController, _ := ctrl.(*api.ReplicationController) + if updatedController.Spec.Replicas != validController.Spec.Replicas+1 { + t.Errorf("Unexpected controller: %#v", ctrl) + } +} + +func TestEtcdDeleteController(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + key, _ := makeControllerKey(ctx, validController.Name) + fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &validController), 0) + obj, err := storage.Delete(ctx, validController.Name, nil) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if status, ok := obj.(*api.Status); !ok { + t.Errorf("Expected status of delete, got %#v", status) + } else if status.Status != api.StatusSuccess { + t.Errorf("Expected success, got %#v", status.Status) + } + if len(fakeClient.DeletedKeys) != 1 { + t.Errorf("Expected 1 delete, found %#v", fakeClient.DeletedKeys) + } + if fakeClient.DeletedKeys[0] != key { + t.Errorf("Unexpected key: %s, expected %s", fakeClient.DeletedKeys[0], key) + } +} + +func TestEtcdListControllers(t *testing.T) { + storage, fakeClient := newStorage(t) + ctx := api.NewDefaultContext() + key := makeControllerListKey(ctx) + controller := validController + controller.Name = "bar" + fakeClient.Data[key] = tools.EtcdResponseWithError{ + R: &etcd.Response{ + Node: &etcd.Node{ + Nodes: []*etcd.Node{ + { + Value: runtime.EncodeOrDie(latest.Codec, &validController), + }, + { + Value: runtime.EncodeOrDie(latest.Codec, &controller), + }, + }, + }, + }, + E: nil, + } + objList, err := storage.List(ctx, labels.Everything(), fields.Everything()) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + controllers, _ := objList.(*api.ReplicationControllerList) + if len(controllers.Items) != 2 || controllers.Items[0].Name != validController.Name || controllers.Items[1].Name != controller.Name { + t.Errorf("Unexpected controller list: %#v", controllers) + } +} + +func TestEtcdListControllersNotFound(t *testing.T) { + storage, fakeClient := newStorage(t) + ctx := api.NewDefaultContext() + key := makeControllerListKey(ctx) + fakeClient.Data[key] = tools.EtcdResponseWithError{ + R: &etcd.Response{}, + E: tools.EtcdErrorNotFound, + } + objList, err := storage.List(ctx, labels.Everything(), fields.Everything()) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + controllers, _ := objList.(*api.ReplicationControllerList) + if len(controllers.Items) != 0 { + t.Errorf("Unexpected controller list: %#v", controllers) + } +} + +func TestEtcdListControllersLabelsMatch(t *testing.T) { + storage, fakeClient := newStorage(t) + ctx := api.NewDefaultContext() + key := makeControllerListKey(ctx) + + controller := validController + controller.Labels = map[string]string{"k": "v"} + controller.Name = "bar" + + fakeClient.Data[key] = tools.EtcdResponseWithError{ + R: &etcd.Response{ + Node: &etcd.Node{ + Nodes: []*etcd.Node{ + { + Value: runtime.EncodeOrDie(latest.Codec, &validController), + }, + { + Value: runtime.EncodeOrDie(latest.Codec, &controller), + }, + }, + }, + }, + E: nil, + } + testLabels := labels.SelectorFromSet(labels.Set(controller.Labels)) + objList, err := storage.List(ctx, testLabels, fields.Everything()) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + controllers, _ := objList.(*api.ReplicationControllerList) + if len(controllers.Items) != 1 || controllers.Items[0].Name != controller.Name || + !testLabels.Matches(labels.Set(controllers.Items[0].Labels)) { + t.Errorf("Unexpected controller list: %#v for query with labels %#v", + controllers, testLabels) + } +} + +func TestEtcdWatchController(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + watching, err := storage.Watch(ctx, + labels.Everything(), + fields.Everything(), + "1", + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + fakeClient.WaitForWatchCompletion() + + select { + case _, ok := <-watching.ResultChan(): + if !ok { + t.Errorf("watching channel should be open") + } + default: + } + fakeClient.WatchInjectError <- nil + if _, ok := <-watching.ResultChan(); ok { + t.Errorf("watching channel should be closed") + } + watching.Stop() +} + +func TestEtcdWatchControllersMatch(t *testing.T) { + ctx := api.WithNamespace(api.NewDefaultContext(), validController.Namespace) + storage, fakeClient := newStorage(t) + fakeClient.ExpectNotFoundGet(etcdgeneric.NamespaceKeyRootFunc(ctx, "/registry/pods")) + + watching, err := storage.Watch(ctx, + labels.SelectorFromSet(validController.Spec.Selector), + fields.Everything(), + "1", + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + fakeClient.WaitForWatchCompletion() + + // The watcher above is waiting for these Labels, on receiving them it should + // apply the ControllerStatus decorator, which lists pods, causing a query against + // the /registry/pods endpoint of the etcd client. + controller := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + Labels: validController.Spec.Selector, + Namespace: "default", + }, + } + controllerBytes, _ := latest.Codec.Encode(controller) + fakeClient.WatchResponse <- &etcd.Response{ + Action: "create", + Node: &etcd.Node{ + Value: string(controllerBytes), + }, + } + select { + case _, ok := <-watching.ResultChan(): + if !ok { + t.Errorf("watching channel should be open") + } + case <-time.After(time.Millisecond * 100): + t.Error("unexpected timeout from result channel") + } + watching.Stop() +} + +func TestEtcdWatchControllersNotMatch(t *testing.T) { + ctx := api.NewDefaultContext() + storage, fakeClient := newStorage(t) + fakeClient.ExpectNotFoundGet(etcdgeneric.NamespaceKeyRootFunc(ctx, "/registry/pods")) + + watching, err := storage.Watch(ctx, + labels.SelectorFromSet(labels.Set{"name": "foo"}), + fields.Everything(), + "1", + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + fakeClient.WaitForWatchCompletion() + + controller := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "bar", + Labels: map[string]string{ + "name": "bar", + }, + }, + } + controllerBytes, _ := latest.Codec.Encode(controller) + fakeClient.WatchResponse <- &etcd.Response{ + Action: "create", + Node: &etcd.Node{ + Value: string(controllerBytes), + }, + } + + select { + case <-watching.ResultChan(): + t.Error("unexpected result from result channel") + case <-time.After(time.Millisecond * 100): + // expected case + } +} + +func TestCreate(t *testing.T) { + storage, fakeClient := newStorage(t) + test := resttest.New(t, storage, fakeClient.SetError) + test.TestCreate( + // valid + &api.ReplicationController{ + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"a": "b"}, + Template: &validPodTemplate.Spec, + }, + }, + // invalid + &api.ReplicationController{ + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{}, + Template: &validPodTemplate.Spec, + }, + }, + ) +} + +func TestDelete(t *testing.T) { + storage, fakeClient := newStorage(t) + test := resttest.New(t, storage, fakeClient.SetError) + + createFn := func() runtime.Object { + rc := validController + rc.ResourceVersion = "1" + fakeClient.Data["/registry/controllers/default/foo"] = tools.EtcdResponseWithError{ + R: &etcd.Response{ + Node: &etcd.Node{ + Value: runtime.EncodeOrDie(latest.Codec, &rc), + ModifiedIndex: 1, + }, + }, + } + return &rc + } + gracefulSetFn := func() bool { + // If the controller is still around after trying to delete either the delete + // failed, or we're deleting it gracefully. + if fakeClient.Data["/registry/controllers/default/foo"].R.Node != nil { + return true + } + return false + } + + test.TestDelete(createFn, gracefulSetFn) +} diff --git a/pkg/registry/controller/registry.go b/pkg/registry/controller/registry.go index 3dfa8a0d04bbd..8b5a940e439e6 100644 --- a/pkg/registry/controller/registry.go +++ b/pkg/registry/controller/registry.go @@ -17,7 +17,9 @@ limitations under the License. package controller import ( + "fmt" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" @@ -25,10 +27,66 @@ import ( // Registry is an interface for things that know how to store ReplicationControllers. type Registry interface { - ListControllers(ctx api.Context) (*api.ReplicationControllerList, error) + ListControllers(ctx api.Context, label labels.Selector, field fields.Selector) (*api.ReplicationControllerList, error) WatchControllers(ctx api.Context, label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) GetController(ctx api.Context, controllerID string) (*api.ReplicationController, error) CreateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) UpdateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) DeleteController(ctx api.Context, controllerID string) error } + +// storage puts strong typing around storage calls +type storage struct { + rest.StandardStorage +} + +// NewRegistry returns a new Registry interface for the given Storage. Any mismatched +// types will panic. +func NewRegistry(s rest.StandardStorage) Registry { + return &storage{s} +} + +// List obtains a list of ReplicationControllers that match selector. +func (s *storage) ListControllers(ctx api.Context, label labels.Selector, field fields.Selector) (*api.ReplicationControllerList, error) { + if !field.Empty() { + return nil, fmt.Errorf("field selector not supported yet") + } + obj, err := s.List(ctx, label, field) + if err != nil { + return nil, err + } + return obj.(*api.ReplicationControllerList), err +} + +func (s *storage) WatchControllers(ctx api.Context, label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) { + return s.Watch(ctx, label, field, resourceVersion) +} + +func (s *storage) GetController(ctx api.Context, controllerID string) (*api.ReplicationController, error) { + obj, err := s.Get(ctx, controllerID) + if err != nil { + return nil, err + } + return obj.(*api.ReplicationController), nil +} + +func (s *storage) CreateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) { + obj, err := s.Create(ctx, controller) + if err != nil { + return nil, err + } + return obj.(*api.ReplicationController), nil +} + +func (s *storage) UpdateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) { + obj, _, err := s.Update(ctx, controller) + if err != nil { + return nil, err + } + return obj.(*api.ReplicationController), nil +} + +func (s *storage) DeleteController(ctx api.Context, controllerID string) error { + _, err := s.Delete(ctx, controllerID, nil) + return err +} diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index 365606f6300ba..4ca1e22707d06 100644 --- a/pkg/registry/controller/rest.go +++ b/pkg/registry/controller/rest.go @@ -20,13 +20,12 @@ import ( "fmt" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util/fielderrors" - "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" ) // rcStrategy implements verification logic for Replication Controllers. @@ -66,108 +65,19 @@ func (rcStrategy) ValidateUpdate(obj, old runtime.Object) fielderrors.Validation return validation.ValidateReplicationControllerUpdate(old.(*api.ReplicationController), obj.(*api.ReplicationController)) } -// PodLister is anything that knows how to list pods. -type PodLister interface { - ListPods(ctx api.Context, labels labels.Selector) (*api.PodList, error) -} - -// REST implements rest.Storage for the replication controller service. -type REST struct { - registry Registry - podLister PodLister - strategy rcStrategy -} - -// NewStorage returns a new rest.Storage for the given registry and PodLister. -func NewStorage(registry Registry, podLister PodLister) *REST { - return &REST{ - registry: registry, - podLister: podLister, - strategy: Strategy, - } -} - -// Create registers the given ReplicationController with the system, -// which eventually leads to the controller manager acting on its behalf. -func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) { - controller, ok := obj.(*api.ReplicationController) - if !ok { - return nil, fmt.Errorf("not a replication controller: %#v", obj) - } - - if err := rest.BeforeCreate(rs.strategy, ctx, obj); err != nil { - return nil, err - } - - out, err := rs.registry.CreateController(ctx, controller) - if err != nil { - err = rest.CheckGeneratedNameError(rs.strategy, err, controller) - } - return out, err -} - -// Delete asynchronously deletes the ReplicationController specified by its id. -func (rs *REST) Delete(ctx api.Context, id string) (runtime.Object, error) { - return &api.Status{Status: api.StatusSuccess}, rs.registry.DeleteController(ctx, id) -} - -// Get obtains the ReplicationController specified by its id. -func (rs *REST) Get(ctx api.Context, id string) (runtime.Object, error) { - controller, err := rs.registry.GetController(ctx, id) - if err != nil { - return nil, err - } - return controller, err -} - -// List obtains a list of ReplicationControllers that match selector. -func (rs *REST) List(ctx api.Context, label labels.Selector, field fields.Selector) (runtime.Object, error) { - if !field.Empty() { - return nil, fmt.Errorf("field selector not supported yet") - } - controllers, err := rs.registry.ListControllers(ctx) - if err != nil { - return nil, err - } - filtered := []api.ReplicationController{} - for _, controller := range controllers.Items { - if label.Matches(labels.Set(controller.Labels)) { - filtered = append(filtered, controller) - } - } - controllers.Items = filtered - return controllers, err -} - -// New creates a new ReplicationController for use with Create and Update. -func (*REST) New() runtime.Object { - return &api.ReplicationController{} -} - -func (*REST) NewList() runtime.Object { - return &api.ReplicationControllerList{} -} - -// Update replaces a given ReplicationController instance with an existing -// instance in storage.registry. -func (rs *REST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool, error) { - controller, ok := obj.(*api.ReplicationController) - if !ok { - return nil, false, fmt.Errorf("not a replication controller: %#v", obj) - } - existingController, err := rs.registry.GetController(ctx, controller.Name) - if err != nil { - return nil, false, err - } - if err := rest.BeforeUpdate(rs.strategy, ctx, controller, existingController); err != nil { - return nil, false, err - } - out, err := rs.registry.UpdateController(ctx, controller) - return out, false, err -} - -// Watch returns ReplicationController events via a watch.Interface. -// It implements rest.Watcher. -func (rs *REST) Watch(ctx api.Context, label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) { - return rs.registry.WatchControllers(ctx, label, field, resourceVersion) +// MatchController is the filter used by the generic etcd backend to route +// watch events from etcd to clients of the apiserver only interested in specific +// labels/fields. +func MatchController(label labels.Selector, field fields.Selector) generic.Matcher { + return generic.MatcherFunc( + func(obj runtime.Object) (bool, error) { + if !field.Empty() { + return false, fmt.Errorf("field selector not supported yet") + } + controllerObj, ok := obj.(*api.ReplicationController) + if !ok { + return false, fmt.Errorf("Given object is not a replication controller.") + } + return label.Matches(labels.Set(controllerObj.Labels)), nil + }) } diff --git a/pkg/registry/controller/rest_test.go b/pkg/registry/controller/rest_test.go deleted file mode 100644 index a77e633681b3d..0000000000000 --- a/pkg/registry/controller/rest_test.go +++ /dev/null @@ -1,537 +0,0 @@ -/* -Copyright 2014 Google Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controller - -import ( - "encoding/json" - "fmt" - "io/ioutil" - "reflect" - "strings" - "testing" - - "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest" - _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" - "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" - "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" - "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" -) - -func TestListControllersError(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{ - Err: fmt.Errorf("test error"), - } - storage := REST{ - registry: &mockRegistry, - } - ctx := api.NewContext() - controllers, err := storage.List(ctx, labels.Everything(), fields.Everything()) - if err != mockRegistry.Err { - t.Errorf("Expected %#v, Got %#v", mockRegistry.Err, err) - } - if controllers != nil { - t.Errorf("Unexpected non-nil ctrl list: %#v", controllers) - } -} - -func TestListEmptyControllerList(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{ - Controllers: &api.ReplicationControllerList{ListMeta: api.ListMeta{ResourceVersion: "1"}}, - } - storage := REST{ - registry: &mockRegistry, - } - ctx := api.NewContext() - controllers, err := storage.List(ctx, labels.Everything(), fields.Everything()) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - if len(controllers.(*api.ReplicationControllerList).Items) != 0 { - t.Errorf("Unexpected non-zero ctrl list: %#v", controllers) - } - if controllers.(*api.ReplicationControllerList).ResourceVersion != "1" { - t.Errorf("Unexpected resource version: %#v", controllers) - } -} - -func TestListControllerList(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{ - Controllers: &api.ReplicationControllerList{ - Items: []api.ReplicationController{ - { - ObjectMeta: api.ObjectMeta{ - Name: "foo", - }, - }, - { - ObjectMeta: api.ObjectMeta{ - Name: "bar", - }, - }, - }, - }, - } - storage := REST{ - registry: &mockRegistry, - } - ctx := api.NewContext() - controllersObj, err := storage.List(ctx, labels.Everything(), fields.Everything()) - controllers := controllersObj.(*api.ReplicationControllerList) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - if len(controllers.Items) != 2 { - t.Errorf("Unexpected controller list: %#v", controllers) - } - if controllers.Items[0].Name != "foo" { - t.Errorf("Unexpected controller: %#v", controllers.Items[0]) - } - if controllers.Items[1].Name != "bar" { - t.Errorf("Unexpected controller: %#v", controllers.Items[1]) - } -} - -// TODO: remove, this is sufficiently covered by other tests -func TestControllerDecode(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{} - storage := REST{ - registry: &mockRegistry, - } - controller := &api.ReplicationController{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - }, - Spec: api.ReplicationControllerSpec{ - Template: &api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{ - "name": "nginx", - }, - }, - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyAlways, - DNSPolicy: api.DNSClusterFirst, - }, - }, - }, - } - body, err := latest.Codec.Encode(controller) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - controllerOut := storage.New() - if err := latest.Codec.DecodeInto(body, controllerOut); err != nil { - t.Errorf("unexpected error: %v", err) - } - - if !api.Semantic.DeepEqual(controller, controllerOut) { - t.Errorf("Expected %#v, found %#v", controller, controllerOut) - } -} - -// TODO: this is sufficiently covered by other tetss -func TestControllerParsing(t *testing.T) { - expectedController := api.ReplicationController{ - ObjectMeta: api.ObjectMeta{ - Name: "nginx-controller", - Labels: map[string]string{ - "name": "nginx", - }, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 2, - Selector: map[string]string{ - "name": "nginx", - }, - Template: &api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{ - "name": "nginx", - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Image: "dockerfile/nginx", - Ports: []api.ContainerPort{ - { - ContainerPort: 80, - HostPort: 8080, - }, - }, - }, - }, - }, - }, - }, - } - file, err := ioutil.TempFile("", "controller") - fileName := file.Name() - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - data, err := json.Marshal(expectedController) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - _, err = file.Write(data) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - err = file.Close() - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - data, err = ioutil.ReadFile(fileName) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - var controller api.ReplicationController - err = json.Unmarshal(data, &controller) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - if !api.Semantic.DeepEqual(controller, expectedController) { - t.Errorf("Parsing failed: %s %#v %#v", string(data), controller, expectedController) - } -} - -var validPodTemplate = api.PodTemplate{ - Spec: api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{"a": "b"}, - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "test", - Image: "test_image", - ImagePullPolicy: api.PullIfNotPresent, - }, - }, - RestartPolicy: api.RestartPolicyAlways, - DNSPolicy: api.DNSClusterFirst, - }, - }, -} - -// TODO: remove, this is sufficiently covered by other tests -func TestCreateController(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{} - mockPodRegistry := registrytest.PodRegistry{ - Pods: &api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"a": "b"}, - }, - }, - }, - }, - } - storage := REST{ - registry: &mockRegistry, - podLister: &mockPodRegistry, - strategy: Strategy, - } - controller := &api.ReplicationController{ - ObjectMeta: api.ObjectMeta{Name: "test"}, - Spec: api.ReplicationControllerSpec{ - Replicas: 2, - Selector: map[string]string{"a": "b"}, - Template: &validPodTemplate.Spec, - }, - } - ctx := api.NewDefaultContext() - obj, err := storage.Create(ctx, controller) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - if obj == nil { - t.Errorf("unexpected object") - } - if !api.HasObjectMetaSystemFieldValues(&controller.ObjectMeta) { - t.Errorf("storage did not populate object meta field values") - } - -} - -// TODO: remove, covered by TestCreate -func TestControllerStorageValidatesCreate(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{} - storage := REST{ - registry: &mockRegistry, - podLister: nil, - strategy: Strategy, - } - failureCases := map[string]api.ReplicationController{ - "empty ID": { - ObjectMeta: api.ObjectMeta{Name: ""}, - Spec: api.ReplicationControllerSpec{ - Selector: map[string]string{"bar": "baz"}, - }, - }, - "empty selector": { - ObjectMeta: api.ObjectMeta{Name: "abc"}, - Spec: api.ReplicationControllerSpec{}, - }, - } - ctx := api.NewDefaultContext() - for _, failureCase := range failureCases { - c, err := storage.Create(ctx, &failureCase) - if c != nil { - t.Errorf("Expected nil channel") - } - if !errors.IsInvalid(err) { - t.Errorf("Expected to get an invalid resource error, got %v", err) - } - } -} - -func TestControllerValidatesUpdate(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{ - Controllers: &api.ReplicationControllerList{}, - } - storage := REST{ - registry: &mockRegistry, - podLister: nil, - strategy: Strategy, - } - - var validControllerSpec = api.ReplicationControllerSpec{ - Selector: validPodTemplate.Spec.Labels, - Template: &validPodTemplate.Spec, - } - - var validController = api.ReplicationController{ - ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: validControllerSpec, - } - - ctx := api.NewDefaultContext() - storage.Create(ctx, &validController) - ns := "newNamespace" - - updaters := []func(rc api.ReplicationController) (runtime.Object, bool, error){ - func(rc api.ReplicationController) (runtime.Object, bool, error) { - rc.UID = "newUID" - return storage.Update(ctx, &rc) - }, - func(rc api.ReplicationController) (runtime.Object, bool, error) { - rc.Name = "" - return storage.Update(ctx, &rc) - }, - func(rc api.ReplicationController) (runtime.Object, bool, error) { - rc.Namespace = ns - return storage.Update(api.WithNamespace(ctx, ns), &rc) - }, - func(rc api.ReplicationController) (runtime.Object, bool, error) { - rc.Spec.Selector = map[string]string{} - return storage.Update(ctx, &rc) - }, - } - for _, u := range updaters { - c, updated, err := u(validController) - if c != nil || updated { - t.Errorf("Expected nil object and not created") - } - if !errors.IsInvalid(err) { - t.Errorf("Expected to get an invalid resource error, got %v", err) - } - } - // The update should fail if the namespace on the controller is set to something - // other than the namespace on the given context, even if the namespace on the - // controller is valid. - c, updated, err := storage.Update(api.WithNamespace(ctx, ns), &validController) - if c != nil || updated { - t.Errorf("Expected nil object and not created") - } - errSubString := "namespace" - if err == nil { - t.Errorf("Expected an error, but we didn't get one") - } else if !errors.IsBadRequest(err) || - strings.Index(err.Error(), errSubString) == -1 { - t.Errorf("Expected a Bad Request error with the sub string '%s', got %v", errSubString, err) - } -} - -type fakePodLister struct { - e error - l api.PodList - s labels.Selector -} - -func (f *fakePodLister) ListPods(ctx api.Context, s labels.Selector) (*api.PodList, error) { - f.s = s - return &f.l, f.e -} - -// TODO: remove, covered by TestCreate -func TestCreateControllerWithGeneratedName(t *testing.T) { - storage := NewStorage(®istrytest.ControllerRegistry{}, nil) - controller := &api.ReplicationController{ - ObjectMeta: api.ObjectMeta{ - Namespace: api.NamespaceDefault, - GenerateName: "rc-", - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 2, - Selector: map[string]string{"a": "b"}, - Template: &validPodTemplate.Spec, - }, - } - - ctx := api.NewDefaultContext() - _, err := storage.Create(ctx, controller) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if controller.Name == "rc-" || !strings.HasPrefix(controller.Name, "rc-") { - t.Errorf("unexpected name: %#v", controller) - } -} - -// TODO: remove, covered by TestCreate -func TestCreateControllerWithConflictingNamespace(t *testing.T) { - storage := REST{strategy: Strategy} - controller := &api.ReplicationController{ - ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "not-default"}, - } - - ctx := api.NewDefaultContext() - channel, err := storage.Create(ctx, controller) - if channel != nil { - t.Error("Expected a nil channel, but we got a value") - } - errSubString := "namespace" - if err == nil { - t.Errorf("Expected an error, but we didn't get one") - } else if !errors.IsBadRequest(err) || - strings.Index(err.Error(), errSubString) == -1 { - t.Errorf("Expected a Bad Request error with the sub string '%s', got %v", errSubString, err) - } -} - -func TestCreate(t *testing.T) { - registry := ®istrytest.ControllerRegistry{} - test := resttest.New(t, NewStorage(registry, nil), registry.SetError) - test.TestCreate( - // valid - &api.ReplicationController{ - Spec: api.ReplicationControllerSpec{ - Replicas: 2, - Selector: map[string]string{"a": "b"}, - Template: &validPodTemplate.Spec, - }, - }, - // invalid - &api.ReplicationController{ - Spec: api.ReplicationControllerSpec{ - Replicas: 2, - Selector: map[string]string{}, - Template: &validPodTemplate.Spec, - }, - }, - ) -} - -func TestBeforeCreate(t *testing.T) { - failures := []runtime.Object{ - &api.Service{}, - &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: "#$%%invalid", - }, - }, - &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "##&*(&invalid", - Namespace: api.NamespaceDefault, - }, - }, - } - for _, test := range failures { - ctx := api.NewDefaultContext() - err := rest.BeforeCreate(rest.Services, ctx, test) - if err == nil { - t.Errorf("unexpected non-error for %v", test) - } - } - - obj := &api.ReplicationController{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: api.NamespaceDefault, - }, - Spec: api.ReplicationControllerSpec{ - Selector: map[string]string{"name": "foo"}, - Template: &api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{ - "name": "foo", - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "foo", - Image: "foo", - ImagePullPolicy: api.PullAlways, - }, - }, - RestartPolicy: api.RestartPolicyAlways, - DNSPolicy: api.DNSDefault, - }, - }, - }, - Status: api.ReplicationControllerStatus{ - Replicas: 3, - }, - } - ctx := api.NewDefaultContext() - err := rest.BeforeCreate(Strategy, ctx, obj) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if !reflect.DeepEqual(obj.Status, api.ReplicationControllerStatus{}) { - t.Errorf("status was not cleared as expected.") - } - if obj.Name != "foo" || obj.Namespace != api.NamespaceDefault { - t.Errorf("unexpected object metadata: %v", obj.ObjectMeta) - } - - obj.Spec.Replicas = -1 - if err := rest.BeforeCreate(Strategy, ctx, obj); err == nil { - t.Errorf("unexpected non-error for invalid replication controller.") - } -} diff --git a/pkg/registry/generic/etcd/etcd.go b/pkg/registry/generic/etcd/etcd.go index b852fe159135b..1860fbbf6de33 100644 --- a/pkg/registry/generic/etcd/etcd.go +++ b/pkg/registry/generic/etcd/etcd.go @@ -241,6 +241,7 @@ func (e *Etcd) UpdateWithName(ctx api.Context, name string, obj runtime.Object) // Update performs an atomic update and set of the object. Returns the result of the update // or an error. If the registry allows create-on-update, the create flow will be executed. +// A bool is returned along with the object and any errors, to indicate object creation. func (e *Etcd) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool, error) { name, err := e.ObjectNameFunc(obj) if err != nil { diff --git a/pkg/registry/pod/etcd/etcd_test.go b/pkg/registry/pod/etcd/etcd_test.go index 08010967a72dd..bcfdd38c05f8b 100644 --- a/pkg/registry/pod/etcd/etcd_test.go +++ b/pkg/registry/pod/etcd/etcd_test.go @@ -155,7 +155,7 @@ func TestDelete(t *testing.T) { } return fakeEtcdClient.Data["/registry/pods/default/foo"].R.Node.TTL == 30 } - test.TestDeleteNoGraceful(createFn, gracefulSetFn) + test.TestDelete(createFn, gracefulSetFn) } func expectPod(t *testing.T, out runtime.Object) (*api.Pod, bool) {