Skip to content

Commit 03a5d8f

Browse files
rxbnrxbn
committed
chore: refactor dns controller
1 parent 7d3f7c0 commit 03a5d8f

File tree

5 files changed

+109
-167
lines changed

5 files changed

+109
-167
lines changed

‎api/v1/zone_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ type ZoneStatus struct {
4444
Conditions []metav1.Condition `json:"conditions"`
4545
}
4646

47+
const (
48+
ZoneNameIndexKey string = ".spec.name"
49+
)
50+
4751
// +kubebuilder:object:root=true
4852
// +kubebuilder:subresource:status
4953
// +kubebuilder:resource:scope=Cluster

‎cmd/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func main() {
130130
Client: mgr.GetClient(),
131131
Scheme: mgr.GetScheme(),
132132
Cf: &cf,
133-
}).SetupWithManager(mgr); err != nil {
133+
}).SetupWithManager(ctx, mgr); err != nil {
134134
setupLog.Error(err, "unable to create controller", "controller", "Zone")
135135
os.Exit(1)
136136
}
@@ -144,7 +144,7 @@ func main() {
144144
if err = (&controller.IngressReconciler{
145145
Client: mgr.GetClient(),
146146
Scheme: mgr.GetScheme(),
147-
}).SetupWithManager(ctx, mgr); err != nil {
147+
}).SetupWithManager(mgr); err != nil {
148148
setupLog.Error(err, "unable to create controller", "controller", "Ingress")
149149
os.Exit(1)
150150
}

‎internal/controller/dnsrecord_controller.go

Lines changed: 93 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,14 @@ import (
2424
"reflect"
2525
"time"
2626

27-
"github.com/go-logr/logr"
27+
"github.com/fluxcd/pkg/runtime/patch"
2828
"golang.org/x/net/publicsuffix"
2929
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
30+
apierrors "k8s.io/apimachinery/pkg/api/errors"
3031
apimeta "k8s.io/apimachinery/pkg/api/meta"
3132
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3233
"k8s.io/apimachinery/pkg/runtime"
34+
apierrutil "k8s.io/apimachinery/pkg/util/errors"
3335
ctrl "sigs.k8s.io/controller-runtime"
3436
"sigs.k8s.io/controller-runtime/pkg/client"
3537
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -58,6 +60,23 @@ func (r *DNSRecordReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
5860
}); err != nil {
5961
return err
6062
}
63+
if err := mgr.GetFieldIndexer().IndexField(ctx, &cloudflareoperatoriov1.DNSRecord{}, cloudflareoperatoriov1.OwnerRefUIDIndexKey,
64+
func(o client.Object) []string {
65+
obj := o.(*cloudflareoperatoriov1.DNSRecord)
66+
ownerReferences := obj.GetOwnerReferences()
67+
var ownerReferencesUID string
68+
for _, ownerReference := range ownerReferences {
69+
if ownerReference.Kind != "Ingress" {
70+
continue
71+
}
72+
ownerReferencesUID = string(ownerReference.UID)
73+
}
74+
75+
return []string{ownerReferencesUID}
76+
},
77+
); err != nil {
78+
return err
79+
}
6180

