Skip to content
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] Make expapi an API group #12413

Closed
caesarxuchao opened this issue Aug 7, 2015 · 12 comments
Closed

[WIP] Make expapi an API group #12413

caesarxuchao opened this issue Aug 7, 2015 · 12 comments
Assignees
Labels
kind/design Categorizes issue or PR as related to design.

Comments

@caesarxuchao
Copy link
Member

#12001 introduce the experimental api prefix, but it's not introduced as an API group and it thus has to share the version with the official API (i.e., v1).

As suggested in #10009 (comment), we can make API group part of the API version to expedite its prototyping.

Here is a laundry list of works required to refactor expapi as an API group, the list is short for now, but I expect it to grow along the way:

  1. changing the versions in registered.RegisteredVersions to "group/version" and plumbing the changes
  2. fixing the code that assume api version doesn't contain "/" (example)
  3. fixing the TODOs that depend on expapi to be an API group (example)

@bgrant0607 @uluyol @nikhiljindal

@caesarxuchao caesarxuchao self-assigned this Aug 7, 2015
@satnam6502 satnam6502 added kind/design Categorizes issue or PR as related to design. team/api labels Aug 7, 2015
@caesarxuchao
Copy link
Member Author

I have refactored expapi to use groupVersion="expapi/v1alpha1". I didn't change the field name "apiVersion" in TypeMeta, so in a JSON config file, one would write "apiVersion": "expapi/v1alpha1".

I didn't change anything for api/v1, so people would still write "apiVersion": "v1" for a api/v1 object.

Moving forward, we need to test the groupVersion design on api/v1 objects (where I expect we will meet many problems), but the changes should not affect the end-users, i.e., an end-user should still write "apiVersion": "v1" in the config file. Here is what I propose:

  1. internally, we use groupVersion "api/v1". For example, we register "api/v1" in registered.RegisteredVersions, rather than "v1".
  2. treating "v1" as an alias of "api/v1". This requires translating "v1" to "api/v1" in many places.

Comments are more than welcome.

@caesarxuchao
Copy link
Member Author

Some open questions:

  1. What should the server URL look like after we support multiple groups? I assume it will be provider/group/version/kind, e.g., for kubernetes native objects, the URL looks like:
    api/group/version/kind/...
    for openshift objects, the URL looks like:
    osapi/group/version/kind/...
  2. Shall the experimental api be a group, or shall it be a prefix, i.e., shall its URL be:
    expapi/group/version/kind/...
    or
    api/expapi/version/kind/...
  3. Shall we allow conflicting kind names in groups of the same provider? I assume not.
  4. Shall we allow conflicting kind names in different providers? (e.g. openshift can have its own replicationcontroller) I assume so.
  5. depending on 3 do we require users to specify group in JSON config file? If the answer to 3 is no, then I think users don't need to specify group.

@uluyol
Copy link
Contributor

uluyol commented Aug 10, 2015

We use "experimental" not "expapi" for exposing the experimental api, so it should be "experimental/v1alpha1".

I believe that we had decided on provider/group/version/kind scoping (where everything following provider can differ completely depending on the provider—there were no immediate plans for dealing with multiple providers). So api, osapi, experimental are all categorized as groups. There is still the question of should we use HOST/group/version (what we do today) or HOST/api/group/version. The latter would mean that we wouldn't have to worry about conflicts with group names and unrelated functionality (e.g. swagger).

@caesarxuchao
Copy link
Member Author

Thanks. If api, osapi, experimental are all categorized as groups, then we have to deal with conflicting kind names in different groups, and user would need to specify group name in JSON config file.

@caesarxuchao
Copy link
Member Author

