Patch kubeadmconfig objects only if no error occurs during reconciliation

[accepting]

Signed-off-by: JunLi [email protected]

What this PR does / why we need it:
There is no need to update KubeadmConfig objects if any error occurs during the reconciliation.

[k8s-ci-robot]

Hi @accepting. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chuckha]

/ok-to-test

[detiber]

This seems like a departure from other cluster-api related use cases, where we would potentially want to ensure that we persist some type of intermediate or partial state back to the resource. That said, the behaviors of this controller vs those other cases it is probably generally ok.

Are we attempting to set ErrorMessage and ErrorReason anywhere where this change could potentially prevent those changes from persisting?

[chuckha]

We do not currently use ErrorMessage or ErrorReason in this provider

[chuckha]

@accepting This is right. We never set the config value except in the successful case so there is no reason to issue a patch in an error case, but I'm curious why you want this change? Was there a bug you were encountering that this fixes?

[accepting]

@chuckha In our use cases, the bootstrap objects support update. When we try to update a bootstrap object, we found the object was updated even if some error occurs afterwards.

[chuckha]

That's interesting. Does an update to the bootstrap object actually modify the cluster?

[accepting]

Yes, with the help of kubeadm phases, we update the control planes/worker nodes' settings by updating their bootstrap objects.

[chuckha]

I'm fine merging this and reverting or changing behavior back if we need to start populating the error message.

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: accepting, chuckha

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [chuckha]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[detiber]

/lgtm