Skip to content

Commit 79aefac

Browse files
kristofgyuraczkristofgyuracz
committed
log non-critical errors, but don't fail
Signed-off-by: Kristof Gyuracz <[email protected]>
1 parent b0731f6 commit 79aefac

File tree

1 file changed

+38
-45
lines changed

1 file changed

+38
-45
lines changed

‎internal/controller/telemetry/collector_controller.go

+38-45
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,9 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
8383
return ctrl.Result{}, err
8484
}
8585

86-
tenantsToDisown, err := r.getTenantsReferencingCollectorButNotSelected(ctx, collector, tenants)
87-
if err != nil {
88-
return ctrl.Result{}, err
89-
}
86+
tenantsToDisown := r.getTenantsReferencingCollectorButNotSelected(ctx, collector, tenants)
9087

91-
err = r.disownTenants(ctx, tenantsToDisown)
92-
if err != nil {
93-
return ctrl.Result{}, err
94-
}
88+
r.disownTenants(ctx, tenantsToDisown)
9589

9690
tenantNames := []string{}
9791

@@ -101,7 +95,6 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
10195

10296
logger.Info("Setting collector status")
10397

104-
10598
subscriptions := []v1alpha1.Subscription{}
10699

107100
for _, tenant := range tenants {
@@ -124,15 +117,9 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
124117
return ctrl.Result{}, err
125118
}
126119

127-
subscriptionsToDisown, err := r.getSubscriptionsReferencingTenantButNotSelected(ctx, &tenant, subscriptionsForTenant)
128-
if err != nil {
129-
return ctrl.Result{}, err
130-
}
120+
subscriptionsToDisown := r.getSubscriptionsReferencingTenantButNotSelected(ctx, &tenant, subscriptionsForTenant)
131121

132-
err = r.disownSubscriptions(ctx, subscriptionsToDisown)
133-
if err != nil {
134-
return ctrl.Result{}, err
135-
}
122+
r.disownSubscriptions(ctx, subscriptionsToDisown)
136123

137124
subscriptions = append(subscriptions, subscriptionsForTenant...)
138125

@@ -159,7 +146,6 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
159146
slices.Sort(logsourceNamespacesForTenant)
160147
tenant.Status.LogSourceNamespaces = logsourceNamespacesForTenant
161148

162-
163149
if err := r.Status().Update(ctx, &tenant); err != nil {
164150
return ctrl.Result{}, err
165151
}
@@ -172,8 +158,11 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
172158

173159
collector.Status.Tenants = tenantNames
174160

175-
r.Status().Update(ctx, collector)
176161
logger.Info("Setting collector status")
162+
err = r.Status().Update(ctx, collector)
163+
if err != nil {
164+
return ctrl.Result{}, err
165+
}
177166

