log non-critical errors, but don't fail

kube-logging · Jan 30, 2024 · e9d4cfe · e9d4cfe
1 parent b0731f6
commit e9d4cfe
Showing 1 changed file with 23 additions and 30 deletions.
diff --git a/internal/controller/telemetry/collector_controller.go b/internal/controller/telemetry/collector_controller.go
@@ -83,15 +83,9 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{}, err
 	}
 
-	tenantsToDisown, err := r.getTenantsReferencingCollectorButNotSelected(ctx, collector, tenants)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
+	tenantsToDisown := r.getTenantsReferencingCollectorButNotSelected(ctx, collector, tenants)
 
-	err = r.disownTenants(ctx, tenantsToDisown)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
+	r.disownTenants(ctx, tenantsToDisown)
 
 	tenantNames := []string{}
 
@@ -124,15 +118,9 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			return ctrl.Result{}, err
 		}
 
-		subscriptionsToDisown, err := r.getSubscriptionsReferencingTenantButNotSelected(ctx, &tenant, subscriptionsForTenant)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
+		subscriptionsToDisown := r.getSubscriptionsReferencingTenantButNotSelected(ctx, &tenant, subscriptionsForTenant)
 
-		err = r.disownSubscriptions(ctx, subscriptionsToDisown)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
+		r.disownSubscriptions(ctx, subscriptionsToDisown)
 
 		subscriptions = append(subscriptions, subscriptionsForTenant...)
 
@@ -172,8 +160,11 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	collector.Status.Tenants = tenantNames
 
-	r.Status().Update(ctx, collector)
 	logger.Info("Setting collector status")
+	err = r.Status().Update(ctx, collector)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
 
 	outputs, err := r.getAllOutputs(ctx)
 	if err != nil {
@@ -411,6 +402,7 @@ func (r *CollectorReconciler) reconcileServiceAccount(ctx context.Context, colle
 
 	return v1alpha1.NamespacedName{Namespace: serviceAccount.Namespace, Name: serviceAccount.Name}, nil
 }
+
 func (r *CollectorReconciler) reconcileClusterRoleBinding(ctx context.Context, collector *v1alpha1.Collector) error {
 	logger := log.FromContext(ctx)
 
@@ -502,15 +494,17 @@ func (r *CollectorReconciler) getTenantsMatchingSelectors(ctx context.Context, l
 	return tenantsForSelector.Items, nil
 }
 
-func (r *CollectorReconciler) getTenantsReferencingCollectorButNotSelected(ctx context.Context, collector *v1alpha1.Collector, selectedTenants []v1alpha1.Tenant) ([]v1alpha1.Tenant, error) {
+func (r *CollectorReconciler) getTenantsReferencingCollectorButNotSelected(ctx context.Context, collector *v1alpha1.Collector, selectedTenants []v1alpha1.Tenant) []v1alpha1.Tenant {
+	logger := log.FromContext(ctx)
 	var tenantsReferencing v1alpha1.TenantList
 
 	listOpts := &client.ListOptions{
 		FieldSelector: fields.OneTermEqualSelector(collectorReferenceField, collector.Name),
 	}
 
 	if err := r.Client.List(ctx, &tenantsReferencing, listOpts); client.IgnoreNotFound(err) != nil {
-		return nil, err
+		logger.Error(err, "failed to list tenants that need to be detached from collector")
+		return nil
 	}
 
 	tenantsToDisown := []v1alpha1.Tenant{}
@@ -531,32 +525,32 @@ func (r *CollectorReconciler) getTenantsReferencingCollectorButNotSelected(ctx c
 
 	}
 
-	return tenantsToDisown, nil
+	return tenantsToDisown
 
 }
 
-func (r *CollectorReconciler) disownTenants(ctx context.Context, tenantsToDisown []v1alpha1.Tenant) error {
+func (r *CollectorReconciler) disownTenants(ctx context.Context, tenantsToDisown []v1alpha1.Tenant) {
 	logger := log.FromContext(ctx)
 	for _, tenant := range tenantsToDisown {
 		tenant.Status.Collector = ""
 		err := r.Client.Status().Update(ctx, &tenant)
 		if err != nil {
-			return err
+			logger.Error(err, fmt.Sprintf("failed to detach tenant %s from collector", tenant.Name))
 		}
 		logger.Info("Disowning tenant", "tenant", tenant.Name)
 	}
-
-	return nil
 }
 
-func (r *CollectorReconciler) getSubscriptionsReferencingTenantButNotSelected(ctx context.Context, tenant *v1alpha1.Tenant, selectedSubscriptions []v1alpha1.Subscription) ([]v1alpha1.Subscription, error) {
+func (r *CollectorReconciler) getSubscriptionsReferencingTenantButNotSelected(ctx context.Context, tenant *v1alpha1.Tenant, selectedSubscriptions []v1alpha1.Subscription) []v1alpha1.Subscription {
+	logger := log.FromContext(ctx)
 	var subscriptionsReferencing v1alpha1.SubscriptionList
 	listOpts := &client.ListOptions{
 		FieldSelector: fields.OneTermEqualSelector(tenantReferenceField, tenant.Name),
 	}
 
 	if err := r.Client.List(ctx, &subscriptionsReferencing, listOpts); client.IgnoreNotFound(err) != nil {
-		return nil, err
+		logger.Error(err, "failed to list subscriptions that need to be detached from tenant")
+		return nil
 	}
 
 	var subscriptionsToDisown []v1alpha1.Subscription
@@ -577,22 +571,21 @@ func (r *CollectorReconciler) getSubscriptionsReferencingTenantButNotSelected(ct
 
 	}
 
-	return subscriptionsToDisown, nil
+	return subscriptionsToDisown
 
 }
 
-func (r *CollectorReconciler) disownSubscriptions(ctx context.Context, subscriptionsToDisown []v1alpha1.Subscription) error {
+func (r *CollectorReconciler) disownSubscriptions(ctx context.Context, subscriptionsToDisown []v1alpha1.Subscription) {
 	logger := log.FromContext(ctx)
 	for _, subscription := range subscriptionsToDisown {
 		subscription.Status.Tenant = ""
 		err := r.Client.Status().Update(ctx, &subscription)
 		if err != nil {
-			return err
+			logger.Error(err, fmt.Sprintf("failed to detach subscription %s/%s from collector", subscription.Namespace, subscription.Name))
 		}
 		logger.Info("Disowning subscription", "subscription", fmt.Sprintf("%s/%s", subscription.Namespace, subscription.Name))
 	}
 
-	return nil
 }
 
 func (r *CollectorReconciler) getAllOutputs(ctx context.Context) ([]v1alpha1.OtelOutput, error) {