From d7a772a8dee46c970e4b131af5db9b34011935d0 Mon Sep 17 00:00:00 2001 From: Mike Spreitzer Date: Mon, 16 Mar 2020 17:38:14 -0400 Subject: [PATCH] Documented mutation restriction for informer clients Also brushed up some other informer comments for readability and nitpicking accuracy. Kubernetes-commit: c4774de94a80bb7d0f54a8a728ad3e97ad1a07b9 --- tools/cache/controller.go | 12 +++++++---- tools/cache/shared_informer.go | 38 ++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/tools/cache/controller.go b/tools/cache/controller.go index 5d582119..9a706821 100644 --- a/tools/cache/controller.go +++ b/tools/cache/controller.go @@ -183,9 +183,11 @@ func (c *controller) processLoop() { } } -// ResourceEventHandler can handle notifications for events that happen to a -// resource. The events are informational only, so you can't return an -// error. +// ResourceEventHandler can handle notifications for events that +// happen to a resource. The events are informational only, so you +// can't return an error. The handlers MUST NOT modify the objects +// received; this concerns not only the top level of structure but all +// the data structures reachable from it. // * OnAdd is called when an object is added. // * OnUpdate is called when an object is modified. Note that oldObj is the // last known state of the object-- it is possible that several changes @@ -205,7 +207,8 @@ type ResourceEventHandler interface { // ResourceEventHandlerFuncs is an adaptor to let you easily specify as many or // as few of the notification functions as you want while still implementing -// ResourceEventHandler. +// ResourceEventHandler. This adapter does not remove the prohibition against +// modifying the objects. type ResourceEventHandlerFuncs struct { AddFunc func(obj interface{}) UpdateFunc func(oldObj, newObj interface{}) @@ -237,6 +240,7 @@ func (r ResourceEventHandlerFuncs) OnDelete(obj interface{}) { // in, ensuring the appropriate nested handler method is invoked. An object // that starts passing the filter after an update is considered an add, and an // object that stops passing the filter after an update is considered a delete. +// Like the handlers, the filter MUST NOT modify the objects it is given. type FilteringResourceEventHandler struct { FilterFunc func(obj interface{}) bool Handler ResourceEventHandler diff --git a/tools/cache/shared_informer.go b/tools/cache/shared_informer.go index df8c67dc..8caa818e 100644 --- a/tools/cache/shared_informer.go +++ b/tools/cache/shared_informer.go @@ -34,29 +34,31 @@ import ( // SharedInformer provides eventually consistent linkage of its // clients to the authoritative state of a given collection of // objects. An object is identified by its API group, kind/resource, -// namespace, and name; the `ObjectMeta.UID` is not part of an -// object's ID as far as this contract is concerned. One +// namespace (if any), and name; the `ObjectMeta.UID` is not part of +// an object's ID as far as this contract is concerned. One // SharedInformer provides linkage to objects of a particular API // group and kind/resource. The linked object collection of a -// SharedInformer may be further restricted to one namespace and/or by -// label selector and/or field selector. +// SharedInformer may be further restricted to one namespace (if +// applicable) and/or by label selector and/or field selector. // // The authoritative state of an object is what apiservers provide // access to, and an object goes through a strict sequence of states. -// An object state is either "absent" or present with a -// ResourceVersion and other appropriate content. +// An object state is either (1) present with a ResourceVersion and +// other appropriate content or (2) "absent". // -// A SharedInformer maintains a local cache, exposed by GetStore() and -// by GetIndexer() in the case of an indexed informer, of the state of -// each relevant object. This cache is eventually consistent with the -// authoritative state. This means that, unless prevented by -// persistent communication problems, if ever a particular object ID X -// is authoritatively associated with a state S then for every -// SharedInformer I whose collection includes (X, S) eventually either -// (1) I's cache associates X with S or a later state of X, (2) I is -// stopped, or (3) the authoritative state service for X terminates. -// To be formally complete, we say that the absent state meets any -// restriction by label selector or field selector. +// A SharedInformer maintains a local cache --- exposed by GetStore(), +// by GetIndexer() in the case of an indexed informer, and possibly by +// machinery involved in creating and/or accessing the informer --- of +// the state of each relevant object. This cache is eventually +// consistent with the authoritative state. This means that, unless +// prevented by persistent communication problems, if ever a +// particular object ID X is authoritatively associated with a state S +// then for every SharedInformer I whose collection includes (X, S) +// eventually either (1) I's cache associates X with S or a later +// state of X, (2) I is stopped, or (3) the authoritative state +// service for X terminates. To be formally complete, we say that the +// absent state meets any restriction by label selector or field +// selector. // // For a given informer and relevant object ID X, the sequence of // states that appears in the informer's cache is a subsequence of the @@ -98,7 +100,7 @@ import ( // added before `Run()`, eventually either the SharedInformer is // stopped or the client is notified of the update. A client added // after `Run()` starts gets a startup batch of notifications of -// additions of the object existing in the cache at the time that +// additions of the objects existing in the cache at the time that // client was added; also, for every update to the SharedInformer's // local cache after that client was added, eventually either the // SharedInformer is stopped or that client is notified of that