Skip to content

Conversation

@russellb
Copy link
Contributor

This change makes use of a new configuration item on the scheduler CR
that specifies that control plane hosts should be able to run
workloads. This option is off by default, but can be changed through
a new install-config option under controlPlane.

The justification for a new install-config option for this is that
this is a required tunable for some clusters to have a successful
install. A 3-node cluster on bare metal for example will need
schedulable masters.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 17, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: russellb
To complete the pull request process, please assign crawford
You can assign the PR to them by writing /assign @crawford in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@russellb
Copy link
Contributor Author

This is an alternative to #2004

@russellb
Copy link
Contributor Author

Looks like I had a bad rebase of the vendor patch, will fix

@russellb russellb force-pushed the schedulable-masters-alt branch from 8f213ed to 1414269 Compare July 17, 2019 18:27
@ravisantoshgudimetla
Copy link
Contributor

I think this is better than #2004, stating the intention clearly. I'll let @eparis @abhinavdahiya to comment on it though

@stbenjam
Copy link
Member

Looks like this needs a rebase. Also, I like this approach much better as well.

russellb added 2 commits July 18, 2019 16:34
The scheduler config includes a new MastersSchedulable option.  It's
false by default, but include it in the manifest to make it a little
easier for someone to change if they want to after running "create
manifests".
This change makes use of a new configuration item on the scheduler CR
that specifies that control plane hosts should be able to run
workloads.  This option is off by default, but can be changed through
a new install-config option under controlPlane.

The justification for a new install-config option for this is that
this is a required tunable for some clusters to have a successful
install.  A 3-node cluster on bare metal for example will need
schedulable masters.
@russellb russellb force-pushed the schedulable-masters-alt branch from 1414269 to ba32a62 Compare July 18, 2019 20:51
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 18, 2019
@DanyC97
Copy link
Contributor

DanyC97 commented Jul 18, 2019

A 3-node cluster on bare metal for example will need schedulable masters

@russellb curious, why on BM the control plane needs to allow running pods ? is it for cost reasons?

@markmc
Copy link
Contributor

markmc commented Jul 19, 2019

A 3-node cluster on bare metal for example will need schedulable masters

@russellb curious, why on BM the control plane needs to allow running pods ? is it for cost reasons?

Great question. Yes - we believe that with bare metal machines, they will commonly have substantially more capacity than what is required for masters. We also believe it makes sense to support bare metal clusters that don't have separate worker nodes at all, at least for the initial install.

None of those concerns are specific to bare metal, though. It's just we believe they will be more common on bare metal.

@smarterclayton
Copy link
Contributor

I prefer to have one less knob (the alternative), since 3 node clusters are a thing we want to make first class.

As we tune resource usage of the control plane I would expect the most common ootb deploy configuration to be a 3 node clusters with burstable workers. That also forces us to create resilient control planes and to invest in proper resource scheduling preemption.

@eparis
Copy link
Member

eparis commented Jul 22, 2019

/retest

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 22, 2019

@russellb: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws ba32a62 link /test e2e-aws
ci/prow/e2e-aws-scaleup-rhel7 ba32a62 link /test e2e-aws-scaleup-rhel7
ci/prow/e2e-aws-upgrade ba32a62 link /test e2e-aws-upgrade

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@stbenjam
Copy link
Member

We're going to take #2004 instead of this one

/close

@openshift-ci-robot
Copy link
Contributor

@stbenjam: Closed this PR.

Details

In response to this:

We're going to take #2004 instead of this one

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants