feat: check dependency for state of the world reconciler #870
Conversation
| // TODO: add object links | ||
| ) | ||
| // TODO: add specific tasks to workflow |
There was a problem hiding this comment.
Future TODOs that would be completed by other issues as functionality is migrated to the workflow
Signed-off-by: KevFan <[email protected]>
KevFan
requested review from
guicassolato and
Boomatang
and removed request for
guicassolato
| @@ -59,6 +64,71 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D | |||
| controller.WithReconcile(buildReconciler(client)), | |||
| } | |||
|
|
|||
| ok, err := kuadrantgatewayapi.IsGatewayAPIInstalled(manager.GetRESTMapper()) | |||
There was a problem hiding this comment.
What should happen if there is no gateway currently installed that we can use? We would need at least one, I am asking the same question of the cert manger if that is not installed should we try reconcile. I am aware that the user may install these resources later, but that would mean restarting the operator (which I am okay with).
I guess my question is; should we even start the operator if there is no gateway that we can use installed?
There was a problem hiding this comment.
Yeah, not sure what's the best approach but I've went with the approach of the existing controllers which gets disabled rather than crash when a dependency CRD is not detected (which we have existing integration tests for) 🤔
Personally, I think it's better not to crash and have this reported in the Policy Status also since a user may not initially look through logs on why things are not working as expected.
There was a problem hiding this comment.
I agree, the operator should not crash. Personally, for now, what I would do is to disable watchers and maybe steps in the workflow. There should be a task responsible for the kuadrant CR status that watches for changes on Kuadrant CRDs (which we can assume they exist). Then, the task checks for GatewayAPI being installed and reports in the status if not. Same for policies.
There was a problem hiding this comment.
After the call I am happy that we have a plan to get the errors these missing CRDs and resources can cause into the status of kuadrant and / or the related policies. Would be good to create issue for any follow up work this may need.
|
LGTM I leave approval to @Boomatang |
Description
Closes: #811
Checks for dependencies for whether specific watchers and object kinds should be added to the controller opts of the state of the world reconciler