[WIP] Uber api server #21190

jianhuiz · 2016-02-12T21:59:40Z

create separate uber api server with cluster api object added
duplicate /cmd/kube-apiserver and /pkg/master/ as suggested

@quinton-hoole, @deepak-vij, @XiaoningDing, @alfred-huangjian
#19313
#20733

k8s-bot · 2016-02-12T22:00:14Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

k8s-bot · 2016-02-12T22:01:14Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

k8s-github-robot · 2016-02-12T22:12:20Z

Labelling this PR as size/XXL

bgrant0607 · 2016-02-12T23:29:12Z

@jianhuiz Reference to proposal or issue?

Please also see:
https://github.com/kubernetes/kubernetes/blob/master/docs/devel/faster_reviews.md

cc @quinton-hoole

bgrant0607 · 2016-02-12T23:31:13Z

Also note that we're frozen while trying to converge the 1.2 release at the moment.

ghost · 2016-02-13T22:05:04Z

@bgrant0607 the proposal is #19313. I can take this one.

bgrant0607 · 2016-02-13T22:50:14Z

Thanks @quinton-hoole. Let me know when you want to discuss the API.

@lavalamp and/or @caesarxuchao may have suggestions about how to factor the apiserver code.

k8s-github-robot · 2016-02-13T23:03:02Z

PR needs rebase

mfanjie · 2016-02-14T05:11:43Z

cmd/uber-apiserver/apiserver.go

+	"runtime"
+	"time"
+
+	"k8s.io/kubernetes/cmd/uber-apiserver/app"


Should you reconsider the package name as ube instead of uber? same pattern with kubernetes -> kube

lavalamp · 2016-02-16T20:19:09Z

I'm swamped today. I will try to look at this tomorrow.

huangyuqi · 2016-03-08T04:08:24Z

Hi, @jianhuiz

In ubernetes's proposal, cluster controller and scheduler will both operate the resource sub-RC. For example, cluster controller watches sub-RC and create corresponding RC in kubernetes cluster.
So, i think the resource sub-RC needs to be added in apiserver like cluster. Do you agree?

Would you please give us some suggestion? @quinton-hoole
Thanks.

jianhuiz · 2016-03-15T23:25:54Z

Regarding the test failure at https://github.com/kubernetes/kubernetes/blob/master/pkg/apimachinery/registered/registered.go#L170)

controlplane/v1alpha1 was not included in KUBE_TEST_API_VERSIONS or KUBE_TEST_API so registered.GroupOrDie panics when trying to get API groups of controlplane/v1alpha1
add controlplane/v1alpha1 to KUBE_TEST_API_VERSIONS and KUBE_TEST_API, then it panics at https://github.com/kubernetes/kubernetes/blob/master/pkg/client/unversioned/import_known_versions.go#L35 since controlplane was not installed
add _ "k8s.io/kubernetes/pkg/apis/controlplane/install" to pkg/client/unversioned/import_known_versions.go, the tests passes.
However it will cause the kube-apiserver description mentioning controlplane/v1alpha1 at https://github.com/kubernetes/kubernetes/blob/master/cmd/kube-apiserver/app/options/options.go#L199 which will also trigger the generation of docs/admin/kube-apiserver.md to include that.

All this happens because that now we have two apiservers but other packages and the scripts are dedicated to work with only the kube-apiserver.

Advises?
@quinton-hoole @lavalamp @caesarxuchao @colhom

lavalamp · 2016-03-15T23:57:38Z

@nikhiljindal

k8s-github-robot · 2016-03-25T07:08:47Z

@jianhuiz PR needs rebase

nikhiljindal · 2016-03-25T21:43:20Z

cmd/ube-apiserver/app/controlplane.go