6281
return ctrl.NewControllerManagedBy(mgr).
6382
For(&cloudflareoperatoriov1.DNSRecord{}).
@@ -71,110 +90,93 @@ func (r *DNSRecordReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
7190

7291
// Reconcile is part of the main kubernetes reconciliation loop which aims to
7392
// move the current state of the cluster closer to the desired state.
74-
func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
93+
func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) {
7594
log := ctrl.LoggerFrom(ctx)
7695

7796
dnsrecord := &cloudflareoperatoriov1.DNSRecord{}
7897
if err := r.Get(ctx, req.NamespacedName, dnsrecord); err != nil {
7998
return ctrl.Result{}, client.IgnoreNotFound(err)
8099
}
81100

82-
if r.Cf.APIToken == "" {
83-
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
84-
Type: "Ready",
85-
Status: "False",
86-
Reason: "NotReady",
87-
Message: "Cloudflare account is not yet ready",
88-
ObservedGeneration: dnsrecord.Generation,
89-
})
90-
if err := r.Status().Update(ctx, dnsrecord); err != nil {
91-
log.Error(err, "Failed to update DNSRecord status")
92-
return ctrl.Result{}, err
101+
patchHelper := patch.NewSerialPatcher(dnsrecord, r.Client)
102+
103+
defer func() {
104+
patchOpts := []patch.Option{}
105+
106+
if errors.Is(retErr, reconcile.TerminalError(nil)) || (retErr == nil && (result.IsZero() || !result.Requeue)) {
107+
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
93108
}
94-
return ctrl.Result{RequeueAfter: time.Second * 5}, nil
95-
}
109+
110+
if err := patchHelper.Patch(ctx, dnsrecord, patchOpts...); err != nil {
111+
if !dnsrecord.DeletionTimestamp.IsZero() {
112+
err = apierrutil.FilterOut(err, func(e error) bool { return apierrors.IsNotFound(e) })
113+
}
114+
retErr = apierrutil.Reduce(apierrutil.NewAggregate([]error{retErr, err}))
115+
}
116+
}()
96117

97118
zoneName, _ := publicsuffix.EffectiveTLDPlusOne(dnsrecord.Spec.Name)
98119

99120
zones := &cloudflareoperatoriov1.ZoneList{}
100-
if err := r.List(ctx, zones); err != nil {
101-
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
102-
log.Error(err, "Failed to update DNSRecord status")
103-
return ctrl.Result{}, err
104-
}
121+
if err := r.List(ctx, zones, client.MatchingFields{cloudflareoperatoriov1.ZoneNameIndexKey: zoneName}); err != nil {
122+
log.Error(err, "Failed to list zones")
105123
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
106124
}
107125

