Improve region validation, only print log message in create cluster command

[errordeveloper]

Description

Improve region validation, only print log message in create cluster command

Checklist

• Code compiles correctly (i.e make build)
• All tests passing (i.e. make test)

[errordeveloper]

As you cannot declare const []string, we prefer a function here. The previous version was written before we've adopted this convention. Just FYI :)

[gotjosh]

Noted! 🔥

[gotjosh]

Looks good from this side of fence. 🎉 🥇

Stricly for my understanding, what's the policy on tests? Noticed we don't have any for either of these files. Altough I recognise something like IsSupportedRegion is quite trivial, my brain was expecting a test for it.

[gotjosh]

👍 - I noticed this as part of my previous change but decided to leave it out as I wasn't really sure if it was relevant for other calls. Seems like it is when we are actually creating the cluster.

[gotjosh]

Noted! 🔥

[errordeveloper]

Stricly for my understanding, what's the policy on tests? Noticed we don't have any for either of these files.

Good question 👍. It'd be nice to have a clear policy stated somewhere. At the moment we avoid trivial tests, and favour testing critical code path and integration tests are getting into a shape, but need more love.

Integration tests live in integration/creategetdelete_test.go. These will need to get expanded in several different ways, but we've not had a chance to think it trough just yet.

Packages like pkg/az and pkg/cloudconfig have an okay test coverage, and those are sort of clean and simple packages, each of which does one very specific thing. Packages like pkg/eks and pkg/eks/api are much broader, but also largely trivial (except from a few funcs, like eks.New and it's newSession helper, which largely rely on the AWS SDK and will need some mocking, but I'm not sure if it'd worse it).

Most of critical path test are in pkg/cfn/builder/api_test.go, and those had been growing, but also getting a little wild and surely in need of a cleanup.

What I think would be very nice is a kind of introspective integration test, where cluster would be created in stages and in the process of that several checks would be done. Perhaps we can do this in the current integration suite in a way, but that may need new code that fetches state from CloudFormation etc, as we exec eksctl and thereby don't have access to internal structures unless we over-engineer some kind of channel to it... However, some kind of dry-run mode that spews out CloudFormation definitions maybe a thing to think about, that could be useful for this and in few other cases, but that may need review of certain assumptions, which maybe a good thing anyway. What I'm thinking of is the coupling of cluster and nodegroup stacks, I don't think we can create nodegroup stack template without having a live cluster (or a mock of it), which is fine for most uses, but obviously isn't ideal for proper dry-run mode.

Anyway, a bit of a brain dump, but hopefully still useful. So to sum up, I'd rather we work on end-to-end testing with introspection, then test trivial units in a race for some coverage figure. Hope it makes sense :)