+		ParameterCodec:         api.ParameterCodec,
+		NegotiatedSerializer:   api.Codecs,
+	}
+	if err := m.InstallAPIGroup(&apiGroupInfo); err != nil {


Call InstallAPIGroups() It does more than just calling InstallAPIGroup.
It also adds swagger support. We can potentially add more stuff there as we move more logic to genericapiserver.

InstallAPIGroups() however can be called only once (due to this swagger support) which had been called in master.New()

Since ube-apiserver were to be compatible with uber-apiserver for most of the APIs, those codes are duplicated from uber-apiserver as suggested. I'd rather not to make much changes but try to keep sync with uber-apiserver while it's in refactoring. I therefore call InstallAPIGroup() instead to avoid installing swagger twice.

I disagree that swagger (as well as other potential stuff) should be put inside InstallAPIGroups() which makes the functions behaviors surprises than they look. It would be better move them to init() like the installGroupsDiscoveryHandler()

We cant move the swagger stuff to init since it has to be called after all api groups have been added.

It can be modified to be re-installable, or lazy installed, or It can be moved to Run()
Rather than having InstallAPIGroups() can be called only once and all APIs have to be installed at that time.

Yes moving it to Run is reasonable.
#23554.
Thanks!

nikhiljindal · 2016-03-25T22:25:55Z

I have added some comments in the code.

This is a huge PR which we should split. Its going to be hard for anyone to understand or review this and going to be hard for you to keep in sync.

I think we can have one PR that just adds the ubernetes server (cmd/ube-apiserver code).
With that PR we should be able to do go run cmd/ube-apiserver/server/main.go which should bring up the server with common endpoints as provided by genericapiserver. It wont have any API group version. This should largely be a simple PR that we should be able to get in pretty soon.
The advantage of doing that first is that once it is submitted, we can work in parallel to remove the code duplication between cmd/ube-apiserver and cmd/kube-apiserver (by moving common code to genericapiserver). I can help you with that.
In another PR, we can add the group and then the API objects. I expect that adding API objects might take the most review time. We can get everything else in while API objects PR is being reviewed rather than holding everything for that (if all code is in one PR).

Does that sound reasonable to you?

nikhiljindal · 2016-03-25T22:31:38Z

cmd/ube-apiserver/app/server.go

+		glog.Fatalf("Failure to start kubelet client: %v", err)
+	}
+
+	apiGroupVersionOverrides, err := parseRuntimeConfig(s)


Just so that you know there is #23067 which is refactoring a few things here

nikhiljindal · 2016-03-25T23:02:01Z

Also, so that you know, there is another option than to duplicate cmd/api-server: We can start with code as in https://github.com/kubernetes/kubernetes/blob/master/examples/apiserver/apiserver.go. That will give us an apiserver to start with. We can add our group and api objects, etc.
And in parallel move more functionality from cmd/kube-apiserver to genericapiserver.

That will save us from keeping the duplicate code in sync (as i pointed there are PRs in flight that are changing that code)

cc @lavalamp

jianhuiz · 2016-03-25T23:13:30Z

@nikhiljindal I personaly wouldn't mind either way, and didn't see either way is better than the other, I just don't want to struggle with the ongoing refactoring and would like to focus on the api objects to support the development of the cluster controler, u7s scheduler, etc. I think I can catch up with the apiserver it self later (when the refactoring of generic apiserver becomes clear)

@nikhiljindal

Automatic merge from submit-queue duplicate kube-apiserver to federated-apiserver duplicate the kube-apiserver source code to ube-apiserver and update references cluster specific api objects will be in separate PRs #19313, #21190 #21190 (comment) @nikhiljindal

@jianhuiz

Automatic merge from submit-queue genericapiserver: Moving InstallSwaggerAPI to Run Ref #21190 (comment) Moving InstallSwaggerAPI() from InstallAPIGroups() to Run(). This allows the use of InstallAPIGroups() multiple times or using InstallAPIGroup() directly. cc @jianhuiz @kubernetes/sig-api-machinery

googlebot added the cla: yes label Feb 12, 2016

k8s-github-robot assigned bgrant0607 Feb 12, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 12, 2016

jianhuiz mentioned this pull request Feb 12, 2016

[WIP] Uber cluster controller #20733

Closed

