Skip to content

Commit

Permalink
fix: report correct state update
Browse files Browse the repository at this point in the history 
Signed-off-by: Bence Csati <bence.csati@axoflow.com>
  • Loading branch information
@csatib02
csatib02 committed Nov 12, 2024
1 parent cfbd86b commit 5867f09
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 37 deletions.
35 changes: 28 additions & 7 deletions internal/controller/telemetry/collector_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,27 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
logger.Info(err.Error())
return ctrl.Result{}, nil
}
logger.Error(errors.WithStack(err), "invalid otel config input")

collector.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "invalid otel config input")
if updateErr := r.updateStatus(ctx, collector); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating collector status")
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, err
}

otelConfig := otelConfigInput.AssembleConfig(ctx)
if err := validator.ValidateAssembledConfig(otelConfig); err != nil {
collector.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "invalid otel config")

collector.Status.State = v1alpha1.StateFailed
if updateErr := r.updateStatus(ctx, collector); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating collector status")
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, err
}

Expand Down Expand Up @@ -280,8 +291,14 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
resourceReconciler := reconciler.NewReconcilerWith(r.Client, reconciler.WithLog(logger))
_, err = resourceReconciler.ReconcileResource(&otelCollector, reconciler.StatePresent)
if err != nil {
logger.Error(errors.WithStack(err), "failed reconciling collector")

collector.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "failed reconciling otel collector")
if updateErr := r.updateStatus(ctx, collector); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating collector status")
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, err
}

Expand All @@ -294,10 +311,10 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
collector.Status.State = v1alpha1.StateReady
if !reflect.DeepEqual(originalCollectorStatus, collector.Status) {
logger.Info("collector status changed")
if err = r.Client.Status().Update(ctx, collector); err != nil {
collector.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "failed updating collector status")
return ctrl.Result{}, err

if updateErr := r.updateStatus(ctx, collector); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating collector status")
return ctrl.Result{}, errors.Append(err, updateErr)
}
}

Expand Down Expand Up @@ -535,6 +552,10 @@ func (r *CollectorReconciler) getTenantsMatchingSelectors(ctx context.Context, l
return tenantsForSelector.Items, nil
}

func (r *CollectorReconciler) updateStatus(ctx context.Context, obj client.Object) error {
return r.Status().Update(ctx, obj)
}

func normalizeStringSlice(inputList []string) []string {
allKeys := make(map[string]bool)
uniqueList := []string{}
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/telemetry/otel_conf_gen/otel_conf_gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
"golang.org/x/exp/maps"
)

var ErrNoResources = errors.New("there are no resources deployed that the controller can use")
var ErrNoResources = errors.New("there are no resources deployed that the collector(s) can use")