108-
zone := cloudflareoperatoriov1.Zone{}
109-
for _, z := range zones.Items {
110-
if z.Spec.Name == zoneName {
111-
zone = z
112-
break
113-
}
126+
if len(zones.Items) == 0 {
127+
r.markFailed(dnsrecord, fmt.Errorf("Zone %q not found", zoneName))
128+
return ctrl.Result{}, nil
114129
}
115130

116-
if zone.Spec.Name == "" {
117-
if err := r.markFailed(ctx, dnsrecord, fmt.Errorf("Zone %q not found", zoneName)); err != nil {
118-
log.Error(err, "Failed to update DNSRecord status")
131+
zone := &zones.Items[0]
132+
133+
if !dnsrecord.DeletionTimestamp.IsZero() {
134+
if err := r.reconcileDelete(ctx, zone.Status.ID, dnsrecord); err != nil {
135+
log.Error(err, "Failed to delete DNS record in Cloudflare, record may still exist in Cloudflare")
119136
return ctrl.Result{}, err
120137
}
121138
return ctrl.Result{}, nil
122139
}
123140

124-
if condition := apimeta.FindStatusCondition(zone.Status.Conditions, "Ready"); condition == nil || condition.Status != "True" {
141+
if !controllerutil.ContainsFinalizer(dnsrecord, common.CloudflareOperatorFinalizer) {
142+
controllerutil.AddFinalizer(dnsrecord, common.CloudflareOperatorFinalizer)
143+
return ctrl.Result{Requeue: true}, nil
144+
}
145+
146+
return r.reconcileDNSRecord(ctx, dnsrecord, zone), nil
147+
}
148+
149+
// reconcileDNSRecord reconciles the dnsrecord
150+
func (r *DNSRecordReconciler) reconcileDNSRecord(ctx context.Context, dnsrecord *cloudflareoperatoriov1.DNSRecord, zone *cloudflareoperatoriov1.Zone) ctrl.Result {
151+
if r.Cf.APIToken == "" {
125152
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
126153
Type: "Ready",
127154
Status: "False",
128155
Reason: "NotReady",
129-
Message: "Zone is not yet ready",
156+
Message: "Cloudflare account is not yet ready",
130157
ObservedGeneration: dnsrecord.Generation,
131158
})
132-
if err := r.Status().Update(ctx, dnsrecord); err != nil {
133-
log.Error(err, "Failed to update DNSRecord status")
134-
return ctrl.Result{}, err
135-
}
136-
return ctrl.Result{RequeueAfter: time.Second * 5}, nil
159+
return ctrl.Result{RequeueAfter: time.Second * 5}
137160
}
138161

139-
if !controllerutil.ContainsFinalizer(dnsrecord, common.CloudflareOperatorFinalizer) {
140-
controllerutil.AddFinalizer(dnsrecord, common.CloudflareOperatorFinalizer)
141-
if err := r.Update(ctx, dnsrecord); err != nil {
142-
log.Error(err, "Failed to update DNSRecord finalizer")
143-
return ctrl.Result{}, err
144-
}
145-
}
146-
147-
if !dnsrecord.DeletionTimestamp.IsZero() {
148-
if controllerutil.ContainsFinalizer(dnsrecord, common.CloudflareOperatorFinalizer) {
149-
if err := r.finalizeDNSRecord(ctx, zone.Status.ID, log, dnsrecord); err != nil && err.Error() != "Record does not exist. (81044)" {
150-
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
151-
log.Error(err, "Failed to update DNSRecord status")
152-
return ctrl.Result{}, err
153-
}
154-
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
155-
}
156-
metrics.DnsRecordFailureCounter.DeleteLabelValues(dnsrecord.Namespace, dnsrecord.Name, dnsrecord.Spec.Name)
157-
}
158-
159-
controllerutil.RemoveFinalizer(dnsrecord, common.CloudflareOperatorFinalizer)
160-
if err := r.Update(ctx, dnsrecord); err != nil {
161-
return ctrl.Result{}, err
162-
}
163-
return ctrl.Result{}, nil
162+
if condition := apimeta.FindStatusCondition(zone.Status.Conditions, "Ready"); condition == nil || condition.Status != "True" {
163+
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
164+
Type: "Ready",
165+
Status: "False",
166+
Reason: "NotReady",
167+
Message: "Zone is not yet ready",
168+
ObservedGeneration: dnsrecord.Generation,
169+
})
170+
return ctrl.Result{RequeueAfter: time.Second * 5}
164171
}
165172

166-
metrics.DnsRecordFailureCounter.WithLabelValues(dnsrecord.Namespace, dnsrecord.Name, dnsrecord.Spec.Name).Set(0)
167-
168173
var existingRecord cloudflare.DNSRecord
169174
if dnsrecord.Status.RecordID != "" {
170175
var err error
171176
existingRecord, err = r.Cf.GetDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), dnsrecord.Status.RecordID)
172177
if err != nil {
173-
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
174-
log.Error(err, "Failed to update DNSRecord status")
175-
return ctrl.Result{}, err
176-
}
177-
return ctrl.Result{}, err
178+
r.markFailed(dnsrecord, err)
179+
return ctrl.Result{}
178180
}
179181
} else {
180182
cfExistingRecords, _, err := r.Cf.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.ListDNSRecordsParams{
@@ -183,11 +185,8 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
183185
Content: dnsrecord.Spec.Content,
184186
})
185187
if err != nil {
186-
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
187-
log.Error(err, "Failed to update DNSRecord status")
188-
return ctrl.Result{}, err
189-
}
190-
return ctrl.Result{}, err
188+
r.markFailed(dnsrecord, err)
189+
return ctrl.Result{}
191190
}
192191
if len(cfExistingRecords) > 0 {
193192
existingRecord = cfExistingRecords[0]
@@ -197,27 +196,17 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
197196
if (dnsrecord.Spec.Type == "A" || dnsrecord.Spec.Type == "AAAA") && dnsrecord.Spec.IPRef.Name != "" {
198197
ip := &cloudflareoperatoriov1.IP{}
199198
if err := r.Get(ctx, client.ObjectKey{Name: dnsrecord.Spec.IPRef.Name}, ip); err != nil {
200-
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
201-
log.Error(err, "Failed to update DNSRecord status")
202-
return ctrl.Result{}, err
203-
}
204-
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
199+
r.markFailed(dnsrecord, err)
200+
return ctrl.Result{RequeueAfter: time.Second * 30}
205201
}
206202
if ip.Spec.Address != dnsrecord.Spec.Content {
207203
dnsrecord.Spec.Content = ip.Spec.Address
208-
if err := r.Update(ctx, dnsrecord); err != nil {
209-
log.Error(err, "Failed to update DNSRecord")
210-
return ctrl.Result{}, err
211-
}
212204
}
213205
}
214206

