-
Notifications
You must be signed in to change notification settings - Fork 707
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
Update go deps: k8s + helm (+ memory increase) #3651
Conversation
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Helm 3.7 depends on k8s >= 1.22, so we have to update both Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> Conflicts: go.mod go.sum
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Todo: check OCI support is still working here |
Two scenarios:
If it turns out we can only do one of these, we may want to chat with the bitnami content team and decide when we upgrade to 3.7 in our code. If we can support both, great. |
I've successfully tested this PR is not "breaking anything" related to OCI:
OK - I've installed our TAC OCI demo repo and I'm able to sync the repo and install chars from there.
OK - I've built the OCI artifact with Helm 3.7.1 and uploaded it to a harbor instance. Kind of fortunately, since we are copying the OCI code from the Helm codebase (as a temporary fix given that it is not -yet- exported), with a simple change (#3606 (comment)), we are able to support post-3.7 artifacts (and pre-3.7 ones as well) So, as a conclusion, this PR is not affecting the OCI support, thus being ready to be landed as soon it is +1ed. |
Sorry, thought I already had. |
Description of the change
As commented in previous PRs (#3501 (comment)), let's split this change into smaller PRs, so that we can better track which dep is causing the memory issue.
This PR is upgrading the minimum number of deps, but helm+k8s are bounded... so we have to bump them up at the same time.
Benefits
Updating deps + increasing from 216MI to 512Mi the memory of the kubeappsapis' pod.
Possible drawbacks
N/A
Applicable issues
getChart
fetches the whole repo each time it's invoked #3567Additional information
This PR supersedes #3632
According to what we discovered, this PR should pass the tests as is. I haven't included any of the fix attempts I performed in the aforementioned PR since they are not proven to be useful.