remove support for 1.18

[aclevername]

Description

Closes #5062

eksctl create cluster --name jk --version 1.18
2022-04-05 16:44:00 [ℹ]  eksctl version 0.93.0-dev+2333ab45f.2022-04-05T16:43:52Z
2022-04-05 16:44:00 [ℹ]  using region us-west-2
Error: invalid version, 1.18 is no longer supported, supported values: 1.19, 1.20, 1.21, 1.22
see also: https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-versions.html

[Skarlso]

Please also update dry-run.md and addons.md. :)

stale

[Skarlso]

thanks!

[Himangini]

Couple of questions on removal of supported versions checks for addons

[Himangini]

Also i noticed some specific checks in tests around 1.18, do we need to update them to 1.19 to make sure everything works fine for 1.19 in the following files?
pkg/actions/addon/create_test.go
pkg/actions/addon/delete_test.go
pkg/actions/addon/get_test.go
pkg/actions/addon/update_test.go
pkg/actions/addon/describe_versions_test.go

[aclevername]

I will take a look but I have a feeling it doesn't matte for the tests, any version can be used for unit-testing, since we aren't talking to the real EKS API. We have lots of tests referencing old versions since its just not worth changing. E.g. kube-proxy tests reference 1.15/6 https://github.com/weaveworks/eksctl/blob/main/pkg/addons/default/kube_proxy_test.go

[Himangini]

I will take a look but I have a feeling it doesn't matte for the tests, any version can be used for unit-testing, since we aren't talking to the real EKS API. We have lots of tests referencing old versions since its just not worth changing. E.g. kube-proxy tests reference 1.15/6 https://github.com/weaveworks/eksctl/blob/main/pkg/addons/default/kube_proxy_test.go

As I mentioned in the earlier comment, the files refer to specific version 1.18 in tests. If we remove the supported version checks and greaterThanOrEqualTo1_18 checks, then we should remove those tests since it's irrelevant now or update them to use 1.19 otherwise it's confusing. Eg., Why would you test the creation of an addon with a non-supported cluster version?

[Skarlso]

As I mentioned in the earlier comment, the files refer to specific version 1.18 in tests. If we remove the supported version checks and greaterThanOrEqualTo1_18 checks, then we should remove those tests since it's irrelevant now or update them to use 1.19 otherwise it's confusing. Eg., Why would you test the creation of an addon with a non-supported cluster version?

In those tests, the version is irrelevant. :)
For example:

		clusterConfig = &api.ClusterConfig{Metadata: &api.ClusterMeta{
			Version: "1.18",
			Name:    "my-cluster",
		}}

The version there literally could be 1 and this test would still pass. :) The important things are different in nature. Things like, sorting, and that the right thing is returned / created / deleted / updated. Those versions don't really do anything in these tests.

We usually just leave them be as it's not worth continuously updating. Such as versions like these:

						{
							AddonVersion: aws.String("v1.0.0-eksbuild.1"),
						},
						{
							AddonVersion: aws.String("v1.7.5-eksbuild.1"),
						},
						{
							AddonVersion: aws.String("v1.7.5-eksbuild.2"),
						},
						{
							//not sure if all versions come with v prefix or not, so test a mix
							AddonVersion: aws.String("v1.7.7-eksbuild.2"),
						},
						{
							AddonVersion: aws.String("v1.7.6"),

We never update these either, because they don't matter. They aren't version checked any longer. The tests are testing a different thing. The relevant tests in kube_proxy have been updated however. Those do matter for version reasons.

[aclevername]

+1 to what @Skarlso said 😄

[Himangini]

I didn't know there's a spokesperson for you @aclevername :D

[aclevername]

I didn't know there's a spokesperson for you @aclevername :D

tbf all of the responses yesterday were while I was at the doctors, which I appreciate ❤️

[Skarlso]

I didn't know there's a spokesperson for you @aclevername :D

1. I reviewed and approved this pr so I thought I'd share my findings with you.
2. Jake was mostly out because of medical reasons and I wanted to move the PR review forward so you aren't blocked.
3. Since Jake will be out, I take over most of his Prs so I kind of want the context.

[cPu1]

I left a comment about keeping the existing validation for addons but otherwise LGTM.