Add integration tests for kube-apiextensions-server #45721

nikhita · 2017-05-12T11:24:31Z

What this PR does / why we need it: Add the following integration tests for kube-apiextensions-server:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): for #45511

Special notes for your reviewer:

Release note:

NONE

@sttts @deads2k

k8s-ci-robot · 2017-05-12T11:24:39Z

Hi @nikhita. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

sttts · 2017-05-12T11:25:37Z

@k8s-bot ok to test

sttts · 2017-05-12T11:27:31Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/basic_test.go

@@ -186,3 +188,204 @@ func TestSimpleCRUD(t *testing.T) {
 		t.Errorf("missing watch event")
 	}
 }
+
+func TestNonNamespaced(t *testing.T) {


I guess there is quite some overlap with TestSimpleCRUD. Can you try to factor out the equal pieces?

sttts · 2017-05-12T11:29:53Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/testserver/resources.go

+	}
+}
+
+func NewNoxuClusterCustomResourceDefinition() *apiextensionsv1alpha1.CustomResource {


Also here I would merge these two funcs and add scope parameter to NewNoxuCustomResourceDefinition.

sttts · 2017-05-12T13:35:39Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/basic_test.go


 	noxuInstanceToCreate := testserver.NewNoxuInstance(ns, "foo")
-	createdNoxuInstance, err := noxuNamespacedResourceClient.Create(noxuInstanceToCreate)
+	createdNoxuInstance, err := noxuResourceClient.Create(noxuInstanceToCreate)


A check is missing below whether ObjectMeta.Namespace is set correctly.

Also check that resources with the wrong scope are rejected.

And check that creating another resource with another namespace, but same name works or fails (depending on scope).

sttts · 2017-05-12T13:38:12Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/basic_test.go

+
+	s := []string{"shortnoxu", "shortnoxu2"}
+	if !reflect.DeepEqual(r.ShortNames, s) {
+		t.Fatalf("Expected exactly the shortnames \"shortnoxu\", \"shortnoxu2\" in group version %v/%v via discovery, got: %v", group, version, r.ShortNames)


Nit:

`foo bar "abc" def`

is a bit more elegant than quoting.

sttts · 2017-05-12T13:53:01Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/basic_test.go

@@ -186,3 +198,49 @@ func TestSimpleCRUD(t *testing.T) {
 		t.Errorf("missing watch event")
 	}
 }
+func TestDiscovery(t *testing.T) {


sttts · 2017-05-16T13:24:21Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/basic_test.go

+func TestSameNameDiffNamespace(t *testing.T) {
+	scope := apiextensionsv1alpha1.NamespaceScoped
+	ns1 := "namespace-1"
+	testSimpleCRUD(t, scope, ns1)


each of these creates its own server, doesn't it? We have to share it though for this test.

sttts · 2017-05-16T13:24:39Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/basic_test.go

+	}
+
+	createdNoxuInstance, err := instantiateCustomResource(t, testserver.NewNoxuInstance(ns, "foo"), noxuResourceClient, noxuDefinition)
+	if err == nil {


which error do we expect?

sttts · 2017-05-17T06:41:19Z

staging/src/k8s.io/kube-apiextensions-server/test/integration/basic_test.go

+	}
+
+	// Second namespace
+	ns2 := "namespace-2"


Is this much different from namespace-1? Can we merge these two into a (local) func?

* test namespace scoped resources * test cluster scoped resources * test discovery * test no namespace rejects * test same name different namespace

nikhita · 2017-05-17T08:11:00Z

@k8s-bot pull-kubernetes-federation-e2e-gce test this

sttts · 2017-05-17T08:29:55Z

/lgtm
/approve

k8s-github-robot · 2017-05-17T08:30:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhita, sttts

Needs approval from an approver in each of these OWNERS Files:

~~staging/src/k8s.io/kube-apiextensions-server/OWNERS~~ [sttts]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

nikhita · 2017-05-17T08:42:18Z

@k8s-bot pull-kubernetes-federation-e2e-gce test this

k8s-github-robot · 2017-05-17T09:28:48Z

Automatic merge from submit-queue (batch tested with PRs 45771, 45721)

k8s-ci-robot · 2017-05-17T09:48:32Z

@nikhita: The following test(s) failed:

Test name	Commit	Details	Rerun command
pull-kubernetes-federation-e2e-gce	`62421cd`	link	`@k8s-bot pull-kubernetes-federation-e2e-gce test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 12, 2017

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 12, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 12, 2017

sttts reviewed May 12, 2017

View reviewed changes

k8s-github-robot assigned sttts and deads2k May 12, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels May 12, 2017

nikhita force-pushed the kubeapiserver-int-test branch from 07c0f9e to 3a99b21 Compare May 12, 2017 12:52

nikhita mentioned this pull request May 12, 2017

Add integration tests for kube-apiextensions-server nikhita/gsoc-meta-k8s#2

Closed

10 tasks

sttts reviewed May 12, 2017

View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 15, 2017

nikhita force-pushed the kubeapiserver-int-test branch from 3a99b21 to 9e7fc4f Compare May 16, 2017 12:35

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 16, 2017

sttts reviewed May 16, 2017

View reviewed changes

nikhita force-pushed the kubeapiserver-int-test branch from 9e7fc4f to 6f9133a Compare May 16, 2017 15:23

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 16, 2017

sttts reviewed May 17, 2017

View reviewed changes

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 17, 2017

Add integration tests for kube-apiextensions-server

62421cd

* test namespace scoped resources * test cluster scoped resources * test discovery * test no namespace rejects * test same name different namespace

nikhita force-pushed the kubeapiserver-int-test branch from 6f9133a to 62421cd Compare May 17, 2017 07:19

nikhita changed the title ~~[WIP] Add integration tests for kube-apiextensions-server~~ Add integration tests for kube-apiextensions-server May 17, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 17, 2017

k8s-github-robot merged commit 63ec1b2 into kubernetes:master May 17, 2017

nikhita deleted the kubeapiserver-int-test branch May 17, 2017 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add integration tests for kube-apiextensions-server #45721

Add integration tests for kube-apiextensions-server #45721

nikhita commented May 12, 2017 •

edited

k8s-ci-robot commented May 12, 2017

sttts commented May 12, 2017

sttts May 12, 2017

sttts May 12, 2017

sttts May 12, 2017

sttts May 12, 2017

sttts May 12, 2017

sttts May 12, 2017

sttts May 16, 2017

sttts May 16, 2017

sttts May 17, 2017

nikhita commented May 17, 2017

sttts commented May 17, 2017

k8s-github-robot commented May 17, 2017

nikhita commented May 17, 2017

k8s-github-robot commented May 17, 2017

k8s-ci-robot commented May 17, 2017 •

edited

Add integration tests for kube-apiextensions-server #45721

Add integration tests for kube-apiextensions-server #45721

Conversation

nikhita commented May 12, 2017 • edited

k8s-ci-robot commented May 12, 2017

sttts commented May 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikhita commented May 17, 2017

sttts commented May 17, 2017

k8s-github-robot commented May 17, 2017

nikhita commented May 17, 2017

k8s-github-robot commented May 17, 2017

k8s-ci-robot commented May 17, 2017 • edited

nikhita commented May 12, 2017 •

edited

k8s-ci-robot commented May 17, 2017 •

edited