RFC (post 0.18): libvirt and azure helm provisioning support with cleaner API (conflicts with #2809) #2810
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
beraldoleal
changed the title
beraldoleal
changed the title
Once the installation/edit/removal implementation for each provider tends to be very similar with helm, we could eliminate this API from the provider side. This change proposes using common code for install/edit/uninstall across all providers. This way we could eliminate some boiler plate code. Each provider just need to return the dinamic values, or what they "provisioned". Helm will merge those values with the deployment intent. This would eliminates the need for per-provider Install/Uninstall/Edit implementations (that could be removed when kustomize is dropped) and removes unnecessary data transformations and code duplication. Old flow: Workflow creates env vars → properties file → Go map → Provider-specific Configure() maps to --set args → helm install New flow: Workflow → helm values YAML + provisioner adds dynamic values → helm -f Signed-off-by: Beraldo Leal <[email protected]>
libvirt seems to not need any special deployment variable. Signed-off-by: Beraldo Leal <[email protected]>
same as previous commit Signed-off-by: Beraldo Leal <[email protected]>
beraldoleal
force-pushed
the
helm-provisioning-cleanup
branch
from
498706d to
c36f69f
Compare
|
Hi @beraldoleal , I only glanced at the changes and honestly I disliked it because it changes the "philosophy" of the framework. I don't want to go into further details because I think it's not time to make that change, IMO we should focus on what is strictly needed for the 0.18 release. I understand it has boilerplates but it works; IMO we should focus on just covering the other providers. |
This is covering the other providers as well.... but I get it, the focus is totally fair. Let's move on. I will keep it open so we can properly discuss the "philosophy" post 0.18. |
beraldoleal
changed the title
2 participants
Hi folks, this seems big, but I hope its easy to review for those that are involved in the helm code at this point.
Before we implement the remaining providers, I'd like you to consider this approach if possible. This is groundwork for removing several map conversions in the code that can now be avoided with helm. Doing it now would avoid more rework later.
Risk looks low to me (though I might be colorblind to danger)... happy to postpone if needed.
If that makes sense, I can continue testing on that.
Note: I chose to replace helm.go with new code to make reviewing a bit easier, but most of the work there came from Wainer's interface.