Do not return errors from reconciliation when waiting for update job

[Szymongib]

Summary

Before changes are rolled out to Mattermost deployment, the update job is deployed to verify that the image is correct. The Operator waits for the job to complete before starting deployment update.

From reconciler perspective, when the job is still running the error is returned to requeue reconciliation request. If error occur enough number of times, in rare cases this may significantly delay rolling out the deployment due to error reconciliation back off.

We should modify logic that waits for the update job to finish to return some indication that it is not yet finished rather than returning an error and use constant time for requeue delay. For now it should be enough to return bool alongside error and propagate it from checkUpdateJob function to reconciler.

[imskr]

Can I work on this?

[Szymongib]

Sure @imskr, I will assign you.

[imskr]

Thanks @Szymongib After reading the description I have some thoughts:

1. We can return bool alongside error in updateMattermostDeployment and we can maybe return true here after checkUpdateJob

2. Inside Reconcile() I have to use the true bool from above to somehow stop from reconciling.

Am I right?

[gabrieljackson]

Hey @imskr, yeah that seems like a fairly reasonable implementation.

For (2), we would want to take the returned bool and have the reconciler queue up another reconcile loop after a certain specific delay. Something like 5-10 seconds seems about right to me. The idea here is we don't want to have the operator think there was too many errors and fall into delayed reconciliation checks which it will do at a certain threshold.

[Szymongib]

Inside Reconcile() I have to use the true bool from above to somehow stop from reconciling.

Yes, the general idea sound right, we might however consider returning some struct representing the status from checkMattermost instead of simple boolean for better clarity.