get/delete/drain support for non-eksctl created managed nodegroups

[aclevername]

Description

Adds support for get/delete/drain nodegroups not created by eksctl. Managed nodegroups now have a property Unowned (I hate this word, open to suggestions)

#2809

Discussion

• I've moved some logic into pkg/ctl/cmdutils/nodegroup.go, I'm not sure where it should live, open to suggestions.

Checklist

• Added tests that cover your change (if possible)
• Added/modified documentation as required (such as the README.md, or the userdocs directory)
• Manually tested
• Added labels for change area (e.g. area/nodegroup), target version (e.g. version/0.12.0) and kind (e.g. kind/improvement)
• Make sure the title of the PR is a good description that can go into the release notes

[michaelbeaumont]

this is always also managed, right?

[aclevername]

yes @michaelbeaumont . I was tempted to call it NodeGroupTypeManagedUnowned but it felt like too much 😂

[Callisto13]

Maybe use the comment to elaborate?

[michaelbeaumont]

i was just thinking it might be unclear (at first glance) to users if they see Unowned whether it's managed or unmanaged.

[Callisto13]

I think the Unowned abstraction you chose is fine, no alternative is jumping out at me right now.

I've moved some logic into pkg/ctl/cmdutils/nodegroup.go, I'm not sure where it should live, open to suggestions.

Based on where that function is being used, and based on how things are currently organised in that area, it makes sense for now. Maybe as we tidy up more we will see more clearly 🤷‍♀️ . I am generally not a fan of that cmdutils package, it's become a bit of a dumping ground for stuff which doesn't really belong anywhere else.

[aclevername]

I think the Unowned abstraction you chose is fine, no alternative is jumping out at me right now.

👌

Based on where that function is being used, and based on how things are currently organised in that area, it makes sense for now. Maybe as we tidy up more we will see more clearly 🤷‍♀️ . I am generally not a fan of that cmdutils package, it's become a bit of a dumping ground for stuff which doesn't really belong anywhere else.

Yeah I agree, also the cmdutils/filter does a lot of logic now. Feel like the filtering could be moved to somewhere else down the line

[Callisto13]

TEESSSTTSSSSS

[aclevername]

TEESSSTTSSSSS

I do want to add tests, but because this interacts with manager I end up being 2-3 stacks deep testing the manager package instead 😢

[Callisto13]

😭