type OtelColConfigInput struct {
// These must only include resources that are selected by the collector, tenant labelselectors, and listed outputs in the subscriptions
Expand Down
67 changes: 38 additions & 29 deletions internal/controller/telemetry/route_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,14 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl

subscriptionsForTenant, updateList, err := r.getSubscriptionsForTenant(ctx, tenant)
if err != nil {
tenant.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "failed to get subscriptions for tenant", "tenant", tenant.Name)
if updateErr := r.Status().Update(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed update tenant status", "tenant", tenant.Name)
return ctrl.Result{}, err

tenant.Status.State = v1alpha1.StateFailed
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, err
}

Expand Down Expand Up @@ -116,10 +118,9 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
subscription.Status.Outputs = validOutputs

if !reflect.DeepEqual(originalSubscriptionStatus, subscription.Status) {
if updateErr := r.Status().Update(ctx, &subscription); updateErr != nil {
subscription.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(updateErr), "failed update subscription status", "subscription", subscription.NamespacedName().String())
return ctrl.Result{}, err
if updateErr := r.updateStatus(ctx, &subscription); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating subscription status", "subscription", subscription.NamespacedName().String())
return ctrl.Result{}, errors.Append(err, updateErr)
}
}
}
Expand All @@ -128,10 +129,11 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
if err != nil {
tenant.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "failed to get bridges for tenant", "tenant", tenant.Name)
if updateErr := r.Status().Update(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed update tenant status", "tenant", tenant.Name)
return ctrl.Result{}, err
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, err
}
bridgesForTenantNames := getBridgeNamesFromBridges(bridgesForTenant)
Expand All @@ -142,10 +144,11 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
if err := r.checkBridgeConnection(ctx, tenant.Name, &bridge); err != nil {
tenant.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "failed to check bridge connection", "bridge", bridge.Name)
if updateErr := r.Status().Update(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed update tenant status", "tenant", tenant.Name)
return ctrl.Result{}, err
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, err
}
}
Expand All @@ -154,10 +157,11 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
if err != nil {
tenant.Status.State = v1alpha1.StateFailed
logger.Error(errors.WithStack(err), "failed to get logsource namespaces for tenant", "tenant", tenant.Name)
if updateErr := r.Status().Update(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed update tenant status", "tenant", tenant.Name)
return ctrl.Result{}, err
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, err
}
slices.Sort(logsourceNamespacesForTenant)
Expand All @@ -166,11 +170,12 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
tenant.Status.State = v1alpha1.StateReady
if !reflect.DeepEqual(originalTenantStatus, tenant.Status) {
logger.Info("tenant status changed")
if err := r.Status().Update(ctx, tenant); err != nil {
tenant.Status.State = v1alpha1.StateFailed
logger.Error(errors.New("failed update tenant status"), "failed update tenant status", "tenant", tenant.Name)
return ctrl.Result{}, err
if updateErr := r.updateStatus(ctx, tenant); updateErr != nil {
logger.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
return ctrl.Result{}, errors.Append(err, updateErr)
}

return ctrl.Result{}, nil
}

logger.Info("tenant reconciliation complete", "tenant", tenant.Name)
Expand Down Expand Up @@ -360,9 +365,9 @@ func (r *RouteReconciler) disownSubscriptions(ctx context.Context, subscriptions

for _, subscription := range subscriptionsToDisown {
subscription.Status.Tenant = ""
if err := r.Client.Status().Update(ctx, &subscription); err != nil {
if updateErr := r.Status().Update(ctx, &subscription); updateErr != nil {
subscription.Status.State = v1alpha1.StateFailed
logger.Error(err, fmt.Sprintf("failed to detach subscription %s/%s from collector", subscription.Namespace, subscription.Name))
logger.Error(updateErr, fmt.Sprintf("failed to detach subscription %s/%s from collector", subscription.Namespace, subscription.Name))
} else {
logger.Info("disowning subscription", "subscription", fmt.Sprintf("%s/%s", subscription.Namespace, subscription.Name))
}
Expand All @@ -378,9 +383,9 @@ func (r *RouteReconciler) updateSubscriptionsForTenant(ctx context.Context, tena
subscription.Status.Tenant = tenantName
logger.Info("updating subscription status for tenant ownership")

if err := r.Status().Update(ctx, &subscription); err != nil {
if updateErr := r.Status().Update(ctx, &subscription); updateErr != nil {
subscription.Status.State = v1alpha1.StateFailed
logger.Error(err, fmt.Sprintf("failed to set subscription (%s/%s) -> tenant (%s) reference", subscription.Namespace, subscription.Name, tenantName))
logger.Error(updateErr, fmt.Sprintf("failed to set subscription (%s/%s) -> tenant (%s) reference", subscription.Namespace, subscription.Name, tenantName))
} else {
updatedSubscriptions = append(updatedSubscriptions, subscription)
}
Expand Down Expand Up @@ -474,24 +479,28 @@ func (r *RouteReconciler) getTenants(ctx context.Context, listOpts *client.ListO
}

func (r *RouteReconciler) checkBridgeConnection(ctx context.Context, tenantName string, bridge *v1alpha1.Bridge) error {
for _, tenant := range []string{bridge.Spec.SourceTenant, bridge.Spec.TargetTenant} {
if tenant != tenantName {
for _, tenantReference := range []string{bridge.Spec.SourceTenant, bridge.Spec.TargetTenant} {
if tenantReference != tenantName {
listOpts := &client.ListOptions{
FieldSelector: fields.OneTermEqualSelector(tenantNameField, tenant),
FieldSelector: fields.OneTermEqualSelector(tenantNameField, tenantReference),
}
tenant, err := r.getTenants(ctx, listOpts)
if err != nil {
return err
}
if len(tenant) == 0 {
return errors.Errorf("bridge %s has a dangling tenant reference %s", bridge.Name, tenant)
return errors.Errorf("bridge %s has a dangling tenant reference %s", bridge.Name, tenantReference)
}
}
}

return nil
}

func (r *RouteReconciler) updateStatus(ctx context.Context, obj client.Object) error {
return r.Client.Status().Update(ctx, obj)
}

func normalizeNamespaceSlice(inputList []apiv1.Namespace) []apiv1.Namespace {
allKeys := make(map[string]bool)
uniqueList := []apiv1.Namespace{}
Expand Down

0 comments on commit 5867f09

Please sign in to comment.