-
Notifications
You must be signed in to change notification settings - Fork 793
feat: add HTTP trigger API for on-demand descheduling cycles #1857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -532,23 +532,44 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer | |
| return err | ||
| } | ||
|
|
||
| wait.NonSlidingUntil(func() { | ||
| // A next context is created here intentionally to avoid nesting the spans via context. | ||
| sCtx, sSpan := tracing.Tracer().Start(ctx, "NonSlidingUntil") | ||
| executeCycle := func() error { | ||
| sCtx, sSpan := tracing.Tracer().Start(ctx, "DeschedulingCycle") | ||
| defer sSpan.End() | ||
|
|
||
| if err := runLoop(sCtx); err != nil { | ||
| sSpan.AddEvent("Descheduling loop failed", trace.WithAttributes(attribute.String("err", err.Error()))) | ||
| klog.Error(err) | ||
| return | ||
| } | ||
| // If there was no interval specified, send a signal to the stopChannel to end the wait.Until loop after 1 iteration | ||
| if rs.DeschedulingInterval.Seconds() == 0 { | ||
| cancel() | ||
| return err | ||
| } | ||
| }, rs.DeschedulingInterval, ctx.Done()) | ||
| return nil | ||
| } | ||
|
|
||
| return nil | ||
| executeCycle() //nolint:errcheck | ||
|
|
||
| if rs.DeschedulingInterval.Seconds() == 0 && rs.TriggerCh == nil { | ||
| return nil | ||
| } | ||
|
|
||
| var tickerC <-chan time.Time | ||
| if rs.DeschedulingInterval.Seconds() > 0 { | ||
| ticker := time.NewTicker(rs.DeschedulingInterval) | ||
| defer ticker.Stop() | ||
| tickerC = ticker.C | ||
| } | ||
|
|
||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return nil | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. case <-ctx.Done():
// Drain any pending trigger so the caller doesn't hang.
select {
case resultCh := <-rs.TriggerCh:
resultCh <- fmt.Errorf("descheduler shutting down")
default:
}
return nil edge case: if a request is sitting in TriggerCh (buffer 1) when shutdown fires, this returns and the handler's <-resultCh blocks until the request context is torn down — which usually shows up as a 504 to the caller during a graceful shutdown. Maybe drain it on the way out? 🤔
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, missed this case :) i will fix it! |
||
| case <-tickerC: | ||
| executeCycle() //nolint:errcheck | ||
| case resultCh := <-rs.TriggerCh: | ||
| klog.V(1).Info("Descheduling cycle triggered via API") | ||
| err := executeCycle() | ||
| if resultCh != nil { | ||
| resultCh <- err | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func GetPluginConfig(pluginName string, pluginConfigs []api.PluginConfig) (*api.PluginConfig, int) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What sort of authorization is done ? i.e. how does this code distinguishes between authorized and unauthorized callers ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The meaning here is ambiguous; I meant that only those with access to the cluster and port can run it. But as @googs1025 asked, I'll add authorization. :)