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

kubeadm join: Added support for config file. #34885

Merged
merged 1 commit into from Oct 15, 2016
Merged

kubeadm join: Added support for config file. #34885

merged 1 commit into from Oct 15, 2016

Conversation

pires
Copy link
Contributor

@pires pires commented Oct 15, 2016 •
edited by k8s-oncall

As more behavior (#34719, #34807, fix for #33641) is added to kubeadm join, this will be eventually very much needed. Makes sense to go in sooner rather than later.

Also references #34501 and #34884.

/cc @luxas @mikedanese

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Oct 15, 2016
@luxas luxas added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Oct 15, 2016
@luxas
Copy link
Member

luxas commented Oct 15, 2016

@pires Have you tested it manually and verified that it works?

luxas
luxas approved these changes Oct 15, 2016
View reviewed changes
Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM

If @errordeveloper or @mikedanese could take a very quick look now over the weekend, it would be nice

@luxas
Copy link
Member

luxas commented Oct 15, 2016

LGTM, this doesn't work yet, but I'll follow it up so it works for both init and join eventually

@luxas luxas added lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. labels Oct 15, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@mikedanese mikedanese removed lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. labels Oct 15, 2016
@mikedanese
Copy link
Member

mikedanese commented Oct 15, 2016 •
edited

Can you explain why it doesn't work? Why don't we just fix it?

@luxas
Copy link
Member

luxas commented Oct 15, 2016

@mikedanese Because "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/install" isn't registered anywhere in cmd/kubeadm, but I soon have a PR up for that.

I meant that kubeadm init --config doesn't work now
This change is ok, but we also need to do _ "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/install" for init and join to work with --config

Can you reapply the lgtm label?

@mikedanese mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2016
@mikedanese
Copy link
Member

Ok please give details when you say stuff like that in the future.

@luxas
Copy link
Member

luxas commented Oct 15, 2016

Yep, actually I wasn't 100% sure what the root cause was 30 mins ago, but I knew I needed this change which just duplicates the code that is for kubeadm init now

@luxas luxas added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Oct 15, 2016
@luxas luxas mentioned this pull request Oct 15, 2016
Merged
@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit ab14c31 into kubernetes:master Oct 15, 2016
@pires pires deleted the kubeadm_join_configuration branch October 15, 2016 17:15
@k8s-github-robot k8s-github-robot mentioned this pull request Oct 15, 2016
Closed
k8s-github-robot pushed a commit that referenced this pull request Oct 16, 2016
Merge pull request #34891 from luxas/kubeadm_configfile
fb83dcc 
Automatic merge from submit-queue

Register the kubeadm api group in cmd/kubeadm

Fixes the issue mentioned in: #34885 (comment)

My test config:
```yaml
apiVersion: kubeadm.k8s.io/v1alpha1
kind: MasterConfiguration
kubernetesVersion: v1.4.2
networking:
  podSubnet: 10.244.0.0/16
  dnsDomain: k8s.somecompany.com
  serviceSubnet: 10.16.0.0/12
api:
  externalDNSNames:
   - myawesomek8snode
secrets:
  givenToken: 92f7e2.3685fe2e01f799f3
```
```console
# kubeadm init --config kubeadm.yaml
```
@kubernetes/sig-cluster-lifecycle
@luxas luxas mentioned this pull request Oct 16, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luxas luxas luxas approved these changes

Assignees

@errordeveloper errordeveloper

@luxas luxas

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@pires @luxas @k8s-github-robot @mikedanese @errordeveloper @googlebot