Skip to content

Commit 26ec927

Browse files
csatib02csatib02
committed
refactor: handle owned resources
Signed-off-by: Bence Csati <[email protected]>
1 parent a4930a0 commit 26ec927

File tree

1 file changed

+98
-66
lines changed

1 file changed

+98
-66
lines changed

‎controllers/telemetry/route_controller.go‎

Lines changed: 98 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ import (
3939
"github.com/kube-logging/telemetry-controller/pkg/sdk/utils"
4040
)
4141

42+
// tenantReconcileStep represents a step in the reconciliation process for a Tenant resource.
43+
// This solution is sufficient for the current use case, where we have a few steps to execute.
44+
type tenantReconcileStep struct {
45+
name string
46+
fn func() error
47+
}
48+
4249
// RouteReconciler is responsible for reconciling Tenant resources
4350
// It also watches for changes to Subscriptions, Outputs, and Namespaces
4451
// to trigger the appropriate reconciliation logic when related resources change.
@@ -69,28 +76,36 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
6976
originalTenantStatus := tenant.Status
7077
baseManager.Info(fmt.Sprintf("reconciling tenant: %q", tenant.Name))
7178

72-
if err := handleOwnedResources(ctx, baseManager.GetTenantResourceManager(), tenant); err != nil {
73-
tenant.Status.State = state.StateFailed
74-
baseManager.Error(errors.WithStack(err), "failed to handle resources owned by tenant", "tenant", tenant.Name)
75-
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
76-
baseManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
77-
return ctrl.Result{}, errors.Append(err, updateErr)
78-
}
79+
steps := []tenantReconcileStep{
80+
{
81+
name: "handle log source namespaces",
82+
fn: func() error {
83+
return handleLogSourceNamespaces(ctx, baseManager.GetTenantResourceManager(), tenant)
84+
},
85+
},
86+
{
87+
name: "handle owned resources",
88+
fn: func() error {
89+
return handleOwnedResources(ctx, baseManager.GetTenantResourceManager(), tenant)
90+
},
91+
},
92+
{
93+
name: "handle bridge resources",
94+
fn: func() error {
95+
return handleBridgeResources(ctx, baseManager.GetBridgeManager(), tenant)
96+
},
97+
},
7998
}
80-
81-
if err := handleBridgeResources(ctx, baseManager.GetBridgeManager(), tenant); err != nil {
82-
tenant.Status.State = state.StateFailed
83-
baseManager.Error(errors.WithStack(err), "failed to handle bridge resources", "tenant", tenant.Name)
84-
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
85-
baseManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
86-
return ctrl.Result{}, errors.Append(err, updateErr)
99+
for _, step := range steps {
100+
if err := step.fn(); err != nil {
101+
return r.handleReconcileError(ctx, baseManager, tenant, step.name, err)
87102
}
88103
}
89104

90105
tenant.Status.State = state.StateReady
91106
if !reflect.DeepEqual(originalTenantStatus, tenant.Status) {
92107
baseManager.Info("tenant status changed")
93-
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
108+
if updateErr := r.Status().Update(ctx, tenant); updateErr != nil {
94109
baseManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
95110
return ctrl.Result{}, updateErr
96111
}
@@ -202,81 +217,95 @@ func (r *RouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
202217
return builder.Complete(r)
203218
}
204219

205-
func (r *RouteReconciler) updateStatus(ctx context.Context, obj client.Object) error {
206-
return r.Status().Update(ctx, obj)
220+
// handleReconcileError handles errors that occur during reconciliation steps
221+
func (r *RouteReconciler) handleReconcileError(ctx context.Context, baseManager manager.BaseManager, tenant *v1alpha1.Tenant, stepName string, err error) (ctrl.Result, error) {
222+
wrappedErr := errors.Wrapf(err, "failed to %s for tenant %s", stepName, tenant.Name)
223+
224+
tenant.Status.Problems = append(tenant.Status.Problems, wrappedErr.Error())
225+
tenant.Status.ProblemsCount = len(tenant.Status.Problems)
226+
tenant.Status.State = state.StateFailed
227+
228+
baseManager.Error(errors.WithStack(err), fmt.Sprintf("failed to %s", stepName), "tenant", tenant.Name)
229+
if updateErr := r.Status().Update(ctx, tenant); updateErr != nil {
230+
baseManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
231+
return ctrl.Result{}, errors.Append(err, updateErr)
232+
}
233+
234+
return ctrl.Result{}, wrappedErr
207235
}
208236

209-
func handleOwnedResources(ctx context.Context, tenantResManager *manager.TenantResourceManager, tenant *v1alpha1.Tenant) error {
237+
func handleLogSourceNamespaces(ctx context.Context, tenantResManager *manager.TenantResourceManager, tenant *v1alpha1.Tenant) error {
210238
logsourceNamespacesForTenant, err := tenantResManager.GetLogsourceNamespaceNamesForTenant(ctx, tenant)
211239
if err != nil {
212-
tenant.Status.State = state.StateFailed
213240
tenantResManager.Error(errors.WithStack(err), "failed to get logsource namespaces for tenant", "tenant", tenant.Name)
214-
if updateErr := tenantResManager.Status().Update(ctx, tenant); updateErr != nil {
215-
tenantResManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
216-
return errors.Append(err, updateErr)
217-
}
218-
219-
return err
241+
return fmt.Errorf("failed to get logsource namespaces for tenant %s: %w", tenant.Name, err)
220242
}
221243
slices.Sort(logsourceNamespacesForTenant)
222244
tenant.Status.LogSourceNamespaces = logsourceNamespacesForTenant
223245

224-
subscriptionsForTenant, subscriptionUpdateList, err := tenantResManager.GetResourceOwnedByTenant(ctx, &v1alpha1.Subscription{}, tenant)
225-
if err != nil {
226-
tenantResManager.Error(errors.WithStack(err), "failed to get subscriptions for tenant", "tenant", tenant.Name)
227-
228-
tenant.Status.State = state.StateFailed
229-
if updateErr := tenantResManager.Status().Update(ctx, tenant); updateErr != nil {
230-
tenantResManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
231-
return errors.Append(err, updateErr)
232-
}
246+
return nil
247+
}
233248

234-
return err
249+
func handleOwnedResources(ctx context.Context, tenantResManager *manager.TenantResourceManager, tenant *v1alpha1.Tenant) error {
250+
tenantOwnedResources := []model.ResourceOwnedByTenant{
251+
&v1alpha1.Subscription{},
252+
&v1alpha1.Output{},
235253
}
254+
for _, resource := range tenantOwnedResources {
255+
resourcesForTenant, resourceUpdateList, err := tenantResManager.GetResourceOwnedByTenant(ctx, resource, tenant)
256+
if err != nil {
257+
tenantResManager.Error(errors.WithStack(err), fmt.Sprintf("failed to get %T for tenant", resource), "tenant", tenant.Name)
258+
if updateErr := tenantResManager.Status().Update(ctx, tenant); updateErr != nil {
259+
tenantResManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
260+
return errors.Append(err, updateErr)
261+
}
262+
return err
263+
}
236264

237-
// add all newly updated subscriptions here
238-
subscriptionsForTenant = append(subscriptionsForTenant, tenantResManager.UpdateResourcesForTenant(ctx, tenant.Name, subscriptionUpdateList)...)
239-
subscriptionsToDisown, err := tenantResManager.GetResourcesReferencingTenantButNotSelected(ctx, tenant, &v1alpha1.Subscription{}, subscriptionsForTenant)
240-
if err != nil {
241-
tenantResManager.Error(errors.WithStack(err), "failed to get subscriptions to disown", "tenant", tenant.Name)
242-
}
243-
tenantResManager.DisownResources(ctx, subscriptionsToDisown)
265+
// Add all newly updated resources here
266+
resourcesForTenant = append(resourcesForTenant, tenantResManager.UpdateResourcesForTenant(ctx, tenant.Name, resourceUpdateList)...)
244267

245-
subscriptionNames := manager.GetResourceNamesFromResource(subscriptionsForTenant)
246-
components.SortNamespacedNames(subscriptionNames)
247-
tenant.Status.Subscriptions = subscriptionNames
268+
resourcesToDisown, err := tenantResManager.GetResourcesReferencingTenantButNotSelected(ctx, tenant, &v1alpha1.Subscription{}, resourcesForTenant)
269+
if err != nil {
270+
tenantResManager.Error(errors.WithStack(err), fmt.Sprintf("failed to get %T to disown", resource), "tenant", tenant.Name)
271+
}
272+
tenantResManager.DisownResources(ctx, resourcesToDisown)
248273

249-
// Check outputs for tenant
250-
outputsForTenant, outputUpdateList, err := tenantResManager.GetResourceOwnedByTenant(ctx, &v1alpha1.Output{}, tenant)
251-
if err != nil {
252-
tenantResManager.Error(errors.WithStack(err), "failed to get outputs for tenant", "tenant", tenant.Name)
274+
if _, ok := resource.(*v1alpha1.Subscription); ok {
275+
subscriptionNames := manager.GetResourceNamesFromResource(resourcesForTenant)
276+
components.SortNamespacedNames(subscriptionNames)
277+
tenant.Status.Subscriptions = subscriptionNames
253278

254-
tenant.Status.State = state.StateFailed
255-
if updateErr := tenantResManager.Status().Update(ctx, tenant); updateErr != nil {
256-
tenantResManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
257-
return errors.Append(err, updateErr)
279+
if err := validateSubscriptionOutputs(ctx, tenantResManager, tenant, resourcesForTenant); err != nil {
280+
return err
281+
}
258282
}
259-
260-
return err
261283
}
262284

263-
// add all newly updated outputs here
264-
outputsForTenant = append(outputsForTenant, tenantResManager.UpdateResourcesForTenant(ctx, tenant.Name, outputUpdateList)...)
265-
outputsToDisown, err := tenantResManager.GetResourcesReferencingTenantButNotSelected(ctx, tenant, &v1alpha1.Output{}, outputsForTenant)
266-
if err != nil {
267-
tenantResManager.Error(errors.WithStack(err), "failed to get outputs to disown", "tenant", tenant.Name)
268-
}
269-
tenantResManager.DisownResources(ctx, outputsToDisown)
285+
return nil
286+
}
270287

271-
// Check outputs for subscriptions
288+
func validateSubscriptionOutputs(ctx context.Context, tenantResManager *manager.TenantResourceManager, tenant *v1alpha1.Tenant, subscriptionsForTenant []model.ResourceOwnedByTenant) error {
272289
realSubscriptionsForTenant, err := utils.GetConcreteTypeFromList[*v1alpha1.Subscription](utils.ToObject(subscriptionsForTenant))
273290
if err != nil {
274291
tenantResManager.Error(errors.WithStack(err), "failed to get concrete type from list", "tenant", tenant.Name)
292+
return err
275293
}
276294

277295
for _, subscription := range realSubscriptionsForTenant {
278296
originalSubscriptionStatus := subscription.Status.DeepCopy()
279-
subscription.Status.Outputs = tenantResManager.ValidateSubscriptionOutputs(ctx, subscription)
297+
validOutputs, invalidOutputs := tenantResManager.ValidateSubscriptionOutputs(ctx, subscription)
298+
299+
if len(invalidOutputs) > 0 {
300+
subscription.Status.Problems = append(subscription.Status.Problems, fmt.Sprintf("invalid outputs for subscription %s: %v", subscription.Name, invalidOutputs))
301+
subscription.Status.ProblemsCount = len(subscription.Status.Problems)
302+
subscription.Status.State = state.StateFailed
303+
tenantResManager.UpdateOutputs(ctx, tenant, invalidOutputs)
304+
}
305+
306+
components.SortNamespacedNames(validOutputs)
307+
subscription.Status.Outputs = validOutputs
308+
280309
if !reflect.DeepEqual(originalSubscriptionStatus, subscription.Status) {
281310
if updateErr := tenantResManager.Status().Update(ctx, subscription); updateErr != nil {
282311
tenantResManager.Error(errors.WithStack(updateErr), "failed updating subscription status", "subscription", subscription.NamespacedName().String())
@@ -306,8 +335,11 @@ func handleBridgeResources(ctx context.Context, bridgeManager *manager.BridgeMan
306335
tenant.Status.ConnectedBridges = bridgesForTenantNames
307336

308337
for _, bridge := range bridgesForTenant {
309-
if err := bridgeManager.CheckBridgeConnection(ctx, tenant.Name, &bridge); err != nil {
310-
tenant.Status.State = state.StateFailed
338+
if err := bridgeManager.ValidateBridgeConnection(ctx, tenant.Name, &bridge); err != nil {
339+
bridge.Status.Problems = append(bridge.Status.Problems, errors.Wrapf(err, "bridge %s validation failed", bridge.Name).Error())
340+
bridge.Status.ProblemsCount = len(bridge.Status.Problems)
341+
bridge.Status.State = state.StateFailed
342+
311343
bridgeManager.Error(errors.WithStack(err), "failed to check bridge connection", "bridge", bridge.Name)
312344
if updateErr := bridgeManager.Status().Update(ctx, tenant); updateErr != nil {
313345
bridgeManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)

0 commit comments

Comments
 (0)