Skip to content

Commit 2bb946e

Browse files
committed
Update doc after sig-api-machinery discussion
1 parent ca7439e commit 2bb946e

File tree

1 file changed

+12
-15
lines changed

1 file changed

+12
-15
lines changed

designs/conditional-controllers.md

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -183,29 +183,29 @@ breaks the clean design of the cache.
183183

184184
### Stoppable Informers and EventHandler removal natively in client-go
185185

186+
*This proposal was discussed with sig-api-machinery on 11/5 and has been
187+
tentatively accepted. See the [design
188+
doc](https://docs.google.com/document/d/17QrTaxfIEUNOEAt61Za3Mu0M76x-iEkcmR51q0a5lis/edit) and [WIP implementation](https://github.com/kevindelgado/kubernetes/pull/1) for more detail.*
189+
186190
A few changes to the underlying SharedInformer interface in client-go could save
187191
us from a lot of work in controller-runtime.
188192

189193
One is to add a second `Run` method on the `SharedInformer` interface such as
190194
```
191195
// RunWithStopOptions runs the informer and provides options to be checked that
192196
// would indicate under what conditions the informer should stop
193-
RunWithStopOptions(stopOptions StopOptions) StopReason
197+
RunWithStopOptions(stopOptions StopOptions)
194198
195199
// StopOptions let the caller specifcy which conditions to stop the informer.
196200
type StopOptions struct {
197-
// StopChannel stops the Informer when it receives a close signal
198-
StopChannel <-chan struct{}
201+
// ExternalStop stops the Informer when it receives a close signal
202+
ExternalStop <-chan struct{}
199203
200204
// OnListError inspects errors returned from the informer's underlying
201205
// reflector, and based on the error determines whether or not to stop the
202206
// informer.
203207
OnListError func(err) bool
204208
}
205-
206-
// StopReason is a custom typed error that indicates how the informer was
207-
// stopped
208-
type StopReason error
209209
```
210210

211211
This could be plumbed through controller-runtime allowing users to pass informer
@@ -221,19 +221,13 @@ similar to the `EventHandler` reference counting interface we discussed above th
221221
```
222222
type SharedInformer interface {
223223
...
224-
CountEventHandlers() int
225-
RemoveEventHandler(id) error
224+
RemoveEventHandler(handler ResourceEventHandler) bool
225+
EventHandlerCount() int
226226
}
227227
```
228-
(where `id`` is a to be determined identifer of the specific handler to be
229-
removed).
230228

231229
This would remove the need to track it ourselves in controller-runtime.
232230

233-
TODO: Bring this design up (potentially with a proof-of-concept branch) at an
234-
upcoming sig-api-machinery meeting to begin getting feedback and see whether it
235-
is feasible to land the changes in client-go.
236-
237231
## Open Questions
238232

239233
### Multiple Restarts
@@ -298,4 +292,7 @@ controller-runtime
298292
* 10/8/2020: Discuss idea in community meeting
299293
* 10/19/2020: Proposal updated to add EventHandler count tracking and client-go
300294
based alternative.
295+
* 11/4/2020: Propopsal to add RunWithStopOptions, RemoveEventHandler, and
296+
EventHandlerCount discussed at sig-api-machinery meeting and was tentatively
297+
accepted. See the [design doc](https://docs.google.com/document/d/17QrTaxfIEUNOEAt61Za3Mu0M76x-iEkcmR51q0a5lis/edit) and [WIP implementation](https://github.com/kevindelgado/kubernetes/pull/1) for more detail.
301298

0 commit comments

Comments
 (0)