cleanup package `registered` and `install` #18925

caesarxuchao · 2015-12-19T05:42:09Z

Some highlights of the PR:

in package registered, I remove the hard-coded list of valid versions and the hard-coded list of versions that are supported by default
The install package in each group initiate the group installation process. It depends on the registered package to check if a version should be enabled and keep records of enabled groups. The init() function in the register package in each version is changed to an exported function named Register(), and install will call it to add the version to the Scheme.
The import_known_versions.go in each binary imports the install package and calls a function exported by the registered package to check if all versions specified in KUBE_API_VERSIONS are installed.
[edit] The order of versions in KUBE_API_VERSIONS is not significant anymore. The preference of versions is specified in availableVersions in each install package (https://github.com/kubernetes/kubernetes/blob/master/pkg/api/install/install.go#L42).

Note this PR doesn't implement exactly what I wrote in #18531. Specifically this PR doesn't distinguish registered versions from enabled versions, because I find it's useless to have versions that are registered but not enabled.

cc @nikhiljindal

k8s-github-robot · 2015-12-19T05:44:37Z

Labelling this PR as size/L

k8s-bot · 2015-12-19T06:16:00Z

GCE e2e build/test failed for commit 489eb9c678f62c957fb27c2772054105ff7c3a3c.

k8s-bot · 2015-12-19T06:27:57Z

GCE e2e test build/test passed for commit e67fe849a91e3b9afb381609c78c8c75c2aac88b.

k8s-bot · 2015-12-19T06:52:42Z

GCE e2e test build/test passed for commit f1fb4675f9d19c63bf1e6a9e86aa48d203c50bbd.

k8s-bot · 2015-12-21T00:41:27Z

GCE e2e test build/test passed for commit 9b98872d6a8e749f7d2d0dc77b8d59e40c54528b.

k8s-bot · 2015-12-21T00:50:57Z

GCE e2e test build/test passed for commit 7799c19a413cf99a5b28efa6f9d79bef035f22d1.

deads2k · 2015-12-21T13:00:41Z

pkg/api/registered/registered.go

+// IsEnvReuqestedVersion returns if the version is requested via the
+// KUBE_API_VERSIONS environment variable. If the environment variable is empty,
+// then it always returns true.
+func IsEnvReuqestedVersion(v unversioned.GroupVersion) bool {


IsEnvExcludedVersion to make the "no env set" case be more expected?

But I'm afraid that will lead people to think the versions listed in the Env are the excluded versions. I think explain the "no env set" case in the comment is clearer.

deads2k · 2015-12-21T13:18:02Z

I like this direction a lot. I think making the distinction between enabledVersions and registeredVersions here will help a lot since you're ripping up those methods now.

deads2k · 2015-12-21T20:20:14Z

pkg/api/registered/registered.go

+// IsEnvReuqestedVersion returns if the version is requested via the
+// KUBE_API_VERSIONS environment variable. If the environment variable is empty,
+// then it always returns true.
+func IsEnvReuqestedVersion(v unversioned.GroupVersion) bool {


But I'm afraid that will lead people to think the versions listed in the Env are the excluded versions. I think explain the "no env set" case in the comment is clearer.

Ok, how about IsAllowedVersion? Also, the name is currently typoed: Requested

I'll change to IsAllowedVersion. Thanks.

k8s-bot · 2015-12-21T20:38:08Z

GCE e2e test build/test passed for commit eb801a83665f66bef78f981e9559f55461219932.

caesarxuchao · 2015-12-21T20:46:51Z

docs/admin/kube-apiserver.md

@@ -99,14 +99,14 @@ kube-apiserver
      --service-node-port-range=: A port range to reserve for services with NodePort visibility.  Example: '30000-32767'.  Inclusive at both ends of the range.
      --ssh-keyfile="": If non-empty, use secure SSH proxy to the nodes, using this user keyfile
      --ssh-user="": If non-empty, use secure SSH proxy to the nodes, using this user name
-      --storage-versions="componentconfig/v1alpha1,extensions/v1beta1,v1": The versions to store resources with. Different groups may be stored in different versions. Specified in the format "group1/version1,group2/version2...". This flag expects a complete list of storage versions of ALL groups registered in the server. It defaults to a list of preferred versions of all registered groups, which is derived from the KUBE_API_VERSIONS environment variable.
+      --storage-versions="componentconfig/v1alpha1,extensions/v1beta1,metrics/v1alpha1,v1": The versions to store resources with. Different groups may be stored in different versions. Specified in the format "group1/version1,group2/version2...". This flag expects a complete list of storage versions of ALL groups registered in the server. It defaults to a list of preferred versions of all registered groups, which is derived from the KUBE_API_VERSIONS environment variable.


This addition is caused by the apiserver's dependency on the client, so it will install all the versions listed in pkg/client/unversioned/import_known_versions.sh.

I'm not sure why the metrics group isn't there before. Having it in the list looks correct anyway.

k8s-bot · 2015-12-21T21:09:25Z

GCE e2e test build/test passed for commit d8da3d65be25c435f388a6e3d02aa52877682a45.

deads2k · 2015-12-21T21:15:07Z

pkg/api/install/install.go

@@ -56,6 +54,15 @@ func init() {
 		return
 	}

+	registered.RegisterVersions(availableVersions...)


This should be done first. You always want to register even if you have no external versions enabled. In this order, you might return before registering.

k8s-bot · 2015-12-22T22:03:06Z

GCE e2e test build/test passed for commit 43b60c9f4d995d117dfed1c51f1eadcf4bccd2c6.

k8s-github-robot · 2015-12-23T22:53:32Z

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

k8s-bot · 2015-12-23T23:23:25Z

GCE e2e test build/test passed for commit 43b60c9f4d995d117dfed1c51f1eadcf4bccd2c6.

k8s-github-robot · 2015-12-24T06:47:14Z

PR changed after LGTM, removing LGTM.

k8s-github-robot · 2015-12-24T07:17:13Z

Labelling this PR as size/XL

k8s-bot · 2015-12-24T07:17:18Z

GCE e2e test build/test passed for commit 2b5b708.

caesarxuchao · 2015-12-24T07:24:51Z

Rebased and added a quite mechanical change to address caesarxuchao@43b60c9#commitcomment-15144868. Adding back the LGTM.

k8s-bot · 2015-12-24T07:43:59Z

GCE e2e test build/test passed for commit 3a2cede942e221d5b2fec5804f579e145d488cb7.

k8s-bot · 2015-12-24T07:53:46Z

GCE e2e test build/test passed for commit ad484c7.

k8s-github-robot · 2015-12-24T09:42:59Z

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

k8s-bot · 2015-12-24T10:10:05Z

GCE e2e test build/test passed for commit ad484c7.

caesarxuchao · 2015-12-24T21:57:54Z

@k8s-bot unit test this

k8s-github-robot · 2015-12-24T22:38:51Z

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

k8s-bot · 2015-12-24T23:08:50Z

GCE e2e test build/test passed for commit ad484c7.

k8s-github-robot · 2015-12-24T23:19:36Z

Automatic merge from submit-queue

Auto commit by PR queue bot

…herry-pick-18919-to-release-3.9 [release-3.9] UPSTREAM: 60978: Fix use of "-w" flag to iptables-restore Origin-commit: a771ff97d4ba42233f34582aa6e14cb32a65fec9

caesarxuchao added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 19, 2015

caesarxuchao assigned deads2k Dec 19, 2015

googlebot added the cla: yes label Dec 19, 2015

k8s-github-robot added kind/new-api size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 19, 2015

caesarxuchao force-pushed the refactor-registration branch from 489eb9c to e67fe84 Compare December 19, 2015 06:00

caesarxuchao force-pushed the refactor-registration branch from f1fb467 to 9b98872 Compare December 21, 2015 00:11

deads2k reviewed Dec 21, 2015
View reviewed changes

caesarxuchao force-pushed the refactor-registration branch from 7799c19 to eb801a8 Compare December 21, 2015 20:02

deads2k reviewed Dec 21, 2015
View reviewed changes

caesarxuchao reviewed Dec 21, 2015
View reviewed changes

deads2k reviewed Dec 21, 2015
View reviewed changes

caesarxuchao mentioned this pull request Dec 22, 2015

master.Config.StorageVersion is redundant with KUBE_API_VERSIONS #19032

Closed

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

Refactor the API registration and installation

2b5b708

caesarxuchao force-pushed the refactor-registration branch from 43b60c9 to 2b5b708 Compare December 24, 2015 06:44

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 24, 2015

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 24, 2015

pass runtime.Scheme as parameter to AddToScheme

ad484c7

caesarxuchao force-pushed the refactor-registration branch from 3a2cede to ad484c7 Compare December 24, 2015 07:21

caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 24, 2015

k8s-github-robot pushed a commit that referenced this pull request Dec 24, 2015

Merge pull request #18925 from caesarxuchao/refactor-registration

500493a

Auto commit by PR queue bot

k8s-github-robot merged commit 500493a into kubernetes:master Dec 24, 2015

caesarxuchao mentioned this pull request Dec 29, 2015

api server library: moving API registration logic to generic api server #19040

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup package `registered` and `install` #18925

cleanup package `registered` and `install` #18925

caesarxuchao commented Dec 19, 2015

k8s-github-robot commented Dec 19, 2015

k8s-bot commented Dec 19, 2015

k8s-bot commented Dec 19, 2015

k8s-bot commented Dec 19, 2015

k8s-bot commented Dec 21, 2015

k8s-bot commented Dec 21, 2015

deads2k Dec 21, 2015

caesarxuchao Dec 21, 2015

deads2k commented Dec 21, 2015

deads2k Dec 21, 2015

caesarxuchao Dec 21, 2015

caesarxuchao Dec 21, 2015

k8s-bot commented Dec 21, 2015

caesarxuchao Dec 21, 2015

k8s-bot commented Dec 21, 2015

deads2k Dec 21, 2015

caesarxuchao Dec 22, 2015

k8s-bot commented Dec 22, 2015

k8s-github-robot commented Dec 23, 2015

k8s-bot commented Dec 23, 2015

k8s-github-robot commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

caesarxuchao commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

caesarxuchao commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

cleanup package registered and install #18925

cleanup package registered and install #18925

Conversation

caesarxuchao commented Dec 19, 2015

k8s-github-robot commented Dec 19, 2015

k8s-bot commented Dec 19, 2015

k8s-bot commented Dec 19, 2015

k8s-bot commented Dec 19, 2015

k8s-bot commented Dec 21, 2015

k8s-bot commented Dec 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Dec 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Dec 21, 2015

Choose a reason for hiding this comment

k8s-bot commented Dec 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Dec 22, 2015

k8s-github-robot commented Dec 23, 2015

k8s-bot commented Dec 23, 2015

k8s-github-robot commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

caesarxuchao commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

caesarxuchao commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

k8s-bot commented Dec 24, 2015

k8s-github-robot commented Dec 24, 2015

cleanup package `registered` and `install` #18925

cleanup package `registered` and `install` #18925