Set default container runtime to containerd

[bmelbourne]

What this PR does / why we need it:
As Kubernetes will deprecate Docker runtime support from version 1.20 and will be removed in a future release (currently planned for the 1.22 release in late 2021), set the default container runtime to containerd from kOps 1.20.

https://github.com/kubernetes/kubernetes/tree/master/CHANGELOG/CHANGELOG-1.20.md#deprecation

Which issue(s) this PR fixes
#10356

Special notes for your reviewer:
Merge this PR in to kOps 1.20 release milestone.

[k8s-ci-robot]

Hi @bmelbourne. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[olemarkus]

This only changes the default for new clusters. It does not change anything for existing.

I would think that you need a check for k8s version. We only support/test containerd as of k8s 1.18. I suspect we want the default to change only for 1.20+ clusters.

[bmelbourne]

This only changes the default for new clusters. It does not change anything for existing.

I would think that you need a check for k8s version. We only support/test containerd as of k8s 1.18. I suspect we want the default to change only for 1.20+ clusters.

I'm working on exactly that scenario and testing various cluster configs including Terraform to ensure the default container runtime remains as docker for kOps versions up to 1.19.

I pushed my initial commits somewhat early to the PR before completing testing of the final solution.

[olemarkus]

Right now, it looks like we are setting defaults in two places. The one in create_cluster.go that you already changed, and in pkg/model/components/defaults.go. I would completely remove the one in create_cluster.go and rely on the other one instead. Here you have better access to the kubernetes version.

[hakman]

The point of setting the container runtime in 2 places was to be able to change the default only for newly created clusters.
At the moment, there is a pending discussion in the Office Hours meeting about when would be the right time and how to approach this change in kOps.

That being said, I think getting this to ready is a good step and I think 1.20 is a good goal to change the default for newly created clusters.

/ok-to-test
/hold until agreement is reached

[olemarkus]

It makes sense to perhaps to make the change only for new clusers, perhaps not. At some point we need to change the default for everyone anyway.

You can have a look at new_cluster.go then. There are some options there that are set based on k8s version.

[hakman]

I think for now it's ok to not think about k8s version. I think we will switch it on for all new clusters.
To make this work, this will have to be relaxed and not require a containerd version and run hack/update-expected.sh:

kops/pkg/model/components/containerd.go

Lines 55 to 61 in c86e509

	if b.IsKubernetesGTE("1.19") {
	containerd.Version = fi.String("1.4.3")
	} else if b.IsKubernetesGTE("1.18") {
	containerd.Version = fi.String("1.3.4")
	} else {
	return fmt.Errorf("containerd version is required")
	}

[bmelbourne]

It makes sense to perhaps to make the change only for new clusers, perhaps not. At some point we need to change the default for everyone anyway.

You can have a look at new_cluster.go then. There are some options there that are set based on k8s version.

My initial testing had been based on new code introduced in new_cluster.go but with the deprecation release note, I thought this would result in some potentially confusing scenarios for users creating new and upgrading existing clusters to K8s 1.20+, so I moved the code to defaults.go where it could be better maintained going forward.

If users then choose to continue using Docker as their (unsupported) container runtime beyond K8s and kOps versions 1.20+, then they would need to override the default setting using either --container-runtime=docker or containerRuntime: docker. From an Operations perspective, this is very clear and obvious to all support engineers maintaining K8s/kOps clusters.

[bmelbourne]

/retest

[hakman]

/test pull-kops-e2e-kubernetes-aws

[hakman]

@bmelbourne after discussing a bit, decided to set the k8s version from integration tests to "1.20.0" and thought it was best to do it before this PR, to see changes. Also, the networking is set to "cni" to avoid extra customizations for that.
Does this sound good to you? If so, can you rebase and update the PR? I plan to take a look at it this weekend anyway.

[bmelbourne]

@bmelbourne after discussing a bit, decided to set the k8s version from integration tests to "1.20.0" and thought it was best to do it before this PR, to see changes. Also, the networking is set to "cni" to avoid extra customizations for that.
Does this sound good to you? If so, can you rebase and update the PR? I plan to take a look at it this weekend anyway.

That was good timing, I'm just in the process of rebasing the other containerd changes now.
Once I've completed my own integration testing, I should have everything committed sometime tomorrow.

[hakman]

Thanks! No worries, I can look at the code without the tests part too.

[hakman]

As promised, I made another pass over the PR. Looks pretty well, just small changes needed.
Please rebase and use the k8s versions and network settings that were updated in #10591. The changes to the integration tests should be smaller like this.

[bmelbourne]

@bmelbourne after discussing a bit, decided to set the k8s version from integration tests to "1.20.0" and thought it was best to do it before this PR, to see changes. Also, the networking is set to "cni" to avoid extra customizations for that.
Does this sound good to you? If so, can you rebase and update the PR? I plan to take a look at it this weekend anyway.

That was good timing, I'm just in the process of rebasing the other containerd changes now.
Once I've completed my own integration testing, I should have everything committed sometime tomorrow.

I've completed the rebasing of the latest master changes and local integration testing.

I've also updated tests\integration\create_cluster\minimal\options.yaml to K8s v1.20.0 as I believe this was missed in previous commits.

[hakman]

This looks perfect now @bmelbourne. Thanks!
/retest
/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bmelbourne, hakman

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:

• OWNERS [hakman]

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

[hakman]

/test pull-kops-e2e-k8s-containerd