controller/engine: add queueing index method to NamedController #672
Conversation
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@upbound.io>
| GetCache() cache.Cache | ||
| IndexField(obj client.Object, field string, extractValue client.IndexerFunc) |
There was a problem hiding this comment.
Could you add a doc string for this method, since the others all have them.
| @@ -281,7 +305,18 @@ func (c *namedController) Start(ctx context.Context) error { | |||
| return nil | |||
| } | |||
|
|
|||
| // IndexField queues an index for addition to the cache on start. | |||
There was a problem hiding this comment.
Would QueueIndexField or IndexFieldOnStart or something similar make this more self-descriptive?
What happens if this is called after the controller/cache is started? Nothing I guess? (Worth mentioning in doc string.)
There was a problem hiding this comment.
Actually, standby with this PR. The context passed to GetInformer and with that to IndexField is only used for waiting, not for the informer life-cycle. I think this was a misunderstanding.
| indexes []index | ||
| } | ||
|
|
||
| type index struct { |
There was a problem hiding this comment.
Would it make sense to export this type and have Start take a variadic array of it? I'm not sure how this is called, so I'm happy if it's more ergonomic to use the method approach instead.
e.g.:
c.Start(ctx,
Index{Object: o, Field: "spec.hi", ExtractValue: ifn},
Index{Object: o, Field: "spec.hello", ExtractValue: ifn2})
sttts
force-pushed
the
sttts-controller-engine-indexes
branch
from
ffc8dce
to
c4f6831
Compare
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@upbound.io>
sttts
force-pushed
the
sttts-controller-engine-indexes
branch
from
c4f6831
to
956937e
Compare
|
#689 moved the engine implementation to c/c. The implementation is being reworked there so this PR may no longer be relevant but either way it won't happen in this repo. 🙂 |
Description of your changes
Using
GetCache()'sIndexFieldmethod will use the passed context for the life-cycle of informers started at that moment. This is usually not what is desired, but the life-cycle of the cache should match the life-cycle of the controller, and this applies to all informers of course.This PR adds a
IndexFieldmethod to queue up index creation until the controller has been started.Note that the
GetCache()method is still useful to e.g. wire it up into other components that want to read from the cache later.I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Tested in Crossplane fixing realtime composition issues.