List/Watch/Get of objects associated with node #443

wojtek-t · 2017-03-10T14:19:05Z

@lavalamp @liggitt @kubernetes/sig-api-machinery-misc
@dchen1107

shyamjvs · 2017-03-10T14:58:02Z

I have a (probably silly) high-level question about this mechanism. Since we are aiming for read/access of secrets at the node level, that iiuc means that we are fine with pods within a node being able to read each other's secrets potentially. If that is the case, then consider the following scenario:

Pods A and B (needing secrets P and Q respectively) were running on Node-1 initially. Since the secret is authorized to view at node level, potentially both A and B can read each other's secrets
Now pod B gets evicted for some reason and is rescheduled onto Node-2
The backend gets updated with these changes and assuming everything goes fine, Node-1 should have access only to secret P now. But Q could still be possibly read from its cache, or pod A would be probably be knowing it already from before.

Does this mean that we either need to make secrets authorized at pod level?
Otherwise, if we totally ignore the risk of pods from within the same node being able to read each other's secrets, then why really bother about pods from different nodes being able to read each other's secrets (as it's just a matter of scheduling)?

liggitt · 2017-03-10T15:04:31Z

Since we are aiming for read/access of secrets at the node level, that iiuc means that we are fine with pods within a node being able to read each other's secrets potentially.

Pods do not get to make API calls with the node's identity. PodSecurityPolicy can be used to disallow privileged pods, host IPC, and volume mounts that would let a pod access the node host environment.

deads2k · 2017-03-10T15:19:25Z

Have I missed the bit describing how the resourceVersion is honored on the watch? This will effectively be a watch that a resourceVersion that is a tuple of multiple resource versions since the determination of which secrets have been added since RV=6 is dependent upon the state of the observed bound pods when you asked. You also end up with odd behavior where you see an ADD with RV=10, then a different pod is bound and you see an ADD with RV=6. That messes with reflector behavior which uses the lastSyncVersion.

We've done this in openshift with the projects resource and found the semantics to be challenging.

wojtek-t · 2017-03-10T15:35:15Z

@deads2k - thanks for pointing to it; I though about the former and I thought it's not a big deal. But I didn't fully realize the problem of the second one (which is kind of under the TODO in the doc). I will think more about it.

deads2k · 2017-03-10T15:38:34Z

@deads2k - thanks for pointing to it; I though about the former and I thought it's not a big deal.

I think it's still a big deal since your list isn't synchronized to your watch. Normal resources allow a coherent and consistent view when doing

list and get RV
watch using RV

With the situation as described, your watch will produce different output depending on whether its called one millisecond later or one second later, which is a big semantic difference for list/watch behavior.

wojtek-t · 2017-03-10T15:43:52Z

yeah - makes sense.

@deads2k - is there any place where I can read what you did in openshift for projects ? Just for my own education?

deads2k · 2017-03-10T15:47:40Z

@deads2k - is there any place where I can read what you did in openshift for projects? Just for my own education?

I didn't solve it. I found the problem and just punted. The watch for projects doesn't respect resource version at all. It just starts a "watch from now-ish" (yeah, it's not even "now"). Since its consumer is for human interaction, it didn't much matter. I think its a bigger issue for the thing driving our workloads.

wojtek-t · 2017-03-10T18:22:24Z

@deads2k - got it. Thanks I will think about it and hopefully will get back with something next week.

wojtek-t · 2017-03-13T08:14:19Z

I though about it, and so far don't have a good solution for it. One thing that we can potentially do (that reduces the amount of bad scenarios, but doesnt eliminate all of them is):

add a precondition to NodeSelector that requires "PodInformer to be synced to at least RV=X"
Change (define differently) semantic for the watch so that:
"if bounding a pod to a node results in bounding some object (secret/...) to it, this doesn't result in sending ADD event for this object (similarly for deleting a pod that unbounds an object).
It is responsibility of a watcher to grab the current object version when it is newly bound to a node.
We only guarantee to send add/update/delete events for secrets that were already bound to a node".

With those changes:

It should be pretty easy to modify the watchers in a node to issue GET for a secret/... for secret/... whenever a pod spec changes (and it's only a constant number of requests per pod modification, so it's not a big deal). I will describe in more details if/when we solve all other issues, but that's not very difficult.
Calling list/watch with a precondition for RV of pod informer in watch equal to current RV of pods in the node (kubelet) will result in returning all secrets/... of pods that kubelet is already aware. Which solves a bunch of issues.
I think that the only remaining problem that I don't yet know how to solve is scenario like this:
"bound a pod to node X that is referencing secret Y, a moment later create secret Y"

If the PodInformer in the watch implementation is lagging, we can potentially observe a secret creation before bounding a pod to node, which would be that we will not deliver the "add secret" event to the watcher.
That is probably solvable but it would require doing "multi-object-type" watch on the etcd level" (otherwise we would potentially do the serialization on our side, but it woudln't work in non-HA setups).

liggitt · 2017-03-13T17:16:12Z

add a precondition to NodeSelector that requires "PodInformer to be synced to at least RV=X"

the issue is that there are lots of informers (at least pods, pvcs, pvs, secrets) that determine the objects a node should be able to see, and all of those factor in to what shows up in the list or watch...

Change (define differently) semantic for the watch

I don't think different semantics are a good idea... it means the existing list/watch utilities could not be used to keep a cache store up to date.

wojtek-t · 2017-03-13T18:05:56Z

the issue is that there are lots of informers (at least pods, pvcs, pvs, secrets) that determine the objects a node should be able to see, and all of those factor in to what shows up in the list or watch...

but secrets, pvcs and config maps are kind of independent. What matters for them is only pods. I.e. all of them depends on pods, and nothing else.

I don't think different semantics are a good idea... it means the existing list/watch utilities could not be used to keep a cache store up to date.

I think we can reuse most of them, probably with some small layer on top of it. But wanted to avoid describing it, as long as not all issues are resolved. We need to solve everything to make it useful.

liggitt · 2017-03-13T18:46:57Z

but secrets, pvcs and config maps are kind of independent. What matters for them is only pods. I.e. all of them depends on pods, and nothing else.

A node can gain access to a secret for the following reasons:

a pod bound to the node references the secret directly (image pull secret, secret volume mount, secret used by other volume mount, env, or envfrom)
a pod bound to the node references a PVC which references a PV which references the secret needed to mount the PV (e.g. RBD, Ceph, Gluster)

For the node->pod->pvc->pv->secret case, all items in the chain matter, and changes can add/remove a secret from the set a node is allowed to get.

wojtek-t · 2017-03-14T09:04:47Z

@liggitt - thanks for pointing on it. I wasn't aware of it.

i think I have some ideas how to solve it - I will try to update the doc today.

wojtek-t · 2017-03-14T10:58:15Z

@liggitt @deads2k - I've significantly rewrite the doc and changed the design. The new approach has some assumptions, but should address all the concerns that you mentioned above.

PTAL when you will find some time.

deads2k · 2017-03-14T12:27:04Z

contributors/design-proposals/secure_watch.md

+
+ Fortunately, we can solve it in much simpler way, with one additional assumption:
+
+1. All object types necessary to determine the in-memory mapping share the same


This statement means that we're explicitly giving up on being able to shard etcd for any resource used by a node and that any new backend would have to provide the same ordering guarantee. That's significant departure from our previous posture of "we'll shard on resource types first".

I agree that this is limitation. Though it doesn't block sharding completely - it's still possible to shard a bunch of different resource types into separate etcd (e.g. Nodes, Services, Endpoints, ...).

So maybe it's something we can leave with? [obviously, if you have any better suggestion how to solve it, I'm more than happy to hear it.]

deads2k · 2017-03-14T12:51:04Z

contributors/design-proposals/secure_watch.md

+detail hidden in the code (see more details in the next section).
+
+
+### Implementation details


I haven't yet thought about whether I think this works. The "pod references PVC which references PV which references secret" and I update the resource links worries me for ordering concerns. I think that in such a case, you have to keep getting the secret until the watch finally returns it you. Even so, a later update to a PV could remove the secret (you should get a DELETE I think) and a subsequent update to the PV would expose it to you again and we wouldn't be able to show you the ADD because it could be moving back in time.

I haven't yet thought about whether I think this works. The "pod references PVC which references PV which references secret" and I update the resource links worries me for ordering concerns.

Sorry, I don't understand this part.

I think that in such a case, you have to keep getting the secret until the watch finally returns it you.

Why? If at the moment with rv=X you call GET for a secret and retrieve its current version (because you already have access to it, so there is a pod that bounds it to the node via some path), the when the resource graph will reach rv=X too and this secret will change the modification of it will be delivered to the user. Or did i misunderstand your concern?

Even so, a later update to a PV could remove the secret (you should get a DELETE I think).

I'm not sure it is that crucial. Even we deliver the delete to the watcher, it potentially may still keep it in memory. So it's more important to disallow it from getting it again.

and a subsequent update to the PV would expose it to you again and we wouldn't be able to show you the ADD because it could be moving back in time.

Are we allowing for such operations? Is every PV/PVC update a valid one? Should it be?

Are we allowing for such operations? Is every PV/PVC update a valid one? Should it be?

I think changing a secret reference is a pretty reasonable thing to do and allow. Particularly on a resource where the cost for deletion is high.

I think that we can't really drop the assumptions that RVs of objects returned via watch are in non-decreasing order, because it would break all the machinery we have.

That said, one potential thing that comes to my mind is to say that we deliver all those ADD and DELETE events being result of pod/pv/.. modification (that I described that we don't deliver) but with the following caveat:

if a pod change in rv=X (or any other object) is causing add or delete of secrets (or anything else) s1, s2, s3, then we also deliver them as watch events, but all of them with the same RV=X (equal to the one of changed pod).

However, that has two main implications:

we end up with objects that has RVs not being their real RVs (though I don't think this will have any bad implications)

we end up with (potentially) multiple watch events with the same RV. This is a problem if the watch breaks/closes in between of sending them.

A workaround then would be to resend all of them when watch is renewed.
So when we get a request with rv=x, instead of starting from x, we start the watch by sending all events from x-1, and then user needs to protect himself for processing re-adding already added object or deleting the non-existing one. Though in thing like ThreadSafeStore it would just work.
But that is again change in the semantics.

Another potential modification to the above would be, instead of having a watch event to be a secret (or any other object), change it to actually be list of secrets. Then the above problem disappears, but we end up with a completely different API. So then it probably can't be extension to existing WATCH, but should be some different API endpoint (like WATCHLIST, though this name has already been used for a different purpose :)). And we end up in a situation when our machinery (e.g. reflector) again doesn't work (it's possible to make it work, but it requires work).

@deads2k WDYT?

I think my suggestion about touching objects and letting them go through the watch machinery again is much simpler.

mutating objects in response to ACL changes that change visibility to them does not seem like a good idea (or even possible, depending on the authorization system in use)

deads2k · 2017-03-20T13:19:55Z

we end up with objects that has RVs not being their real RVs (though I don't think this will have any bad implications)

This would concern me. It means that anyone using that resource version won't see the behavior they expect. As a for instance, a watch with it won't properly watch the particular resource. I think even a get which hits the cache would be suspect too.

lavalamp · 2017-03-20T17:31:22Z

contributors/design-proposals/secure_watch.md

+
+1. There can be more objects referenced by a given pod (so we can't send all of
+them with a rv corresponding to that pod add/update/delete)
+2. If we decide for sending those with their original rv`s, then we could


nit: unmatched backtick

lavalamp · 2017-03-20T17:34:55Z

contributors/design-proposals/secure_watch.md

+
+# Detailed design
+
+ We will introduce the following ```node selector ``` filtering mechanism:


Alternative proposal:

Set up a controller that actively gives kubelets read permission on objects they need to view.

Make API server filter by permission when a user requests to watch across namespaces.

Kubelets just watch everything.

This approach is extendable to help other users with the same problem.

@lavalamp - thanks for the suggestion. However, I think there are few things we need to think about it in this solution. The main thing is:
"are we going to reflect this information in etcd"

If not (e.g. it will be just in-memory), then it might be tricky to synchronize it between apiservers in HA setups.

If it will be in etcd, then:

having "allowed kubelets" be listed together with the object doesn't seem like a good idea

if we are not going to store the "newly allowed/disallowed kubelet" together with the "contentless update", then how we are going to derive its purpose in apiserver?

Maybe these questions are stupid, but I guess I'm not following your thoughts here.

And it seems that @liggitt and @deads2k are against having "artificial" resource versions for the purpose of changing ACLs.

Recording third alternative here (the approach discussed by david and jordan):

pods come from LIST / WATCH on a field selector

all other resources from kubelet are direct GETs, possibly implemented by batch GET on the server, protected by ACL, retry after some window on 403 (maybe double the backoff)

protect ACL via a separate authorizer, use the reference graph

Retry of 403 is really no different than any other retry - you can't start unless you can access it, and malicious clients are the ones we reject.

In the future batch GET could be useful, but not strictly necessary here.

However, I think there are few things we need to think about it in this solution. The main thing is: "are we going to reflect this information in etcd"

E.g. RBAC permissions are stored in etcd afaik.

having "allowed kubelets" be listed together with the object doesn't seem like a good idea

We shouldn't add the permissions directly to the secret/configmap/etc objects.

if we are not going to store the "newly allowed/disallowed kubelet" together with the "contentless update", then how we are going to derive its purpose in apiserver?

The contentless update would just exist for the purpose of getting all existing watches to reevaluate the object. In this model, we've added a visibility-checking filter to the watches. The filter would have to do a consistent read of the permission source.

Recording third alternative here

I could live with that, but fixing visibility for watches seems more useful long-term.

So there's a sig-auth discussion about allowing selective retrieval of secrets for the purposes of using them. In that model you'd have to say up front who you are and access them via a subresource (potentially so that you can get redirected to a third party system). So we're already considering decoupling secrets strongly, which makes watches impossible.

Part of the reason for direct access is because it keeps the core simple. Yes, we could do all sorts of crazy filtering, and watch for changing ACL rules, and match those up on the fly to access. The question is - why bother, if we can put in place a reasonably performant "lazy" access?

kubernetes/kubernetes#40403 had gone into some of the options here, but the sheer simplicity of having only two types of filter on LIST WATCH:

field/label

can you start watching X

means that the scale complexity is at least tractable - you can effectively detect when ACL changes and break the watch (which we need to start doing at some point in the future). But when you bring in linking and relationships, you have to do a much more complex join, and the potential security risks are much higher if you get the join ordering wrong.

That said, if we can define a maximum window for watch breaking, we could in theory do the ACL check on a single item watch every X minutes (at the window for maximum time open) and look at allowing bulk WATCH (watch these keys). Not sure how computationally tractable that is, but it separates WATCH from ACL still, which I think is a net win.

a much more complex join, and the potential security risks are much higher if you get the join ordering wrong.

That's a fair point.

can you start watching X

look at allowing bulk WATCH (watch these keys)

If we want a bulk watch it seems like we'd have to do the ACL check there, in which case doing it up front is redundant.

Sorry by bulk watch I mean the long standing request of multiplexing watches:

open session

add watch 1

add watch 2

add watch 3

close watch 2

close session

I think that can be efficiently optimized, but each watch has its own acl check - so we move the distribution of the check from inside the watch to outside the watch. I think we need to close watches when security rules change anyway (it's just a matter of time until that becomes broken), hence why I think we could potentially start down the ACL path, then do watch with ACL checks breaking watch, then do bulk watch.

But in the meantime that would mean that we have to do GET / poll / cache from the kubelet.

@smarterclayton - thanks for explanation. The bulk watch that you suggested above makes a lot of sense - I like the idea.
Do you think that we can work on 'bulk watch" in parallel with other two things? I didn't think carefully about it, but it seems that "bulk watch" is supposed to be some wrapper (wrapper is probably not a good word here) around regular single-resource watches. If so, I think adding ACL to watches seems kind of independent to me.
Maybe I can help with designing and implementing bulk watch?

lavalamp · 2017-03-20T17:35:24Z

contributors/design-proposals/secure_watch.md

+type NodeSelector struct {
+  // TODO: Should this be repeated field to allow for some fancy controllers
+  // that will have access to multiple nodes?
+  nodeName string


This is very single-purpose.

lavalamp · 2017-03-20T17:40:00Z

contributors/design-proposals/secure_watch.md

+of events is always the same (e.g. very slow laggin watch may not cause dropped
+events). 
+
+ To satisfy the above requirements, we can't really rely only on the existing


This is going to be really hard to get right, I think. An alternative that comes to mind:

When the controller adds or removes a permission to view an object, it "touches" the object, e.g. increments a counter in a new field.

Then the normal filtering mechanism will work with no changes whatsoever.

lavalamp · 2017-03-20T17:43:57Z

contributors/design-proposals/secure_watch.md

+
+ One potential solution would be to identify this watch by a resource version
+combined from resource versions coming from different object kinds (e.g.
+pods have rv = rv1, secrets have rv = rv2, ...). Then we could keep the history


I do not think we should produce a solution that depends on any particular fact about RVs. We don't let users do it, we don't do it in the GC even though it would be very nice, we shouldn't start doing it now.

Does this one depend on any fact? I think it doesn't (though this was kind of "considered alternative" and I don't think we should proceed with this one anyway).

lavalamp · 2017-03-20T17:59:45Z

contributors/design-proposals/secure_watch.md

+}
+```
+
+ The NodeSelector field will be added to ```ListOptions ``` (next to label & field


Yeah, I really think this is the wrong direction to go, and it's much better to build reusable visibility rules.

Yeah I am -1 on this - I want generic visibility, or selective ACL + field rules.

wojtek-t

@smarterclayton @lavalamp - PTAL

wojtek-t · 2017-05-15T08:23:49Z

contributors/design-proposals/bulk_watch.md

+
+```
+type BulkListOptions struct {
+	Selectors []ListSelector


Yeah - I really prefer this one. I would like to avoid having a single endpoint supporting different kind of operations.
Since it will still be alpha, I agree that if we learn that this is painful we will be able to change it.

So, I'm changing it to GetOperations and GetOption for now - we can revisit in the future (while still being in alpha).

wojtek-t · 2017-05-15T08:53:25Z

contributors/design-proposals/bulk_watch.md

+}
+```
+
+ We will create a dedicated admission plugin (or other filter mechanism) that


@smarterclayton - I see. So i think that I inconsciously mixed two different things here.

Basically, if we ignore watch for a moment, we still need to be able to answer whether a given GET request is allowed or not. And this is what I was referring to as a "dedicated admission plugin" (though maybe it' won't be that dedicated).
The second thing is watch, and for that we also need to do periodic checking (or lazy checking as you suggested, which is a nice optimization). But as I wrote somewhere at the beginning of a doc, this isn't bulk-specific, we need pretty much the same mechanism for a regular watch. If we do it low enough in our machinery, we should be able to simply reuse it.

I just clarified this part of a doc to reflect those thoughts.

wojtek-t · 2017-05-15T08:53:35Z

contributors/design-proposals/bulk_watch.md

+will be responsible for detecting whether a given user is allowed to list/watch
+objects requested by a given `BulkdSelector` (the exact mechanism for it is out
+of scope for this document) and either rejecting the whole request or letting it
+go. We are not going to support partial rejections (e.g. you cannot proceed with


This should be clarified now.

wojtek-t · 2017-05-15T08:54:14Z

contributors/design-proposals/bulk_watch.md

+
+ The the high level, the propocol will look:
+1. client opens a new websocket connection to a bulk watch endpoint to the
+server via gttp GET


wojtek-t · 2017-05-15T08:54:39Z

contributors/design-proposals/bulk_watch.md

+ The the high level, the propocol will look:
+1. client opens a new websocket connection to a bulk watch endpoint to the
+server via gttp GET
+1. this results in creating in creating a single channel that is used only


lavalamp · 2017-06-09T21:30:48Z

contributors/design-proposals/bulk_watch.md

+ We will start with introducing `getoperation` resource and supporting the
+following operation:
+```
+POST /apis/bulk.k8s.io/v1/getoperation <body defines filtering>


How about "subscriptions" as the resource name, if the only only thing one can do is subscribe and unsubscribe?

When I'm not thinking about it, subscriptions would be good for watch, but not necessarily for get in my opinion.
@smarterclayton - WDYT?

If we're going to call it subscriptions, we could just call it watches. However, I think BulkGetOperation is probably appropriate - it's not too generic, and because it supports multiple watches, the incremental "subscribe" style works as well, and leaves the door open. I originally worried about stuttering, but I think given the novelty of this operation BulkGetOperation (for the endpoint) and GetOperation (for the incremental watch) is appropriate.

Just to clarify I understand correctly. You suggest the following change:

s/getoperation/bulkgetoperation/

or I misunderstood it? And if so, should i also change the structs in lines 140 into:
GetOperations -> BulkGetOperations
GetOperation -> BulkGetOperation

?

The top level object would be BulkGetOperation, the resource would be bulkgetoperations, and the thing users send over the watch would be GetOperation (singular)

Thanks for clarification - makes perfect sense to me.
Fixed.

lavalamp · 2017-06-09T21:36:43Z

contributors/design-proposals/bulk_watch.md

+# Detailed design
+
+ As stated in above requirements, we need to make bulk operations work across
+different resource types (e.g. watch pod P and secret S within a single watch


So this means that we probably need to implement this in the aggregation layer. The first step in the implementation could just be issuing a watch for every subscription request. Later, we could optimize to also do bulk watch between aggregator and backing apiserver, but that is pure optimization.

Incorporated this comment into "Implementation details" section.

lavalamp · 2017-06-09T21:46:37Z

contributors/design-proposals/bulk_watch.md

+1. to subscribe for a watch of a given (set of) objects, user sends `Watch`
+object over the channel; in response a new channel is created and the message
+with the channel identifier is send back to the user (we will be using integers
+as channel identifiers).


TODO: check if integers are mandatory or if we could do something like "ch1", "ch2"...

Added as a TODO for now.

lavalamp · 2017-06-09T21:47:49Z

contributors/design-proposals/bulk_watch.md

+1. to stop watching for a given (set of) objects, user sends `CloseWatch`
+object over the the channel; in response the corresponding watch is broken and
+corresponding channel within websocket is closed
+1. once done, user can close the whole websocket connection (this results in


should/must instead of can?

lavalamp · 2017-06-09T21:48:35Z

contributors/design-proposals/bulk_watch.md

+```
+type Request struct {
+	// Only one of those is set.
+	Watch      Watch


Watch *Watch (optional)?

lavalamp · 2017-06-09T21:50:09Z

contributors/design-proposals/bulk_watch.md

+}
+
+// Depending on the request, channel that was created or deleted.
+type Response struct {


TODO: add some way of correlating the responses with the requests.

My preference is to have an int that the client increments with every request which the server echos back in the response.

lavalamp · 2017-06-09T21:56:59Z

contributors/design-proposals/bulk_watch.md

+	Channel Identifier
+}
+```
+With the following structure we can guarantee that we only send and receive


s/following/previous/

wojtek-t

Comments applied. PTAL

wojtek-t · 2017-07-04T12:47:42Z

contributors/design-proposals/bulk_watch.md

+ We will start with introducing `getoperation` resource and supporting the
+following operation:
+```
+POST /apis/bulk.k8s.io/v1/getoperation <body defines filtering>


When I'm not thinking about it, subscriptions would be good for watch, but not necessarily for get in my opinion.
@smarterclayton - WDYT?

wojtek-t · 2017-07-04T12:49:16Z

contributors/design-proposals/bulk_watch.md

+1. to stop watching for a given (set of) objects, user sends `CloseWatch`
+object over the the channel; in response the corresponding watch is broken and
+corresponding channel within websocket is closed
+1. once done, user can close the whole websocket connection (this results in


wojtek-t · 2017-07-04T12:50:17Z

contributors/design-proposals/bulk_watch.md

+1. to subscribe for a watch of a given (set of) objects, user sends `Watch`
+object over the channel; in response a new channel is created and the message
+with the channel identifier is send back to the user (we will be using integers
+as channel identifiers).


Added as a TODO for now.

wojtek-t · 2017-07-04T12:50:35Z

contributors/design-proposals/bulk_watch.md

+```
+type Request struct {
+	// Only one of those is set.
+	Watch      Watch


wojtek-t · 2017-07-04T12:51:02Z

contributors/design-proposals/bulk_watch.md

+	Channel Identifier
+}
+```
+With the following structure we can guarantee that we only send and receive


wojtek-t · 2017-07-04T13:03:46Z

contributors/design-proposals/bulk_watch.md

+# Detailed design
+
+ As stated in above requirements, we need to make bulk operations work across
+different resource types (e.g. watch pod P and secret S within a single watch


Incorporated this comment into "Implementation details" section.

wojtek-t · 2017-07-04T13:08:38Z

contributors/design-proposals/bulk_watch.md

+}
+
+// Depending on the request, channel that was created or deleted.
+type Response struct {


cben · 2017-07-05T08:03:43Z

contributors/design-proposals/bulk_watch.md

+ As stated in above requirements, we need to make bulk operations work across
+different resource types (e.g. watch pod P and secret S within a single watch
+call). Spanning multiple resources, resource types or conditions will be more
+and more important for large number of watches. As an example, deferation will


s/deferation/federation/

smarterclayton · 2017-07-05T20:35:22Z

contributors/design-proposals/bulk_watch.md

+As a result, we need another API for watch that will also support incremental
+subscriptions - it will look as following:
+```
+websocket /apis/bulk.k8s.io/v1/getoperation?watch=1


Should be v1alpha1

In line 98 above I have:
"In all text below, we are assuming v1 version of the API, but it will obviously go through alpha and beta stages before (it will start as v1alpha1)."

smarterclayton · 2017-07-05T20:38:41Z

contributors/design-proposals/bulk_watch.md

+// Depending on the request, channel that was created or deleted.
+type Response struct {
+	// Propagated from the Request.
+	RequestId Identified


id and requestID externally, ID and RequestID internally (style)

smarterclayton · 2017-07-05T20:39:32Z

A few minor api quibbles, but overall I'm pretty happy with this.

wojtek-t

Thanks Clayton!

PTAL

wojtek-t · 2017-07-06T10:47:34Z

contributors/design-proposals/bulk_watch.md

+ As stated in above requirements, we need to make bulk operations work across
+different resource types (e.g. watch pod P and secret S within a single watch
+call). Spanning multiple resources, resource types or conditions will be more
+and more important for large number of watches. As an example, deferation will


wojtek-t · 2017-07-06T10:49:59Z

contributors/design-proposals/bulk_watch.md

+As a result, we need another API for watch that will also support incremental
+subscriptions - it will look as following:
+```
+websocket /apis/bulk.k8s.io/v1/getoperation?watch=1


In line 98 above I have:
"In all text below, we are assuming v1 version of the API, but it will obviously go through alpha and beta stages before (it will start as v1alpha1)."

wojtek-t · 2017-07-06T10:50:35Z

contributors/design-proposals/bulk_watch.md

+// Depending on the request, channel that was created or deleted.
+type Response struct {
+	// Propagated from the Request.
+	RequestId Identified


wojtek-t · 2017-07-06T10:53:03Z

contributors/design-proposals/bulk_watch.md

+ We will start with introducing `getoperation` resource and supporting the
+following operation:
+```
+POST /apis/bulk.k8s.io/v1/getoperation <body defines filtering>


Just to clarify I understand correctly. You suggest the following change:

s/getoperation/bulkgetoperation/

or I misunderstood it? And if so, should i also change the structs in lines 140 into:
GetOperations -> BulkGetOperations
GetOperation -> BulkGetOperation

?

smarterclayton · 2017-07-06T20:28:36Z

LGTM, squash and i'll merge

wojtek-t · 2017-07-07T06:40:24Z

@smarterclayton - thanks a lot! Commits squashed.

smarterclayton · 2017-07-07T20:34:39Z

One more thing (we can do in a follow up). The web sockets impl must correctly support protocols, and also support base64 token encoding in the protocol (fixes a security hole) kubernetes/kubernetes#47967.

wojtek-t assigned smarterclayton and thockin Mar 10, 2017

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 10, 2017

wojtek-t mentioned this pull request Mar 10, 2017

kubelet access to API server should be subdivided kubernetes/kubernetes#40476

Closed

wojtek-t changed the title ~~List/Watch/Get of objects associated with node [1.7]~~ [WIP] List/Watch/Get of objects associated with node [1.7] Mar 13, 2017

wojtek-t force-pushed the kubelet_watch branch from a19ba32 to cb15b6f Compare March 14, 2017 10:56

deads2k reviewed Mar 14, 2017

View reviewed changes

lavalamp reviewed Mar 20, 2017

View reviewed changes

wojtek-t force-pushed the kubelet_watch branch from aedd6c2 to d590c30 Compare May 15, 2017 09:29

wojtek-t commented May 15, 2017

View reviewed changes

shyamjvs mentioned this pull request May 25, 2017

Watching a list of resource doesn't work kubernetes/kubernetes#15665

Closed

smarterclayton mentioned this pull request Jun 1, 2017

Feature Request: Ability to WATCH more than one resource type kubernetes/kubernetes#1685

Closed

lavalamp reviewed Jun 9, 2017

View reviewed changes

wojtek-t mentioned this pull request Jul 4, 2017

Add scalability good practices doc #740

Merged

wojtek-t force-pushed the kubelet_watch branch from d590c30 to 6b34c98 Compare July 4, 2017 13:10

wojtek-t commented Jul 4, 2017

View reviewed changes

cben reviewed Jul 5, 2017

View reviewed changes

cben mentioned this pull request Jul 5, 2017

switch http client to something that does not have a native extension ManageIQ/kubeclient#237

Open

smarterclayton reviewed Jul 5, 2017

View reviewed changes

wojtek-t force-pushed the kubelet_watch branch from 6b34c98 to 93c1c97 Compare July 6, 2017 10:53

wojtek-t commented Jul 6, 2017

View reviewed changes

Bulk get and bulk watch proposal

64f185b

wojtek-t force-pushed the kubelet_watch branch from c791aa6 to 64f185b Compare July 7, 2017 06:39

smarterclayton merged commit 794c44c into kubernetes:master Jul 7, 2017

davidopp mentioned this pull request Jul 18, 2017

Add policy ConfigMap monitoring and restart to the scheduler. kubernetes/kubernetes#44805

Closed

wojtek-t deleted the kubelet_watch branch July 3, 2018 12:54

danehans pushed a commit to danehans/community that referenced this pull request Jul 18, 2023

Fix Neeraj's LinkedIn link (kubernetes#443)

d6397fc


		Fortunately, we can solve it in much simpler way, with one additional assumption:

		1. All object types necessary to determine the in-memory mapping share the same

		detail hidden in the code (see more details in the next section).


		### Implementation details


		# Detailed design

		We will introduce the following ```node selector ``` filtering mechanism:

List/Watch/Get of objects associated with node #443

List/Watch/Get of objects associated with node #443

Conversation

wojtek-t commented Mar 10, 2017

shyamjvs commented Mar 10, 2017 • edited Loading

liggitt commented Mar 10, 2017

deads2k commented Mar 10, 2017

wojtek-t commented Mar 10, 2017

deads2k commented Mar 10, 2017

wojtek-t commented Mar 10, 2017

deads2k commented Mar 10, 2017

wojtek-t commented Mar 10, 2017

wojtek-t commented Mar 13, 2017 • edited Loading

liggitt commented Mar 13, 2017 • edited Loading

wojtek-t commented Mar 13, 2017

liggitt commented Mar 13, 2017

wojtek-t commented Mar 14, 2017

wojtek-t commented Mar 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Mar 20, 2017

Choose a reason for hiding this comment

lavalamp Mar 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtek-t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtek-t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shyamjvs commented Mar 10, 2017 •

edited

Loading

wojtek-t commented Mar 13, 2017 •

edited

Loading

liggitt commented Mar 13, 2017 •

edited

Loading

lavalamp Mar 20, 2017 •

edited

Loading