pkg
pkg copied to clipboard
Generate reconcileFuncs
The generated reconcilers/controllers had me so hyped up, that I now want to generate all the things 😂.
In all seriousness: What if we could generate the Reconcile${Type} functions as well? They usually follow a common schema:
func ReconcileT(ctx context.Context, desired *type.T) (*type.T, error) {
lister := type.GetListerForT(ctx)
client := type.GetClientForT(ctx)
actual, err := lister.Ts(desired.namespace).Get(desired.name)
if errors.IsNotFound(err) {
created, err = client.Ts(desired.namespace).Create(desired)
if err != nil {
return nil, fmt.Errorf("error creating T: %w", err)
}
return created, nil
} else if err != nil {
return nil, fmt.Errorf("error fetching T: %w", err)
} else if *metav1.GetControllerOf(actual) != *metav1.GetControllerOf(desired) {
return nil, NewNotControlledError(...)
} else {
if !equality.Semantic.DeepEqual(desired.Spec, actual.Spec) {
want := actual.DeepCopy()
want.Spec = desired.Spec
updated, err := client.Ts(want.namespace).Update(want)
if err != nil {
return nil, fmt.Errorf("error updating T: %w", err)
}
return updated, nil
}
}
}
They do have diverged quite a bit in Serving though. I looked a bit through all of our instances of such functions and noticed a few (maybe key) differences between some of them:
- Some of the emit events when creating/updating succeeds/fails.
- Some of them determine
DeepEqualincluding the annotations/labels. - Some of them alter the parent object's Status based on the action that happened.
I wonder if there is room for us to converge on a common set of actions we want to do for each of these reconcile functions and whether or not we can generate them. Writing them is usually mindless boilerplate.
Some ideas on the divergences above: We could add a return value "Event", which can be used to determine what happened in the function to send events (if the reconciler wants to do that) or set status based on that event too.
Note: I don't think the clients/informers are currently available on the context being passed through the reconcile loop, so the interface I've shown is not quite right.
I prototyped this very roughly in Serving here https://github.com/knative/serving/compare/master...markusthoemmes:codegen-reconcilefunc?expand=1.
/cc @vaikas @n3wscott @mattmoor
As discussed in slack, I think this is a great idea!! Couple of thoughts, wonder if we should diff labels/annotations as well since at least some of the resources utilize those and it would be good to get them updated properly. I think Event being returned would be good escape hatch to return information that the reconciler might want to act upon, as you touch, sending an event is a good one for sure, there might be others? :)
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.
/lifecycle frozen
I also think that this would be very useful, but we should ensure that it is able to capture the vast majority of our needs.
cc @ImJasonH @vdemeester @afrittoli too as an FYI.