Skip to content

Commit 159aa44

Browse files
felix-kaestnerfelix-kaestner
committed
Remove provider.Result Response type from provider integration
This changes removes the `provider.Result` return argument historically used on the "EnsureX" provider methods. This type contained a `RequeueAfter` field that mimicked the equally named field of the `ctrl.Result` returned from the reconciliation loop. The reason for this changes is that we decided to keep the orchestration flow, incl. the decision for and how requeuing should be done up to the controller and should avoid leaking such kubernetes specific logic into the provider code.
1 parent 42541be commit 159aa44

16 files changed

Lines changed: 206 additions & 266 deletions

‎hack/provider/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func main() {
205205
case "create":
206206
switch resource := obj.(type) {
207207
case *v1alpha1.Interface:
208-
_, err = ip.EnsureInterface(ctx, &provider.InterfaceRequest{
208+
err = ip.EnsureInterface(ctx, &provider.InterfaceRequest{
209209
Interface: resource,
210210
ProviderConfig: nil,
211211
})

‎internal/controller/acl_controller.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,12 @@ func (r *AccessControlListReconciler) Reconcile(ctx context.Context, req ctrl.Re
161161
}
162162
}()
163163

164-
res, err := r.reconcile(ctx, s)
165-
if err != nil {
164+
if err := r.reconcile(ctx, s); err != nil {
166165
log.Error(err, "Failed to reconcile resource")
167166
return ctrl.Result{}, err
168167
}
169168

170-
return res, nil
169+
return ctrl.Result{}, nil
171170
}
172171

173172
// SetupWithManager sets up the controller with the Manager.
@@ -198,7 +197,7 @@ type aclScope struct {
198197
Provider provider.ACLProvider
199198
}
200199

201-
func (r *AccessControlListReconciler) reconcile(ctx context.Context, s *aclScope) (_ ctrl.Result, reterr error) {
200+
func (r *AccessControlListReconciler) reconcile(ctx context.Context, s *aclScope) (reterr error) {
202201
if s.ACL.Labels == nil {
203202
s.ACL.Labels = make(map[string]string)
204203
}
@@ -208,12 +207,12 @@ func (r *AccessControlListReconciler) reconcile(ctx context.Context, s *aclScope
208207
// Ensure the AccessControlList is owned by the Device.
209208
if !controllerutil.HasControllerReference(s.ACL) {
210209
if err := controllerutil.SetOwnerReference(s.Device, s.ACL, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
211-
return ctrl.Result{}, err
210+
return err
212211
}
213212
}
214213

215214
if err := s.Provider.Connect(ctx, s.Connection); err != nil {
216-
return ctrl.Result{}, fmt.Errorf("failed to connect to provider: %w", err)
215+
return fmt.Errorf("failed to connect to provider: %w", err)
217216
}
218217
defer func() {
219218
if err := s.Provider.Disconnect(ctx, s.Connection); err != nil {
@@ -222,7 +221,7 @@ func (r *AccessControlListReconciler) reconcile(ctx context.Context, s *aclScope
222221
}()
223222

224223
// Ensure the AccessControlList is realized on the provider.
225-
res, err := s.Provider.EnsureACL(ctx, &provider.EnsureACLRequest{
224+
err := s.Provider.EnsureACL(ctx, &provider.EnsureACLRequest{
226225
ACL: s.ACL,
227226
ProviderConfig: s.ProviderConfig,
228227
})
@@ -232,11 +231,7 @@ func (r *AccessControlListReconciler) reconcile(ctx context.Context, s *aclScope
232231
cond.Type = v1alpha1.ReadyCondition
233232
conditions.Set(s.ACL, cond)
234233

235-
if err != nil {
236-
return ctrl.Result{}, err
237-
}
238-
239-
return ctrl.Result{RequeueAfter: res.RequeueAfter}, nil
234+
return err
240235
}
241236

242237
func (r *AccessControlListReconciler) finalize(ctx context.Context, s *aclScope) (reterr error) {

‎internal/controller/banner_controller.go‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -168,13 +168,12 @@ func (r *BannerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ c
168168
}
169169
}()
170170

171-
res, err := r.reconcile(ctx, s)
172-
if err != nil {
171+
if err := r.reconcile(ctx, s); err != nil {
173172
log.Error(err, "Failed to reconcile resource")
174173
return ctrl.Result{}, err
175174
}
176175

177-
return res, nil
176+
return ctrl.Result{}, nil
178177
}
179178

180179
// SetupWithManager sets up the controller with the Manager.
@@ -217,7 +216,7 @@ type bannerScope struct {
217216
Provider provider.BannerProvider
218217
}
219218

220-
func (r *BannerReconciler) reconcile(ctx context.Context, s *bannerScope) (_ ctrl.Result, reterr error) {
219+
func (r *BannerReconciler) reconcile(ctx context.Context, s *bannerScope) (reterr error) {
221220
if s.Banner.Labels == nil {
222221
s.Banner.Labels = make(map[string]string)
223222
}
@@ -227,12 +226,12 @@ func (r *BannerReconciler) reconcile(ctx context.Context, s *bannerScope) (_ ctr
227226
// Ensure the Banner is owned by the Device.
228227
if !controllerutil.HasControllerReference(s.Banner) {
229228
if err := controllerutil.SetOwnerReference(s.Device, s.Banner, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
230-
return ctrl.Result{}, err
229+
return err
231230
}
232231
}
233232

234233
if err := s.Provider.Connect(ctx, s.Connection); err != nil {
235-
return ctrl.Result{}, fmt.Errorf("failed to connect to provider: %w", err)
234+
return fmt.Errorf("failed to connect to provider: %w", err)
236235
}
237236
defer func() {
238237
if err := s.Provider.Disconnect(ctx, s.Connection); err != nil {
@@ -242,11 +241,11 @@ func (r *BannerReconciler) reconcile(ctx context.Context, s *bannerScope) (_ ctr
242241

243242
msg, err := clientutil.NewClient(r, s.Banner.Namespace).Template(ctx, &s.Banner.Spec.Message)
244243
if err != nil {
245-
return ctrl.Result{}, err
244+
return err
246245
}
247246

248247
// Ensure the Banner is realized on the provider.
249-
res, err := s.Provider.EnsureBanner(ctx, &provider.BannerRequest{
248+
err = s.Provider.EnsureBanner(ctx, &provider.BannerRequest{
250249
Message: string(msg),
251250
ProviderConfig: s.ProviderConfig,
252251
})
@@ -256,11 +255,7 @@ func (r *BannerReconciler) reconcile(ctx context.Context, s *bannerScope) (_ ctr
256255
cond.Type = v1alpha1.ReadyCondition
257256
conditions.Set(s.Banner, cond)
258257

259-
if err != nil {
260-
return ctrl.Result{}, err
261-
}
262-
263-
return ctrl.Result{RequeueAfter: res.RequeueAfter}, nil
258+
return err
264259
}
265260

266261
func (r *BannerReconciler) finalize(ctx context.Context, s *bannerScope) (reterr error) {

‎internal/controller/certificate_controller.go‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,12 @@ func (r *CertificateReconciler) Reconcile(ctx context.Context, req ctrl.Request)
167167
}
168168
}()
169169

170-
res, err := r.reconcile(ctx, s)
171-
if err != nil {
170+
if err := r.reconcile(ctx, s); err != nil {
172171
log.Error(err, "Failed to reconcile resource")
173172
return ctrl.Result{}, err
174173
}
175174

176-
return res, nil
175+
return ctrl.Result{}, nil
177176
}
178177

179178
// SetupWithManager sets up the controller with the Manager.
@@ -210,7 +209,7 @@ type certificateScope struct {
210209
Provider provider.CertificateProvider
211210
}
212211

213-
func (r *CertificateReconciler) reconcile(ctx context.Context, s *certificateScope) (_ ctrl.Result, reterr error) {
212+
func (r *CertificateReconciler) reconcile(ctx context.Context, s *certificateScope) (reterr error) {
214213
if s.Certificate.Labels == nil {
215214
s.Certificate.Labels = make(map[string]string)
216215
}
@@ -220,12 +219,12 @@ func (r *CertificateReconciler) reconcile(ctx context.Context, s *certificateSco
220219
// Ensure the Certificate is owned by the Device.
221220
if !controllerutil.HasControllerReference(s.Certificate) {
222221
if err := controllerutil.SetOwnerReference(s.Device, s.Certificate, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
223-
return ctrl.Result{}, err
222+
return err
224223
}
225224
}
226225

227226
if err := s.Provider.Connect(ctx, s.Connection); err != nil {
228-
return ctrl.Result{}, fmt.Errorf("failed to connect to provider: %w", err)
227+
return fmt.Errorf("failed to connect to provider: %w", err)
229228
}
230229
defer func() {
231230
if err := s.Provider.Disconnect(ctx, s.Connection); err != nil {
@@ -235,11 +234,11 @@ func (r *CertificateReconciler) reconcile(ctx context.Context, s *certificateSco
235234

236235
cert, err := clientutil.NewClient(r, s.Certificate.Namespace).Certificate(ctx, &s.Certificate.Spec.SecretRef)
237236
if err != nil {
238-
return ctrl.Result{}, err
237+
return err
239238
}
240239

241240
// Ensure the Certificate is realized on the provider.
242-
res, err := s.Provider.EnsureCertificate(ctx, &provider.EnsureCertificateRequest{
241+
err = s.Provider.EnsureCertificate(ctx, &provider.EnsureCertificateRequest{
243242
ID: s.Certificate.Spec.ID,
244243
Certificate: cert,
245244
ProviderConfig: s.ProviderConfig,
@@ -250,11 +249,7 @@ func (r *CertificateReconciler) reconcile(ctx context.Context, s *certificateSco
250249
cond.Type = v1alpha1.ReadyCondition
251250
conditions.Set(s.Certificate, cond)
252251

253-
if err != nil {
254-
return ctrl.Result{}, err
255-
}
256-
257-
return ctrl.Result{RequeueAfter: res.RequeueAfter}, nil
252+
return err
258253
}
259254

260255
func (r *CertificateReconciler) finalize(ctx context.Context, s *certificateScope) (reterr error) {

‎internal/controller/dns_controller.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,12 @@ func (r *DNSReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl
161161
}
162162
}()
163163

164-
res, err := r.reconcile(ctx, s)
165-
if err != nil {
164+
if err := r.reconcile(ctx, s); err != nil {
166165
log.Error(err, "Failed to reconcile resource")
167166
return ctrl.Result{}, err
168167
}
169168

170-
return res, nil
169+
return ctrl.Result{}, nil
171170
}
172171

173172
// SetupWithManager sets up the controller with the Manager.
@@ -198,7 +197,7 @@ type dnsScope struct {
198197
Provider provider.DNSProvider
199198
}
200199

201-
func (r *DNSReconciler) reconcile(ctx context.Context, s *dnsScope) (_ ctrl.Result, reterr error) {
200+
func (r *DNSReconciler) reconcile(ctx context.Context, s *dnsScope) (reterr error) {
202201
if s.DNS.Labels == nil {
203202
s.DNS.Labels = make(map[string]string)
204203
}
@@ -208,12 +207,12 @@ func (r *DNSReconciler) reconcile(ctx context.Context, s *dnsScope) (_ ctrl.Resu
208207
// Ensure the DNS is owned by the Device.
209208
if !controllerutil.HasControllerReference(s.DNS) {
210209
if err := controllerutil.SetOwnerReference(s.Device, s.DNS, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
211-
return ctrl.Result{}, err
210+
return err
212211
}
213212
}
214213

215214
if err := s.Provider.Connect(ctx, s.Connection); err != nil {
216-
return ctrl.Result{}, fmt.Errorf("failed to connect to provider: %w", err)
215+
return fmt.Errorf("failed to connect to provider: %w", err)
217216
}
218217
defer func() {
219218
if err := s.Provider.Disconnect(ctx, s.Connection); err != nil {
@@ -222,7 +221,7 @@ func (r *DNSReconciler) reconcile(ctx context.Context, s *dnsScope) (_ ctrl.Resu
222221
}()
223222

224223
// Ensure the DNS is realized on the provider.
225-
res, err := s.Provider.EnsureDNS(ctx, &provider.EnsureDNSRequest{
224+
err := s.Provider.EnsureDNS(ctx, &provider.EnsureDNSRequest{
226225
DNS: s.DNS,
227226
ProviderConfig: s.ProviderConfig,
228227
})
@@ -232,11 +231,7 @@ func (r *DNSReconciler) reconcile(ctx context.Context, s *dnsScope) (_ ctrl.Resu
232231
cond.Type = v1alpha1.ReadyCondition
233232
conditions.Set(s.DNS, cond)
234233

235-
if err != nil {
236-
return ctrl.Result{}, err
237-
}
238-
239-
return ctrl.Result{RequeueAfter: res.RequeueAfter}, nil
234+
return err
240235
}
241236

242237
func (r *DNSReconciler) finalize(ctx context.Context, s *dnsScope) (reterr error) {

‎internal/controller/interface_controller.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func (r *InterfaceReconciler) reconcile(ctx context.Context, s *scope) (_ ctrl.R
236236
}()
237237

238238
// Ensure the Interface is realized on the provider.
239-
res, err := s.Provider.EnsureInterface(ctx, &provider.InterfaceRequest{
239+
err := s.Provider.EnsureInterface(ctx, &provider.InterfaceRequest{
240240
Interface: s.Interface,
241241
ProviderConfig: s.ProviderConfig,
242242
})
@@ -248,10 +248,6 @@ func (r *InterfaceReconciler) reconcile(ctx context.Context, s *scope) (_ ctrl.R
248248
return ctrl.Result{}, err
249249
}
250250

251-
if res.RequeueAfter > 0 {
252-
return ctrl.Result{RequeueAfter: res.RequeueAfter}, nil
253-
}
254-
255251
status, err := s.Provider.GetInterfaceStatus(ctx, &provider.InterfaceRequest{
256252
Interface: s.Interface,
257253
ProviderConfig: s.ProviderConfig,

‎internal/controller/isis_controller.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func (r *ISISReconciler) reconcile(ctx context.Context, s *isisScope) (_ ctrl.Re
258258
}()
259259

260260
// Ensure the ISIS is realized on the provider.
261-
res, err := s.Provider.EnsureISIS(ctx, &provider.EnsureISISRequest{
261+
err := s.Provider.EnsureISIS(ctx, &provider.EnsureISISRequest{
262262
ISIS: s.ISIS,
263263
Interfaces: interfaces,
264264
ProviderConfig: s.ProviderConfig,
@@ -269,11 +269,7 @@ func (r *ISISReconciler) reconcile(ctx context.Context, s *isisScope) (_ ctrl.Re
269269
cond.Type = v1alpha1.ReadyCondition
270270
conditions.Set(s.ISIS, cond)
271271

272-
if err != nil {
273-
return ctrl.Result{}, err
274-
}
275-
276-
return ctrl.Result{RequeueAfter: res.RequeueAfter}, nil
272+
return ctrl.Result{}, err
277273
}
278274

279275
func (r *ISISReconciler) finalize(ctx context.Context, s *isisScope) (reterr error) {

‎internal/controller/managementaccess_controller.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,12 @@ func (r *ManagementAccessReconciler) Reconcile(ctx context.Context, req ctrl.Req
161161
}
162162
}()
163163

164-
res, err := r.reconcile(ctx, s)
165-
if err != nil {
164+
if err := r.reconcile(ctx, s); err != nil {
166165
log.Error(err, "Failed to reconcile resource")
167166
return ctrl.Result{}, err
168167
}
169168

170-
return res, nil
169+
return ctrl.Result{}, nil
171170
}
172171

173172
// SetupWithManager sets up the controller with the Manager.
@@ -198,7 +197,7 @@ type managementAccessScope struct {
198197
Provider provider.ManagementAccessProvider
199198
}
200199

201-
func (r *ManagementAccessReconciler) reconcile(ctx context.Context, s *managementAccessScope) (_ ctrl.Result, reterr error) {
200+
func (r *ManagementAccessReconciler) reconcile(ctx context.Context, s *managementAccessScope) (reterr error) {
202201
if s.ManagementAccess.Labels == nil {
203202
s.ManagementAccess.Labels = make(map[string]string)
204203
}
@@ -208,12 +207,12 @@ func (r *ManagementAccessReconciler) reconcile(ctx context.Context, s *managemen
208207
// Ensure the ManagementAccess is owned by the Device.
209208
if !controllerutil.HasControllerReference(s.ManagementAccess) {
210209
if err := controllerutil.SetOwnerReference(s.Device, s.ManagementAccess, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
211-
return ctrl.Result{}, err
210+
return err
212211
}
213212
}
214213

215214
if err := s.Provider.Connect(ctx, s.Connection); err != nil {
216-
return ctrl.Result{}, fmt.Errorf("failed to connect to provider: %w", err)
215+
return fmt.Errorf("failed to connect to provider: %w", err)
217216
}
218217
defer func() {
219218
if err := s.Provider.Disconnect(ctx, s.Connection); err != nil {
@@ -222,7 +221,7 @@ func (r *ManagementAccessReconciler) reconcile(ctx context.Context, s *managemen
222221
}()
223222

224223
// Ensure the ManagementAccess is realized on the provider.
225-
res, err := s.Provider.EnsureManagementAccess(ctx, &provider.EnsureManagementAccessRequest{
224+
err := s.Provider.EnsureManagementAccess(ctx, &provider.EnsureManagementAccessRequest{
226225
ManagementAccess: s.ManagementAccess,
227226
ProviderConfig: s.ProviderConfig,
228227
})
@@ -232,11 +231,7 @@ func (r *ManagementAccessReconciler) reconcile(ctx context.Context, s *managemen
232231
cond.Type = v1alpha1.ReadyCondition
233232
conditions.Set(s.ManagementAccess, cond)
234233

235-
if err != nil {
236-
return ctrl.Result{}, err
237-
}
238-
239-
return ctrl.Result{RequeueAfter: res.RequeueAfter}, nil
234+
return err
240235
}
241236

242237
func (r *ManagementAccessReconciler) finalize(ctx context.Context, s *managementAccessScope) (reterr error) {

0 commit comments

Comments
 (0)