refactor: use watch api and instant poll new or modified secrets #107
Conversation
|
@Flydiverny an alternative to polling is using the Kubernetes API watch endpoints. I think we want to use those eventually, but hadn't gotten around to the implementation. If we're going to make improvements to this code, it would be good for those improvements to work with the watch implementation (or maybe we do that one first?). @satish-ravi, thoughts? |
|
yeah I was wondering why it didn't use watch, but still tried to simulate it somehow? :D So I went with yielding event types based on what watch should provide, not sure about the payload tho. I'm not sure how to use watch with the kubernetes-client though Figured out how to watch, but seems like abit of a headache to combine subscribing to a stream with generators, as you cant yield inside the event listener. |
|
@silasbw made some updates, now using the watch api instead. |
|
Hi @Flydiverny. I'm taking a look at this today |
There was a problem hiding this comment.
@Flydiverny - I've been running this locally, and the other than the tests needing to be updated, this is looking pretty good to me
|
Yeah, I'll update the test to work with the stream. |
There was a problem hiding this comment.
To handle stream errors/disconnects, we could just reconnect by calling getStream again and attaching listeners. This would trigger ADDED events for all existing resources, which would allow us to rebuild pollers.
Re getStream deprecation, @silasbw should we add getObjectStream for CRDs in kubernetes-client?
lib/external-secret.js
Outdated
| }) | ||
|
|
||
| jsonStream.on('end', () => { | ||
| logger.warn('Watch stream ended') |
There was a problem hiding this comment.
This could potentially make the logs noisy. The kube-apiserver disconnects streams periodically (see min-request-timeout here https://kubernetes.io/docs/reference/command-line-tools-reference/kube-apiserver/)
|
Yes, we should add |
Flydiverny
changed the title
Flydiverny
force-pushed
the
refactor/allow-faster-api-polling
branch
2 times, most recently
from
497097b
to
5220628
Compare
Flydiverny
force-pushed
the
refactor/allow-faster-api-polling
branch
from
5220628
to
572a2e2
Compare
|
Please check again if there is something more to be done, or that you want reverted :) |
|
Hi @Flydiverny sorry for the delay. I'll take another look today. |
There was a problem hiding this comment.
This looks good to me, but I'd like @satish-ravi to look again
Co-Authored-By: Satish Ravi <satishmufc@gmail.com>
|
On vacation so can't follow up on the failed build for some days not sure why it decided to fail suddenly :) |
Currently you can't set a lower events interval than poll interval, as this would result in never polling anything as the poller would be recreated everytime the events are polled.
Not sure if this is the best way to go about it but I tested refactoring so the events poller tracks which external secrets it has seen and yields appropriate events instead of always doing a delete all.
It would be nice if we could in some way instantly poll if an external secret has been modified or added. As if you have a long poll time you would currently have to wait for that to trigger, which feels slow when adding a new external secret. Not sure how to deal with pod restarts tho, as you wouldn't necessarily want to poll instantly when that happens, one alternative would be for the poller to check if the secret exists when started, and if it doesn't then trigger a poll right away.
Let me know what you think :)
ping @silasbw