Implement first set of federated service e2e tests. #26636

madhusudancs · 2016-06-01T14:46:12Z

These tests are untested and there is no guarantee that they work. The ongoing auth problems is blocking these e2es from being tested and upon @quinton-hoole's request I am submitting them now.

Only the last commit here needs review.

Depends on #26953

cc @nikhiljindal @colhom @mfanjie @kubernetes/sig-cluster-federation

colhom · 2016-06-01T22:09:58Z

federation/cluster/common.sh

 FEDERATION_IMAGE_REPO_BASE=${FEDERATION_IMAGE_REPO_BASE:-'gcr.io/google_containers'}
+FEDERATION_PUSH_REPO_BASE=${FEDERATION_PUSH_REPO_BASE:-"gcr.io/${PROJECT}"}


wonderful! thanks for doing that.

goltermann · 2016-06-06T17:30:32Z

@madhusudancs When do you think you'll have this one ready for review? We cut beta on June 10.

madhusudancs · 2016-06-07T09:33:20Z

@goltermann it is ready now.

colhom · 2016-06-07T15:19:51Z

@madhusudancs e2e is failing due to #23545

madhusudancs · 2016-06-07T19:23:14Z

@colhom Thanks for pointing it out.

ghost · 2016-06-08T20:12:35Z

test/e2e/federated-service.go

+		}
+
+		By("Obtaining a list of all the clusters")
+		clusterList, err := f.FederationClientset.Federation().Clusters().List(api.ListOptions{})


I think you need a check here that the number of clusters matches the number of contexts above, with a few retries on the list operation, given that the cluster create call is theoretically asynchronous.

Also, given that you register/create cluster objects in BeforeEach() (i.e. before every test, i.e. before every It() clause), you also need to unregister/delete the cluster objects after every test (i.e. in AfterEach()). Either that, or you need to register the clusters once before all the tests, and delete them once after all the tests (i.e in BeforeSuite() and AfterSuite()) See https://onsi.github.io/ginkgo/#global-setup-and-teardown-beforesuite-and-aftersuite

Done. Didn't put it in Before/AfterSuite because that would run the setup for all the tests.

madhusudancs · 2016-06-13T12:45:26Z

@quinton-hoole Addressed the comments, PTAL.

nikhiljindal · 2016-06-13T17:43:10Z

Thanks @madhusudancs
Can you please rebase now that #26636 is merged.

cc @mml who was trying to patch and run this.

@mml or @madhusudancs Have you guys been able to verify that these pass?

madhusudancs · 2016-06-13T17:58:17Z

Can you please rebase now that #26636 is merged.

@nikhiljindal Which PR did you mean? This seems like a self-recursion :)

nikhiljindal · 2016-06-13T18:00:27Z

Sorry I meant #26953.
The first commit in this PR is already submitted right?

madhusudancs · 2016-06-13T18:15:13Z

Ah Ok. Thanks. Yes, I will rebase on top of that PR. It shouldn't affect trying this PR though, since that commit is included here.

I haven't been able to verify that the tests pass. I am trying to bring up a new federation based on the recent merges.

@quinton-hoole

Note that these tests are untested and there is no guarantee that they work. The ongoing auth problems is blocking these e2es from being tested and upon @quinton-hoole's request I am submitting them now.

k8s-bot · 2016-06-13T19:22:29Z

GCE e2e build/test passed for commit 839d98a.

ghost · 2016-06-13T20:18:23Z

test/e2e/federated-service.go

