kubeadm: Control plane config moved to substructs #70371
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
release-note
k8s-ci-robot
added
sig/cluster-lifecycle
k8s-ci-robot
added
size/L
neolit123
approved these changes
Oct 29, 2018
k8s-ci-robot
added
the
needs-rebase
rosti
force-pushed
the
control-plane-substructs
branch
from
1b0c1ad to
dfa69f6
Compare
k8s-ci-robot
removed
the
needs-rebase
3 tasks
fabriziopandini
suggested changes
Oct 31, 2018
Member
fabriziopandini
left a comment
fabriziopandini
left a comment
There was a problem hiding this comment.
@rosti thanks for this PR!
I have no strong opinion on the usage of a shared struct vs dedicated struct.
Everything else seems ok
Only one question. The Change about HostPathMount.Writable to ReadOnly is going to be addressed in another PR?
rosti
force-pushed
the
control-plane-substructs
branch
from
dfa69f6 to
b0bdb41
Compare
Contributor
Author
|
/test pull-kubernetes-e2e-gce-100-performance |
1 task
timothysc
removed
the
kind/api-change
k8s-ci-robot
added
the
needs-kind
timothysc
added
the
kind/cleanup
k8s-ci-robot
removed
the
needs-kind
timothysc
approved these changes
Nov 1, 2018
Contributor
timothysc
left a comment
timothysc
left a comment
There was a problem hiding this comment.
/approve
LGTM but will defer to @fabriziopandini for final call
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rosti, timothysc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Contributor
|
This appears to be blocking a number of your other PRs. |
Member
|
@rosti please rebase and then ping me for lgtm |
In v1alpha3's, control plane component config options were nested directly into the ClusterConfiguration structure. This is cluttering the config structure and makes it hard to maintain. Therefore the control plane config options must be separated into different substructures in order to graduate the format to beta. This change does the following: - Introduces a new structure called ControlPlaneComponent, that contains fields common to all control plane component types. These are currently extra args and extra volumes. - Introduce a new structure called APIServer that contains ControlPlaneComponent and APIServerCertSANs field (from ClusterConfiguration) - Replace all API Server, Scheduler and Controller Manager options in ClusterConfiguration with APIServer, ControllerManager and Scheduler fields of APIServer and ControlPlaneComponent types. Signed-off-by: Rostislav M. Georgiev <[email protected]>
rosti
force-pushed
the
control-plane-substructs
branch
from
b0bdb41 to
d14c27a
Compare
Member
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Contributor
Author
|
/test pull-kubernetes-e2e-kops-aws |
Member
|
/test pull-kubernetes-integration |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind api-change
What this PR does / why we need it:
In v1alpha3's, control plane component config options were nested directly into the ClusterConfiguration structure. This is cluttering the config structure and makes it hard to maintain. Therefore the control plane config options must be separated into different substructures in order to graduate the format to beta.
This change does the following:
Introduces a new structure called ControlPlaneComponent, that contains fields common to all control plane component types. These are currently extra args and extra volumes.
Introduce a new structure called APIServer that contains ControlPlaneComponent and APIServerCertSANs field (from ClusterConfiguration)
Replace all API Server, Scheduler and Controller Manager options in ClusterConfiguration with APIServer, ControllerManager and Scheduler fields of APIServer and ControlPlaneComponent types.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Refs kubernetes/kubeadm#911
Special notes for your reviewer:
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/assign @luxas
/assign @timothysc
/assign @fabriziopandini
This will be followed by a PR that introduces an API server timeout option (as discussed at office hours on Oct 24th.
Does this PR introduce a user-facing change?: