Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Task] Move functionality out of nodegroup and nodegroup_service #5475

Closed
Himangini opened this issue Jul 4, 2022 · 0 comments
Closed

[Task] Move functionality out of nodegroup and nodegroup_service #5475

Himangini opened this issue Jul 4, 2022 · 0 comments
Labels
area/tech-debt Leftover improvements in code, testing and building good first issue Good for newcomers priority/important-longterm Important over the long term, but may not be currently staffed and/or may require multiple releases

Comments

@Himangini
Copy link
Collaborator

Description

This ticket to refactor functionality as suggested below and original ticket here

nodegroup.go

Methods for validating that managed nodegroup configurations are supported by the EKS versions. Seems reasonable & EKS related ✔️

func ValidateFeatureCompatibility(clusterConfig *api.ClusterConfig, kubeNodeGroups []KubeNodeGroup) error
func ValidateBottlerocketSupport(controlPlaneVersion string, kubeNodeGroups []KubeNodeGroup) error
func ValidateManagedNodesSupport(clusterConfig *api.ClusterConfig) error
func VersionSupportsManagedNodes(controlPlaneVersion string) (bool, error)
func ValidateWindowsCompatibility(kubeNodeGroups []KubeNodeGroup, controlPlaneVersion string) error
func ValidateKMSSupport(clusterConfig *api.ClusterConfig, eksVersion string) error
func SupportsWindowsWorkloads(nodeGroups []KubeNodeGroup) bool
func LogWindowsCompatibility(nodeGroups []KubeNodeGroup, clusterMeta *api.ClusterMeta)

Also contains a random stack-related func that should not be here ❎

func (c *ClusterProvider) GetNodeGroupIAM(stackManager manager.StackManager, ng *api.NodeGroup) error

nodegroup_service.go

Methods for translating managed nodegroup configuration into real information, e.g. taking instaceSelector fields and expanding them, resolving AMIs for when AMIFamily is specified etc. All of this code seems to apply to managed and unmanaged nodegroup, which means its not necessarily EKS specific knowledge. Consider moving ❎ , but it might also be okay to leave here ✔️ 🤷

func NewNodeGroupService(provider api.ClusterProvider, instanceSelector InstanceSelector) *NodeGroupService
func (m *NodeGroupService) NewAWSSelectorSession(provider api.ClusterProvider)
func (m *NodeGroupService) Normalize(nodePools []api.NodePool, clusterMeta *api.ClusterMeta) error
func (m *NodeGroupService) ExpandInstanceSelectorOptions(nodePools []api.NodePool, clusterAZs []string) error
func (m *NodeGroupService) ValidateLegacySubnetsForNodeGroups(spec *api.ClusterConfig, provider api.ClusterProvider) error

Also contains some stray validation logic that should/code be in nodegroup.go

func (m *NodeGroupService) ValidateExistingNodeGroupsForCompatibility(cfg *api.ClusterConfig, stackManager manager.StackManager) error
@Himangini Himangini added the area/tech-debt Leftover improvements in code, testing and building label Jul 4, 2022
@Himangini Himangini added good first issue Good for newcomers priority/important-longterm Important over the long term, but may not be currently staffed and/or may require multiple releases labels Jul 22, 2022
@Himangini Himangini closed this as not planned Won't fix, can't repro, duplicate, stale Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tech-debt Leftover improvements in code, testing and building good first issue Good for newcomers priority/important-longterm Important over the long term, but may not be currently staffed and/or may require multiple releases
Projects
None yet
Development

No branches or pull requests

1 participant