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.

Sign up for GitHub

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

Jump to bottom

Set the kubelet --hostname-override flag conditionally at kubeadm init time #846

Closed
luxas opened this issue May 22, 2018 · 3 comments · Fixed by kubernetes/kubernetes#64706
Closed

Set the kubelet --hostname-override flag conditionally at kubeadm init time #846

luxas opened this issue May 22, 2018 · 3 comments · Fixed by kubernetes/kubernetes#64706
Assignees
@timothysc @detiber @liztio
Labels
area/security area/UX kind/bug Categorizes issue or PR as related to a bug. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone
v1.11

Comments

@luxas
Copy link
Member

luxas commented May 22, 2018

When kubernetes/kubernetes#63887 merges, we should set the kubelet's --hostname-override flag to what's passed to kubeadm via the --node-name flag conditionally if cfg.NodeName != nodeutil.GetHostname("") (if the flag differs from the default) at kubeadm init time, in cmd/kubeadm/app/phases/kubelet/flags.go.

This will automagically fix #584
xref related issue: #822
/assign @liztio
as you're assigned to #584. LMK if you don't have time to do this.
@luxas luxas added kind/enhancement priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/security area/UX labels May 22, 2018
@luxas luxas added this to the v1.11 milestone May 22, 2018
@luxas luxas mentioned this issue May 22, 2018
Closed
@detiber
Copy link
Member

detiber commented May 23, 2018

There is another tricky little aspect to this. Since we currently deploy kube-proxy as a daemonset with a single config map, we also need to have a way to pass this value along to the kube-proxy config as well, otherwise Services of type NodePort or LoadBalancer configured with externalTrafficPolicy: local will fail.

@luxas
Copy link
Member Author

luxas commented May 23, 2018

@detiber do you have more information on this somewhere? Would this mean every kube-proxy daemon needs to be configured differently?

@detiber
Copy link
Member

detiber commented May 24, 2018

@luxas I created a separate issue to track the kube-proxy related config: #857

@timothysc timothysc assigned timothysc and detiber and unassigned liztio May 29, 2018
@timothysc timothysc added kind/bug Categorizes issue or PR as related to a bug. and removed kind/enhancement labels May 29, 2018
@liztio liztio self-assigned this Jun 4, 2018
@liztio liztio added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Jun 4, 2018
@liztio liztio mentioned this issue Jun 5, 2018
Merged
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Jun 5, 2018
Merge pull request #64706 from liztio/kubeadm-hostname-override
c46e8a9 
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>.

kubeadm: Let the `--node-name` flag flow down to `--hostname-override` for the kubelet

**What this PR does / why we need it**:

Kubeadm-initialised kubelet uses provided hostname if present

If --node-name is passed in to `kubeadm init`, `--hostname-override` will be
passed to kubelet. This prevents timeout errors for `kubeadm init`.

**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 kubernetes/kubeadm#846

**Special notes for your reviewer**:
Depends on #64624 to work fully, but can safely merged before hand.

**Release note**:

```release-note
[action required] The `--node-name` flag for kubeadm now dictates the Node API object name the
kubelet uses for registration, in all cases but where you might use an in-tree cloud provider.
If you're not using an in-tree cloud provider, `--node-name` will set the Node API object name.
If you're using an in-tree cloud provider, you MUST make `--node-name` match the name the
in-tree cloud provider decides to use.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@timothysc timothysc

@detiber detiber

@liztio liztio

Labels
area/security area/UX kind/bug Categorizes issue or PR as related to a bug. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Milestone
v1.11
Development

Successfully merging a pull request may close this issue.

kubeadm: Let the --node-name flag flow down to --hostname-override for the kubelet liztio/kubernetes
4 participants
@timothysc @detiber @luxas @liztio