215207
if *dnsrecord.Spec.Proxied && dnsrecord.Spec.TTL != 1 {
216-
if err := r.markFailed(ctx, dnsrecord, errors.New("TTL must be 1 when proxied")); err != nil {
217-
log.Error(err, "Failed to update DNSRecord status")
218-
return ctrl.Result{}, err
219-
}
220-
return ctrl.Result{}, nil
208+
r.markFailed(dnsrecord, errors.New("TTL must be 1 when proxied"))
209+
return ctrl.Result{}
221210
}
222211

223212
if existingRecord.ID == "" {
@@ -231,30 +220,13 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
231220
Data: dnsrecord.Spec.Data,
232221
})
233222
if err != nil {
234-
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
235-
log.Error(err, "Failed to update DNSRecord status")
236-
return ctrl.Result{}, err
237-
}
238-
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
223+
r.markFailed(dnsrecord, err)
224+
return ctrl.Result{RequeueAfter: time.Second * 30}
239225
}
240-
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
241-
Type: "Ready",
242-
Status: "True",
243-
Reason: "Ready",
244-
Message: "DNS record synced",
245-
ObservedGeneration: dnsrecord.Generation,
246-
})
247226
dnsrecord.Status.RecordID = newDNSRecord.ID
248-
if err := r.Status().Update(ctx, dnsrecord); err != nil {
249-
log.Error(err, "Failed to update DNSRecord status")
250-
return ctrl.Result{}, err
251-
}
252-
return ctrl.Result{RequeueAfter: dnsrecord.Spec.Interval.Duration}, nil
253-
}
254-
255-
if !compareDNSRecord(dnsrecord.Spec, existingRecord) {
227+
} else if !compareDNSRecord(dnsrecord.Spec, existingRecord) {
256228
if _, err := r.Cf.UpdateDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.UpdateDNSRecordParams{
257-
ID: existingRecord.ID,
229+
ID: dnsrecord.Status.RecordID,
258230
Name: dnsrecord.Spec.Name,
259231
Type: dnsrecord.Spec.Type,
260232
Content: dnsrecord.Spec.Content,
@@ -263,26 +235,9 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
263235
Priority: dnsrecord.Spec.Priority,
264236
Data: dnsrecord.Spec.Data,
265237
}); err != nil {
266-
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
267-
log.Error(err, "Failed to update DNSRecord status")
268-
return ctrl.Result{}, err
269-
}
270-
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
238+
r.markFailed(dnsrecord, err)
239+
return ctrl.Result{RequeueAfter: time.Second * 30}
271240
}
272-
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
273-
Type: "Ready",
274-
Status: "True",
275-
Reason: "Ready",
276-
Message: "DNS record synced",
277-
ObservedGeneration: dnsrecord.Generation,
278-
})
279-
dnsrecord.Status.RecordID = existingRecord.ID
280-
if err := r.Status().Update(ctx, dnsrecord); err != nil {
281-
log.Error(err, "Failed to update DNSRecord status")
282-
return ctrl.Result{}, err
283-
}
284-
log.Info("DNS record updated in cloudflare", "name", existingRecord.Name, "id", existingRecord.ID)
285-
return ctrl.Result{RequeueAfter: dnsrecord.Spec.Interval.Duration}, nil
286241
}
287242

