Add --sleep-interval=infinite support to GFD for running as a pod#1603
Add --sleep-interval=infinite support to GFD for running as a pod#1603rajathagasthya merged 1 commit intomainfrom
--sleep-interval=infinite support to GFD for running as a pod#1603Conversation
rajathagasthya
requested review from
ArangoGutierrez,
cdesiniotis,
elezar and
tariq1890
rajathagasthya
changed the title
--oneshot-daemon flag to GFD for running as a pod
rajathagasthya
force-pushed
the
gfd-oneshot
branch
from
df514dc to
1761101
Compare
rajathagasthya
force-pushed
the
gfd-oneshot
branch
2 times, most recently
from
dcca988 to
97c875f
Compare
There was a problem hiding this comment.
instead of adding a new flag, does it not make sense to reuse the sleep interval? We could add support for infinite as a special string that disables the main loop.
See:
k8s-device-plugin/api/config/v1/duration.go
Lines 43 to 50 in 1e44e57
Alternatively, a user COULD aready get close to this behaviour by setting this duration to something really long (years?).
rajathagasthya
force-pushed
the
gfd-oneshot
branch
from
97c875f to
5a398b6
Compare
rajathagasthya
changed the title
--oneshot-daemon flag to GFD for running as a pod--sleep-interval=infinite support to GFD for running as a pod
rajathagasthya
force-pushed
the
gfd-oneshot
branch
from
5a398b6 to
18b4d02
Compare
|
|
||
| // IsInfinite returns true if the duration represents an infinite sleep interval. | ||
| func (d *Duration) IsInfinite() bool { | ||
| return d != nil && time.Duration(*d) == math.MaxInt64 |
There was a problem hiding this comment.
Are we concerned with overvflow if we set this to MaxInt64? We could also reaslistically use 0 or -1 as values here.
There was a problem hiding this comment.
I don't think there's an overflow risk here. We always check IsInfinite() before using the duration in time.After(), When infinite, we skip the timer entirely. time.Duration(MaxInt64) is also a valid duration, so I'd prefer to keep it this way than using 0 or -1 sentinel values.
There was a problem hiding this comment.
nit: We could introduce a constant for Infinite as well. Not a blocker for this PR.
There was a problem hiding this comment.
Ack. I'll add that if there are other comments I need to address. If not, will do it in a follow up.
elezar
left a comment
elezar
left a comment
There was a problem hiding this comment.
Thanks @rajathagasthya. I think this has a much better UX than the other proposal.
I would recommend updating the Duration.MarshalJSON function to also output infinite when we save or log the config.
Reuse the existing --sleep-interval flag with support for 'infinite' as a special value. This causes GFD to label once and sleep indefinitely, which is useful for running as a Kubernetes pod that should not exit. Signed-off-by: Rajath Agasthya <[email protected]>
rajathagasthya
force-pushed
the
gfd-oneshot
branch
from
18b4d02 to
ae64126
Compare
5 participants
Reuse the existing --sleep-interval flag with support for 'infinite' as a special value. This causes GFD to label once and sleep indefinitely, which is useful for running as a Kubernetes pod that should not exit.