Refactor TemplateBuilder in CloudStack/VSphere to avoid unintuitive behavior
- Dominant language
- Go
- Stars
- 2.1k
- Forks
- 328
- Avg merge
- 1d 4h
- Merged PRs (30d)
- 9
Description
**What would you like to be added**:
In both the CloudStack and VSphere providers, the templateBuilder construct is maintaining a map of worker machine config specs. This would be well and good if it was passed in or generated in the constructor, and then made read-only. However, this map is currently required to be modified at runtime, which is unintuitive and goes against best practices. Instead, we should either be able to pass in the machineConfigs as input to the `GenerateCAPISpecWorkers` method, or generate the machine configs map in the template builder constructor.
See https://github.com/aws/eks-anywhere/pull/1675/files#r837713863 for additional context.
**Why is this needed**:
The current implementation introduces an implicit dependency between two public methods `GenerateCAPISpecWorkers` and `GenerateCAPISpecForUpgrade` on the TemplateBuilder which makes it more difficult to test and reason about
Contributor guide
Research direction
Locate the Go TemplateBuilder implementations for the CloudStack and VSphere providers and read GenerateCAPISpecWorkers and GenerateCAPISpecForUpgrade first, along with the referenced pull request discussion. Compare whether machineConfigs should be passed to GenerateCAPISpecWorkers or initialized in the constructor; done means removing the implicit method-order dependency and making the behavior independently testable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- infrastructure
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100