Skip to content
This repository was archived by the owner on Jul 30, 2021. It is now read-only.

Patch kubeadmconfig objects only if no error occurs during reconciliation #242

Merged
merged 1 commit into from
Sep 23, 2019
Merged

Patch kubeadmconfig objects only if no error occurs during reconciliation #242

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions controllers/kubeadmconfig_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,11 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
if err != nil {
return ctrl.Result{}, err
}
// Always attempt to Patch the KubeadmConfig object and status after each reconciliation.
// Attempt to Patch the KubeadmConfig object and status after each reconciliation if no error occurs.
defer func() {
if err := patchHelper.Patch(ctx, config); err != nil {
log.Error(err, "failed to patch config")
if rerr == nil {
rerr = err
if rerr == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not currently use ErrorMessage or ErrorReason in this provider

if rerr = patchHelper.Patch(ctx, config); rerr != nil {
log.Error(rerr, "failed to patch config")
}
}
}()
Expand Down