Skip to content

Commit

Permalink
session: implement ci timeouts, defaulting to 30m (#6004)
Browse files Browse the repository at this point in the history
Fixes #3783

Signed-off-by: Nick Santos <[email protected]>

Signed-off-by: Nick Santos <[email protected]>
  • Loading branch information
nicks authored Jan 6, 2023
1 parent bfe8403 commit f53e573
Show file tree
Hide file tree
Showing 20 changed files with 181 additions and 44 deletions.
4 changes: 4 additions & 0 deletions internal/cli/ci.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ See blog post for additional information: https://blog.tilt.dev/2020/04/16/how-t
cmd.Flags().Lookup("logactions").Hidden = true
cmd.Flags().StringVar(&c.outputSnapshotOnExit, "output-snapshot-on-exit", "",
"If specified, Tilt will dump a snapshot of its state to the specified path when it exits")
cmd.Flags().DurationVar(&ciTimeout, "timeout", model.CITimeoutDefault,
"Timeout to wait for CI to pass. Set to 0 for no timeout.")

return cmd
}
Expand Down Expand Up @@ -101,3 +103,5 @@ func (c *ciCmd) run(ctx context.Context, args []string) error {
}
return err
}

var ciTimeout time.Duration
5 changes: 5 additions & 0 deletions internal/cli/wire.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ var BaseWireSet = wire.NewSet(

controllers.WireSet,

provideCITimeoutFlag,
provideWebVersion,
provideWebMode,
provideWebURL,
Expand Down Expand Up @@ -354,3 +355,7 @@ func wireLsp(ctx context.Context, l logger.Logger, subcommand model.TiltSubcomma
wire.Build(UpWireSet, newLspDeps, newAnalytics)
return cmdLspDeps{}, nil
}

func provideCITimeoutFlag() model.CITimeoutFlag {
return model.CITimeoutFlag(ciTimeout)
}
38 changes: 28 additions & 10 deletions internal/cli/wire_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 29 additions & 1 deletion internal/controllers/core/session/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,30 @@ func TestExitControlCI_GracePeriod(t *testing.T) {
f.requireDoneWithError("exceeded grace period: Pod pod-a in error state due to container c1: ErrImagePull")
}

func TestExitControlCI_Timeout(t *testing.T) {
f := newFixture(t, store.EngineModeCI)

var session v1alpha1.Session
f.MustGet(types.NamespacedName{Name: "Tiltfile"}, &session)
session.Spec.CI = &v1alpha1.SessionCISpec{Timeout: &metav1.Duration{Duration: time.Minute}}
f.Update(&session)

m := manifestbuilder.New(f, "fe").WithK8sYAML(testyaml.SanchoYAML).Build()
f.upsertManifest(m)

f.MustReconcile(sessionKey)
f.requireNotDone()

f.clock.Advance(50 * time.Second)

f.MustReconcile(sessionKey)
f.requireNotDone()

f.clock.Advance(20 * time.Second)
f.MustReconcile(sessionKey)
f.requireDoneWithError("Timeout after 1m0s")
}

func TestExitControlCI_PodRunningContainerError(t *testing.T) {
f := newFixture(t, store.EngineModeCI)

Expand Down Expand Up @@ -619,7 +643,11 @@ func newFixture(t testing.TB, engineMode store.EngineMode) *fixture {
clock := clockwork.NewFakeClock()
r := NewReconciler(cfb.Client, st, clock)
cf := cfb.Build(r)
cf.Create(sessions.FromTiltfile(tf, nil, engineMode))

session := sessions.FromTiltfile(tf, nil, model.CITimeoutFlag(model.CITimeoutDefault), engineMode)
session.Status.StartTime = apis.NewMicroTime(clock.Now())
cf.Create(session)

return &fixture{
ControllerFixture: cf,
tf: tdf,
Expand Down
19 changes: 19 additions & 0 deletions internal/controllers/core/session/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ func (r *Reconciler) makeLatestStatus(session *v1alpha1.Session, result *ctrl.Re
})

r.processExitCondition(session.Spec, &state, &status)

// If there's a global timeout, schedule a requeue.
ci := session.Spec.CI
if ci != nil && ci.Timeout != nil && ci.Timeout.Duration > 0 {
timeout := ci.Timeout.Duration
requeueAfter := timeout - r.clock.Since(session.Status.StartTime.Time)
if result.RequeueAfter == 0 || result.RequeueAfter < requeueAfter {
result.RequeueAfter = requeueAfter
}
}

return status
}

Expand Down Expand Up @@ -90,6 +101,14 @@ func (r *Reconciler) processExitCondition(spec v1alpha1.SessionSpec, state *stor
if allResourcesOK && len(status.Targets) > 1 {
status.Done = true
}

// Enforce a global timeout.
ci := spec.CI
if status.Error == "" && ci != nil && ci.Timeout != nil && ci.Timeout.Duration > 0 &&
r.clock.Since(status.StartTime.Time) > ci.Timeout.Duration {
status.Done = true
status.Error = fmt.Sprintf("Timeout after %s", ci.Timeout.Duration)
}
}

// errToString returns a stringified version of an error or an empty string if the error is nil.
Expand Down
10 changes: 6 additions & 4 deletions internal/controllers/core/tiltfile/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func updateOwnedObjects(
tf *v1alpha1.Tiltfile,
tlr *tiltfile.TiltfileLoadResult,
changeEnabledResources bool,
ciTimeoutFlag model.CITimeoutFlag,
mode store.EngineMode,
defaultK8sConnection *v1alpha1.KubernetesClusterConnection,
) error {
Expand All @@ -78,7 +79,7 @@ func updateOwnedObjects(
}
}

apiObjects := toAPIObjects(nn, tf, tlr, mode, defaultK8sConnection, disableSources)
apiObjects := toAPIObjects(nn, tf, tlr, ciTimeoutFlag, mode, defaultK8sConnection, disableSources)

// Propagate labels and owner references from the parent tiltfile.
for _, objMap := range apiObjects {
Expand Down Expand Up @@ -223,6 +224,7 @@ func toAPIObjects(
nn types.NamespacedName,
tf *v1alpha1.Tiltfile,
tlr *tiltfile.TiltfileLoadResult,
ciTimeoutFlag model.CITimeoutFlag,
mode store.EngineMode,
defaultK8sConnection *v1alpha1.KubernetesClusterConnection,
disableSources disableSourceMap,
Expand All @@ -248,7 +250,7 @@ func toAPIObjects(
result.AddSetForType(&v1alpha1.UIButton{}, toCancelButtons(tlr))
}

result.AddSetForType(&v1alpha1.Session{}, toSessionObjects(nn, tf, tlr, mode))
result.AddSetForType(&v1alpha1.Session{}, toSessionObjects(nn, tf, tlr, ciTimeoutFlag, mode))
result.AddSetForType(&v1alpha1.UIResource{}, toUIResourceObjects(tf, tlr, disableSources))

watchInputs := WatchInputs{
Expand Down Expand Up @@ -589,12 +591,12 @@ func toImageMapObjects(tlr *tiltfile.TiltfileLoadResult, disableSources disableS
}

// Creates an object representing the tilt session and exit conditions.
func toSessionObjects(nn types.NamespacedName, tf *v1alpha1.Tiltfile, tlr *tiltfile.TiltfileLoadResult, mode store.EngineMode) apiset.TypedObjectSet {
func toSessionObjects(nn types.NamespacedName, tf *v1alpha1.Tiltfile, tlr *tiltfile.TiltfileLoadResult, ciTimeoutFlag model.CITimeoutFlag, mode store.EngineMode) apiset.TypedObjectSet {
result := apiset.TypedObjectSet{}
if nn.Name != model.MainTiltfileManifestName.String() {
return result
}
result[sessions.DefaultSessionName] = sessions.FromTiltfile(tf, tlr, mode)
result[sessions.DefaultSessionName] = sessions.FromTiltfile(tf, tlr, ciTimeoutFlag, mode)
return result
}

Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/core/tiltfile/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ func TestReconciledTypesCompleteness(t *testing.T) {
fe := manifestbuilder.New(f, "fe").WithK8sYAML(testyaml.SanchoYAML).Build()
tlr := &tiltfile.TiltfileLoadResult{Manifests: []model.Manifest{fe}}
ds := toDisableSources(tlr)
objs := toAPIObjects(nn, tf, tlr, store.EngineModeCI, &v1alpha1.KubernetesClusterConnection{}, ds)
objs := toAPIObjects(nn, tf, tlr, 0, store.EngineModeCI, &v1alpha1.KubernetesClusterConnection{}, ds)

reconciledTypes := make(map[schema.GroupVersionResource]bool)
for _, t := range typesToReconcile {
Expand Down Expand Up @@ -414,7 +414,7 @@ func newAPIFixture(t testing.TB) *apiFixture {
}

func (f *apiFixture) updateOwnedObjects(nn types.NamespacedName, tf *v1alpha1.Tiltfile, tlr *tiltfile.TiltfileLoadResult) error {
return updateOwnedObjects(f.ctx, f.c, nn, tf, tlr, false, store.EngineModeUp,
return updateOwnedObjects(f.ctx, f.c, nn, tf, tlr, false, 0, store.EngineModeUp,
&v1alpha1.KubernetesClusterConnection{})
}

Expand Down
Loading

0 comments on commit f53e573

Please sign in to comment.