Ensure that kctrl picks up status tail after secrets are updated #713
Conversation
| @@ -310,6 +312,20 @@ func (o CreateOrUpdateOptions) update(client kubernetes.Interface, kcClient kccl | |||
| return nil | |||
| } | |||
|
|
|||
| // Pause reconciliation so that kctrl can tail status if an existing values secret is updated | |||
| reconciliationPaused := false | |||
There was a problem hiding this comment.
This variable is not entirely necessary, we can chuck it out
I thought it improves readability but happy to remove it
| } | ||
|
|
||
| if reconciliationPaused { | ||
| err = o.unpauseReconciliation(kcClient) |
There was a problem hiding this comment.
Pausing and unpausing if needed at the beginning and the end is the best way I could find. Maybe we should start logging pauses and unpauses, so that if things error out in the middle the user is aware that reconciliation is paused?
Not sure if it makes sense to defer a function which unpauses the app if paused :/
There was a problem hiding this comment.
keeping it paused it probably good (since they may want to keep on reconfiguring packageinstall) but good thought on indicating that to the user.
| @@ -743,6 +768,69 @@ func (o *CreateOrUpdateOptions) updateDataValuesSecret(client kubernetes.Interfa | |||
| return nil | |||
| } | |||
|
|
|||
| func (o *CreateOrUpdateOptions) pauseReconciliation(client kcclient.Interface) (*kcpkgv1alpha1.PackageInstall, error) { | |||
There was a problem hiding this comment.
I think we should add a TODO item or create an issue to remove duplication.
What this PR does / why we need it:
It ensures that kctrl picks up on a reconciliation aftera secret is updated as kctrl might poll the resource and error out / exit successfully before the reconciliation due to a secret update might start
Which issue(s) this PR fixes:
Fixes #670
Does this PR introduce a user-facing change?
Additional Notes for your reviewer:
Review Checklist:
a link to that PR
change
Additional documentation e.g., Proposal, usage docs, etc.: