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

Improve gotk reconcile helmrelease readiness check #215

Closed
hiddeco opened this issue Sep 7, 2020 · 4 comments · Fixed by #281
Closed

Improve gotk reconcile helmrelease readiness check #215

hiddeco opened this issue Sep 7, 2020 · 4 comments · Fixed by #281
Labels
area/helm Helm related issues and pull requests enhancement New feature or request

Comments

@hiddeco
Copy link
Member

hiddeco commented Sep 7, 2020

As the helm-controller does not reset the Ready==True condition without a state change, the current readiness check for the command may result in a false positive.

To prevent this, the Status fields in the HelmRelease can be consulted to ensure reported state is from after the reconciliation request was made (via Status.LastObservedTime).

@hiddeco hiddeco added enhancement New feature or request area/UX area/helm Helm related issues and pull requests labels Sep 7, 2020
@seaneagan
Copy link

See also #217

I think the current check would still have a race condition even if the helm-controller reset the Ready condition on each reconciliation, since it doesn't check that the reconcilAt annotation has actually resulted in a new reconciliation.

@stefanprodan
Copy link
Member

stefanprodan commented Sep 12, 2020

The race condition could be solved for all the toolkit objects if we check the ObservedGeneration. An annotation change will bump the generation, then from gotk we wait for ObservedGeneration to equal to Generation, then check the Ready condition.

@hiddeco
Copy link
Member Author

hiddeco commented Sep 12, 2020

I do not think that annotating a resource causes a generation bump for custom resources.

See the in-code comments in kubernetes-sigs/controller-runtime#553

@stefanprodan
Copy link
Member

Ah I was looking at the Deployment kind but that's the exception.

ybelleguic pushed a commit to ybelleguic/flux2 that referenced this issue Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants