diff --git a/.local-dev/config/ns.yaml b/.local-dev/config/ns.yaml index dc3151b7..dd342ab2 100644 --- a/.local-dev/config/ns.yaml +++ b/.local-dev/config/ns.yaml @@ -35,6 +35,7 @@ components: controller: url: http://ns-controller:10000 priority: 0 + stepTimeout: "1h" controller: port: 10000 diff --git a/cmd/config.go b/cmd/config.go index 0f986142..7bfbc14f 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -51,9 +51,10 @@ type BuilderConfig struct { Buildkit struct { Address string `mapstructure:"address" yaml:"address"` } `mapstructure:"buildkit" yaml:"buildkit"` - Buildpack buildpack.Config `mapstructure:"buildpack" yaml:"buildpack"` - Controller grpc.ControllerServiceClientConfig `mapstructure:"controller" yaml:"controller"` - Priority int `mapstructure:"priority" yaml:"priority"` + Buildpack buildpack.Config `mapstructure:"buildpack" yaml:"buildpack"` + Controller grpc.ControllerServiceClientConfig `mapstructure:"controller" yaml:"controller"` + Priority int `mapstructure:"priority" yaml:"priority"` + StepTimeout string `mapstructure:"stepTimeout" yaml:"stepTimeout"` } type ControllerConfig struct { @@ -151,6 +152,7 @@ func init() { viper.SetDefault("components.builder.controller.url", "http://ns-controller:10000") viper.SetDefault("components.builder.priority", 0) + viper.SetDefault("components.builder.stepTimeout", "1h") viper.SetDefault("components.controller.port", 10000) viper.SetDefault("components.controller.tokenHeader", "X-NS-Controller-Token") diff --git a/cmd/providers.go b/cmd/providers.go index e585e2b8..6d10ca4c 100644 --- a/cmd/providers.go +++ b/cmd/providers.go @@ -101,6 +101,7 @@ var providers = wire.NewSet( provideAuthDevServer, provideBuildpackHelperServer, buildpack.NewBuildpackBackend, + provideBuilderConfig, provideBuildkitClient, provideControllerServer, provideContainerLogger, @@ -172,6 +173,20 @@ func provideBuildpackHelperServer( return &buildpackhelper.APIServer{H2CServer: web.NewH2CServer(wc)} } +func provideBuilderConfig(c Config) (*ubuilder.Config, error) { + stepTimeoutStr := c.Components.Builder.StepTimeout + stepTimeout, err := time.ParseDuration(stepTimeoutStr) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("failed to parse components.builder.stepTimeout value: %s", stepTimeoutStr)) + } + if stepTimeout <= 0 { + return nil, errors.Errorf("components.builder.stepTimeout must be positive: %s", stepTimeoutStr) + } + return &ubuilder.Config{ + StepTimeout: stepTimeout, + }, nil +} + func provideBuildkitClient(c Config) (*buildkit.Client, error) { cc := c.Components.Builder ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index c2743513..2b849e5d 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -55,17 +55,21 @@ func NewBuilder(c Config) (component, error) { if err != nil { return nil, err } + builderConfig, err := provideBuilderConfig(c) + if err != nil { + return nil, err + } tokenAuthInterceptor, err := provideTokenAuthInterceptor(c) if err != nil { return nil, err } controllerBuilderServiceClient := provideControllerBuilderServiceClient(c, tokenAuthInterceptor) componentsConfig := c.Components - builderConfig := componentsConfig.Builder - buildpackConfig := builderConfig.Buildpack + mainBuilderConfig := componentsConfig.Builder + buildpackConfig := mainBuilderConfig.Buildpack buildpackHelperServiceClient := provideBuildpackHelperClient(c) buildpackBackend := buildpack.NewBuildpackBackend(buildpackConfig, buildpackHelperServiceClient) - service, err := builder.NewService(controllerBuilderServiceClient, client, buildpackBackend) + service, err := builder.NewService(builderConfig, controllerBuilderServiceClient, client, buildpackBackend) if err != nil { return nil, err } diff --git a/pkg/usecase/builder/build.go b/pkg/usecase/builder/build.go index 08b7d260..cb094ec0 100644 --- a/pkg/usecase/builder/build.go +++ b/pkg/usecase/builder/build.go @@ -8,7 +8,6 @@ import ( "github.com/friendsofgo/errors" buildkit "github.com/moby/buildkit/client" log "github.com/sirupsen/logrus" - gstatus "google.golang.org/grpc/status" "github.com/traPtitech/neoshowcase/pkg/domain" "github.com/traPtitech/neoshowcase/pkg/infrastructure/grpc/pb" @@ -61,80 +60,80 @@ func (s *builderService) startBuild(req *domain.StartBuildRequest) error { type buildStep struct { desc string - fn func() error + fn func(ctx context.Context) error } -func (s *builderService) buildSteps(ctx context.Context, st *state) ([]buildStep, error) { +func (s *builderService) buildSteps(st *state) ([]buildStep, error) { var steps []buildStep - steps = append(steps, buildStep{"Repository Clone", func() error { + steps = append(steps, buildStep{"Repository Clone", func(ctx context.Context) error { return s.cloneRepository(ctx, st) }}) switch bc := st.app.Config.BuildConfig.(type) { case *domain.BuildConfigRuntimeBuildpack: - steps = append(steps, buildStep{"Build (Runtime Buildpack)", func() error { + steps = append(steps, buildStep{"Build (Runtime Buildpack)", func(ctx context.Context) error { return s.buildRuntimeBuildpack(ctx, st, bc) }}) case *domain.BuildConfigRuntimeCmd: - steps = append(steps, buildStep{"Build (Runtime Command)", func() error { + steps = append(steps, buildStep{"Build (Runtime Command)", func(ctx context.Context) error { return withBuildkitProgress(ctx, st.logWriter, func(ctx context.Context, ch chan *buildkit.SolveStatus) error { return s.buildRuntimeCmd(ctx, st, ch, bc) }) }}) case *domain.BuildConfigRuntimeDockerfile: - steps = append(steps, buildStep{"Build (Runtime Dockerfile)", func() error { + steps = append(steps, buildStep{"Build (Runtime Dockerfile)", func(ctx context.Context) error { return withBuildkitProgress(ctx, st.logWriter, func(ctx context.Context, ch chan *buildkit.SolveStatus) error { return s.buildRuntimeDockerfile(ctx, st, ch, bc) }) }}) case *domain.BuildConfigStaticBuildpack: - steps = append(steps, buildStep{"Build (Static Buildpack)", func() error { + steps = append(steps, buildStep{"Build (Static Buildpack)", func(ctx context.Context) error { return s.buildStaticBuildpackPack(ctx, st, bc) }}) - steps = append(steps, buildStep{"Extract from Temporary Image", func() error { + steps = append(steps, buildStep{"Extract from Temporary Image", func(ctx context.Context) error { return withBuildkitProgress(ctx, st.logWriter, func(ctx context.Context, ch chan *buildkit.SolveStatus) error { return s.buildStaticExtract(ctx, st, ch) }) }}) - steps = append(steps, buildStep{"Cleanup Temporary Image", func() error { + steps = append(steps, buildStep{"Cleanup Temporary Image", func(ctx context.Context) error { return s.buildStaticCleanup(ctx, st) }}) - steps = append(steps, buildStep{"Save Artifact", func() error { + steps = append(steps, buildStep{"Save Artifact", func(ctx context.Context) error { return s.saveArtifact(ctx, st) }}) case *domain.BuildConfigStaticCmd: - steps = append(steps, buildStep{"Build (Static Command)", func() error { + steps = append(steps, buildStep{"Build (Static Command)", func(ctx context.Context) error { return withBuildkitProgress(ctx, st.logWriter, func(ctx context.Context, ch chan *buildkit.SolveStatus) error { return s.buildStaticCmd(ctx, st, ch, bc) }) }}) - steps = append(steps, buildStep{"Extract from Temporary Image", func() error { + steps = append(steps, buildStep{"Extract from Temporary Image", func(ctx context.Context) error { return withBuildkitProgress(ctx, st.logWriter, func(ctx context.Context, ch chan *buildkit.SolveStatus) error { return s.buildStaticExtract(ctx, st, ch) }) }}) - steps = append(steps, buildStep{"Cleanup Temporary Image", func() error { + steps = append(steps, buildStep{"Cleanup Temporary Image", func(ctx context.Context) error { return s.buildStaticCleanup(ctx, st) }}) - steps = append(steps, buildStep{"Save Artifact", func() error { + steps = append(steps, buildStep{"Save Artifact", func(ctx context.Context) error { return s.saveArtifact(ctx, st) }}) case *domain.BuildConfigStaticDockerfile: - steps = append(steps, buildStep{"Build (Static Dockerfile)", func() error { + steps = append(steps, buildStep{"Build (Static Dockerfile)", func(ctx context.Context) error { return withBuildkitProgress(ctx, st.logWriter, func(ctx context.Context, ch chan *buildkit.SolveStatus) error { return s.buildStaticDockerfile(ctx, st, ch, bc) }) }}) - steps = append(steps, buildStep{"Extract from Temporary Image", func() error { + steps = append(steps, buildStep{"Extract from Temporary Image", func(ctx context.Context) error { return withBuildkitProgress(ctx, st.logWriter, func(ctx context.Context, ch chan *buildkit.SolveStatus) error { return s.buildStaticExtract(ctx, st, ch) }) }}) - steps = append(steps, buildStep{"Cleanup Temporary Image", func() error { + steps = append(steps, buildStep{"Cleanup Temporary Image", func(ctx context.Context) error { return s.buildStaticCleanup(ctx, st) }}) - steps = append(steps, buildStep{"Save Artifact", func() error { + steps = append(steps, buildStep{"Save Artifact", func(ctx context.Context) error { return s.saveArtifact(ctx, st) }}) default: @@ -152,7 +151,7 @@ func (s *builderService) process(ctx context.Context, st *state) domain.BuildSta version, revision := cli.GetVersion() st.WriteLog(fmt.Sprintf("[ns-builder] Version %v (%v)", version, revision)) - steps, err := s.buildSteps(ctx, st) + steps, err := s.buildSteps(st) if err != nil { log.Errorf("calculating build steps: %+v", err) st.WriteLog(fmt.Sprintf("[ns-builder] Error calculating build steps: %v", err)) @@ -162,22 +161,30 @@ func (s *builderService) process(ctx context.Context, st *state) domain.BuildSta for i, step := range steps { st.WriteLog(fmt.Sprintf("[ns-builder] ==> (%d/%d) %s", i+1, len(steps), step.desc)) start := time.Now() - err := step.fn() + + // Execute step with timeout + childCtx, cancel := context.WithTimeout(ctx, s.config.StepTimeout) + err := step.fn(childCtx) + cancel() + + // First, check if ctx was cancelled from parent + // - this (usually) means the user pressed 'Cancel' button to cancel the build if cerr := ctx.Err(); cerr != nil { st.WriteLog(fmt.Sprintf("[ns-builder] Build cancelled by user: %v", cerr)) return domain.BuildStatusCanceled } - if errors.Is(err, context.Canceled) || - errors.Is(err, context.DeadlineExceeded) || - errors.Is(err, gstatus.FromContextError(context.Canceled).Err()) { - st.WriteLog(fmt.Sprintf("[ns-builder] Build cancelled: %v", err)) - return domain.BuildStatusCanceled + // Check if the childCtx timed out from the configured "StepTimeout" + if errors.Is(err, context.DeadlineExceeded) { + st.WriteLog(fmt.Sprintf("[ns-builder] Build timed out: %v", err)) + return domain.BuildStatusFailed } + // Other reasons such as build script failure if err != nil { log.Errorf("Build failed for %v: %+v", st.build.ID, err) st.WriteLog(fmt.Sprintf("[ns-builder] Build failed: %v", err)) return domain.BuildStatusFailed } + st.WriteLog(fmt.Sprintf("[ns-builder] ==> (%d/%d) Done (%v).", i+1, len(steps), time.Since(start))) } diff --git a/pkg/usecase/builder/service.go b/pkg/usecase/builder/service.go index 100195ee..2d30e746 100644 --- a/pkg/usecase/builder/service.go +++ b/pkg/usecase/builder/service.go @@ -18,12 +18,17 @@ import ( "github.com/traPtitech/neoshowcase/pkg/util/retry" ) +type Config struct { + StepTimeout time.Duration +} + type Service interface { Start(ctx context.Context) error Shutdown(ctx context.Context) error } type builderService struct { + config *Config client domain.ControllerBuilderServiceClient buildkit *buildkit.Client buildpack builder.BuildpackBackend @@ -39,6 +44,7 @@ type builderService struct { } func NewService( + config *Config, client domain.ControllerBuilderServiceClient, buildkit *buildkit.Client, buildpack builder.BuildpackBackend, @@ -52,6 +58,7 @@ func NewService( return nil, errors.Wrap(err, "failed to convert into public key") } return &builderService{ + config: config, client: client, buildkit: buildkit, buildpack: buildpack,