diff --git a/internal/controllers/reconciliation/integration_test.go b/internal/controllers/reconciliation/integration_test.go index f284db41..3ad14bec 100644 --- a/internal/controllers/reconciliation/integration_test.go +++ b/internal/controllers/reconciliation/integration_test.go @@ -22,6 +22,8 @@ import ( "github.com/Azure/eno/internal/testutil" ) +// TODO: Sometimes the resource is patched to the create phase after the external update but before the update + type crudTestCase struct { Name string Empty, Initial, Updated client.Object @@ -245,18 +247,6 @@ func setImage(t *testing.T, upstream client.Client, syn *apiv1.Synthesizer, comp return upstream.Update(context.Background(), syn) }) require.NoError(t, err) - - // Also pin the composition to >= this synthesizer version. - // This is necessary to avoid deadlock in cases where incoherent cache causes the composition not to be updated on this tick of the rollout loop. - // It isn't a problem in production because we don't expect serialized behavior from the rollout controller. - err = retry.RetryOnConflict(retry.DefaultBackoff, func() error { - if err := upstream.Get(context.Background(), client.ObjectKeyFromObject(comp), comp); err != nil { - return err - } - comp.Spec.Synthesizer.MinGeneration = syn.Generation - return upstream.Update(context.Background(), comp) - }) - require.NoError(t, err) } func newSliceBuilder(t *testing.T, scheme *runtime.Scheme, test *crudTestCase) func(c *apiv1.Composition, s *apiv1.Synthesizer) []*apiv1.ResourceSlice { diff --git a/internal/controllers/synthesis/integration_test.go b/internal/controllers/synthesis/integration_test.go index a27087ca..d131fc30 100644 --- a/internal/controllers/synthesis/integration_test.go +++ b/internal/controllers/synthesis/integration_test.go @@ -203,10 +203,10 @@ func TestControllerSynthesizerRollout(t *testing.T) { require.NoError(t, err) // The first synthesizer update should be applied to the composition - // TODO: Flake + // TODO: Flake because sometimes the pod updates the slice refs but the synth generation somehow still doesn't match this one testutil.Eventually(t, func() bool { require.NoError(t, client.IgnoreNotFound(cli.Get(ctx, client.ObjectKeyFromObject(comp), comp))) - return comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedSynthesizerGeneration == syn.Generation + return comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedSynthesizerGeneration >= syn.Generation }) // Wait for the informer cache to know about the last update diff --git a/internal/controllers/synthesis/rollout.go b/internal/controllers/synthesis/rollout.go index 1df22d47..6f89ecfc 100644 --- a/internal/controllers/synthesis/rollout.go +++ b/internal/controllers/synthesis/rollout.go @@ -31,6 +31,7 @@ func NewRolloutController(mgr ctrl.Manager, cooldownPeriod time.Duration) error } return ctrl.NewControllerManagedBy(mgr). For(&apiv1.Synthesizer{}). + Watches(&apiv1.Composition{}, manager.NewCompositionToSynthesizerHandler(c.client)). WithLogConstructor(manager.NewLogConstructor(mgr, "rolloutController")). Complete(c) } @@ -48,7 +49,7 @@ func (c *rolloutController) Reconcile(ctx context.Context, req ctrl.Request) (ct if syn.Status.LastRolloutTime != nil { remainingCooldown := c.cooldown - time.Since(syn.Status.LastRolloutTime.Time) if remainingCooldown > 0 { - return ctrl.Result{RequeueAfter: remainingCooldown}, nil + return ctrl.Result{RequeueAfter: remainingCooldown}, nil // not ready to continue rollout yet } } @@ -61,6 +62,7 @@ func (c *rolloutController) Reconcile(ctx context.Context, req ctrl.Request) (ct } // randomize list to avoid always rolling out changes in the same order + // TODO: Consider a more efficient approach here rand.Shuffle(len(compList.Items), func(i, j int) { compList.Items[i] = compList.Items[j] }) for _, comp := range compList.Items { @@ -107,7 +109,7 @@ func (c *rolloutController) Reconcile(ctx context.Context, req ctrl.Request) (ct if len(compList.Items) > 0 { // log doesn't make sense if the synthesizer wasn't actually rolled out logger.Info("finished rolling out latest synthesizer version") } - return ctrl.Result{}, nil // TODO: Consider leaving this loop open in case new compositions fell through the cracks earlier + return ctrl.Result{}, nil } return ctrl.Result{}, nil diff --git a/internal/manager/manager.go b/internal/manager/manager.go index d96c7d6d..b58ff953 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -6,9 +6,13 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" + "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -76,3 +80,32 @@ func NewLogConstructor(mgr ctrl.Manager, controllerName string) func(*reconcile. return l } } + +func NewCompositionToSynthesizerHandler(cli client.Client) handler.EventHandler { + return &handler.Funcs{ + CreateFunc: func(ctx context.Context, ce event.CreateEvent, rli workqueue.RateLimitingInterface) { + comp, ok := ce.Object.(*apiv1.Composition) + if !ok { + logr.FromContextOrDiscard(ctx).V(0).Info("unexpected type given to NewCompositionToSynthesizerHandler") + return + } + rli.Add(reconcile.Request{NamespacedName: types.NamespacedName{Name: comp.Spec.Synthesizer.Name}}) + }, + UpdateFunc: func(ctx context.Context, ue event.UpdateEvent, rli workqueue.RateLimitingInterface) { + comp, ok := ue.ObjectNew.(*apiv1.Composition) + if !ok { + logr.FromContextOrDiscard(ctx).V(0).Info("unexpected type given to NewCompositionToSynthesizerHandler") + return + } + rli.Add(reconcile.Request{NamespacedName: types.NamespacedName{Name: comp.Spec.Synthesizer.Name}}) + }, + DeleteFunc: func(ctx context.Context, de event.DeleteEvent, rli workqueue.RateLimitingInterface) { + comp, ok := de.Object.(*apiv1.Composition) + if !ok { + logr.FromContextOrDiscard(ctx).V(0).Info("unexpected type given to NewCompositionToSynthesizerHandler") + return + } + rli.Add(reconcile.Request{NamespacedName: types.NamespacedName{Name: comp.Spec.Synthesizer.Name}}) + }, + } +}