From 5399e1ece4797645c165fb6ffca5a6797270e903 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Mon, 9 Sep 2024 13:59:06 +0100 Subject: [PATCH] Lift the DAG error into the reconcile function. Signed-off-by: Jim Fitzpatrick --- controller/controller.go | 15 ++++----------- controller/controller_test.go | 2 +- controller/subscriber.go | 4 ++-- controller/test_helper.go | 2 +- controller/workflow.go | 8 ++++---- examples/kuadrant/main.go | 2 +- .../reconcilers/effective_policies_reconciler.go | 4 ++-- examples/kuadrant/reconcilers/envoy_gateway.go | 4 ++-- examples/kuadrant/reconcilers/istio.go | 4 ++-- .../reconcilers/topology_file_reconciler.go | 2 +- 10 files changed, 20 insertions(+), 27 deletions(-) diff --git a/controller/controller.go b/controller/controller.go index 89b4a69..2aa6d10 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -61,7 +61,7 @@ func WithRunnable(name string, builder RunnableBuilder) ControllerOption { } } -type ReconcileFunc func(context.Context, []ResourceEvent, *machinery.Topology) +type ReconcileFunc func(context.Context, []ResourceEvent, *machinery.Topology, error) func WithReconcile(reconcile ReconcileFunc) ControllerOption { return func(o *ControllerOptions) { @@ -106,24 +106,20 @@ func NewController(f ...ControllerOption) *Controller { name: "controller", logger: logr.Discard(), runnables: map[string]RunnableBuilder{}, - reconcile: func(context.Context, []ResourceEvent, *machinery.Topology) { + reconcile: func(context.Context, []ResourceEvent, *machinery.Topology, error) { }, } for _, fn := range f { fn(opts) } - allowTopologyLoops := false - if opts.allowTopologyLoops { - allowTopologyLoops = true - } controller := &Controller{ name: opts.name, logger: opts.logger, client: opts.client, manager: opts.manager, cache: &watchableCacheStore{}, - topology: newGatewayAPITopologyBuilder(opts.policyKinds, opts.objectKinds, opts.objectLinks, allowTopologyLoops), + topology: newGatewayAPITopologyBuilder(opts.policyKinds, opts.objectKinds, opts.objectLinks, opts.allowTopologyLoops), runnables: map[string]Runnable{}, reconcile: opts.reconcile, } @@ -251,10 +247,7 @@ func (c *Controller) delete(obj Object) { func (c *Controller) propagate(resourceEvents []ResourceEvent) { topology, err := c.topology.Build(c.cache.List()) - if !c.topology.allowTopologyLoops && err != nil { - panic(err) - } - c.reconcile(LoggerIntoContext(context.TODO(), c.logger), resourceEvents, topology) + c.reconcile(LoggerIntoContext(context.TODO(), c.logger), resourceEvents, topology, err) } func (c *Controller) subscribe() { diff --git a/controller/controller_test.go b/controller/controller_test.go index 6d3f402..1fffbc6 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -25,7 +25,7 @@ func TestControllerOptions(t *testing.T) { name: "controller", logger: logr.Discard(), runnables: map[string]RunnableBuilder{}, - reconcile: func(context.Context, []ResourceEvent, *machinery.Topology) { + reconcile: func(context.Context, []ResourceEvent, *machinery.Topology, error) { }, } diff --git a/controller/subscriber.go b/controller/subscriber.go index ac2cf3a..2afee9c 100644 --- a/controller/subscriber.go +++ b/controller/subscriber.go @@ -16,7 +16,7 @@ type Subscription struct { Events []ResourceEventMatcher } -func (s Subscription) Reconcile(ctx context.Context, resourceEvents []ResourceEvent, topology *machinery.Topology) { +func (s Subscription) Reconcile(ctx context.Context, resourceEvents []ResourceEvent, topology *machinery.Topology, err error) { matchingEvents := lo.Filter(resourceEvents, func(resourceEvent ResourceEvent, _ int) bool { return lo.ContainsBy(s.Events, func(m ResourceEventMatcher) bool { obj := resourceEvent.OldObject @@ -30,6 +30,6 @@ func (s Subscription) Reconcile(ctx context.Context, resourceEvents []ResourceEv }) }) if len(matchingEvents) > 0 && s.ReconcileFunc != nil { - s.ReconcileFunc(ctx, matchingEvents, topology) + s.ReconcileFunc(ctx, matchingEvents, topology, nil) } } diff --git a/controller/test_helper.go b/controller/test_helper.go index 43dc3fb..d40a49b 100644 --- a/controller/test_helper.go +++ b/controller/test_helper.go @@ -51,7 +51,7 @@ func init() { Func: func(_ machinery.Object) []machinery.Object { return []machinery.Object{&RuntimeObject{myObjects[0]}} }, } } - testReconcileFunc = func(_ context.Context, events []ResourceEvent, topology *machinery.Topology) { + testReconcileFunc = func(_ context.Context, events []ResourceEvent, topology *machinery.Topology, err error) { for _, event := range events { testLogger.Info("reconcile", "kind", event.Kind, diff --git a/controller/workflow.go b/controller/workflow.go index 9b7c283..1a6d467 100644 --- a/controller/workflow.go +++ b/controller/workflow.go @@ -15,10 +15,10 @@ type Workflow struct { Postcondition ReconcileFunc } -func (d *Workflow) Run(ctx context.Context, resourceEvents []ResourceEvent, topology *machinery.Topology) { +func (d *Workflow) Run(ctx context.Context, resourceEvents []ResourceEvent, topology *machinery.Topology, err error) { // run precondition reconcile function if d.Precondition != nil { - d.Precondition(ctx, resourceEvents, topology) + d.Precondition(ctx, resourceEvents, topology, err) } // dispatch the event to concurrent tasks @@ -28,13 +28,13 @@ func (d *Workflow) Run(ctx context.Context, resourceEvents []ResourceEvent, topo for _, f := range funcs { go func() { defer waitGroup.Done() - f(ctx, resourceEvents, topology) + f(ctx, resourceEvents, topology, err) }() } waitGroup.Wait() // run precondition reconcile function if d.Postcondition != nil { - d.Postcondition(ctx, resourceEvents, topology) + d.Postcondition(ctx, resourceEvents, topology, err) } } diff --git a/examples/kuadrant/main.go b/examples/kuadrant/main.go index 5aa7afb..2103a20 100644 --- a/examples/kuadrant/main.go +++ b/examples/kuadrant/main.go @@ -241,7 +241,7 @@ func buildReconciler(gatewayProviders []string, client *dynamic.DynamicClient) c } reconciler := &controller.Workflow{ - Precondition: func(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology) { + Precondition: func(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology, err error) { logger := controller.LoggerFromContext(ctx).WithName("event logger") for _, event := range resourceEvents { // log the event diff --git a/examples/kuadrant/reconcilers/effective_policies_reconciler.go b/examples/kuadrant/reconcilers/effective_policies_reconciler.go index 322f7b4..e22fab7 100644 --- a/examples/kuadrant/reconcilers/effective_policies_reconciler.go +++ b/examples/kuadrant/reconcilers/effective_policies_reconciler.go @@ -28,7 +28,7 @@ type EffectivePoliciesReconciler struct { ReconcileFuncs []controller.ReconcileFunc } -func (r *EffectivePoliciesReconciler) Reconcile(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology) { +func (r *EffectivePoliciesReconciler) Reconcile(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology, err error) { targetables := topology.Targetables() // reconcile policies @@ -84,7 +84,7 @@ func (r *EffectivePoliciesReconciler) Reconcile(ctx context.Context, resourceEve for _, f := range funcs { go func() { defer waitGroup.Done() - f(ctx, resourceEvents, topology) + f(ctx, resourceEvents, topology, err) }() } } diff --git a/examples/kuadrant/reconcilers/envoy_gateway.go b/examples/kuadrant/reconcilers/envoy_gateway.go index 477e989..ef14174 100644 --- a/examples/kuadrant/reconcilers/envoy_gateway.go +++ b/examples/kuadrant/reconcilers/envoy_gateway.go @@ -28,7 +28,7 @@ type EnvoyGatewayProvider struct { Client *dynamic.DynamicClient } -func (p *EnvoyGatewayProvider) ReconcileSecurityPolicies(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology) { +func (p *EnvoyGatewayProvider) ReconcileSecurityPolicies(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, err error) { logger := controller.LoggerFromContext(ctx).WithName("envoy gateway").WithName("securitypolicy") ctx = controller.LoggerIntoContext(ctx, logger) @@ -57,7 +57,7 @@ func (p *EnvoyGatewayProvider) ReconcileSecurityPolicies(ctx context.Context, _ } } -func (p *EnvoyGatewayProvider) DeleteSecurityPolicy(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology) { +func (p *EnvoyGatewayProvider) DeleteSecurityPolicy(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology, err error) { for _, resourceEvent := range resourceEvents { gateway := resourceEvent.OldObject p.deleteSecurityPolicy(ctx, topology, gateway.GetNamespace(), gateway.GetName(), nil) diff --git a/examples/kuadrant/reconcilers/istio.go b/examples/kuadrant/reconcilers/istio.go index d670c2b..b54d16f 100644 --- a/examples/kuadrant/reconcilers/istio.go +++ b/examples/kuadrant/reconcilers/istio.go @@ -31,7 +31,7 @@ type IstioGatewayProvider struct { Client *dynamic.DynamicClient } -func (p *IstioGatewayProvider) ReconcileAuthorizationPolicies(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology) { +func (p *IstioGatewayProvider) ReconcileAuthorizationPolicies(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, err error) { logger := controller.LoggerFromContext(ctx).WithName("istio").WithName("authorizationpolicy") ctx = controller.LoggerIntoContext(ctx, logger) @@ -60,7 +60,7 @@ func (p *IstioGatewayProvider) ReconcileAuthorizationPolicies(ctx context.Contex } } -func (p *IstioGatewayProvider) DeleteAuthorizationPolicy(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology) { +func (p *IstioGatewayProvider) DeleteAuthorizationPolicy(ctx context.Context, resourceEvents []controller.ResourceEvent, topology *machinery.Topology, err error) { for _, resourceEvent := range resourceEvents { gateway := resourceEvent.OldObject p.deleteAuthorizationPolicy(ctx, topology, gateway.GetNamespace(), gateway.GetName(), nil) diff --git a/examples/kuadrant/reconcilers/topology_file_reconciler.go b/examples/kuadrant/reconcilers/topology_file_reconciler.go index c1183e4..9ac44fc 100644 --- a/examples/kuadrant/reconcilers/topology_file_reconciler.go +++ b/examples/kuadrant/reconcilers/topology_file_reconciler.go @@ -12,7 +12,7 @@ const topologyFile = "topology.dot" type TopologyFileReconciler struct{} -func (r *TopologyFileReconciler) Reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology) { +func (r *TopologyFileReconciler) Reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, err error) { logger := controller.LoggerFromContext(ctx).WithName("topology file") file, err := os.Create(topologyFile)