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.KubeADM interface #486

Merged
merged 3 commits into from
Jun 20, 2019
Merged

kubeadm.KubeADM interface #486

merged 3 commits into from
Jun 20, 2019

Conversation

kron4eg
Copy link
Member

@kron4eg kron4eg commented Jun 11, 2019 •
edited
Loading

What this PR does / why we need it:
A way to abstract different kubeadm API version.

Fixes #505

NONE

@kron4eg kron4eg requested a review from xmudrii June 11, 2019 11:01
@kubermatic-bot kubermatic-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 11, 2019
@kron4eg kron4eg changed the title WIP: kubeadm.KubeADM interface kubeadm.KubeADM interface Jun 11, 2019
@kubermatic-bot kubermatic-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 11, 2019
@kron4eg
Copy link
Member Author

kron4eg commented Jun 11, 2019

/assigne @xmudrii

@kron4eg
Copy link
Member Author

kron4eg commented Jun 11, 2019

/assign @xmudrii

xmudrii
xmudrii reviewed Jun 12, 2019
View reviewed changes
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

Left some questions, otherwise LGTM 💯

pkg/templates/kubeadm/kubeadmv1beta1.go Outdated Show resolved Hide resolved
pkg/templates/kubeadm/provider.go Outdated Show resolved Hide resolved
pkg/templates/kubeadm/provider.go Outdated
if err != nil {
return nil, errors.Wrap(err, "failed to parse version")
}
switch {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to enforce this check at the validation level too. If you provide not supported version, e.g. 1.16, it's going to pass validation but fail on install.

Copy link
Member Author

Choose a reason for hiding this comment

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

but fail on install

Not necessary. As kubeadm interfaces are stabilizing they are becoming more and more forward compatible. In order to avoid failing with 1.15 I'll add v1beta2 kubeadm APIs to this PR.

pkg/templates/kubeadm/provider.go Outdated
}

// By default use latest known kubeadm API version
return &kubeadmv1beta2{}, nil
Copy link
Member

Choose a reason for hiding this comment

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

Related to the previous comment: should we strong enforce supported versions or not? For example, if 1.16 introduces any breaking change in kubeadm (like drop --experimental), that would render KubeOne unusable.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't use --experimental btw ;)

1.14 is using kubeadm upgrade node experimental-control-plane that's true. But starting from kubeadm 1.15 onward it will became stable kubeadm upgrade node control-plane. So v1beta2 is forward compatible up to the kubeadm version that will drop v1beta2. That's why it's default.

When new versions will be released or at least close to it, we will insert them here also.

pkg/templates/util.go Show resolved Hide resolved
@kubermatic-bot kubermatic-bot added dco-signoff: no Denotes that at least one commit in the pull request doesn't have a valid DCO signoff message. and removed dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. labels Jun 12, 2019
@kron4eg kron4eg changed the title kubeadm.KubeADM interface WIP: kubeadm.KubeADM interface Jun 12, 2019
@kubermatic-bot kubermatic-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 12, 2019
@kron4eg kron4eg force-pushed the kubeadm_interface branch 2 times, most recently from bcbf343 to acbbb67 Compare June 19, 2019 15:58
@xmudrii xmudrii added this to the v0.9 milestone Jun 20, 2019
@xmudrii
Copy link
Member

xmudrii commented Jun 20, 2019

This might be required for the v0.9 release and for 1.15 to work properly with all features
/priority critical-urgent

@kubermatic-bot kubermatic-bot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jun 20, 2019
@kubermatic-bot kubermatic-bot added dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. and removed dco-signoff: no Denotes that at least one commit in the pull request doesn't have a valid DCO signoff message. labels Jun 20, 2019
@kron4eg kron4eg changed the title WIP: kubeadm.KubeADM interface kubeadm.KubeADM interface Jun 20, 2019
@kubermatic-bot kubermatic-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 20, 2019
@xmudrii
Copy link
Member

xmudrii commented Jun 20, 2019

/hold

@kubermatic-bot kubermatic-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2019
xmudrii
xmudrii approved these changes Jun 20, 2019
View reviewed changes
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

/approve

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 20, 2019
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 15dbbbc543ef9787845e8215a2250746a82fbf40

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xmudrii

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 files:

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

@kubermatic-bot kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 20, 2019
@xmudrii
Copy link
Member

xmudrii commented Jun 20, 2019

/hold cancel

@kubermatic-bot kubermatic-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2019
@kubermatic-bot kubermatic-bot merged commit bdc7dfc into master Jun 20, 2019
@kubermatic-bot kubermatic-bot deleted the kubeadm_interface branch June 20, 2019 17:15
xmudrii added a commit that referenced this pull request Jun 23, 2019
@xmudrii
Add #486 to the changelog
74e140f 
Signed-off-by: Marko Mudrinić <[email protected]>
xmudrii added a commit that referenced this pull request Jul 1, 2019
@xmudrii
Add #486 to the changelog
5faa4ac 
Signed-off-by: Marko Mudrinić <[email protected]>
xmudrii added a commit that referenced this pull request Jul 2, 2019
@xmudrii
Add #486 to the changelog
d33ae22 
Signed-off-by: Marko Mudrinić <[email protected]>
kubermatic-bot pushed a commit that referenced this pull request Jul 2, 2019
@xmudrii @kubermatic-bot
Prepare for the release v0.9.0 (#504)
dd2e1ea 
* Update README.md

* Make install steps version-agnostic
* Recommend using officially released versions over running
of the master branch
* Mention that support for package managers is work in progress
* Add support for Kubernetes 1.15 to the compatibility table
* Minor updates and preparation for v0.9

Signed-off-by: Marko Mudrinić <[email protected]>

* Update the release process document

Signed-off-by: Marko Mudrinić <[email protected]>

* Add changelog for release v0.9.0

Signed-off-by: Marko Mudrinić <[email protected]>

* Add #508 to the changelog

Signed-off-by: Marko Mudrinić <[email protected]>

* Add #486 to the changelog

Signed-off-by: Marko Mudrinić <[email protected]>

* Update CHANGELOG.md with recent changes

Signed-off-by: Marko Mudrinić <[email protected]>

* Minor README improvements

Signed-off-by: Marko Mudrinić <[email protected]>

* Revert NodeLocalDNSCache

Signed-off-by: Marko Mudrinić <[email protected]>

* Change v0.9 release date in changelog

Signed-off-by: Marko Mudrinić <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xmudrii xmudrii xmudrii approved these changes

Assignees

@xmudrii xmudrii

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. lgtm Indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v0.9
Development

Successfully merging this pull request may close these issues.

Revisit default admission plugins
3 participants
@kron4eg @xmudrii @kubermatic-bot