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

Links to related object in json responses #3676

Closed
erictune opened this issue Jan 21, 2015 · 32 comments
Closed

Links to related object in json responses #3676

erictune opened this issue Jan 21, 2015 · 32 comments
Labels
area/api Indicates an issue on api area. area/ecosystem area/usability kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@erictune
Copy link
Member

User @abonas requested that a GET of services or replicationcontrollers should include links to the pods that constitute that service or replicationcontrollers. I filed this issue to discuss that idea.

Questions:

  • is this a priority right now given that clients can figure this out through multiple REST calls?
  • is it practical to do this, given that we don't have the ability to e.g. atomically read a service object and select its pods.
  • would links include resourceVersion (probably not until we remember old objects)
  • if we don't do something now, will it be hard to add later?
  • where in our data structures would this information go?
  • would we use something home-grown, like we did with "self-link", or one of many standards for HATEOS-y json linking, such as JSON-LD, HAL, JSON-References (see https://www.mnot.net/blog/2011/11/25/linking_in_json for some discussion)
@bgrant0607
Copy link
Member

Reverse label lookup #1348 is the intended solution. We plan to support this in the server. Kubectl already implements this in the client.

Users have the option of adding identifying annotations or labels if they choose.

@bgrant0607 bgrant0607 added the area/api Indicates an issue on api area. label Jan 21, 2015
@thockin
Copy link
Member

thockin commented Jan 21, 2015

FWIW, I think that showing the result of the selector as part of Service
and ReplicationController status would be good UX.

On Wed, Jan 21, 2015 at 7:56 AM, Brian Grant notifications@github.com
wrote:

Closed #3676
#3676.

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

@bgrant0607
Copy link
Member

Problems with this:

  • GET of any object MUST be constant time. Either the status would be precomputed and stale, or we need to index the reverse lookups, which we plan to do, but it isn't done. And, it would still be racy.
  • I want the Pod API on Kubelet to be THE SAME as the Pod API on the API server (Kubelet to understand pods, and to be able to pull from apiserver #2483), and for it to populate the Pod status (Properly set object status #2726). It won't know about services unless it queries the apiserver.
  • We're planning to convert ReplicationController to a plugin. Pods shouldn't know about them.

I'll reopen, but this is post-1.0 at best.

@bgrant0607 bgrant0607 reopened this Jan 21, 2015
@bgrant0607 bgrant0607 added priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. area/usability labels Jan 21, 2015
@thockin
Copy link
Member

thockin commented Jan 21, 2015

One of us is mis-parsing. I don't want to know which replication
controllers a pod is under, I want to know which pods are under a
replication controller.

I'd be fine with the status being precomputed, if we thing a label
evaluation is too slow in real time. It's guaranteed to be stale by the
time a user reads it, we'r just making that window a little wider.

The kubelet API knows nothing of replication controllers, so there is no
way for me to even ask this question.

As a plugin, replication controller can still either cache the pods IDs it
selects or else run a label query in series with a GET

On Wed, Jan 21, 2015 at 2:04 PM, Brian Grant notifications@github.com
wrote:

Reopened #3676
#3676.

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

@bgrant0607
Copy link
Member

Oh, right, me. Well, the problem with THAT is the size of the list. We could include a link to the list query that would give you the pods.

@thockin
Copy link
Member

thockin commented Jan 21, 2015

I think worrying about the list size is premature.

On Wed, Jan 21, 2015 at 2:17 PM, Brian Grant notifications@github.com
wrote:

Oh, right, me. Well, the problem with THAT is the size of the list. We
could include a link to the list query that would give you the pods.

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

@derekwaynecarr
Copy link
Member

I don't see a pre compute problem here. If I understand the request, they just want a link rel to follow that would be the default lookup. In this manner, their UIs just follow the links rather than them doing manual URL construction.

Sent from my iPhone

On Jan 21, 2015, at 5:26 PM, Tim Hockin notifications@github.com wrote:

I think worrying about the list size is premature.

On Wed, Jan 21, 2015 at 2:17 PM, Brian Grant notifications@github.com
wrote:

Oh, right, me. Well, the problem with THAT is the size of the list. We
could include a link to the list query that would give you the pods.

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


Reply to this email directly or view it on GitHub.

@bgrant0607
Copy link
Member

@thockin I disagree. It's not a scalable pattern. A link to the query is far preferable to a list of links to the pods. Then, the solutions we develop to deal with long lists will just work.

@thockin
Copy link
Member

thockin commented Jan 22, 2015

I'll take either one, but we can agree to disagree. A JSON object with
1000 or 10000 lines of "pod-id" or "ip:port" is not too unreasonable. I
guess I was assuming it would be a list of Ids or links, and not the full
body of each pod. a link to a query sounds plausible, too.

On Wed, Jan 21, 2015 at 3:57 PM, Brian Grant notifications@github.com
wrote:

@thockin https://github.com/thockin I disagree. It's not a scalable
pattern. A link to the query is far preferable to a list of links to the
pods. Then, the solutions we develop to deal with long lists will just work.

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

@abonas
Copy link
Contributor

abonas commented Jan 22, 2015

@thockin @derekwaynecarr indeed just to confirm - you got this right :)
This is NOT about getting from pod which service/rc "stalks" it, but rather getting from each service and each rc the pods they're after.
I think it's logical to get it from the api, and will minimize race conditions of changed entities/labels/selector like what @simon3z mentioned in #3623.

@abonas
Copy link
Contributor

abonas commented Jan 22, 2015

btw, getting the pod uids is enough, no need in full body of pods, since they are retrieved separately anyway.

@thockin
Copy link
Member

thockin commented Jan 22, 2015

What's your thoughts on getting a list of IDs vs getting a link to a query?

On Thu, Jan 22, 2015 at 1:25 AM, abonas notifications@github.com wrote:

btw, getting the pod uids is enough, no need in full body of pods, since
they are retrieved separately anyway.

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

@bgrant0607
Copy link
Member

Our objects are loosely coupled by design. A link to a query is reasonable. A list of pod references is not.

I'm going to cite the service redesign issue #2585, to ensure I don't forget about this.

@abonas
Copy link
Contributor

abonas commented Jan 22, 2015

@thockin list of IDs are more convenient from a link to query.
why? Because it:

  1. saves a redundant request to make the query, the pods are pulled anyway and I can store them with their id as a key.
  2. currently the query language is not exposed so it's not clear how it's doable anyway. see Generalize label selectors #341

@bgrant0607
Copy link
Member

The redundancy will be eliminated: #2740.

You can query using labels already. It is even documented:
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/labels.md
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/apiserver/resthandler.go#L158

#341 is not relevant, since services and replication controllers cannot yet utilize the more general form of queries.

@bgrant0607
Copy link
Member

Group thread about how to identify the pods of a replication controller:
https://groups.google.com/forum/#!topic/google-containers/WccX0G0Awqs

@bgrant0607
Copy link
Member

Continuing discussion from #3872 and #3623:

We're moving away from status computed on-the-fly: #2726.

Also, we're planning to support pruning out fields #1459, but will not add extra information that is not present by default. Such information has to be provided by a separate API endpoint.

Frequently changing, potentially huge lists in object status would be highly problematic, both internally and for the client-facing API, causing churn in etcd and in watches, and greatly increasing the cost of all GETs, even though the data would likely not be used in most cases. Additionally, consistency would be a problem.

We plan to address these problems for list objects, but not for subobjects within API objects.

We can add links (or structured list references) in status to this additional information. At this point, we plan to do this for resources and pods under a replication controller. I could be convinced to support it in some form for Endpoints, also, but not for Service itself, as we're almost certain to break it into multiple parts (#2585). If we do break up Service, its status would contain a link to the corresponding Endpoints. I imagine we'd also do this for any other instances of object references, such as replication controller to pod template (#170).

@bgrant0607
Copy link
Member

/cc @jackgr

@bgrant0607 bgrant0607 added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 17, 2016
@k8s-github-robot
Copy link

@erictune There are no sig labels on this issue. Please add a sig label by:
(1) mentioning a sig: @kubernetes/sig-<team-name>-misc
(2) specifying the label manually: /sig <label>

Note: method (1) will trigger a notification to the team. You can find the team list here.

@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label May 31, 2017
@0xmichalis
Copy link
Contributor

I think owner references are covering this issue now, please reopen if this does not hold true.

@bgrant0607
Copy link
Member

@Kargakis I reopened. Owner references link from pods to their controller. This is about the other direction. The client needs fairly deep knowledge of the API in order to be able to find the selector field, understand what type of resource it refers to, and translate the selector into the syntax used by the API query parameter.

@0xmichalis
Copy link
Contributor

/sig api-machinery

then

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Sep 7, 2017
@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Sep 7, 2017
@bgrant0607 bgrant0607 removed this from the next-candidate milestone Sep 7, 2017
@bgrant0607
Copy link
Member

Related: #22675

@bgrant0607
Copy link
Member

@kubernetes/sig-api-machinery-feature-requests

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 4, 2018
@bgrant0607
Copy link
Member

/remove-lifecycle stale
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 23, 2018
@liggitt liggitt added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 14, 2019
@thockin thockin closed this as completed Aug 20, 2022
@liggitt liggitt closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Indicates an issue on api area. area/ecosystem area/usability kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

No branches or pull requests