ghost assigned ghost and unassigned bgrant0607 Feb 13, 2016

bgrant0607 changed the title ~~Uber api server~~ [WIP] Uber api server Feb 13, 2016

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

mfanjie reviewed Feb 14, 2016
View reviewed changes

ghost added this to the next-candidate milestone Feb 18, 2016

ghost added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/cluster-federation labels Feb 18, 2016

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

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

jianhuiz added 6 commits March 25, 2016 09:22

add api group 'controlplane' and 'cluster' object

a9c3088

add cluster registry

96daf43

duplicate kube-apiserver to ube-apiserver and update reference

621f5fd

register controlplane/clusters to ube-apiserver

5c9346c

switch to new deep copy generator

9a5009c

merge util changes

8c6e6dc

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

nikhiljindal reviewed Mar 25, 2016
View reviewed changes

jianhuiz mentioned this pull request Mar 25, 2016

duplicate kube-apiserver to federated-apiserver #23509

Merged

jianhuiz closed this Mar 25, 2016

jianhuiz deleted the uber-api-server branch March 25, 2016 23:41

nikhiljindal mentioned this pull request Mar 28, 2016

genericapiserver: Moving InstallSwaggerAPI to Run #23554

Merged

huangyuqi mentioned this pull request Apr 1, 2016

Cluster Federation: task list #23653

Closed

20 tasks

jianhuiz mentioned this pull request Apr 20, 2016

Federation apiobject cluster #23847

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Uber api server #21190

[WIP] Uber api server #21190

jianhuiz commented Feb 12, 2016

k8s-bot commented Feb 12, 2016

k8s-bot commented Feb 12, 2016

k8s-github-robot commented Feb 12, 2016

bgrant0607 commented Feb 12, 2016

bgrant0607 commented Feb 12, 2016

ghost commented Feb 13, 2016

bgrant0607 commented Feb 13, 2016

k8s-github-robot commented Feb 13, 2016

mfanjie Feb 14, 2016

lavalamp commented Feb 16, 2016

huangyuqi commented Mar 8, 2016

jianhuiz commented Mar 15, 2016

lavalamp commented Mar 15, 2016

k8s-github-robot commented Mar 25, 2016

nikhiljindal Mar 25, 2016

jianhuiz Mar 25, 2016

nikhiljindal Mar 25, 2016

jianhuiz Mar 26, 2016

nikhiljindal Mar 28, 2016

nikhiljindal commented Mar 25, 2016

nikhiljindal Mar 25, 2016

nikhiljindal commented Mar 25, 2016

jianhuiz commented Mar 25, 2016

[WIP] Uber api server #21190

[WIP] Uber api server #21190

Conversation

jianhuiz commented Feb 12, 2016

k8s-bot commented Feb 12, 2016

k8s-bot commented Feb 12, 2016

k8s-github-robot commented Feb 12, 2016

bgrant0607 commented Feb 12, 2016

bgrant0607 commented Feb 12, 2016

ghost commented Feb 13, 2016

bgrant0607 commented Feb 13, 2016

k8s-github-robot commented Feb 13, 2016

mfanjie Feb 14, 2016

Choose a reason for hiding this comment

lavalamp commented Feb 16, 2016

huangyuqi commented Mar 8, 2016

jianhuiz commented Mar 15, 2016

lavalamp commented Mar 15, 2016

k8s-github-robot commented Mar 25, 2016

nikhiljindal Mar 25, 2016

Choose a reason for hiding this comment

jianhuiz Mar 25, 2016

Choose a reason for hiding this comment

nikhiljindal Mar 25, 2016

Choose a reason for hiding this comment

jianhuiz Mar 26, 2016

Choose a reason for hiding this comment

nikhiljindal Mar 28, 2016

Choose a reason for hiding this comment

nikhiljindal commented Mar 25, 2016

nikhiljindal Mar 25, 2016

Choose a reason for hiding this comment

nikhiljindal commented Mar 25, 2016

jianhuiz commented Mar 25, 2016