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

Remove unneeded status update since we now update the status via defe… #919

Merged

Conversation

jnummelin
Copy link
Member

…r func

This puts the controller into update failing "loop" since we now update the status twice from same reconciliation round.

…r func

This puts the controller into update failing "loop" since we now update the status twice from same reconciliation round.

Signed-off-by: Jussi Nummelin <[email protected]>
@jnummelin
Copy link
Member Author

The change to use defer func to update the status comes from this commit.
@apedriza Could you have a look so that we're not then now missing any expected status fields

@apedriza
Copy link
Contributor

I think the proposed change is correct since the status update is done in the defer but I have tested and still encounter the problem sometimes. I think this issue is similar to #921. I have tried using a client.MergeFrom in the patch operation and I am not encountering the problem again.

As a side note, in other patching operations for other resources we are also encountering this problem. I have not seen it in depth but maybe we should take a look at using https://pkg.go.dev/sigs.k8s.io/[email protected]/util/patch in the future to handle these updates.

@jnummelin jnummelin marked this pull request as ready for review February 17, 2025 11:10
@jnummelin jnummelin requested a review from a team as a code owner February 17, 2025 11:10
@makhov
Copy link
Contributor

makhov commented Feb 17, 2025

@apedriza yeah. I think we can merge this one and I'll check all the merges in my fix for #921

@jnummelin jnummelin merged commit 219dfba into k0sproject:main Feb 17, 2025
42 checks passed
@jnummelin jnummelin deleted the fix/k0smotron-ctrl-status-update branch February 17, 2025 11:12
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.

3 participants