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

Containerd on kubernetes 1.21+ #1205

Merged
merged 18 commits into from
Jan 22, 2021
Merged

Containerd on kubernetes 1.21+ #1205

merged 18 commits into from
Jan 22, 2021

Conversation

kron4eg
Copy link
Member

@kron4eg kron4eg commented Dec 18, 2020 •
edited
Loading

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 #1186 #1187

Special notes for your reviewer:
Initial test uses upgrade from 1.16 for a reason:

  • to check install of old version
  • to check upgrade path

Does this PR introduce a user-facing change?:

NONE

@kron4eg kron4eg requested a review from xmudrii December 18, 2020 15:47
@kubermatic-bot kubermatic-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 18, 2020
@kron4eg kron4eg changed the title Containerd E2E WIP: Containerd E2E Dec 18, 2020
@kubermatic-bot kubermatic-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 18, 2020
@kron4eg
Copy link
Member Author

kron4eg commented Dec 18, 2020

/retest

@kron4eg
Copy link
Member Author

kron4eg commented Dec 21, 2020

/hold

until #1204 is merged

@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 Dec 21, 2020
@kron4eg kron4eg changed the title WIP: Containerd E2E Containerd E2E Dec 21, 2020
@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 Dec 21, 2020
@xmudrii
Copy link
Member

xmudrii commented Dec 21, 2020

Can you refactor this PR so it doesn’t depend on #1204? It might take some time until it’s not merged and I don’t think that we should rush it.

@kron4eg
Copy link
Member Author

kron4eg commented Dec 21, 2020

No, I don't think it makes sense and is not worth the effort. #1204 is logical conclusion of containerd endeavor in KubeOne and so that's why E2E is based on top of it.

@kron4eg
Copy link
Member Author

kron4eg commented Dec 23, 2020

Alternatively, we can close #1204 in favour of this one and close both open issues at once.

@kron4eg kron4eg changed the title Containerd E2E Containerd on kubernetes 1.23+ Dec 28, 2020
@kron4eg kron4eg mentioned this pull request Dec 28, 2020
Closed
@kubermatic-bot kubermatic-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 28, 2020
@kron4eg
Copy link
Member Author

kron4eg commented Dec 28, 2020

/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 Dec 28, 2020
@xmudrii
Copy link
Member

xmudrii commented Dec 29, 2020

/hold

We should discuss this a bit further and double-check what's the plan for removing dockershim.

@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 Dec 29, 2020
@kron4eg
Copy link
Member Author

kron4eg commented Jan 11, 2021

/retest

@kron4eg kron4eg changed the title Containerd on kubernetes 1.23+ Containerd on kubernetes 1.21+ Jan 12, 2021
xmudrii
xmudrii requested changes Jan 13, 2021
View reviewed changes
kron4eg added 11 commits January 20, 2021 18:09
@kron4eg
Use *State.ContainerRuntimeConfig() helper
014a23b 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
*State.ContainerRuntimeConfig() unit tests
284f2d7 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Introduce containerd E2E tests
6ef0f96 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Use github.com/Masterminds/semver/v3 after rebase
fd1c04e 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Use containerd repositories instead of GH releases
0ca848b 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Amazon has own containerd version
7c0ab23 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Force containerd for 1.21 instead 1.23
b5b36e4 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Return defaulting of container-runtime at API level back
4aafb25 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Validate correct container-runtime in API
9c12f5e 
If case when requested kubernetes version is 1.21+ the containerd will
be hard requirement.

Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Drop computed ContainerRuntime() and simply use parsed
e7f283e 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Mutually exclude container runtimes in e2e tests
6007a5e 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Copy link
Member Author

kron4eg commented Jan 20, 2021

/retest

@kron4eg
Default to containerd 1.21+ clusters
14ea617 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Enforce containerd for 1.22+ clusters
a6beee9 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Copy link
Member Author

kron4eg commented Jan 20, 2021

/retest

xmudrii
xmudrii requested changes Jan 21, 2021
View reviewed changes
@kron4eg
New defaulting logic
3558c8e 
* new cluster of kubernetes >=1.21, <1.22 defaults to containerd
* kubernets >=1.22 always gets containerd, unconditionally, specifying docker is an error
* coreos and flatcar always default to docker
* everything else defaults to docker

Signed-off-by: Artiom Diomin <kron82@gmail.com>
xmudrii
xmudrii requested changes Jan 22, 2021
View reviewed changes
@kron4eg
Validate presence of docker in kubernetes >= 1.21
8d1b938 
And default to containerd in case if no docker detected

Signed-off-by: Artiom Diomin <kron82@gmail.com>
xmudrii
xmudrii approved these changes Jan 22, 2021
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.

/lgtm
/approve

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: c7417ab76e9eb01a896f1d0ce5618b44b10fd646

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kron4eg, 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

@xmudrii
Copy link
Member

xmudrii commented Jan 22, 2021

/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 Jan 22, 2021
@kubermatic-bot kubermatic-bot merged commit 357f3a2 into master Jan 22, 2021
@kubermatic-bot kubermatic-bot deleted the containerd_e2e branch January 22, 2021 15:18
hwuethrich added a commit to hwuethrich/kubeone that referenced this pull request Jan 25, 2021
@hwuethrich
Merge remote-tracking branch 'origin/master'
fc44f0a 
* origin/master:
  Containerd on kubernetes 1.21+ (kubermatic#1205)
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. 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
No milestone
Development

Successfully merging this pull request may close these issues.

Containerd add E2E tests
3 participants
@kron4eg @xmudrii @kubermatic-bot