178167
outputs, err := r.getAllOutputs(ctx)
179168
if err != nil {
@@ -292,7 +281,10 @@ func (r *CollectorReconciler) SetupWithManager(mgr ctrl.Manager) error {
292281
tenant, _ := object.(*v1alpha1.Tenant)
293282

294283
collectors := v1alpha1.CollectorList{}
295-
r.List(ctx, &collectors)
284+
err := r.List(ctx, &collectors)
285+
if err != nil {
286+
return nil
287+
}
296288

297289
for _, collector := range collectors.Items {
298290
tenantsForCollector, err := r.getTenantsMatchingSelectors(ctx, collector.Spec.TenantSelector)
@@ -310,10 +302,7 @@ func (r *CollectorReconciler) SetupWithManager(mgr ctrl.Manager) error {
310302
}
311303
}
312304

313-
tenantsToDisown, err := r.getTenantsReferencingCollectorButNotSelected(ctx, &collector, tenantsForCollector)
314-
if err != nil {
315-
return nil
316-
}
305+
tenantsToDisown := r.getTenantsReferencingCollectorButNotSelected(ctx, &collector, tenantsForCollector)
317306

318307
for _, t := range tenantsToDisown {
319308
if t.Name == tenant.Name {
@@ -333,7 +322,10 @@ func (r *CollectorReconciler) SetupWithManager(mgr ctrl.Manager) error {
333322
subscription, _ := object.(*v1alpha1.Subscription)
334323

335324
tenants := v1alpha1.TenantList{}
336-
r.List(ctx, &tenants)
325+
err := r.List(ctx, &tenants)
326+
if err != nil {
327+
return nil
328+
}
337329

338330
for _, tenant := range tenants.Items {
339331
subscriptionsForTenant, err := r.getSubscriptionsForTenant(ctx, &tenant)
@@ -351,10 +343,7 @@ func (r *CollectorReconciler) SetupWithManager(mgr ctrl.Manager) error {
351343
}
352344
}
353345

354-
subscriptionstoDisown, err := r.getSubscriptionsReferencingTenantButNotSelected(ctx, &tenant, subscriptionsForTenant)
355-
if err != nil {
356-
return nil
357-
}
346+
subscriptionstoDisown := r.getSubscriptionsReferencingTenantButNotSelected(ctx, &tenant, subscriptionsForTenant)
358347

359348
for _, s := range subscriptionstoDisown {
360349
if s.Name == subscription.Name {
@@ -411,6 +400,7 @@ func (r *CollectorReconciler) reconcileServiceAccount(ctx context.Context, colle
411400

412401
return v1alpha1.NamespacedName{Namespace: serviceAccount.Namespace, Name: serviceAccount.Name}, nil
413402
}
403+
414404
func (r *CollectorReconciler) reconcileClusterRoleBinding(ctx context.Context, collector *v1alpha1.Collector) error {
415405
logger := log.FromContext(ctx)
416406

@@ -473,7 +463,6 @@ func (r *CollectorReconciler) reconcileClusterRole(ctx context.Context, collecto
473463
return err
474464
}
475465

476-
477466
func getSubscriptionNamesFromSubscription(subscriptions []v1alpha1.Subscription) []v1alpha1.NamespacedName {
478467
subscriptionNames := make([]v1alpha1.NamespacedName, len(subscriptions))
479468
for i, subscription := range subscriptions {
@@ -502,15 +491,17 @@ func (r *CollectorReconciler) getTenantsMatchingSelectors(ctx context.Context, l
502491
return tenantsForSelector.Items, nil
503492
}
504493

505-
func (r *CollectorReconciler) getTenantsReferencingCollectorButNotSelected(ctx context.Context, collector *v1alpha1.Collector, selectedTenants []v1alpha1.Tenant) ([]v1alpha1.Tenant, error) {
494+
func (r *CollectorReconciler) getTenantsReferencingCollectorButNotSelected(ctx context.Context, collector *v1alpha1.Collector, selectedTenants []v1alpha1.Tenant) []v1alpha1.Tenant {
495+
logger := log.FromContext(ctx)
506496
var tenantsReferencing v1alpha1.TenantList
507497

508498
listOpts := &client.ListOptions{
509499
FieldSelector: fields.OneTermEqualSelector(collectorReferenceField, collector.Name),
510500
}
511501

512502
if err := r.Client.List(ctx, &tenantsReferencing, listOpts); client.IgnoreNotFound(err) != nil {
513-
return nil, err
503+
logger.Error(err, "failed to list tenants that need to be detached from collector")
504+
return nil
514505
}
515506

516507
tenantsToDisown := []v1alpha1.Tenant{}
@@ -531,32 +522,32 @@ func (r *CollectorReconciler) getTenantsReferencingCollectorButNotSelected(ctx c
531522

532523
}
533524

534-
return tenantsToDisown, nil
525+
return tenantsToDisown
535526

536527
}
537528

538-
func (r *CollectorReconciler) disownTenants(ctx context.Context, tenantsToDisown []v1alpha1.Tenant) error {
529+
func (r *CollectorReconciler) disownTenants(ctx context.Context, tenantsToDisown []v1alpha1.Tenant) {
539530
logger := log.FromContext(ctx)
540531
for _, tenant := range tenantsToDisown {
541532
tenant.Status.Collector = ""
542533
err := r.Client.Status().Update(ctx, &tenant)
543534
if err != nil {
544-
return err
535+
logger.Error(err, fmt.Sprintf("failed to detach tenant %s from collector", tenant.Name))
545536
}
546537
logger.Info("Disowning tenant", "tenant", tenant.Name)
547538
}
548-
549-
return nil
550539
}
551540

552-
func (r *CollectorReconciler) getSubscriptionsReferencingTenantButNotSelected(ctx context.Context, tenant *v1alpha1.Tenant, selectedSubscriptions []v1alpha1.Subscription) ([]v1alpha1.Subscription, error) {
541+
func (r *CollectorReconciler) getSubscriptionsReferencingTenantButNotSelected(ctx context.Context, tenant *v1alpha1.Tenant, selectedSubscriptions []v1alpha1.Subscription) []v1alpha1.Subscription {
542+
logger := log.FromContext(ctx)
553543
var subscriptionsReferencing v1alpha1.SubscriptionList
554544
listOpts := &client.ListOptions{
555545
FieldSelector: fields.OneTermEqualSelector(tenantReferenceField, tenant.Name),
556546
}
557547

558548
if err := r.Client.List(ctx, &subscriptionsReferencing, listOpts); client.IgnoreNotFound(err) != nil {
559-
return nil, err
549+
logger.Error(err, "failed to list subscriptions that need to be detached from tenant")
550+
return nil
560551
}
561552

562553
var subscriptionsToDisown []v1alpha1.Subscription
@@ -577,22 +568,21 @@ func (r *CollectorReconciler) getSubscriptionsReferencingTenantButNotSelected(ct
577568

578569
}
579570

580-
return subscriptionsToDisown, nil
571+
return subscriptionsToDisown
581572

582573
}
583574

584-
func (r *CollectorReconciler) disownSubscriptions(ctx context.Context, subscriptionsToDisown []v1alpha1.Subscription) error {
575+
func (r *CollectorReconciler) disownSubscriptions(ctx context.Context, subscriptionsToDisown []v1alpha1.Subscription) {
585576
logger := log.FromContext(ctx)
586577
for _, subscription := range subscriptionsToDisown {
587578
subscription.Status.Tenant = ""
588579
err := r.Client.Status().Update(ctx, &subscription)
589580
if err != nil {
590-
return err
581+
logger.Error(err, fmt.Sprintf("failed to detach subscription %s/%s from collector", subscription.Namespace, subscription.Name))
591582
}
592583
logger.Info("Disowning subscription", "subscription", fmt.Sprintf("%s/%s", subscription.Namespace, subscription.Name))
593584
}
594585

595-
return nil
596586
}
597587

598588
func (r *CollectorReconciler) getAllOutputs(ctx context.Context) ([]v1alpha1.OtelOutput, error) {
@@ -632,7 +622,7 @@ func (r *CollectorReconciler) getSubscriptionsForTenant(ctx context.Context, ten
632622

633623
}
634624

635-
var validSubscriptions []v1alpha1.Subscription
625+
validSubscriptions := []v1alpha1.Subscription{}
636626

637627
for _, subscription := range selectedSubscriptions {
638628
if subscription.Status.Tenant != "" && subscription.Status.Tenant != tenant.Name {
@@ -644,8 +634,11 @@ func (r *CollectorReconciler) getSubscriptionsForTenant(ctx context.Context, ten
644634
subscription.Status.Tenant = tenant.Name
645635
validSubscriptions = append(validSubscriptions, subscription)
646636

647-
r.Status().Update(ctx, &subscription)
648637
logger.Info("Setting subscription status")
638+
err := r.Status().Update(ctx, &subscription)
639+
if err != nil {
640+
logger.Error(err, fmt.Sprintf("failed to set subscription (%s/%s) -> tenant (%s) reference", subscription.Namespace, subscription.Name, tenant.Name))
641+
}
649642

650643
}
651644

0 commit comments

Comments
 (0)