288243
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
@@ -292,27 +247,25 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
292247
Message: "DNS record synced",
293248
ObservedGeneration: dnsrecord.Generation,
294249
})
295-
dnsrecord.Status.RecordID = existingRecord.ID
296-
if err := r.Status().Update(ctx, dnsrecord); err != nil {
297-
log.Error(err, "Failed to update DNSRecord status")
298-
return ctrl.Result{}, err
299-
}
300250

301-
return ctrl.Result{RequeueAfter: dnsrecord.Spec.Interval.Duration}, nil
251+
metrics.DnsRecordFailureCounter.WithLabelValues(dnsrecord.Namespace, dnsrecord.Name, dnsrecord.Spec.Name).Set(0)
252+
253+
return ctrl.Result{RequeueAfter: dnsrecord.Spec.Interval.Duration}
302254
}
303255

304-
// finalizeDNSRecord deletes the DNS record from cloudflare
305-
func (r *DNSRecordReconciler) finalizeDNSRecord(ctx context.Context, dnsRecordZoneId string, log logr.Logger, d *cloudflareoperatoriov1.DNSRecord) error {
306-
if err := r.Cf.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(dnsRecordZoneId), d.Status.RecordID); err != nil {
307-
log.Error(err, "Failed to delete DNS record in Cloudflare. Record may still exist in Cloudflare")
256+
// reconcileDelete reconciles the deletion of the dnsrecord
257+
func (r *DNSRecordReconciler) reconcileDelete(ctx context.Context, zoneID string, dnsrecord *cloudflareoperatoriov1.DNSRecord) error {
258+
if err := r.Cf.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(zoneID), dnsrecord.Status.RecordID); err != nil && err.Error() != "Record does not exist. (81044)" {
308259
return err
309260
}
261+
metrics.DnsRecordFailureCounter.DeleteLabelValues(dnsrecord.Namespace, dnsrecord.Name, dnsrecord.Spec.Name)
262+
controllerutil.RemoveFinalizer(dnsrecord, common.CloudflareOperatorFinalizer)
310263

311264
return nil
312265
}
313266

314267
// markFailed marks the dnsrecord as failed
315-
func (r *DNSRecordReconciler) markFailed(ctx context.Context, dnsrecord *cloudflareoperatoriov1.DNSRecord, err error) error {
268+
func (r *DNSRecordReconciler) markFailed(dnsrecord *cloudflareoperatoriov1.DNSRecord, err error) {
316269
metrics.DnsRecordFailureCounter.WithLabelValues(dnsrecord.Namespace, dnsrecord.Name, dnsrecord.Spec.Name).Set(1)
317270
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
318271
Type: "Ready",
@@ -321,8 +274,6 @@ func (r *DNSRecordReconciler) markFailed(ctx context.Context, dnsrecord *cloudfl
321274
Message: err.Error(),
322275
ObservedGeneration: dnsrecord.Generation,
323276
})
324-
325-
return r.Status().Update(ctx, dnsrecord)
326277
}
327278

328279
// comparePriority compares the priority nil safe
@@ -384,11 +335,11 @@ func compareDNSRecord(dnsRecordSpec cloudflareoperatoriov1.DNSRecordSpec, existi
384335
return isEqual
385336
}
386337

387-
// requestsForIPChange returns a list of reconcile.Requests for DNSRecords that need to be reconciled
338+
// requestsForIPChange returns a list of reconcile.Requests for DNSRecords that need to be reconciled if the IP changes
388339
func (r *DNSRecordReconciler) requestsForIPChange(ctx context.Context, o client.Object) []reconcile.Request {
389340
ip, ok := o.(*cloudflareoperatoriov1.IP)
390341
if !ok {
391-
err := fmt.Errorf("expected a DNSRecord, got %T", o)
342+
err := fmt.Errorf("expected an IP, got %T", o)
392343
ctrl.LoggerFrom(ctx).Error(err, "failed to get requests for IP change")
393344
return nil
394345
}

0 commit comments

Comments
 (0)