Skip to content
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

controller-manager: switch to options+config pattern and add https+auth #59582

Merged
merged 5 commits into from
Feb 13, 2018

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Feb 8, 2018

This PR switch the {kube,cloud}-controller-managers to use the Options+Config struct pattern for bootstrapping, as we use it throughout all apiservers. This allows us to easily plug in https and authn/z support.

Fixes parts of #59483

This is equivalent to #59408 after squashing.

Deprecate insecure HTTP port of kube-controller-manager and cloud-controller-manager. Use `--secure-port` and `--bind-address` instead.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 8, 2018
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 8, 2018
@sttts
Copy link
Contributor Author

sttts commented Feb 8, 2018

/cc @stewart-yu

@k8s-ci-robot
Copy link
Contributor

@sttts: GitHub didn't allow me to request PR reviews from the following users: stewart-yu.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @stewart-yu

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.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2018
@sttts sttts added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2018
@sttts
Copy link
Contributor Author

sttts commented Feb 13, 2018

@cheftako @deads2k addressed your comments.

@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 13, 2018
@sttts sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2018
@sttts
Copy link
Contributor Author

sttts commented Feb 13, 2018

/assign @luxas

For approval of the cloud-cfg-mgr changes.

@sttts
Copy link
Contributor Author

sttts commented Feb 13, 2018

/assign @mikedanese

For approval. Also compare #59814 to avoid this in the future for the owner-less generic controller-manager code (which should not stay in cmd/ for long, but that's another story).

@mikedanese
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, mikedanese, sttts

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

The pull request process is described here

Needs approval from an approver in each of these OWNERS Files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 13, 2018
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 59653, 58812, 59582, 59665, 59511). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit bd6b71d into kubernetes:master Feb 13, 2018
mikedanese added a commit to mikedanese/kubernetes that referenced this pull request Feb 15, 2018
k8s-github-robot pushed a commit that referenced this pull request Feb 15, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

apiserver: fix some typos from refactor

introduced in #59582

```release-note
NONE
```
jingxu97 pushed a commit to jingxu97/kubernetes that referenced this pull request Mar 13, 2018
k8s-github-robot pushed a commit that referenced this pull request Apr 10, 2018
…-chain

Automatic merge from submit-queue (batch tested with PRs 60197, 61614, 62074, 62071, 62301). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Refactor controller-manager: turn Serve func into handlerchain builder

**What this PR does / why we need it**:
follow up #59582 fix
> turn Serve func into a handler chain builder #59582 (comment)

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
k8s-github-robot pushed a commit that referenced this pull request Apr 13, 2018
Automatic merge from submit-queue (batch tested with PRs 61306, 60270, 62496, 62181, 62234). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

split up the huge set of flags into smaller option structs

**What this PR does / why we need it**:
To make generic, we do following work:

1.  Spliting `KubeControllerManagerConfiguration` in kube-controller-manager and cloud-controller-manager into fewer smaller struct options order by controller, and modify relative flag. Also part of #59483.
2. Spliting `componentconfig` in controller-manager into fewer smaller config order by controller too.

All works follow #59582, using `option+config` logic.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants