-
Notifications
You must be signed in to change notification settings - Fork 273
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
feat: add basic support for helm charts #118
Conversation
Found a bug after rebase-ing, looking into it |
This npm package-lock issue will be the end of me |
Note: This initial implementation doesn't use helm for installing charts, just for fetching and rendering the manifests. We'll change that in following steps, eventually removing the specs module.
} | ||
|
||
await apiPostOrPut(coreApi(context).namespaces(ns).configmaps, resultKey, body) | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be cleaner to abstract this repeated try/catch
pattern around the coreApi
calls with something similar to the old apiPostOrPut
function that was removed in this commit?
8ccd9a1#diff-1300351cb8f4f405710841426a313b36L37
Maybe something like await coreApiRequest(tryFn, catchFn, statusCode)
?
Implementation note: We're not actually using helm to perform the
install, since it proved unreliable and opaque. We instead use helm
to render the templates and provide an array of specs that we feed
to kubectl apply. This also avoids the need for installing tiller,
which will be removed in Helm 3 either way. We may reconsider this
approach when Helm 3 is out and stable.