Here is a summary of the meeting with @nikhiljindal and @bgrant0607.

  1. api, osapi, experimental are all categorized as groups. Their URLs will be:
    /api/v1/...
    /osapi/vX/...
    /experimental/v1alpha1/...
    and there is no prefix before them.
  2. We will not divide v1 objects into multiple groups, they will be in a monolithic group called "api", so the URL for v1 objects will remain unchanged. We will create groups for v2 objects as they come up.
  3. Regarding the value of APIVersion field in config files, newer objects need to specify group/version, e.g. "APIVersion": "experimental/v1alpha1"; for native v1 objects, users only need to specify "APIVersion": "v1", and internally we will translate it to "api/v1". Because openshift may wants to translate "v1" to "osapi/v1" instread, we should provide a translate function:
func translate(kind, version string) groupVersion string

update: GroupForResource seems solve problem 3.

cc @smarterclayton

@caesarxuchao
Copy link
Member Author

Update: after discussion with @nikhiljindal, we changed our idea about point 1:
the old design is using "/api" as the group name of the native k8s kinds, and leave the prefix (for all APIs) null, but such a design prevents us from adding a prefix afterwards, because it will change the URL for v1 objects and we lose backward compatibility.
In the new design, we regard the group name of the native k8s kinds as null; and we add a prefix "api" for all API groups, so the URLs will be:
/api/v1/..
/api/osapi/vX/..
/api/experimental/v1alpha1/..

I have implemented this work. I'll submit the PR after #12405 is merged.

@caesarxuchao
Copy link
Member Author

