Reduce clock usage + add time and period override in rollout controll…

…er (#50634)

api/proto/teleport/autoupdate/v1/autoupdate.proto

@@ -170,6 +170,7 @@ message AutoUpdateAgentRolloutSpec {
 }
 
 // AutoUpdateAgentRolloutStatus tracks the current agent rollout status.
+// The status is reset if any spec field changes except the mode.
 message AutoUpdateAgentRolloutStatus {
   repeated AutoUpdateAgentRolloutStatusGroup groups = 1;
   AutoUpdateAgentRolloutState state = 2;
@@ -182,6 +183,11 @@ message AutoUpdateAgentRolloutStatus {
   // before the rollout start time and the maintenance window belongs to the previous rollout.
   // When the timestamp is nil, the controller will ignore the start time and check and allow groups to activate.
   google.protobuf.Timestamp start_time = 3;
+
+  // Time override is an optional timestamp making the autoupdate_agent_rollout controller use a specific time instead
+  // of the system clock when evaluating time-based criteria. This field is used for testing and troubleshooting
+  // purposes.
+  google.protobuf.Timestamp time_override = 4;
 }
 
 // AutoUpdateAgentRolloutStatusGroup tracks the current agent rollout status of a specific group.

lib/autoupdate/rollout/controller.go

@@ -91,6 +91,7 @@ func (c *Controller) Run(ctx context.Context) error {
 	ticker := interval.New(config)
 	defer ticker.Stop()
 
+	c.log.InfoContext(ctx, "Starting autoupdate_agent_rollout controller", "period", c.period)
 	for {
 		select {
 		case <-ctx.Done():

lib/autoupdate/rollout/reconciler.go

@@ -166,6 +166,7 @@ func (r *reconciler) tryReconcile(ctx context.Context) error {
 
 	// if there are no existing rollout, we create a new one and set the status
 	if !rolloutExists {
+		r.log.DebugContext(ctx, "creating rollout")
 		rollout, err := update.NewAutoUpdateAgentRollout(newSpec)
 		rollout.Status = newStatus
 		if err != nil {
@@ -175,6 +176,7 @@ func (r *reconciler) tryReconcile(ctx context.Context) error {
 		return trace.Wrap(err, "creating rollout")
 	}
 
+	r.log.DebugContext(ctx, "updating rollout")
 	// If there was a previous rollout, we update its spec and status and do an update.
 	// We don't create a new resource to keep the metadata containing the revision ID.
 	existingRollout.Spec = newSpec
@@ -295,6 +297,16 @@ func (r *reconciler) computeStatus(
 	// compute the group state changes
 	now := r.clock.Now()
 
+	// If timeOverride is set to a non-zero value (we have two potential zeros, go time's zero and timestamppb's zero)
+	// we use this instead of the clock's time.
+	if timeOverride := status.GetTimeOverride().AsTime(); !(timeOverride.IsZero() || timeOverride.Unix() == 0) {
+		r.log.DebugContext(ctx, "reconciling with synthetic time instead of real time",
+			"time_override", timeOverride,
+			"real_time", now,
+		)
+		now = timeOverride
+	}
+
 	// If this is a new rollout or the rollout has been reset, we create groups from the config
 	groups := status.GetGroups()
 	var err error
@@ -306,7 +318,7 @@ func (r *reconciler) computeStatus(
 	}
 	status.Groups = groups
 
-	err = r.progressRollout(ctx, newSpec.GetStrategy(), status)
+	err = r.progressRollout(ctx, newSpec.GetStrategy(), status, now)
 	// Failing to progress the update is not a hard failure.
 	// We want to update the status even if something went wrong to surface the failed reconciliation and potential errors to the user.
 	if err != nil {
@@ -322,10 +334,10 @@ func (r *reconciler) computeStatus(
 // groups are updated in place.
 // If an error is returned, the groups should still be upserted, depending on the strategy,
 // failing to update a group might not be fatal (other groups can still progress independently).
-func (r *reconciler) progressRollout(ctx context.Context, strategyName string, status *autoupdate.AutoUpdateAgentRolloutStatus) error {
+func (r *reconciler) progressRollout(ctx context.Context, strategyName string, status *autoupdate.AutoUpdateAgentRolloutStatus, now time.Time) error {
 	for _, strategy := range r.rolloutStrategies {
 		if strategy.name() == strategyName {
-			return strategy.progressRollout(ctx, status)
+			return strategy.progressRollout(ctx, status, now)
 		}
 	}
 	return trace.NotImplemented("rollout strategy %q not implemented", strategyName)

lib/autoupdate/rollout/reconciler_test.go

@@ -714,7 +714,7 @@ func (f *fakeRolloutStrategy) name() string {
 	return f.strategyName
 }
 
-func (f *fakeRolloutStrategy) progressRollout(ctx context.Context, status *autoupdate.AutoUpdateAgentRolloutStatus) error {
+func (f *fakeRolloutStrategy) progressRollout(ctx context.Context, status *autoupdate.AutoUpdateAgentRolloutStatus, now time.Time) error {
 	f.calls++
 	return nil
 }

lib/autoupdate/rollout/strategy.go

@@ -40,7 +40,7 @@ const (
 // This interface allows us to inject dummy strategies for simpler testing.
 type rolloutStrategy interface {
 	name() string
-	progressRollout(context.Context, *autoupdate.AutoUpdateAgentRolloutStatus) error
+	progressRollout(context.Context, *autoupdate.AutoUpdateAgentRolloutStatus, time.Time) error
 }
 
 func inWindow(group *autoupdate.AutoUpdateAgentRolloutStatusGroup, now time.Time) (bool, error) {

lib/autoupdate/rollout/strategy_haltonerror.go

@@ -24,7 +24,6 @@ import (
 	"time"
 
 	"github.com/gravitational/trace"
-	"github.com/jonboulle/clockwork"
 
 	"github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1"
 	update "github.com/gravitational/teleport/api/types/autoupdate"
@@ -39,29 +38,23 @@ const (
 )
 
 type haltOnErrorStrategy struct {
-	log   *slog.Logger
-	clock clockwork.Clock
+	log *slog.Logger
 }
 
 func (h *haltOnErrorStrategy) name() string {
 	return update.AgentsStrategyHaltOnError
 }
 
-func newHaltOnErrorStrategy(log *slog.Logger, clock clockwork.Clock) (rolloutStrategy, error) {
+func newHaltOnErrorStrategy(log *slog.Logger) (rolloutStrategy, error) {
 	if log == nil {
 		return nil, trace.BadParameter("missing log")
 	}
-	if clock == nil {
-		return nil, trace.BadParameter("missing clock")
-	}
 	return &haltOnErrorStrategy{
-		log:   log.With("strategy", update.AgentsStrategyHaltOnError),
-		clock: clock,
+		log: log.With("strategy", update.AgentsStrategyHaltOnError),
 	}, nil
 }
 
-func (h *haltOnErrorStrategy) progressRollout(ctx context.Context, status *autoupdate.AutoUpdateAgentRolloutStatus) error {
-	now := h.clock.Now()
+func (h *haltOnErrorStrategy) progressRollout(ctx context.Context, status *autoupdate.AutoUpdateAgentRolloutStatus, now time.Time) error {
 	// We process every group in order, all the previous groups must be in the DONE state
 	// for the next group to become active. Even if some early groups are not DONE,
 	// later groups might be ACTIVE and need to transition to DONE, so we cannot

lib/autoupdate/rollout/strategy_haltonerror_test.go

@@ -134,7 +134,7 @@ func Test_canStartHaltOnError(t *testing.T) {
 func Test_progressGroupsHaltOnError(t *testing.T) {
 	clock := clockwork.NewFakeClockAt(testSunday)
 	log := utils.NewSlogLoggerForTests()
-	strategy, err := newHaltOnErrorStrategy(log, clock)
+	strategy, err := newHaltOnErrorStrategy(log)
 	require.NoError(t, err)
 
 	fewMinutesAgo := clock.Now().Add(-5 * time.Minute)
@@ -500,7 +500,7 @@ func Test_progressGroupsHaltOnError(t *testing.T) {
 				State:     0,
 				StartTime: tt.rolloutStartTime,
 			}
-			err := strategy.progressRollout(ctx, status)
+			err := strategy.progressRollout(ctx, status, clock.Now())
 			require.NoError(t, err)
 			// We use require.Equal instead of Elements match because group order matters.
 			// It's not super important for time-based, but is crucial for halt-on-error.

lib/autoupdate/rollout/strategy_timebased.go

@@ -21,9 +21,9 @@ package rollout
 import (
 	"context"
 	"log/slog"
+	"time"
 
 	"github.com/gravitational/trace"
-	"github.com/jonboulle/clockwork"
 
 	"github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1"
 	update "github.com/gravitational/teleport/api/types/autoupdate"
@@ -35,29 +35,23 @@ const (
 )
 
 type timeBasedStrategy struct {
-	log   *slog.Logger
-	clock clockwork.Clock
+	log *slog.Logger
 }
 
 func (h *timeBasedStrategy) name() string {
 	return update.AgentsStrategyTimeBased
 }
 
-func newTimeBasedStrategy(log *slog.Logger, clock clockwork.Clock) (rolloutStrategy, error) {
+func newTimeBasedStrategy(log *slog.Logger) (rolloutStrategy, error) {
 	if log == nil {
 		return nil, trace.BadParameter("missing log")
 	}
-	if clock == nil {
-		return nil, trace.BadParameter("missing clock")
-	}
 	return &timeBasedStrategy{
-		log:   log.With("strategy", update.AgentsStrategyTimeBased),
-		clock: clock,
+		log: log.With("strategy", update.AgentsStrategyTimeBased),
 	}, nil
 }
 
-func (h *timeBasedStrategy) progressRollout(ctx context.Context, status *autoupdate.AutoUpdateAgentRolloutStatus) error {
-	now := h.clock.Now()
+func (h *timeBasedStrategy) progressRollout(ctx context.Context, status *autoupdate.AutoUpdateAgentRolloutStatus, now time.Time) error {
 	// We always process every group regardless of the order.
 	var errs []error
 	for _, group := range status.Groups {

lib/autoupdate/rollout/strategy_timebased_test.go

@@ -34,7 +34,7 @@ import (
 func Test_progressGroupsTimeBased(t *testing.T) {
 	clock := clockwork.NewFakeClockAt(testSunday)
 	log := utils.NewSlogLoggerForTests()
-	strategy, err := newTimeBasedStrategy(log, clock)
+	strategy, err := newTimeBasedStrategy(log)
 	require.NoError(t, err)
 
 	groupName := "test-group"
@@ -332,7 +332,7 @@ func Test_progressGroupsTimeBased(t *testing.T) {
 				State:     0,
 				StartTime: tt.rolloutStartTime,
 			}
-			err := strategy.progressRollout(ctx, status)
+			err := strategy.progressRollout(ctx, status, clock.Now())
 			require.NoError(t, err)
 			// We use require.Equal instead of Elements match because group order matters.
 			// It's not super important for time-based, but is crucial for halt-on-error.

lib/config/configuration.go

@@ -2753,6 +2753,14 @@ func Configure(clf *CommandLineFlags, cfg *servicecfg.Config, legacyAppFlags boo
 		cfg.Proxy.QUICProxyPeering = true
 	}
 
+	if rawPeriod := os.Getenv("TELEPORT_UNSTABLE_AGENT_ROLLOUT_SYNC_PERIOD"); rawPeriod != "" {
+		period, err := time.ParseDuration(rawPeriod)
+		if err != nil {
+			return trace.Wrap(err, "invalid agent rollout period %q", rawPeriod)
+		}
+		cfg.Auth.AgentRolloutControllerSyncPeriod = period
+	}
+
 	return nil
 }