-
Notifications
You must be signed in to change notification settings - Fork 0
feat: pdns recordset reconciler #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| c, err := controller.NewTyped("dnsrecordset-powerdns", mgr, controller.TypedOptions[PowerDNSRecordSetReconcileRequest]{ | ||
| Reconciler: r, | ||
| RateLimiter: rl, | ||
| MaxConcurrentReconciles: 4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a good opportunity to wire this value through the operator config settings.
| return err | ||
| } | ||
|
|
||
| rsHandler := handler.TypedFuncs[*dnsv1alpha1.DNSRecordSet, PowerDNSRecordSetReconcileRequest]{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This handler and the zoneHandler below could be simplified by using handler.TypedEnqueueRequestsFromMapFunc. The example I gave had this - and should function:
func (r *DNSRecordSetPowerDNSReconciler) SetupWithManager(mgr ctrl.Manager) error {
return builder.TypedControllerManagedBy[PowerDNSRecordSetReconcileRequest](mgr).
WatchesRawSource(
source.TypedKind(
mgr.GetCache(), &dnsv1alpha1.DNSRecordSet{},
handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, rs *dnsv1alpha1.DNSRecordSet) []PowerDNSRecordSetReconcileRequest {
var requests []PowerDNSRecordSetReconcileRequest
// Enqueue a request for each (zone, recordtype, name) tuple
for _, record := range rs.Spec.Records {
requests = append(requests, PowerDNSRecordSetReconcileRequest{
Request: ctrl.Request{
NamespacedName: client.ObjectKey{
Namespace: rs.Namespace,
Name: rs.Spec.DNSZoneRef.Name,
},
},
RecordSetType: string(rs.Spec.RecordType),
RecordSetName: record.Name,
})
}
return requests
}),
),
).
Complete(r)
}
joshlreese
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - just left a couple notes. Nice updates to the accepted / programming condition handling.
This pull request introduces several changes to the DNS operator to improve status reporting, error handling, and controller logic. The most significant updates are the addition of per-owner status tracking in the API, refactoring of controller logic to better handle various error scenarios, and improved deep copy support for new status types.
API and Status Improvements:
RecordSetsto theDNSRecordSetStatusstruct, enabling per-owner (per name) status and conditions tracking. This includes a newRecordSetStatustype and corresponding CRD schema updates for better granularity in reporting readiness and errors.RecordSetStatustype and updated the deep copy logic forDNSRecordSetStatusto support the new field, ensuring correct handling during reconciliation and status updates.Controller Logic and Error Handling:
ReasonNotOwner,ReasonPDNSError) for more precise condition reporting in status updates.Controller Registration:
DNSRecordSetPowerDNSReconcilercontroller in the main application entrypoint to support PowerDNS-specific reconciliation flows.Upstream/Downstream Status Mirroring: