Skip to content
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

fix: handle "leader changed" errors #1084

Merged
merged 1 commit into from
Nov 16, 2024
Merged

Conversation

luisdavim
Copy link
Contributor

@luisdavim luisdavim commented Oct 8, 2024

Fixes fluxcd/flux2#4804 by copying the solution used in helm/helm#11426

@luisdavim
Copy link
Contributor Author

see also helm/helm#13052

@luisdavim
Copy link
Contributor Author

I see there's a new helm tag that includes the changes to make this functionality public, should I update this PR to bump the helm dependency and use that to avoid the extra code copied from there?

@stefanprodan
Copy link
Member

should I update this PR to bump the helm dependency and use that to avoid the extra code copied from there?

Yes please

@luisdavim
Copy link
Contributor Author

This is strange, the file is here: https://github.com/helm/helm/blob/main/pkg/kube/roundtripper.go and the latest tag is from 2 days ago, well after that file was added but I don't see the round tripper in https://pkg.go.dev/helm.sh/helm/[email protected]/pkg/kube and even after updating the impoort, go says the type doesn't exist... I even cleaned my mod cache to be sure.

@luisdavim
Copy link
Contributor Author

Ok, I see the issue, my change to make the round tripper public was not picked into the release-3.16 branch and the tag is from there, that's why the type is not available :( for now we'd have to keep the copy of the round tripper code, would it be ok to include this on the next release? we run k8s on the edge with limited resources and run into this issue a few times...

@hiddeco
Copy link
Member

hiddeco commented Nov 16, 2024

This sounds good to me. If you can please add a reference to the PR with a // TODO that we should change this next minor Helm release, we won't forget about it.

Thank you for your patience and effort.

@luisdavim
Copy link
Contributor Author

luisdavim commented Nov 16, 2024

This sounds good to me. If you can please add a reference to the PR with a // TODO that we should change this next minor Helm release, we won't forget about it.

Thank you for your patience and effort.

I do have a TODO comment there already but links to the issue, I can also add a link to the PR.

Fixes fluxcd/flux2/#4804 by copying the solution used in helm/helm#11426

Signed-off-by: Luis Davim <[email protected]>
@luisdavim
Copy link
Contributor Author

I've updated the TODO comment to also reference the PR link and the fact taht though it has been mmerged it wasn't released in any version yet.

@hiddeco hiddeco merged commit 57737ba into fluxcd:main Nov 16, 2024
6 checks passed
@luisdavim luisdavim deleted the client_reties branch November 16, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm release installation fails with etcdserver leader changed
3 participants