[Draft] Migrate the CLI to v2 of the golang sdk

[wilsonge]

Description of changes:

• Initial work on migrating the app to version 2 of the SDK (App has largely been migrated to the new SDK's with equivalent functionality). Obviously this would be a new major version of the product.

Current list of items to finish

• Some functionality has been dropped relating to the injection of regions from different sources which needs restoring (cmd/main.go)
• There's still a hard dependency on the v1 sdk endpoints package in odpricing.go which will likely require some work on the smithy side of things to generate a mapping between regions and descriptions (it's in the smithy generated json file but not in the golang sdk).
• Tests are still a WIP and some more minor restructuring will likely be required as part of that.
• Propagation of contexts through the stack

Why Migrate to v2

• At the end of this the overall binary should be a bit smaller by the modular nature of the new sdk i was wrong it's bigger - what can you do 🤷
• Some projects that depend on this (e.g. eksctl) are currently having to create two sessions - one in the v2 sdk for all their direct dependencies and one in the v1 sdk for this project
• Stronger typehints on the vm wrapper

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[bwagner5]

Hey! Thanks for working on this! Curious what the binary size diff was on the WIP for this PR? Seems valuable to do anyways to support clients using v2 versus v1. I suppose if a client is using v1 though, this change would pull in the additional v2 deps for their binary (I think kOps uses v1). I would guess the binary sizes wouldn't be radically different though since the unused packages would shaken out during compilation.

[wilsonge]

Interestingly bigger. Looks like the ec2 serializers are pretty hefty https://github.com/aws/aws-sdk-go-v2/blob/main/service/ec2/serializers.go about 60k lines :) about 33MB for now - but 3MB of that is the v1 package endpoints - which like I say can probably be removed in favour of smithy generated stuff.

I've committed fixes for the last 2 tests (at least they pass now locally). Would be good to get a retrigger on the tests here to see if they pass on the git workflows

[wilsonge]

Reworked the app to pass contexts around the system as required by the new sdk. I'm struggling with generating the 3rd party license file - the golicense library doesn't seem to support go 1.18 but that's the minimum version for the app.

[snay2]

@wilsonge We've been maintaining the THIRD_PARTY_LICENSES file by hand. You can copy/paste and follow the same formatting for any new libraries.

[wilsonge]

Been struggling to build the checker docker image locally for some reason it just seems to loop so I've hand cranked something but might need some help if it doesn't work. But if you could retrigger and do a first pass that would be amazing.

[wilsonge]

@snay2 / @bwagner5 sorry to be a pain but could i get you to retrigger the workflow please

[wilsonge]

OK So tests here look good. What's the next steps. I think this one needs some proper human code review too because there's an awful lot of changes. I'll look at fixing the region injection support per my list of items above - but not sure how much I can do on the v1 endpoint stuff - it might have to be left in for now - but at least it's internal to a single package and can be swapped out within the major version.

[snay2]

@wilsonge Have you finished the tasks you outlined in the PR description? If so, I'd say check those off and remove "draft" from the title, and we'll start reviewing the code.

[cjerad]

Hi wilsonge, in addition to the outstanding tasks I have a few questions and suggestions.

[TiberiuGC]

Hi 👋 - reaching out on eksctl behalf, we're also looking forward to completely migrate to go sdk v2. Is there any chance the work on this PR will be picked up from where it was left in the foreseeable future?

[jillmon]

hi @TiberiuGC. I wasn't aware that eksctl had a dependency on EC2 Instance Selector. Can you give us more detail as to how you're utilizing this tool. It would help with our prioritization of this PR because it looks like this may require another developer to pick up where @wilsonge has left off.

[TiberiuGC]

@jillmon when creating (clusters with) nodegroups using eksctl, users have the option to specify the properties they want for their nodes e.g.

eksctl create nodegroup --instance-selector-
--instance-selector-cpu-architecture  -- x86_64, or arm64
--instance-selector-gpus              -- an integer value
--instance-selector-memory            -- 4 or 4GiB
--instance-selector-vcpus             -- an integer value (2, 4 etc)

and we let instance selector do its magic on deciding the instance types.

As to why this PR is relevant for us, I think @wilsonge explained it well in the description

Why Migrate to V2
...
Some projects that depend on this (e.g. eksctl) are currently having to create two sessions - one in the v2 sdk for all their direct dependencies and one in the v1 sdk for this project.

Let me know if I can provide any more info 🙏

[wilsonge]

I've rebased this onto the latest main branch and applied most the review comments. There's still my todo's listed above (on me but if someone else wants to pick them up as I don't have huge amounts of time just let me know) and the outstanding item about the region lists which I still believe needs an AWS staff member to be resolved.

[cjerad]

Hi wilsonge, thanks for updating this. Just a few small suggestions.

[wilsonge]

Pushed up the latest changes requested. I've also pushed a change to better handle the aws context usage for the pricing client that will need a quick review on top of the previous stuff.

[cjerad]

Thanks for all your work on this.

[cjerad]

Resolved merge conflicts locally then squashed and pushed all changes.