Skip to content

Commit b462966

Browse files
ThomasK33ThomasK33
committed
🤖 fix: map template build wait failures to status errors
Preserve Kubernetes API error mapping for template build failures/timeouts during active-version promotion.
1 parent b1f3e93 commit b462966

2 files changed

Lines changed: 242 additions & 29 deletions

File tree

‎internal/aggregated/storage/storage_test.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,117 @@ func TestTemplateStorageUpdateWaitsForTemplateVersionBuildBeforePromotion(t *tes
398398
}
399399
}
400400

401+
func TestTemplateStorageUpdateReturnsBadRequestWhenTemplateVersionBuildFails(t *testing.T) {
402+
t.Parallel()
403+
404+
server, state := newMockCoderServer(t)
405+
defer server.Close()
406+
407+
templateStorage := NewTemplateStorage(newTestClientProvider(t, server.URL))
408+
ctx := namespacedContext("control-plane")
409+
410+
createObj := &aggregationv1alpha1.CoderTemplate{
411+
ObjectMeta: metav1.ObjectMeta{Name: "acme.failed-build-template"},
412+
Spec: aggregationv1alpha1.CoderTemplateSpec{
413+
Organization: "acme",
414+
Files: map[string]string{"main.tf": "resource \"null_resource\" \"initial\" {}"},
415+
},
416+
}
417+
createdObj, err := templateStorage.Create(ctx, createObj, rest.ValidateAllObjectFunc, nil)
418+
if err != nil {
419+
t.Fatalf("expected template create to succeed: %v", err)
420+
}
421+
createdTemplate := createdObj.(*aggregationv1alpha1.CoderTemplate)
422+
423+
state.setNextCreatedTemplateVersionStatus(codersdk.ProvisionerJobFailed)
424+
425+
desiredTemplate := createdTemplate.DeepCopy()
426+
desiredTemplate.Spec.Files = map[string]string{"main.tf": "resource \"null_resource\" \"updated\" {}"}
427+
428+
updatedObj, created, updateErr := templateStorage.Update(
429+
ctx,
430+
desiredTemplate.Name,
431+
testUpdatedObjectInfo{obj: desiredTemplate},
432+
nil,
433+
rest.ValidateAllObjectUpdateFunc,
434+
false,
435+
nil,
436+
)
437+
if updateErr == nil {
438+
t.Fatal("expected update to fail when template version build fails")
439+
}
440+
if updatedObj != nil {
441+
t.Fatalf("expected nil updated object on failure, got %T", updatedObj)
442+
}
443+
if created {
444+
t.Fatal("expected update created=false")
445+
}
446+
if !apierrors.IsBadRequest(updateErr) {
447+
t.Fatalf("expected BadRequest for failed build, got %v", updateErr)
448+
}
449+
assertTopLevelStatusError(t, updateErr)
450+
if !strings.Contains(updateErr.Error(), "build ended with status") {
451+
t.Fatalf("expected failed-build error message, got %v", updateErr)
452+
}
453+
}
454+
455+
func TestTemplateStorageUpdateReturnsBadRequestWhenTemplateVersionBuildWaitTimesOut(t *testing.T) {
456+
server, state := newMockCoderServer(t)
457+
defer server.Close()
458+
459+
templateStorage := NewTemplateStorage(newTestClientProvider(t, server.URL))
460+
ctx := namespacedContext("control-plane")
461+
462+
createObj := &aggregationv1alpha1.CoderTemplate{
463+
ObjectMeta: metav1.ObjectMeta{Name: "acme.timeout-build-template"},
464+
Spec: aggregationv1alpha1.CoderTemplateSpec{
465+
Organization: "acme",
466+
Files: map[string]string{"main.tf": "resource \"null_resource\" \"initial\" {}"},
467+
},
468+
}
469+
createdObj, err := templateStorage.Create(ctx, createObj, rest.ValidateAllObjectFunc, nil)
470+
if err != nil {
471+
t.Fatalf("expected template create to succeed: %v", err)
472+
}
473+
createdTemplate := createdObj.(*aggregationv1alpha1.CoderTemplate)
474+
475+
t.Setenv(templateVersionBuildWaitTimeoutEnv, "120ms")
476+
t.Setenv(templateVersionBuildBackoffAfterEnv, "0s")
477+
t.Setenv(templateVersionBuildInitialPollIntervalEnv, "10ms")
478+
t.Setenv(templateVersionBuildMaxPollIntervalEnv, "20ms")
479+
480+
state.setNextCreatedTemplateVersionStatus(codersdk.ProvisionerJobPending)
481+
482+
desiredTemplate := createdTemplate.DeepCopy()
483+
desiredTemplate.Spec.Files = map[string]string{"main.tf": "resource \"null_resource\" \"updated\" {}"}
484+
485+
updatedObj, created, updateErr := templateStorage.Update(
486+
ctx,
487+
desiredTemplate.Name,
488+
testUpdatedObjectInfo{obj: desiredTemplate},
489+
nil,
490+
rest.ValidateAllObjectUpdateFunc,
491+
false,
492+
nil,
493+
)
494+
if updateErr == nil {
495+
t.Fatal("expected update to fail when template version build wait times out")
496+
}
497+
if updatedObj != nil {
498+
t.Fatalf("expected nil updated object on timeout, got %T", updatedObj)
499+
}
500+
if created {
501+
t.Fatal("expected update created=false")
502+
}
503+
if !apierrors.IsBadRequest(updateErr) {
504+
t.Fatalf("expected BadRequest for build wait timeout, got %v", updateErr)
505+
}
506+
assertTopLevelStatusError(t, updateErr)
507+
if !strings.Contains(updateErr.Error(), "did not succeed within") {
508+
t.Fatalf("expected timeout error message, got %v", updateErr)
509+
}
510+
}
511+
401512
func TestLoadTemplateVersionBuildWaitConfigFromEnvDefaults(t *testing.T) {
402513
t.Setenv(templateVersionBuildWaitTimeoutEnv, "")
403514
t.Setenv(templateVersionBuildBackoffAfterEnv, "")
@@ -3082,6 +3193,18 @@ func (s *mockCoderServerState) setFailActiveVersionPromotion(fail bool) {
30823193
s.failActiveVersionPromotion = fail
30833194
}
30843195

3196+
func (s *mockCoderServerState) setNextCreatedTemplateVersionStatus(status codersdk.ProvisionerJobStatus) {
3197+
if status == "" {
3198+
panic("assertion failed: template version status must not be empty")
3199+
}
3200+
3201+
s.mu.Lock()
3202+
defer s.mu.Unlock()
3203+
3204+
s.nextTemplateVersionInitialStatus = status
3205+
s.nextTemplateVersionPendingPolls = 0
3206+
}
3207+
30853208
func (s *mockCoderServerState) setNextCreatedTemplateVersionPendingForPolls(polls int) {
30863209
if polls <= 0 {
30873210
panic(fmt.Sprintf("assertion failed: pending poll count must be > 0, got %d", polls))

‎internal/aggregated/storage/template.go‎

Lines changed: 119 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package storage
33
import (
44
"bytes"
55
"context"
6+
"errors"
67
"fmt"
78
"os"
89
"reflect"
@@ -497,7 +498,7 @@ func (s *TemplateStorage) Update(
497498
}
498499

499500
if waitErr := waitForTemplateVersionBuild(ctx, sdk, newVersion.ID); waitErr != nil {
500-
return nil, false, fmt.Errorf("wait for template version %q build: %w", newVersion.ID.String(), waitErr)
501+
return nil, false, mapTemplateVersionBuildWaitError(waitErr, name)
501502
}
502503

503504
if err := sdk.UpdateActiveTemplateVersion(ctx, templateID, codersdk.UpdateActiveTemplateVersion{ID: newVersion.ID}); err != nil {
@@ -596,6 +597,102 @@ func (s *TemplateStorage) ConvertToTable(ctx context.Context, object, tableOptio
596597
return s.tableConvertor.ConvertToTable(ctx, object, tableOptions)
597598
}
598599

600+
type templateVersionBuildTerminalError struct {
601+
versionID uuid.UUID
602+
status codersdk.ProvisionerJobStatus
603+
detail string
604+
}
605+
606+
func (e *templateVersionBuildTerminalError) Error() string {
607+
if e == nil {
608+
panic("assertion failed: template version build terminal error must not be nil")
609+
}
610+
if e.versionID == uuid.Nil {
611+
panic("assertion failed: template version build terminal error must include version ID")
612+
}
613+
if e.status == "" {
614+
panic("assertion failed: template version build terminal error must include status")
615+
}
616+
617+
detail := e.detail
618+
if detail == "" {
619+
detail = "template version build did not succeed"
620+
}
621+
622+
return fmt.Sprintf(
623+
"template version %q build ended with status %q: %s",
624+
e.versionID.String(),
625+
e.status,
626+
detail,
627+
)
628+
}
629+
630+
type templateVersionBuildWaitTimeoutError struct {
631+
versionID uuid.UUID
632+
waitTimeout time.Duration
633+
lastStatus codersdk.ProvisionerJobStatus
634+
lastError string
635+
cause error
636+
}
637+
638+
func (e *templateVersionBuildWaitTimeoutError) Error() string {
639+
if e == nil {
640+
panic("assertion failed: template version build timeout error must not be nil")
641+
}
642+
if e.versionID == uuid.Nil {
643+
panic("assertion failed: template version build timeout error must include version ID")
644+
}
645+
if e.waitTimeout <= 0 {
646+
panic("assertion failed: template version build timeout error must include positive timeout")
647+
}
648+
if e.lastStatus == "" {
649+
panic("assertion failed: template version build timeout error must include last status")
650+
}
651+
if e.cause == nil {
652+
panic("assertion failed: template version build timeout error must include cause")
653+
}
654+
655+
if e.lastError == "" {
656+
return fmt.Sprintf(
657+
"template version %q did not succeed within %s (last status: %q): %v",
658+
e.versionID.String(),
659+
e.waitTimeout,
660+
e.lastStatus,
661+
e.cause,
662+
)
663+
}
664+
665+
return fmt.Sprintf(
666+
"template version %q did not succeed within %s (last status: %q, last error: %s): %v",
667+
e.versionID.String(),
668+
e.waitTimeout,
669+
e.lastStatus,
670+
e.lastError,
671+
e.cause,
672+
)
673+
}
674+
675+
func mapTemplateVersionBuildWaitError(waitErr error, templateName string) error {
676+
if waitErr == nil {
677+
return fmt.Errorf("assertion failed: wait error must not be nil")
678+
}
679+
if templateName == "" {
680+
return fmt.Errorf("assertion failed: template name must not be empty")
681+
}
682+
683+
var terminalErr *templateVersionBuildTerminalError
684+
if errors.As(waitErr, &terminalErr) {
685+
return apierrors.NewBadRequest(terminalErr.Error())
686+
}
687+
688+
var timeoutErr *templateVersionBuildWaitTimeoutError
689+
if errors.As(waitErr, &timeoutErr) {
690+
return apierrors.NewBadRequest(timeoutErr.Error())
691+
}
692+
693+
return coder.MapCoderError(waitErr, aggregationv1alpha1.Resource("codertemplates"), templateName)
694+
}
695+
599696
func waitForTemplateVersionBuild(ctx context.Context, sdk *codersdk.Client, versionID uuid.UUID) error {
600697
if ctx == nil {
601698
return fmt.Errorf("assertion failed: context must not be nil")
@@ -624,12 +721,13 @@ func waitForTemplateVersionBuild(ctx context.Context, sdk *codersdk.Client, vers
624721
version, err := sdk.TemplateVersion(waitCtx, versionID)
625722
if err != nil {
626723
if waitCtx.Err() != nil {
627-
return fmt.Errorf(
628-
"template version %q build wait canceled after status %q: %w",
629-
versionID.String(),
630-
lastStatus,
631-
waitCtx.Err(),
632-
)
724+
return &templateVersionBuildWaitTimeoutError{
725+
versionID: versionID,
726+
waitTimeout: waitConfig.waitTimeout,
727+
lastStatus: lastStatus,
728+
lastError: lastError,
729+
cause: waitCtx.Err(),
730+
}
633731
}
634732
return fmt.Errorf("fetch template version %q: %w", versionID.String(), err)
635733
}
@@ -648,12 +746,11 @@ func waitForTemplateVersionBuild(ctx context.Context, sdk *codersdk.Client, vers
648746
if lastError == "" {
649747
lastError = "template version build did not succeed"
650748
}
651-
return fmt.Errorf(
652-
"template version %q build ended with status %q: %s",
653-
versionID.String(),
654-
status,
655-
lastError,
656-
)
749+
return &templateVersionBuildTerminalError{
750+
versionID: versionID,
751+
status: status,
752+
detail: lastError,
753+
}
657754
case codersdk.ProvisionerJobPending, codersdk.ProvisionerJobRunning, codersdk.ProvisionerJobCanceling, codersdk.ProvisionerJobUnknown:
658755
// Keep polling below.
659756
default:
@@ -674,23 +771,16 @@ func waitForTemplateVersionBuild(ctx context.Context, sdk *codersdk.Client, vers
674771

675772
select {
676773
case <-waitCtx.Done():
677-
if lastError == "" {
678-
return fmt.Errorf(
679-
"template version %q did not succeed within %s (last status: %q): %w",
680-
versionID.String(),
681-
waitConfig.waitTimeout,
682-
lastStatus,
683-
waitCtx.Err(),
684-
)
774+
if waitCtx.Err() == nil {
775+
return fmt.Errorf("assertion failed: wait context finished without an error")
776+
}
777+
return &templateVersionBuildWaitTimeoutError{
778+
versionID: versionID,
779+
waitTimeout: waitConfig.waitTimeout,
780+
lastStatus: lastStatus,
781+
lastError: lastError,
782+
cause: waitCtx.Err(),
685783
}
686-
return fmt.Errorf(
687-
"template version %q did not succeed within %s (last status: %q, last error: %s): %w",
688-
versionID.String(),
689-
waitConfig.waitTimeout,
690-
lastStatus,
691-
lastError,
692-
waitCtx.Err(),
693-
)
694784
case <-time.After(pollInterval):
695785
}
696786
}

0 commit comments

Comments
 (0)