Reworked phase/status-field handling #609
Conversation
| return | ||
| } | ||
| dkState.Update(upd, defaultUpdateInterval, "infra monitoring reconciled") |
There was a problem hiding this comment.
dkState.Update is a bit weird, at this point its a logging util
also the last setting of the interval will win, (also if we always use defaultUpdateInterval always then why pass it in 😅 )
There was a problem hiding this comment.
On that note, afaik, upd is a boolean. Assuming defaultUpdateInterval is refactored to a constant, it would seem prudent to combine the two remaining parameters into a struct, which may or may not also take dkState, and call Update with it or, if dkState is also in that struct, call it as a receiver function
There was a problem hiding this comment.
I agree that the interval parameter should be removed as it does not make sense.
I think it's fine that it's kind of a logging util, as it adds the benefit that it sets the updated field for you on updates.
There was a problem hiding this comment.
For combining the 2 variables into a struct, what would be our advantage?
There was a problem hiding this comment.
For combining the 2 variables into a struct, what would be our advantage?
increased read- and maintainability of the code base.
Also it's prudent to combine parameters into logical units, i.e., structs, and reduce parameter counts of functions.
Description
In our current implementation the phase dynakube status field should indicate if all oneagents were deployed successfully. This was not the case because our code was searching for an old daemonset name which was no longer there. Therefore if the field was once set to error, it was never reseted. The bug was fixed in this PR.
Additionally, the operator and the deployment of the dynakube is more than just generating oneagents => Advanced the phase field checking: Now, the dynakube status field is
To quickly show the state of the deployment, the phase field was added to
additionalPrinterColumnsin our dynakube crd.Besides of that, a reconciliation of the dynakube was reworked a little but, to decrease the number of reconciliation the operator needs and unit tests for the dynakube reconciler were added.
How can this be tested?
Apply the new CRD and start in a monitoring mode you prefer.
In the operator logs you can see:
With
kubectl get dynakubeyou should see the additional printer column.Checklist