Skip to content

Commit 315ac8d

Browse files
rxbnrxbn
committed
fix: improve status condition handling
1 parent f43bb04 commit 315ac8d

File tree

5 files changed

+50
-64
lines changed

5 files changed

+50
-64
lines changed

‎internal/controller/account_controller.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controller
1818

1919
import (
2020
"context"
21+
"errors"
2122
"time"
2223

2324
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -89,7 +90,7 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
8990

9091
secret := &corev1.Secret{}
9192
if err := r.Get(ctx, client.ObjectKey{Namespace: account.Spec.ApiToken.SecretRef.Namespace, Name: account.Spec.ApiToken.SecretRef.Name}, secret); err != nil {
92-
if err := r.markFailed(ctx, account, "Failed to get secret"); err != nil {
93+
if err := r.markFailed(ctx, account, err); err != nil {
9394
log.Error(err, "Failed to update Account status")
9495
return ctrl.Result{}, err
9596
}
@@ -98,7 +99,7 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
9899

99100
cfApiToken := string(secret.Data["apiToken"])
100101
if cfApiToken == "" {
101-
if err := r.markFailed(ctx, account, "Secret has no 'apiToken' key"); err != nil {
102+
if err := r.markFailed(ctx, account, errors.New("Secret has no 'apiToken' key")); err != nil {
102103
log.Error(err, "Failed to update Account status")
103104
return ctrl.Result{}, err
104105
}
@@ -108,7 +109,7 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
108109
if r.Cf.APIToken != cfApiToken {
109110
cf, err := cloudflare.NewWithAPIToken(cfApiToken)
110111
if err != nil {
111-
if err := r.markFailed(ctx, account, err.Error()); err != nil {
112+
if err := r.markFailed(ctx, account, err); err != nil {
112113
log.Error(err, "Failed to update Account status")
113114
return ctrl.Result{}, err
114115
}
@@ -133,18 +134,15 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
133134
}
134135

135136
// markFailed marks the reconciled object as failed
136-
func (r *AccountReconciler) markFailed(ctx context.Context, account *cloudflareoperatoriov1.Account, message string) error {
137+
func (r *AccountReconciler) markFailed(ctx context.Context, account *cloudflareoperatoriov1.Account, err error) error {
137138
metrics.AccountFailureCounter.WithLabelValues(account.Name).Set(1)
138139
apimeta.SetStatusCondition(&account.Status.Conditions, metav1.Condition{
139140
Type: "Ready",
140141
Status: "False",
141142
Reason: "Failed",
142-
Message: message,
143+
Message: err.Error(),
143144
ObservedGeneration: account.Generation,
144145
})
145-
if err := r.Status().Update(ctx, account); err != nil {
146-
return err
147-
}
148146

149-
return nil
147+
return r.Status().Update(ctx, account)
150148
}

‎internal/controller/dnsrecord_controller.go

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ package controller
1919
import (
2020
"context"
2121
"encoding/json"
22+
"errors"
23+
"fmt"
2224
"reflect"
2325
"time"
2426

2527
"github.com/go-logr/logr"
2628
"golang.org/x/net/publicsuffix"
2729
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
28-
"k8s.io/apimachinery/pkg/api/errors"
2930
apimeta "k8s.io/apimachinery/pkg/api/meta"
3031
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3132
"k8s.io/apimachinery/pkg/runtime"
@@ -86,14 +87,10 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
8687

8788
zones := &cloudflareoperatoriov1.ZoneList{}
8889
if err := r.List(ctx, zones); err != nil {
89-
if errors.IsNotFound(err) {
90-
if err := r.markFailed(ctx, dnsrecord, "Failed to fetch Zones"); err != nil {
91-
log.Error(err, "Failed to update DNSRecord status")
92-
return ctrl.Result{}, err
93-
}
94-
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
90+
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
91+
log.Error(err, "Failed to update DNSRecord status")
92+
return ctrl.Result{}, err
9593
}
96-
log.Error(err, "Failed to fetch Zone resources")
9794
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
9895
}
9996

@@ -106,7 +103,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
106103
}
107104

108105
if zone.Spec.Name == "" {
109-
if err := r.markFailed(ctx, dnsrecord, "Zone not found"); err != nil {
106+
if err := r.markFailed(ctx, dnsrecord, fmt.Errorf("Zone %q not found", zoneName)); err != nil {
110107
log.Error(err, "Failed to update DNSRecord status")
111108
return ctrl.Result{}, err
112109
}
@@ -139,7 +136,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
139136
if !dnsrecord.DeletionTimestamp.IsZero() {
140137
if controllerutil.ContainsFinalizer(dnsrecord, common.CloudflareOperatorFinalizer) {
141138
if err := r.finalizeDNSRecord(ctx, zone.Status.ID, log, dnsrecord); err != nil && err.Error() != "Record does not exist. (81044)" {
142-
if err := r.markFailed(ctx, dnsrecord, err.Error()); err != nil {
139+
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
143140
log.Error(err, "Failed to update DNSRecord status")
144141
return ctrl.Result{}, err
145142
}
@@ -163,7 +160,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
163160
Content: dnsrecord.Spec.Content,
164161
})
165162
if err != nil {
166-
if err := r.markFailed(ctx, dnsrecord, err.Error()); err != nil {
163+
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
167164
log.Error(err, "Failed to update DNSRecord status")
168165
return ctrl.Result{}, err
169166
}
@@ -178,7 +175,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
178175
if (dnsrecord.Spec.Type == "A" || dnsrecord.Spec.Type == "AAAA") && dnsrecord.Spec.IPRef.Name != "" {
179176
ip := &cloudflareoperatoriov1.IP{}
180177
if err := r.Get(ctx, client.ObjectKey{Name: dnsrecord.Spec.IPRef.Name}, ip); err != nil {
181-
if err := r.markFailed(ctx, dnsrecord, "IP object not found"); err != nil {
178+
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
182179
log.Error(err, "Failed to update DNSRecord status")
183180
return ctrl.Result{}, err
184181
}
@@ -187,14 +184,14 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
187184
if ip.Spec.Address != dnsrecord.Spec.Content {
188185
dnsrecord.Spec.Content = ip.Spec.Address
189186
if err := r.Update(ctx, dnsrecord); err != nil {
190-
log.Error(err, "Failed to update DNSRecord resource")
187+
log.Error(err, "Failed to update DNSRecord")
191188
return ctrl.Result{}, err
192189
}
193190
}
194191
}
195192

196193
if *dnsrecord.Spec.Proxied && dnsrecord.Spec.TTL != 1 {
197-
if err := r.markFailed(ctx, dnsrecord, "TTL must be 1 when proxied"); err != nil {
194+
if err := r.markFailed(ctx, dnsrecord, errors.New("TTL must be 1 when proxied")); err != nil {
198195
log.Error(err, "Failed to update DNSRecord status")
199196
return ctrl.Result{}, err
200197
}
@@ -212,7 +209,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
212209
Data: dnsrecord.Spec.Data,
213210
})
214211
if err != nil {
215-
if err := r.markFailed(ctx, dnsrecord, err.Error()); err != nil {
212+
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
216213
log.Error(err, "Failed to update DNSRecord status")
217214
return ctrl.Result{}, err
218215
}
@@ -244,7 +241,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
244241
Priority: dnsrecord.Spec.Priority,
245242
Data: dnsrecord.Spec.Data,
246243
}); err != nil {
247-
if err := r.markFailed(ctx, dnsrecord, err.Error()); err != nil {
244+
if err := r.markFailed(ctx, dnsrecord, err); err != nil {
248245
log.Error(err, "Failed to update DNSRecord status")
249246
return ctrl.Result{}, err
250247
}
@@ -293,20 +290,17 @@ func (r *DNSRecordReconciler) finalizeDNSRecord(ctx context.Context, dnsRecordZo
293290
}
294291

295292
// markFailed marks the reconciled object as failed
296-
func (r *DNSRecordReconciler) markFailed(ctx context.Context, dnsrecord *cloudflareoperatoriov1.DNSRecord, message string) error {
293+
func (r *DNSRecordReconciler) markFailed(ctx context.Context, dnsrecord *cloudflareoperatoriov1.DNSRecord, err error) error {
297294
metrics.DnsRecordFailureCounter.WithLabelValues(dnsrecord.Namespace, dnsrecord.Name, dnsrecord.Spec.Name).Set(1)
298295
apimeta.SetStatusCondition(&dnsrecord.Status.Conditions, metav1.Condition{
299296
Type: "Ready",
300297
Status: "False",
301298
Reason: "Failed",
302-
Message: message,
299+
Message: err.Error(),
303300
ObservedGeneration: dnsrecord.Generation,
304301
})
305-
if err := r.Status().Update(ctx, dnsrecord); err != nil {
306-
return err
307-
}
308302

309-
return nil
303+
return r.Status().Update(ctx, dnsrecord)
310304
}
311305

312306
// comparePriority compares the priority nil safe

‎internal/controller/ingress_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
7979

8080
dnsRecords := &cloudflareoperatoriov1.DNSRecordList{}
8181
if err := r.List(ctx, dnsRecords, client.InNamespace(ingress.Namespace), client.MatchingFields{"metadata.ownerReferences.uid": string(ingress.UID)}); err != nil {
82-
log.Error(err, "Failed to fetch DNSRecord")
82+
log.Error(err, "Failed to list DNSRecords")
8383
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
8484
}
8585

‎internal/controller/ip_controller.go

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"context"
2222
"crypto/tls"
2323
"encoding/json"
24+
"errors"
2425
"fmt"
2526
"io"
2627
"math/rand"
@@ -97,15 +98,15 @@ func (r *IPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
9798

9899
if ip.Spec.Type == "static" {
99100
if ip.Spec.Address == "" {
100-
if err := r.markFailed(ctx, ip, "Address is required for static IPs"); err != nil {
101-
log.Error(err, "Failed to update IP resource")
101+
if err := r.markFailed(ctx, ip, errors.New("Address is required for static IPs")); err != nil {
102+
log.Error(err, "Failed to update IP status")
102103
return ctrl.Result{}, err
103104
}
104105
return ctrl.Result{}, nil
105106
}
106107
if net.ParseIP(ip.Spec.Address) == nil {
107-
if err := r.markFailed(ctx, ip, "Address is not a valid IP address"); err != nil {
108-
log.Error(err, "Failed to update IP resource")
108+
if err := r.markFailed(ctx, ip, errors.New("Address is not a valid IP address")); err != nil {
109+
log.Error(err, "Failed to update IP status")
109110
return ctrl.Result{}, err
110111
}
111112
return ctrl.Result{}, nil
@@ -116,14 +117,14 @@ func (r *IPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
116117
if ip.Spec.Interval == nil {
117118
ip.Spec.Interval = &metav1.Duration{Duration: time.Minute * 5}
118119
if err := r.Update(ctx, ip); err != nil {
119-
log.Error(err, "Failed to update IP resource")
120+
log.Error(err, "Failed to update IP")
120121
return ctrl.Result{}, err
121122
}
122123
}
123124

124125
if len(ip.Spec.IPSources) == 0 {
125-
if err := r.markFailed(ctx, ip, "IPSources is required for dynamic IPs"); err != nil {
126-
log.Error(err, "Failed to update IP resource")
126+
if err := r.markFailed(ctx, ip, errors.New("IP sources are required for dynamic IPs")); err != nil {
127+
log.Error(err, "Failed to update IP status")
127128
return ctrl.Result{}, err
128129
}
129130
return ctrl.Result{}, nil
@@ -135,21 +136,20 @@ func (r *IPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
135136
})
136137
}
137138

138-
var ipSourceError string
139+
var ipSourceError error
139140
for _, source := range ip.Spec.IPSources {
140141
response, err := r.getIPSource(ctx, source, log)
141142
if err != nil {
142-
ipSourceError = err.Error()
143+
ipSourceError = err
143144
continue
144145
}
145146
ip.Spec.Address = response
146-
ipSourceError = ""
147147
break
148148
}
149149

150-
if ipSourceError != "" {
150+
if ipSourceError != nil {
151151
if err := r.markFailed(ctx, ip, ipSourceError); err != nil {
152-
log.Error(err, "Failed to update IP resource")
152+
log.Error(err, "Failed to update IP status")
153153
return ctrl.Result{}, err
154154
}
155155
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
@@ -158,12 +158,12 @@ func (r *IPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
158158

159159
if ip.Spec.Address != ip.Status.LastObservedIP {
160160
if err := r.Update(ctx, ip); err != nil {
161-
log.Error(err, "Failed to update IP resource")
161+
log.Error(err, "Failed to update IP status")
162162
return ctrl.Result{}, err
163163
}
164164
ip.Status.LastObservedIP = ip.Spec.Address
165165
if err := r.Status().Update(ctx, ip); err != nil {
166-
log.Error(err, "Failed to update IP resource")
166+
log.Error(err, "Failed to update IP status")
167167
return ctrl.Result{}, err
168168
}
169169
}
@@ -195,7 +195,7 @@ func (r *IPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
195195
ObservedGeneration: ip.Generation,
196196
})
197197
if err := r.Status().Update(ctx, ip); err != nil {
198-
log.Error(err, "Failed to update IP resource")
198+
log.Error(err, "Failed to update IP status")
199199
return ctrl.Result{}, err
200200
}
201201

@@ -308,18 +308,15 @@ func (r *IPReconciler) getIPSource(ctx context.Context, source cloudflareoperato
308308
}
309309

310310
// markFailed marks the reconciled object as failed
311-
func (r *IPReconciler) markFailed(ctx context.Context, ip *cloudflareoperatoriov1.IP, message string) error {
311+
func (r *IPReconciler) markFailed(ctx context.Context, ip *cloudflareoperatoriov1.IP, err error) error {
312312
metrics.IpFailureCounter.WithLabelValues(ip.Name, ip.Spec.Type).Set(1)
313313
apimeta.SetStatusCondition(&ip.Status.Conditions, metav1.Condition{
314314
Type: "Ready",
315315
Status: "False",
316316
Reason: "Failed",
317-
Message: message,
317+
Message: err.Error(),
318318
ObservedGeneration: ip.Generation,
319319
})
320-
if err := r.Status().Update(ctx, ip); err != nil {
321-
return err
322-
}
323320

324-
return nil
321+
return r.Status().Update(ctx, ip)
325322
}

‎internal/controller/zone_controller.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,11 @@ func (r *ZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
102102

103103
zoneID, err := r.Cf.ZoneIDByName(zone.Spec.Name)
104104
if err != nil {
105-
if err := r.markFailed(ctx, zone, err.Error()); err != nil {
105+
if err := r.markFailed(ctx, zone, err); err != nil {
106106
log.Error(err, "Failed to update Zone status")
107107
return ctrl.Result{}, err
108108
}
109-
return ctrl.Result{}, err
109+
return ctrl.Result{}, nil
110110
}
111111

112112
if zone.Status.ID != zoneID {
@@ -120,13 +120,13 @@ func (r *ZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
120120
if zone.Spec.Prune {
121121
dnsRecords := &cloudflareoperatoriov1.DNSRecordList{}
122122
if err := r.List(ctx, dnsRecords); err != nil {
123-
log.Error(err, "Failed to list DNSRecord resources")
123+
log.Error(err, "Failed to list DNSRecords")
124124
return ctrl.Result{RequeueAfter: time.Second * 30}, nil
125125
}
126126

127127
cfDnsRecords, _, err := r.Cf.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.ListDNSRecordsParams{})
128128
if err != nil {
129-
if err := r.markFailed(ctx, zone, err.Error()); err != nil {
129+
if err := r.markFailed(ctx, zone, err); err != nil {
130130
log.Error(err, "Failed to update Zone status")
131131
return ctrl.Result{}, err
132132
}
@@ -145,7 +145,7 @@ func (r *ZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
145145

146146
if _, found := dnsRecordMap[cfDnsRecord.ID]; !found {
147147
if err := r.Cf.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cfDnsRecord.ID); err != nil {
148-
if err := r.markFailed(ctx, zone, err.Error()); err != nil {
148+
if err := r.markFailed(ctx, zone, err); err != nil {
149149
log.Error(err, "Failed to update Zone status")
150150
}
151151
}
@@ -170,18 +170,15 @@ func (r *ZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
170170
}
171171

172172
// markFailed marks the reconciled object as failed
173-
func (r *ZoneReconciler) markFailed(ctx context.Context, zone *cloudflareoperatoriov1.Zone, message string) error {
173+
func (r *ZoneReconciler) markFailed(ctx context.Context, zone *cloudflareoperatoriov1.Zone, err error) error {
174174
metrics.ZoneFailureCounter.WithLabelValues(zone.Name, zone.Spec.Name).Set(1)
175175
apimeta.SetStatusCondition(&zone.Status.Conditions, metav1.Condition{
176176
Type: "Ready",
177177
Status: "False",
178178
Reason: "Failed",
179-
Message: message,
179+
Message: err.Error(),
180180
ObservedGeneration: zone.Generation,
181181
})
182-
if err := r.Status().Update(ctx, zone); err != nil {
183-
return err
184-
}
185182

186-
return nil
183+
return r.Status().Update(ctx, zone)
187184
}

0 commit comments

Comments
 (0)