Repository navigation
fix: Enable TF-based reconciler for SQLInstances without clone source - #2731
google-oss-prow[bot] merged 1 commit into
Conversation
|
For the cases we found in #2706, I think we also want to add the support in the direct SQLInstance reconciler to remove system labels, and recognize/propagate the 0 int value. But I'm good if you want to do that in separate PRs. |
Yeah, this is just a quick fix to unblock the release. |
| if (hasTerraformController || hasDCLController) && useDirectReconcilerPredicate == nil { | ||
| } else { | ||
| logger.Error(fmt.Errorf("no predicate where we have multiple controllers"), "skipping direct controller registration", "group", gvk.Group, "version", gvk.Version, "kind", gvk.Kind) | ||
| hasDirectController = false |
There was a problem hiding this comment.
Previously we decouple the CRDs and controllers and let the labels on CRD to choose the controller. It only moves from TF to direct but not vice versa (which is an important precondition).
Now the controller decision is switched to the CR object.
I like the reconcile gate idea overall (except that it is very challenge to figure out which reconciler is being used, that could make triaging more difficult). I think my concern is mainly that the reconcile gate has changed the concept about some basic concept about the direct registry and label based reconciliations, that is not covered by any of our guides, tools, and test.
yuwenma
left a comment
yuwenma
left a comment
There was a problem hiding this comment.
/lgtm
/approve
I'd like to have this PR goes in to unblock users and continue the reconcile gate discussion.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
2 participants
Fixes #2706