Support scheduler config options so custom schedulers can be used #10064
Conversation
|
[test] |
|
👍 |
| // SchedulerConfigFile points to a file that describes how to set up the scheduler. If empty, you get the default scheduling rules. | ||
| SchedulerConfigFile string `json:"schedulerConfigFile"` | ||
| // SchedulerName is the name this scheduler uses to assign pods. |
There was a problem hiding this comment.
can we group scheduler config options?
edit: actually, we should go the schedulerArguments route
There was a problem hiding this comment.
We can group them - except we'd have to move scheduler config (to be
consistent with upstream). Upstream has two configs - the policy file, and
then the "component config" that wraps it. I don't like having scheduler
config file at the root and the other scheduler config in a sub object, but
I could be convinced. I'd argue we'd fix this by splitting it out
eventually.
On Wed, Jul 27, 2016 at 3:52 PM, Jordan Liggitt [email protected]
wrote:
In pkg/cmd/server/api/v1/types.go
#10064 (comment):// SchedulerConfigFile points to a file that describes how to set up the scheduler. If empty, you get the default scheduling rules. SchedulerConfigFile string `json:"schedulerConfigFile"`
- // SchedulerName is the name this scheduler uses to assign pods.
I'm sort of surprised these options weren't already part of the scheduler
config object read from the file—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/openshift/origin/pull/10064/files/b4296d1dbcfd5294ac9467d4d42ad735d1cfbc5d#r72510463,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p4Lwhdvxg6K-Glh-pzVW032YvinVks5qZ7bwgaJpZM4JWjvW
.
There was a problem hiding this comment.
I'd rather have schedulerArguments, override the default scheduler file with the first-class field from our config, and be perfectly consistent with no effort
There was a problem hiding this comment.
I think doing that is going to require fixing the upstream scheduler args,
because it includes the scheduler, so it would get added into oc.
On Wed, Jul 27, 2016 at 4:04 PM, Jordan Liggitt [email protected]
wrote:
In pkg/cmd/server/api/v1/types.go
#10064 (comment):// SchedulerConfigFile points to a file that describes how to set up the scheduler. If empty, you get the default scheduling rules. SchedulerConfigFile string `json:"schedulerConfigFile"`
- // SchedulerName is the name this scheduler uses to assign pods.
I'd rather have schedulerArguments, override the default scheduler file
with the first-class field from our config, and be perfectly consistent
with no effort—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/openshift/origin/pull/10064/files/b4296d1dbcfd5294ac9467d4d42ad735d1cfbc5d#r72512606,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p_GrH3yWraronSVzq3cZSn9zYJ3wks5qZ7nogaJpZM4JWjvW
.
There was a problem hiding this comment.
I mean schedulerArguments map[string][]string, like we do for other components...
There was a problem hiding this comment.
Apparently we were already including the upstream scheduler, so it's not
any worse.
On Wed, Jul 27, 2016 at 4:16 PM, Clayton Coleman [email protected]
wrote:
I think doing that is going to require fixing the upstream scheduler args,
because it includes the scheduler, so it would get added into oc.On Wed, Jul 27, 2016 at 4:04 PM, Jordan Liggitt [email protected]
wrote:In pkg/cmd/server/api/v1/types.go
#10064 (comment):// SchedulerConfigFile points to a file that describes how to set up the scheduler. If empty, you get the default scheduling rules.
SchedulerConfigFile stringjson:"schedulerConfigFile"
- // SchedulerName is the name this scheduler uses to assign pods.
I'd rather have schedulerArguments, override the default scheduler file
with the first-class field from our config, and be perfectly consistent
with no effort—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/openshift/origin/pull/10064/files/b4296d1dbcfd5294ac9467d4d42ad735d1cfbc5d#r72512606,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p_GrH3yWraronSVzq3cZSn9zYJ3wks5qZ7nogaJpZM4JWjvW
.
There was a problem hiding this comment.
Updated
On Wed, Jul 27, 2016 at 4:30 PM, Clayton Coleman [email protected]
wrote:
Apparently we were already including the upstream scheduler, so it's not
any worse.On Wed, Jul 27, 2016 at 4:16 PM, Clayton Coleman [email protected]
wrote:I think doing that is going to require fixing the upstream scheduler
args, because it includes the scheduler, so it would get added into oc.On Wed, Jul 27, 2016 at 4:04 PM, Jordan Liggitt <[email protected]
wrote:
In pkg/cmd/server/api/v1/types.go
#10064 (comment):// SchedulerConfigFile points to a file that describes how to set up the scheduler. If empty, you get the default scheduling rules.
SchedulerConfigFile stringjson:"schedulerConfigFile"
- // SchedulerName is the name this scheduler uses to assign pods.
I'd rather have schedulerArguments, override the default scheduler file
with the first-class field from our config, and be perfectly consistent
with no effort—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/openshift/origin/pull/10064/files/b4296d1dbcfd5294ac9467d4d42ad735d1cfbc5d#r72512606,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p_GrH3yWraronSVzq3cZSn9zYJ3wks5qZ7nogaJpZM4JWjvW
.
smarterclayton
force-pushed
the
scheduler_config
branch
from
b4296d1 to
9db658c
Compare
| // resolve extended arguments | ||
| // TODO: this should be done in config validation (along with the above) so we can provide | ||
| // proper errors | ||
| schedulerserver := scheduleroptions.NewSchedulerServer() |
There was a problem hiding this comment.
thanks, can you make an equivalent to TestCMServerDefaults that smacks us when the defaults change so we can decide if any need overriding?
|
test needed, LGTM otherwise |
smarterclayton
force-pushed
the
scheduler_config
branch
2 times, most recently
from
9fb9f1f to
c26c204
Compare
smarterclayton
force-pushed
the
scheduler_config
branch
from
c26c204 to
02d7060
Compare
|
test added, rereview while [merge] #9959 flake |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7283/) (Image: devenv-rhel7_4714) |
|
Evaluated for origin merge up to 02d7060 |
|
LGTM |
|
[test] #9959 |
|
Evaluated for origin test up to 02d7060 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7283/) |
Will allow us to be consistent with upstream on this.
@ncdc