diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go index 106237ddc92..f8c1716ef6a 100644 --- a/pkg/api/rest/resttest/resttest.go +++ b/pkg/api/rest/resttest/resttest.go @@ -91,39 +91,99 @@ func copyOrDie(obj runtime.Object) runtime.Object { return out } +// Test creating an object. func (t *Tester) TestCreate(valid runtime.Object, invalid ...runtime.Object) { - t.TestCreateHasMetadata(copyOrDie(valid)) - t.TestCreateGeneratesName(copyOrDie(valid)) - t.TestCreateGeneratesNameReturnsServerTimeout(copyOrDie(valid)) + t.testCreateHasMetadata(copyOrDie(valid)) + t.testCreateGeneratesName(copyOrDie(valid)) + t.testCreateGeneratesNameReturnsServerTimeout(copyOrDie(valid)) if t.clusterScope { - t.TestCreateDiscardsObjectNamespace(copyOrDie(valid)) - t.TestCreateIgnoresContextNamespace(copyOrDie(valid)) - t.TestCreateIgnoresMismatchedNamespace(copyOrDie(valid)) + t.testCreateDiscardsObjectNamespace(copyOrDie(valid)) + t.testCreateIgnoresContextNamespace(copyOrDie(valid)) + t.testCreateIgnoresMismatchedNamespace(copyOrDie(valid)) } else { - t.TestCreateRejectsMismatchedNamespace(copyOrDie(valid)) + t.testCreateRejectsMismatchedNamespace(copyOrDie(valid)) } - t.TestCreateInvokesValidation(invalid...) + t.testCreateInvokesValidation(invalid...) } -func (t *Tester) TestCreateResetsUserData(valid runtime.Object) { - objectMeta := t.getObjectMetaOrFail(valid) - now := util.Now() - objectMeta.UID = "bad-uid" - objectMeta.CreationTimestamp = now +// Test updating an object. +func (t *Tester) TestUpdate(valid runtime.Object, existing, older runtime.Object) { + t.testUpdateFailsOnNotFound(copyOrDie(valid)) + t.testUpdateFailsOnVersion(copyOrDie(older)) +} - obj, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) +// Test deleting an object. +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. +} + +// Test graceful deletion. +func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) { + t.testDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn) + t.testDeleteGracefulUsesZeroOnNil(createFn(), 0) +} + +// Test getting object. +func (t *Tester) TestGet(obj runtime.Object) { + t.testGetFound(obj) + t.testGetNotFound(obj) + t.testGetMimatchedNamespace(obj) + if !t.clusterScope { + t.testGetDifferentNamespace(obj) + } +} + +// ============================================================================= +// Creation tests. + +func (t *Tester) testCreateDiscardsObjectNamespace(valid runtime.Object) { + objectMeta := t.getObjectMetaOrFail(valid) + + // Ignore non-empty namespace in object meta + objectMeta.Namespace = "not-default" + + // Ideally, we'd get an error back here, but at least verify the namespace wasn't persisted + created, err := t.storage.(rest.Creater).Create(t.TestContext(), copyOrDie(valid)) if err != nil { t.Fatalf("Unexpected error: %v", err) } - if obj == nil { - t.Fatalf("Unexpected object from result: %#v", obj) - } - if objectMeta.UID == "bad-uid" || objectMeta.CreationTimestamp == now { - t.Errorf("ObjectMeta did not reset basic fields: %#v", objectMeta) + createdObjectMeta := t.getObjectMetaOrFail(created) + if createdObjectMeta.Namespace != api.NamespaceNone { + t.Errorf("Expected empty namespace on created object, got '%v'", createdObjectMeta.Namespace) } } -func (t *Tester) TestCreateHasMetadata(valid runtime.Object) { +func (t *Tester) testCreateGeneratesName(valid runtime.Object) { + objectMeta := t.getObjectMetaOrFail(valid) + objectMeta.Name = "" + objectMeta.GenerateName = "test-" + + _, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if objectMeta.Name == "test-" || !strings.HasPrefix(objectMeta.Name, "test-") { + t.Errorf("unexpected name: %#v", valid) + } +} + +func (t *Tester) testCreateGeneratesNameReturnsServerTimeout(valid runtime.Object) { + objectMeta := t.getObjectMetaOrFail(valid) + objectMeta.Name = "" + objectMeta.GenerateName = "test-" + t.withStorageError(errors.NewAlreadyExists("kind", "thing"), func() { + _, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) + if err == nil || !errors.IsServerTimeout(err) { + t.Fatalf("Unexpected error: %v", err) + } + }) +} + +func (t *Tester) testCreateHasMetadata(valid runtime.Object) { objectMeta := t.getObjectMetaOrFail(valid) objectMeta.Name = "" objectMeta.GenerateName = "test-" @@ -141,72 +201,7 @@ func (t *Tester) TestCreateHasMetadata(valid runtime.Object) { } } -func (t *Tester) TestCreateGeneratesName(valid runtime.Object) { - objectMeta := t.getObjectMetaOrFail(valid) - objectMeta.Name = "" - objectMeta.GenerateName = "test-" - - _, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - if objectMeta.Name == "test-" || !strings.HasPrefix(objectMeta.Name, "test-") { - t.Errorf("unexpected name: %#v", valid) - } -} - -func (t *Tester) TestCreateGeneratesNameReturnsServerTimeout(valid runtime.Object) { - objectMeta := t.getObjectMetaOrFail(valid) - objectMeta.Name = "" - objectMeta.GenerateName = "test-" - t.withStorageError(errors.NewAlreadyExists("kind", "thing"), func() { - _, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) - if err == nil || !errors.IsServerTimeout(err) { - t.Fatalf("Unexpected error: %v", err) - } - }) -} - -func (t *Tester) TestCreateInvokesValidation(invalid ...runtime.Object) { - for i, obj := range invalid { - ctx := t.TestContext() - _, err := t.storage.(rest.Creater).Create(ctx, obj) - if !errors.IsInvalid(err) { - t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err) - } - } -} - -func (t *Tester) TestCreateRejectsMismatchedNamespace(valid runtime.Object) { - objectMeta := t.getObjectMetaOrFail(valid) - objectMeta.Namespace = "not-default" - - _, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) - if err == nil { - t.Errorf("Expected an error, but we didn't get one") - } else if !strings.Contains(err.Error(), "does not match the namespace sent on the request") { - t.Errorf("Expected 'does not match the namespace sent on the request' error, got '%v'", err.Error()) - } -} - -func (t *Tester) TestCreateDiscardsObjectNamespace(valid runtime.Object) { - objectMeta := t.getObjectMetaOrFail(valid) - - // Ignore non-empty namespace in object meta - objectMeta.Namespace = "not-default" - - // Ideally, we'd get an error back here, but at least verify the namespace wasn't persisted - created, err := t.storage.(rest.Creater).Create(t.TestContext(), copyOrDie(valid)) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - createdObjectMeta := t.getObjectMetaOrFail(created) - if createdObjectMeta.Namespace != api.NamespaceNone { - t.Errorf("Expected empty namespace on created object, got '%v'", createdObjectMeta.Namespace) - } -} - -func (t *Tester) TestCreateIgnoresContextNamespace(valid runtime.Object) { +func (t *Tester) testCreateIgnoresContextNamespace(valid runtime.Object) { // Ignore non-empty namespace in context ctx := api.WithNamespace(api.NewContext(), "not-default2") @@ -221,7 +216,7 @@ func (t *Tester) TestCreateIgnoresContextNamespace(valid runtime.Object) { } } -func (t *Tester) TestCreateIgnoresMismatchedNamespace(valid runtime.Object) { +func (t *Tester) testCreateIgnoresMismatchedNamespace(valid runtime.Object) { objectMeta := t.getObjectMetaOrFail(valid) // Ignore non-empty namespace in object meta @@ -239,12 +234,50 @@ func (t *Tester) TestCreateIgnoresMismatchedNamespace(valid runtime.Object) { } } -func (t *Tester) TestUpdate(valid runtime.Object, existing, older runtime.Object) { - t.TestUpdateFailsOnNotFound(copyOrDie(valid)) - t.TestUpdateFailsOnVersion(copyOrDie(older)) +func (t *Tester) testCreateInvokesValidation(invalid ...runtime.Object) { + for i, obj := range invalid { + ctx := t.TestContext() + _, err := t.storage.(rest.Creater).Create(ctx, obj) + if !errors.IsInvalid(err) { + t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err) + } + } } -func (t *Tester) TestUpdateFailsOnNotFound(valid runtime.Object) { +func (t *Tester) testCreateRejectsMismatchedNamespace(valid runtime.Object) { + objectMeta := t.getObjectMetaOrFail(valid) + objectMeta.Namespace = "not-default" + + _, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) + if err == nil { + t.Errorf("Expected an error, but we didn't get one") + } else if !strings.Contains(err.Error(), "does not match the namespace sent on the request") { + t.Errorf("Expected 'does not match the namespace sent on the request' error, got '%v'", err.Error()) + } +} + +func (t *Tester) testCreateResetsUserData(valid runtime.Object) { + objectMeta := t.getObjectMetaOrFail(valid) + now := util.Now() + objectMeta.UID = "bad-uid" + objectMeta.CreationTimestamp = now + + obj, err := t.storage.(rest.Creater).Create(t.TestContext(), valid) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if obj == nil { + t.Fatalf("Unexpected object from result: %#v", obj) + } + if objectMeta.UID == "bad-uid" || objectMeta.CreationTimestamp == now { + t.Errorf("ObjectMeta did not reset basic fields: %#v", objectMeta) + } +} + +// ============================================================================= +// Update tests. + +func (t *Tester) testUpdateFailsOnNotFound(valid runtime.Object) { _, _, err := t.storage.(rest.Updater).Update(t.TestContext(), valid) if err == nil { t.Errorf("Expected an error, but we didn't get one") @@ -253,7 +286,7 @@ func (t *Tester) TestUpdateFailsOnNotFound(valid runtime.Object) { } } -func (t *Tester) TestUpdateFailsOnVersion(older runtime.Object) { +func (t *Tester) testUpdateFailsOnVersion(older runtime.Object) { _, _, err := t.storage.(rest.Updater).Update(t.TestContext(), older) if err == nil { t.Errorf("Expected an error, but we didn't get one") @@ -262,7 +295,10 @@ func (t *Tester) TestUpdateFailsOnVersion(older runtime.Object) { } } -func (t *Tester) TestDeleteInvokesValidation(invalid ...runtime.Object) { +// ============================================================================= +// Deletion tests. + +func (t *Tester) testDeleteInvokesValidation(invalid ...runtime.Object) { for i, obj := range invalid { objectMeta := t.getObjectMetaOrFail(obj) ctx := t.TestContext() @@ -273,15 +309,7 @@ func (t *Tester) TestDeleteInvokesValidation(invalid ...runtime.Object) { } } -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) { +func (t *Tester) testDeleteNonExist(createFn func() runtime.Object) { existing := createFn() objectMeta := t.getObjectMetaOrFail(existing) context := t.TestContext() @@ -294,12 +322,7 @@ func (t *Tester) TestDeleteNonExist(createFn func() runtime.Object) { }) } -func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) { - t.TestDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn) - t.TestDeleteGracefulUsesZeroOnNil(createFn(), 0) -} - -func (t *Tester) TestDeleteNoGraceful(createFn func() runtime.Object, wasGracefulFn func() bool) { +func (t *Tester) testDeleteNoGraceful(createFn func() runtime.Object, wasGracefulFn func() bool) { existing := createFn() objectMeta := t.getObjectMetaOrFail(existing) ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace) @@ -315,7 +338,10 @@ func (t *Tester) TestDeleteNoGraceful(createFn func() runtime.Object, wasGracefu } } -func (t *Tester) TestDeleteGracefulHasDefault(existing runtime.Object, expectedGrace int64, wasGracefulFn func() bool) { +// ============================================================================= +// Graceful Deletion tests. + +func (t *Tester) testDeleteGracefulHasDefault(existing runtime.Object, expectedGrace int64, wasGracefulFn func() bool) { objectMeta := t.getObjectMetaOrFail(existing) ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace) _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, &api.DeleteOptions{}) @@ -330,7 +356,7 @@ func (t *Tester) TestDeleteGracefulHasDefault(existing runtime.Object, expectedG } } -func (t *Tester) TestDeleteGracefulUsesZeroOnNil(existing runtime.Object, expectedGrace int64) { +func (t *Tester) testDeleteGracefulUsesZeroOnNil(existing runtime.Object, expectedGrace int64) { objectMeta := t.getObjectMetaOrFail(existing) ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace) _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil) @@ -342,68 +368,11 @@ func (t *Tester) TestDeleteGracefulUsesZeroOnNil(existing runtime.Object, expect } } -func (t *Tester) TestGetFound(obj runtime.Object) { - ctx := t.TestContext() - objMeta := t.getObjectMetaOrFail(obj) - objMeta.Name = "foo1" - objMeta.Namespace = api.NamespaceValue(ctx) +// ============================================================================= +// Get tests. - existing, err := t.storage.(rest.Creater).Create(ctx, obj) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - existingMeta := t.getObjectMetaOrFail(existing) - - got, err := t.storage.(rest.Getter).Get(ctx, "foo1") - if err != nil { - t.Errorf("unexpected error: %v", err) - } - gotMeta := t.getObjectMetaOrFail(got) - gotMeta.ResourceVersion = existingMeta.ResourceVersion - if e, a := existing, got; !api.Semantic.DeepEqual(e, a) { - t.Errorf("unexpected obj: %#v, expected %#v", e, a) - } -} - -func (t *Tester) TestGetNotFound(obj runtime.Object) { - ctx := t.TestContext() - objMeta := t.getObjectMetaOrFail(obj) - objMeta.Name = "foo2" - objMeta.Namespace = api.NamespaceValue(ctx) - _, err := t.storage.(rest.Creater).Create(ctx, obj) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - _, err = t.storage.(rest.Getter).Get(ctx, "foo3") - if !errors.IsNotFound(err) { - t.Errorf("unexpected error returned: %#v", err) - } -} - -func (t *Tester) TestGetMimatchedNamespace(obj runtime.Object) { - ctx1 := api.WithNamespace(api.NewContext(), "bar1") - ctx2 := api.WithNamespace(api.NewContext(), "bar2") - objMeta := t.getObjectMetaOrFail(obj) - objMeta.Name = "foo4" - objMeta.Namespace = api.NamespaceValue(ctx1) - _, err := t.storage.(rest.Creater).Create(ctx1, obj) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - _, err = t.storage.(rest.Getter).Get(ctx2, "foo4") - if t.clusterScope { - if err != nil { - t.Errorf("unexpected error: %v", err) - } - } else { - if !errors.IsNotFound(err) { - t.Errorf("unexpected error returned: %#v", err) - } - } -} - -// TestGetDifferentNamespace ensures same-name objects in different namespaces do not clash -func (t *Tester) TestGetDifferentNamespace(obj runtime.Object) { +// testGetDifferentNamespace ensures same-name objects in different namespaces do not clash +func (t *Tester) testGetDifferentNamespace(obj runtime.Object) { if t.clusterScope { t.Fatalf("the test does not work in in cluster-scope") } @@ -450,11 +419,62 @@ func (t *Tester) TestGetDifferentNamespace(obj runtime.Object) { } } -func (t *Tester) TestGet(obj runtime.Object) { - t.TestGetFound(obj) - t.TestGetNotFound(obj) - t.TestGetMimatchedNamespace(obj) - if !t.clusterScope { - t.TestGetDifferentNamespace(obj) +func (t *Tester) testGetFound(obj runtime.Object) { + ctx := t.TestContext() + objMeta := t.getObjectMetaOrFail(obj) + objMeta.Name = "foo1" + objMeta.Namespace = api.NamespaceValue(ctx) + + existing, err := t.storage.(rest.Creater).Create(ctx, obj) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + existingMeta := t.getObjectMetaOrFail(existing) + + got, err := t.storage.(rest.Getter).Get(ctx, "foo1") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + gotMeta := t.getObjectMetaOrFail(got) + gotMeta.ResourceVersion = existingMeta.ResourceVersion + if e, a := existing, got; !api.Semantic.DeepEqual(e, a) { + t.Errorf("unexpected obj: %#v, expected %#v", e, a) + } +} + +func (t *Tester) testGetMimatchedNamespace(obj runtime.Object) { + ctx1 := api.WithNamespace(api.NewContext(), "bar1") + ctx2 := api.WithNamespace(api.NewContext(), "bar2") + objMeta := t.getObjectMetaOrFail(obj) + objMeta.Name = "foo4" + objMeta.Namespace = api.NamespaceValue(ctx1) + _, err := t.storage.(rest.Creater).Create(ctx1, obj) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + _, err = t.storage.(rest.Getter).Get(ctx2, "foo4") + if t.clusterScope { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } else { + if !errors.IsNotFound(err) { + t.Errorf("unexpected error returned: %#v", err) + } + } +} + +func (t *Tester) testGetNotFound(obj runtime.Object) { + ctx := t.TestContext() + objMeta := t.getObjectMetaOrFail(obj) + objMeta.Name = "foo2" + objMeta.Namespace = api.NamespaceValue(ctx) + _, err := t.storage.(rest.Creater).Create(ctx, obj) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + _, err = t.storage.(rest.Getter).Get(ctx, "foo3") + if !errors.IsNotFound(err) { + t.Errorf("unexpected error returned: %#v", err) } } diff --git a/pkg/registry/endpoint/etcd/etcd_test.go b/pkg/registry/endpoint/etcd/etcd_test.go index 63a7b2ec6ce..dc521545e79 100644 --- a/pkg/registry/endpoint/etcd/etcd_test.go +++ b/pkg/registry/endpoint/etcd/etcd_test.go @@ -110,7 +110,7 @@ func TestDelete(t *testing.T) { } return fakeEtcdClient.Data[key].R.Node.TTL == 30 } - test.TestDeleteNoGraceful(createFn, gracefulSetFn) + test.TestDelete(createFn, gracefulSetFn) } func TestEtcdListEndpoints(t *testing.T) { diff --git a/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go b/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go index 8a114b83a3b..57ae83d30b1 100644 --- a/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go +++ b/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go @@ -133,7 +133,7 @@ func TestDelete(t *testing.T) { } return fakeEtcdClient.Data[key].R.Node.TTL == 30 } - test.TestDeleteNoGraceful(createFn, gracefulSetFn) + test.TestDelete(createFn, gracefulSetFn) } func TestGet(t *testing.T) { diff --git a/pkg/registry/minion/etcd/etcd_test.go b/pkg/registry/minion/etcd/etcd_test.go index d5180ee9a07..fcaed9bd7a3 100644 --- a/pkg/registry/minion/etcd/etcd_test.go +++ b/pkg/registry/minion/etcd/etcd_test.go @@ -127,7 +127,7 @@ func TestDelete(t *testing.T) { } return fakeEtcdClient.Data[key].R.Node.TTL == 30 } - test.TestDeleteNoGraceful(createFn, gracefulSetFn) + test.TestDelete(createFn, gracefulSetFn) } func TestEtcdListNodes(t *testing.T) { diff --git a/pkg/registry/persistentvolume/etcd/etcd_test.go b/pkg/registry/persistentvolume/etcd/etcd_test.go index b8dc5f0626f..07da7b165b8 100644 --- a/pkg/registry/persistentvolume/etcd/etcd_test.go +++ b/pkg/registry/persistentvolume/etcd/etcd_test.go @@ -113,7 +113,7 @@ func TestDelete(t *testing.T) { } return fakeEtcdClient.Data[key].R.Node.TTL == 30 } - test.TestDeleteNoGraceful(createFn, gracefulSetFn) + test.TestDelete(createFn, gracefulSetFn) } func TestEtcdListPersistentVolumes(t *testing.T) { diff --git a/pkg/registry/persistentvolumeclaim/etcd/etcd_test.go b/pkg/registry/persistentvolumeclaim/etcd/etcd_test.go index aa7488ad951..e6fecd389e5 100644 --- a/pkg/registry/persistentvolumeclaim/etcd/etcd_test.go +++ b/pkg/registry/persistentvolumeclaim/etcd/etcd_test.go @@ -111,7 +111,7 @@ func TestDelete(t *testing.T) { } return fakeEtcdClient.Data[key].R.Node.TTL == 30 } - test.TestDeleteNoGraceful(createFn, gracefulSetFn) + test.TestDelete(createFn, gracefulSetFn) } func TestEtcdListPersistentVolumeClaims(t *testing.T) {