-
Notifications
You must be signed in to change notification settings - Fork 38.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] add more support for multiple groups; move experimental/v1 to experimental/v1alpha1 #12755
Conversation
GCE e2e build/test failed for commit 11c2904efcd1fd054f52b57a1278505caf3e4c66. |
11c2904
to
9c1fe3d
Compare
GCE e2e build/test failed for commit 9c1fe3d95da135ab32846b14d7d7479a62d725bc. |
9c1fe3d
to
d6a5bdf
Compare
GCE e2e build/test failed for commit d6a5bdfaf991c64660864bb4c154c56a5a6f2421. |
d6a5bdf
to
d0df366
Compare
move experimental/v1 to api/experimental/v1alpha1
d0df366
to
2906eb5
Compare
GCE e2e build/test passed for commit d0df366aee47de621250d3f878ae4b833ef5b4c4. |
GCE e2e build/test passed for commit 2906eb5. |
@@ -120,8 +119,7 @@ func NewAPIServer() *APIServer { | |||
SecurePort: 6443, | |||
APIRate: 10.0, | |||
APIBurst: 200, | |||
APIPrefix: "/api", | |||
ExpAPIPrefix: "/experimental", | |||
APIPrefix: "/kube-api", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't change the meaning of the existing APIPrefix flag. We should create a new flag that would apply to new groups.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, creation of a new KubeAPIPrefix flag should be moved to a separate PR.
@derekwaynecarr if you can look over this |
|
||
import "strings" | ||
|
||
func GetVersion(groupVersion string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/util is not a good place for this. This is api-specific code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, I suggest somewhere under pkg/api.
GCE e2e build/test passed for commit 16b2e94. |
versionPath := path.Join(pkgBase, group, version) | ||
versionPath := path.Join(pkgBase, *groupVersion) | ||
if group == "experimental" { | ||
//TODO: we should rename the direcotry /expapi to /experimental, so directory path match groupVerson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why /expapi can't be moved to pkg/api/experimental?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. It's in the TODO list (#12413 (comment) point 4).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually in the TODO list I'm planning moving it to pkg/experiemental. pkg/api/experimental makes more sense.
#12888 superseded this. Closing. |
ref #12413
changelog:
commit e2c5575:
enforces the URL exposed by apiserver to be in the form of APIPrefix/group/version/...
Most changes in this commit is just name changes.
commit 148a67d
stops apiserver from encoding api.Status to a specific version unless the version is v1.
commit cf8cfdc
move api.Status from api/types.go to api/unversioned.go, because the apiserver sends unversioned Status for non-v1 objects since commit 148a67d. We cannot make non-backward compatible change to api.Status, but we can always make additive change.
We may move other common kinds shared by groups (e.g., deleteoptions, listoptions) to unversioned.go, but that will be another PR
commit 11c2904
A toy experimental kind for testing purposes, copied from uluyol's branch. Will remove this commit before merge.
@uluyol @nikhiljindal @bgrant0607 @krousey
This PR is marked as WIP because I haven't run e2e tests yet.
There is a Todo list in #12413 (comment) trakcing other things needed to support multiple groups.