Add Helm 3 UpgradeRelease #1421
Conversation
|
|
||
| func upgradeRelease(cfg agent.Config, w http.ResponseWriter, req *http.Request, params handlerutil.Params) { | ||
| releaseName := params[nameParam] | ||
| chartDetails, chartMulti, err := handlerutil.ParseAndGetChart(req, cfg.ChartClient, requireV1Support) |
There was a problem hiding this comment.
here requireV1Support is a bit of a readability issue. It seems that it's requiring v1 support. It would more readable if the var is called requireV2Support but it may be confusing if its value is false. Maybe notRequireV1Support?
There was a problem hiding this comment.
I was bugged by the exact same thought! But what do? Wouldn't it be just as confusing to write const dontRequireV1Support = false? Or would a comment suffice to mitigate that confusion?
There was a problem hiding this comment.
yes, a comment when setting the variable would be enough in my opinion
There was a problem hiding this comment.
I felt naming the variable dontRequireV1Support or similar would make the code WET, since it would tie the constant name very closely to its value. I've renamed it to isV1SupportRequired instead; WDYT?
Description of the change
This PR implements
UpgradeReleaseusing the Helm 3 API.Benefits
Possible drawbacks
OperateReleaseonly supports upgrading at the moment (but it will support testing and rolling back later).Applicable issues
Additional information
The
switchstatement inhandler.OperateReleaseis currently redundant, but it will make sense when support for testing and rolling back is added (see its Tiller Proxy counterpart).