Todo:

  1. define what URL to expose sub-resource which doesn't belong to the parent-resource's group.
    Experimental Scale subresource #12217 (comment)
  2. VersionAndKindForResource() only takes resource as input, this prevent us from defining conflicting kind names in different groups. see here
  3. the current way we add client for experimental API does not scale (Add experimental api support to kubectl #12405 (comment))
  4. move directory expapi/ to experimental/, so that groupVersion = directory name. This is an assumption in update-generated-conversions.sh etc.
  5. change the versioning in package testapi.
  6. GetAPIRequestInfo assumes api/version/... it needs to handle api/group/version
  7. let kubectl recognize group, e.g., kubectl get group/resource/name [Add API group concept to kubectl #13929]
  8. Conversion and deep-copy evolution #12598
  9. conversion of top-level API objects across groups, e.g. compute/v2/pod <---> v1/pod
  10. We should have a "group registry" somewhere. It could be a section of api-conventions.md.

@smarterclayton
Copy link
Contributor

/api/v1/..
/api/osapi/vX/..
/api/experimental/v1alpha1/..

Except OpenShift is really going to be:

https://someotherserver.somewhere/oapi/v1

The translate function is going to be kind of ugly too - that's something we can manage on our server based on legacy support without necessarily it having to cross over into the server codebase, and in our client code it's really something you should discover from the server, not make up on your own (client code shouldn't need translate).

I guess I keep circling back to what the group is good for. Is it a prefix to divide API versions? Yes. Is it a mechanism whereby two identically named kinds could be separated? Yes. Does all of it still require a single global indirection to find? Yes.

The map of "group+version+kind" is either discovery (against a known endpoint with a well known traversal path) or embedded into client code, or embedded into a config file on a client. Since client code is the only thing we've embedded it in, and API endpoints can infer group based on context, are we truly blocked by preserving the nil -> behavior?

I don't want to choose URLs based on weird backwards compatibility - I'd rather have well structured URLs that recognize that URL conventions are just that, and that discovery is the problem we want to solve. Any client that wants to be generic to kube to talk to OpenShift already has to walk through some discovery path. I would just expect the client to be able to build the map of kind -> version -> group via the information provided during discovery, rather than make this something that has to be enforced via code.

@caesarxuchao
Copy link
Member Author

Except OpenShift is really going to be:
https://someotherserver.somewhere/oapi/v1

You are right. I changed my design to keep the openshift API URL. Here is the new design:
URL will be APIPrefix/group/version/...

  • for api/v1, we set the APIPrefix="api", group="", version="v1", so the URL is api/v1
  • for experimental api, we set the APIPrefix="kube-api" (the exact name is open to discussion), group="experimental", version="v1alpha1", so the URL is kube-api/experimental/v1alpha1. And all future API groups will have the same APIPrefix.
  • [updated] for openshift and other third party API, we can either make APIPrefix="", group="oapi", or APIPrefix="oapi", gropu="", so the URL will not be changed.

The translate function is going to be kind of ugly too - that's something we can manage on our server based on legacy support without necessarily it having to cross over into the server codebase, and in our client code it's really something you should discover from the server, not make up on your own (client code shouldn't need translate).

Doesn't the GroupForResource implement the translation? For future API groups, because we require the user to specify the APIVersion field using group/version format in their JSON file, we won't need any translation.

The map of "group+version+kind" is either discovery (against a known endpoint with a well known traversal path) or embedded into client code, or embedded into a config file on a client. Since client code is the only thing we've embedded it in, and API endpoints can infer group based on context, are we truly blocked by preserving the nil -> behavior?

I don't follow. What do you mean by nil-> behavior?

I don't want to choose URLs based on weird backwards compatibility - I'd rather have well structured URLs that recognize that URL conventions are just that, and that discovery is the problem we want to solve. Any client that wants to be generic to kube to talk to OpenShift already has to walk through some discovery path. I would just expect the client to be able to build the map of kind -> version -> group via the information provided during discovery, rather than make this something that has to be enforced via code.

Do you have in mind how we shall do the discovery?

@lavalamp
Copy link
Member

Except OpenShift is really going to be: https://someotherserver.somewhere/oapi/v1

@smarterclayton My plan for this was to allow open shift (or any plugin) to claim part of the URL space (e.g., an entire group), so that e.g. /api/openshift/... would be redirected to https://someotherserver.somewhere/api/openshift/. This way the client doesn't have to do any additional discovery. (We can solve the backwards compatibility case in a number of ways, including by just letting you have the old /oapi/ redirected, too.)

@smarterclayton
Copy link
Contributor

I'm not sure that's something that is generally useful or what I would want
as a plugin to Kube. First, the load balancers that serve OpenShift may be
entirely distinct from the master's load balancers (running on a service in
the cluster). Second, API plugins may have different security rules.
Third, every client having to do a redirect or proxy hop adds unnecessary
latency in each client. Some clients can't even detect when redirects
happen, which means that they may cross security boundaries and fail
(browser cross origin, or different load balancer rules). And a client
still has to know that OpenShift exists in order to find that sub path it
exists under, so we still have to have the list that says "i'm looking for
X, where is it at".

Redirection sits in the uncomfortable space between proxying and true
disconnected API endpoints - proxying has advantages for unifying a
security domain but performance downsides, disconnected API endpoints are
harder to find but more resilient. Redirection suffers from an
uncomfortable balance there. Redirection is a convenience, but I don't
think it's a substitute for a concrete discovery strategy. If we have
discovery, then the sub path doesn't add much.

On Mon, Sep 14, 2015 at 4:07 PM, Daniel Smith notifications@github.com
wrote:

Except OpenShift is really going to be:
https://someotherserver.somewhere/oapi/v1

@smarterclayton https://github.com/smarterclayton My plan for this was
to allow open shift (or any plugin) to claim part of the URL space (e.g.,
an entire group), so that e.g. /api/openshift/... would be redirected to
https://someotherserver.somewhere/api/openshift/. This way the client
doesn't have to do any additional discovery. (We can solve the backwards
compatibility case in a number of ways, including by just letting you have
the old /oapi/ redirected, too.)


Reply to this email directly or view it on GitHub
#12413 (comment)
.

Clayton Coleman | Lead Engineer, OpenShift

@caesarxuchao
Copy link
Member Author

We have experimental group and its version is independent of the legacy v1 now. Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design.
Projects
None yet
Development

No branches or pull requests

5 participants