+// service reaches the expected value, i.e. numSvcs in the given individual Kubernetes
+// cluster. If the shard count, i.e. numSvcs is expected to be at least one, then
+// it also checks if the first shard's name and spec matches that of the given service.
+func waitForFederatedServiceShard(cs *release_1_3.Clientset, namespace string, service *api.Service, numSvcs int) {


nit: I don't understand how one could ever have more than 1 instance of the service in a given cluster at the same time. There would be a name clash, right? In which case, is the numSvcs parameter essentially a boolean? Would it ever make sense to call this function with that parameter not equal to either 0 or 1?

ghost · 2016-06-13T21:17:29Z

Thanks @madhusudancs . All my remaining comments can be addressed in a followup PR. Please do. LGTM for this PR.

nikhiljindal · 2016-06-13T21:58:38Z

test/e2e/federated-service.go

+		framework.SkipUnlessFederated(f.Client)
+
+		// TODO: Federation API server should be able to answer this.
+		if federationName = os.Getenv("FEDERATION_NAME"); federationName == "" {


No one is setting this env var anywhere?
So the tests right now will definitely fail?

Yeah, I wasn't expecting this PR to merge before waking up.

I have #27333 to fix this

k8s-github-robot · 2016-06-13T22:00:17Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-06-13T23:03:08Z

GCE e2e build/test passed for commit 839d98a.

k8s-github-robot · 2016-06-13T23:03:34Z

Automatic merge from submit-queue

googlebot added the cla: yes label Jun 1, 2016

madhusudancs assigned ghost Jun 1, 2016

madhusudancs added this to the v1.3 milestone Jun 1, 2016

madhusudancs added the area/cluster-federation label Jun 1, 2016

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Jun 1, 2016

colhom reviewed Jun 1, 2016
View reviewed changes

madhusudancs force-pushed the fed-svc-e2e branch from 3f4416a to 2bd9974 Compare June 1, 2016 22:25

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

madhusudancs force-pushed the fed-svc-e2e branch 2 times, most recently from c13b2ca to 2f3e978 Compare June 2, 2016 20:23

madhusudancs changed the title ~~[WIP] Federated Service Controller e2e~~ Implement first set of federated service e2e tests. Jun 7, 2016

madhusudancs assigned nikhiljindal Jun 7, 2016

madhusudancs force-pushed the fed-svc-e2e branch from 2f3e978 to 1988dbf Compare June 7, 2016 09:40

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

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

ghost reviewed Jun 8, 2016
View reviewed changes

madhusudancs added 2 commits June 13, 2016 11:17

Implement first set of federated service e2e tests.

c4dcb42

Note that these tests are untested and there is no guarantee that they work. The ongoing auth problems is blocking these e2es from being tested and upon @quinton-hoole's request I am submitting them now.

Address review comments.

839d98a

madhusudancs force-pushed the fed-svc-e2e branch from 748cd7c to 839d98a Compare June 13, 2016 18:18

ghost added the team/control-plane label Jun 13, 2016

nikhiljindal mentioned this pull request Jun 13, 2016

federation: fix dns provider initialization issues #27252

Merged

nikhiljindal added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jun 13, 2016

nikhiljindal mentioned this pull request Jun 13, 2016

Cluster Federation: task list #23653

Closed

20 tasks

ghost reviewed Jun 13, 2016
View reviewed changes

ghost added lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jun 13, 2016

nikhiljindal reviewed Jun 13, 2016
View reviewed changes

k8s-github-robot merged commit 239f627 into kubernetes:master Jun 13, 2016

ghost mentioned this pull request Jun 15, 2016

federation-apiserver test: Cleaning up clusters after the test is run #27407

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement first set of federated service e2e tests. #26636

Implement first set of federated service e2e tests. #26636

madhusudancs commented Jun 1, 2016 •

edited

colhom Jun 1, 2016

goltermann commented Jun 6, 2016

madhusudancs commented Jun 7, 2016

colhom commented Jun 7, 2016

madhusudancs commented Jun 7, 2016

ghost Jun 8, 2016

ghost Jun 8, 2016

madhusudancs Jun 13, 2016

madhusudancs commented Jun 13, 2016

nikhiljindal commented Jun 13, 2016

madhusudancs commented Jun 13, 2016

nikhiljindal commented Jun 13, 2016

madhusudancs commented Jun 13, 2016

k8s-bot commented Jun 13, 2016

ghost Jun 13, 2016

ghost commented Jun 13, 2016

nikhiljindal Jun 13, 2016

madhusudancs Jun 14, 2016

nikhiljindal Jun 14, 2016

madhusudancs Jun 14, 2016

k8s-github-robot commented Jun 13, 2016

k8s-bot commented Jun 13, 2016

k8s-github-robot commented Jun 13, 2016

		FEDERATION_IMAGE_REPO_BASE=${FEDERATION_IMAGE_REPO_BASE:-'gcr.io/google_containers'}
		FEDERATION_PUSH_REPO_BASE=${FEDERATION_PUSH_REPO_BASE:-"gcr.io/${PROJECT}"}

Implement first set of federated service e2e tests. #26636

Implement first set of federated service e2e tests. #26636

Conversation

madhusudancs commented Jun 1, 2016 • edited

Choose a reason for hiding this comment

goltermann commented Jun 6, 2016

madhusudancs commented Jun 7, 2016

colhom commented Jun 7, 2016

madhusudancs commented Jun 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madhusudancs commented Jun 13, 2016

nikhiljindal commented Jun 13, 2016

madhusudancs commented Jun 13, 2016

nikhiljindal commented Jun 13, 2016

madhusudancs commented Jun 13, 2016

k8s-bot commented Jun 13, 2016

Choose a reason for hiding this comment

ghost commented Jun 13, 2016

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

k8s-github-robot commented Jun 13, 2016

k8s-bot commented Jun 13, 2016

k8s-github-robot commented Jun 13, 2016

madhusudancs commented Jun 1, 2016 •

edited