Always send a status back to forge (#1751)

server/api/pipeline.go

@@ -257,6 +257,7 @@ func GetPipelineConfig(c *gin.Context) {
 func CancelPipeline(c *gin.Context) {
 	_store := store.FromContext(c)
 	repo := session.Repo(c)
+	user := session.User(c)
 	num, _ := strconv.ParseInt(c.Params.ByName("number"), 10, 64)
 
 	pl, err := _store.GetPipelineNumber(repo, num)
@@ -265,7 +266,7 @@ func CancelPipeline(c *gin.Context) {
 		return
 	}
 
-	if err := pipeline.Cancel(c, _store, repo, pl); err != nil {
+	if err := pipeline.Cancel(c, _store, repo, user, pl); err != nil {
 		handlePipelineErr(c, err)
 	} else {
 		c.Status(http.StatusNoContent)

server/forge/gitea/gitea.go

@@ -565,12 +565,14 @@ func getStatus(status model.StatusValue) gitea.StatusState {
 		return gitea.StatusPending
 	case model.StatusSuccess:
 		return gitea.StatusSuccess
-	case model.StatusFailure, model.StatusError:
+	case model.StatusFailure:
 		return gitea.StatusFailure
 	case model.StatusKilled:
 		return gitea.StatusFailure
 	case model.StatusDeclined:
 		return gitea.StatusWarning
+	case model.StatusError:
+		return gitea.StatusError
 	default:
 		return gitea.StatusFailure
 	}

server/grpc/rpc.go

@@ -136,10 +136,12 @@ func (s *RPC) Update(c context.Context, id string, state rpc.State) error {
 		return err
 	}
 
-	if _, err = pipeline.UpdateStepStatus(s.store, *step, state, currentPipeline.Started); err != nil {
+	if step, err = pipeline.UpdateStepStatus(s.store, *step, state, currentPipeline.Started); err != nil {
 		log.Error().Err(err).Msg("rpc.update: cannot update step")
 	}
 
+	s.updateForgeStatus(c, repo, currentPipeline, step)
+
 	currentPipeline.Steps, err = s.store.StepList(currentPipeline)
 	if err != nil {
 		log.Error().Err(err).Msg("can not get step list from store")
@@ -275,6 +277,8 @@ func (s *RPC) Init(c context.Context, id string, state rpc.State) error {
 		}
 	}
 
+	s.updateForgeStatus(c, repo, currentPipeline, step)
+
 	defer func() {
 		currentPipeline.Steps, _ = s.store.StepList(currentPipeline)
 		message := pubsub.Message{
@@ -292,8 +296,12 @@ func (s *RPC) Init(c context.Context, id string, state rpc.State) error {
 		}
 	}()
 
-	_, err = pipeline.UpdateStepToStatusStarted(s.store, *step, state)
+	step, err = pipeline.UpdateStepToStatusStarted(s.store, *step, state)
-	return err
+	if err != nil {
+		return err
+	}
+	s.updateForgeStatus(c, repo, currentPipeline, step)
+	return nil
 }
 
 // Done implements the rpc.Done function

server/pipeline/approve.go

@@ -26,7 +26,7 @@ import (
 )
 
 // Approve update the status to pending for blocked pipeline because of a gated repo
-// and start them afterwards
+// and start them afterward
 func Approve(ctx context.Context, store store.Store, currentPipeline *model.Pipeline, user *model.User, repo *model.Repo) (*model.Pipeline, error) {
 	if currentPipeline.Status != model.StatusBlocked {
 		return nil, ErrBadRequest{Msg: fmt.Sprintf("cannot decline a pipeline with status %s", currentPipeline.Status)}

server/pipeline/cancel.go

@@ -27,7 +27,7 @@ import (
 )
 
 // Cancel the pipeline and returns the status.
-func Cancel(ctx context.Context, store store.Store, repo *model.Repo, pipeline *model.Pipeline) error {
+func Cancel(ctx context.Context, store store.Store, repo *model.Repo, user *model.User, pipeline *model.Pipeline) error {
 	if pipeline.Status != model.StatusRunning && pipeline.Status != model.StatusPending && pipeline.Status != model.StatusBlocked {
 		return &ErrBadRequest{Msg: "Cannot cancel a non-running or non-pending or non-blocked pipeline"}
 	}
@@ -84,20 +84,22 @@ func Cancel(ctx context.Context, store store.Store, repo *model.Repo, pipeline *
 		}
 	}
 
-	killedBuild, err := UpdateToStatusKilled(store, *pipeline)
+	killedPipeline, err := UpdateToStatusKilled(store, *pipeline)
 	if err != nil {
 		log.Error().Err(err).Msgf("UpdateToStatusKilled: %v", pipeline)
 		return err
 	}
 
-	steps, err = store.StepList(killedBuild)
+	updatePipelineStatus(ctx, pipeline, repo, user)
+
+	steps, err = store.StepList(killedPipeline)
 	if err != nil {
 		return &ErrNotFound{Msg: err.Error()}
 	}
-	if killedBuild.Steps, err = model.Tree(steps); err != nil {
+	if killedPipeline.Steps, err = model.Tree(steps); err != nil {
 		return err
 	}
-	if err := publishToTopic(ctx, killedBuild, repo); err != nil {
+	if err := publishToTopic(ctx, killedPipeline, repo); err != nil {
 		log.Error().Err(err).Msg("publishToTopic")
 	}
 
@@ -109,6 +111,7 @@ func cancelPreviousPipelines(
 	_store store.Store,
 	pipeline *model.Pipeline,
 	repo *model.Repo,
+	user *model.User,
 ) error {
 	// check this event should cancel previous pipelines
 	eventIncluded := false
@@ -162,7 +165,7 @@ func cancelPreviousPipelines(
 			continue
 		}
 
-		if err = Cancel(ctx, _store, repo, active); err != nil {
+		if err = Cancel(ctx, _store, repo, user, active); err != nil {
 			log.Error().
 				Err(err).
 				Str("Ref", active.Ref).

server/pipeline/create.go

@@ -123,9 +123,7 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, pipeline
 			log.Error().Err(err).Msg("publishToTopic")
 		}
 
-		if err := updatePipelineStatus(ctx, pipeline, repo, repoUser); err != nil {
+		updatePipelineStatus(ctx, pipeline, repo, repoUser)
-			log.Error().Err(err).Msg("updatePipelineStatus")
-		}
 
 		return pipeline, nil
 	}
@@ -142,9 +140,7 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, pipeline
 			log.Error().Err(err).Msg("publishToTopic")
 		}
 
-		if err := updatePipelineStatus(ctx, pipeline, repo, repoUser); err != nil {
+		updatePipelineStatus(ctx, pipeline, repo, repoUser)
-			log.Error().Err(err).Msg("updatePipelineStatus")
-		}
 
 		return pipeline, nil
 	}

server/pipeline/decline.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	"github.com/rs/zerolog/log"
+
 	"github.com/woodpecker-ci/woodpecker/server/model"
 	"github.com/woodpecker-ci/woodpecker/server/store"
 )
@@ -42,9 +43,7 @@ func Decline(ctx context.Context, store store.Store, pipeline *model.Pipeline, u
 		log.Error().Err(err).Msg("can not build tree from step list")
 	}
 
-	if err := updatePipelineStatus(ctx, pipeline, repo, user); err != nil {
+	updatePipelineStatus(ctx, pipeline, repo, user)
-		log.Error().Err(err).Msg("updateBuildStatus")
-	}
 
 	if err := publishToTopic(ctx, pipeline, repo); err != nil {
 		log.Error().Err(err).Msg("publishToTopic")

server/pipeline/helper.go

@@ -23,7 +23,7 @@ import (
 	"github.com/woodpecker-ci/woodpecker/server/model"
 )
 
-func updatePipelineStatus(ctx context.Context, pipeline *model.Pipeline, repo *model.Repo, user *model.User) error {
+func updatePipelineStatus(ctx context.Context, pipeline *model.Pipeline, repo *model.Repo, user *model.User) {
 	for _, step := range pipeline.Steps {
 		// skip child steps
 		if !step.IsParent() {
@@ -33,9 +33,7 @@ func updatePipelineStatus(ctx context.Context, pipeline *model.Pipeline, repo *m
 		err := server.Config.Services.Forge.Status(ctx, user, repo, pipeline, step)
 		if err != nil {
 			log.Error().Err(err).Msgf("error setting commit status for %s/%d", repo.FullName, pipeline.Number)
-			return err
+			return
 		}
 	}
-
-	return nil
 }

server/pipeline/items.go

@@ -28,7 +28,7 @@ import (
 	"github.com/woodpecker-ci/woodpecker/server/store"
 )
 
-func createPipelineItems(_ context.Context, store store.Store,
+func createPipelineItems(c context.Context, store store.Store,
 	currentPipeline *model.Pipeline, user *model.User, repo *model.Repo,
 	yamls []*forge_types.FileMeta, envs map[string]string,
 ) (*model.Pipeline, []*pipeline.Item, error) {
@@ -83,6 +83,8 @@ func createPipelineItems(_ context.Context, store store.Store,
 		currentPipeline, uerr := UpdateToStatusError(store, *currentPipeline, err)
 		if uerr != nil {
 			log.Error().Err(err).Msgf("Error setting error status of pipeline for %s#%d", repo.FullName, currentPipeline.Number)
+		} else {
+			updatePipelineStatus(c, currentPipeline, repo, user)
 		}
 		return currentPipeline, nil, err
 	}

server/pipeline/restart.go

@@ -30,17 +30,17 @@ import (
 )
 
 // Restart a pipeline by creating a new one out of the old and start it
-func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline, user *model.User, repo *model.Repo, envs map[string]string) (*model.Pipeline, error) {
+func Restart(ctx context.Context, store store.Store, lastPipeline *model.Pipeline, user *model.User, repo *model.Repo, envs map[string]string) (*model.Pipeline, error) {
-	switch lastBuild.Status {
+	switch lastPipeline.Status {
 	case model.StatusDeclined,
 		model.StatusBlocked:
-		return nil, &ErrBadRequest{Msg: fmt.Sprintf("cannot restart a pipeline with status %s", lastBuild.Status)}
+		return nil, &ErrBadRequest{Msg: fmt.Sprintf("cannot restart a pipeline with status %s", lastPipeline.Status)}
 	}
 
 	var pipelineFiles []*forge_types.FileMeta
 
 	// fetch the old pipeline config from database
-	configs, err := store.ConfigsForPipeline(lastBuild.ID)
+	configs, err := store.ConfigsForPipeline(lastPipeline.ID)
 	if err != nil {
 		msg := fmt.Sprintf("failure to get pipeline config for %s. %s", repo.FullName, err)
 		log.Error().Msgf(msg)
@@ -58,7 +58,7 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline,
 			currentFileMeta[i] = &forge_types.FileMeta{Name: cfg.Name, Data: cfg.Data}
 		}
 
-		newConfig, useOld, err := server.Config.Services.ConfigService.FetchConfig(ctx, repo, lastBuild, currentFileMeta)
+		newConfig, useOld, err := server.Config.Services.ConfigService.FetchConfig(ctx, repo, lastPipeline, currentFileMeta)
 		if err != nil {
 			return nil, &ErrBadRequest{
 				Msg: fmt.Sprintf("On fetching external pipeline config: %s", err),
@@ -69,10 +69,10 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline,
 		}
 	}
 
-	newBuild := createNewOutOfOld(lastBuild)
+	newPipeline := createNewOutOfOld(lastPipeline)
-	newBuild.Parent = lastBuild.ID
+	newPipeline.Parent = lastPipeline.ID
 
-	err = store.CreatePipeline(newBuild)
+	err = store.CreatePipeline(newPipeline)
 	if err != nil {
 		msg := fmt.Sprintf("failure to save pipeline for %s", repo.FullName)
 		log.Error().Err(err).Msg(msg)
@@ -80,36 +80,38 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline,
 	}
 
 	if len(configs) == 0 {
-		newBuild, uerr := UpdateToStatusError(store, *newBuild, errors.New("pipeline definition not found"))
+		newPipeline, uerr := UpdateToStatusError(store, *newPipeline, errors.New("pipeline definition not found"))
 		if uerr != nil {
 			log.Debug().Err(uerr).Msg("failure to update pipeline status")
+		} else {
+			updatePipelineStatus(ctx, newPipeline, repo, user)
 		}
-		return newBuild, nil
+		return newPipeline, nil
 	}
-	if err := persistPipelineConfigs(store, configs, newBuild.ID); err != nil {
+	if err := persistPipelineConfigs(store, configs, newPipeline.ID); err != nil {
 		msg := fmt.Sprintf("failure to persist pipeline config for %s.", repo.FullName)
 		log.Error().Err(err).Msg(msg)
 		return nil, fmt.Errorf(msg)
 	}
 
-	newBuild, pipelineItems, err := createPipelineItems(ctx, store, newBuild, user, repo, pipelineFiles, envs)
+	newPipeline, pipelineItems, err := createPipelineItems(ctx, store, newPipeline, user, repo, pipelineFiles, envs)
 	if err != nil {
 		if errors.Is(err, &yaml.PipelineParseError{}) {
-			return newBuild, nil
+			return newPipeline, nil
 		}
 		msg := fmt.Sprintf("failure to createBuildItems for %s", repo.FullName)
 		log.Error().Err(err).Msg(msg)
 		return nil, fmt.Errorf(msg)
 	}
 
-	newBuild, err = start(ctx, store, newBuild, user, repo, pipelineItems)
+	newPipeline, err = start(ctx, store, newPipeline, user, repo, pipelineItems)
 	if err != nil {
 		msg := fmt.Sprintf("failure to start pipeline for %s", repo.FullName)
 		log.Error().Err(err).Msg(msg)
 		return nil, fmt.Errorf(msg)
 	}
 
-	return newBuild, nil
+	return newPipeline, nil
 }
 
 // TODO: reuse at create.go too

server/pipeline/start.go

@@ -27,7 +27,7 @@ import (
 // start a pipeline, make sure it was stored persistent in the store before
 func start(ctx context.Context, store store.Store, activePipeline *model.Pipeline, user *model.User, repo *model.Repo, pipelineItems []*pipeline.Item) (*model.Pipeline, error) {
 	// call to cancel previous pipelines if needed
-	if err := cancelPreviousPipelines(ctx, store, activePipeline, repo); err != nil {
+	if err := cancelPreviousPipelines(ctx, store, activePipeline, repo, user); err != nil {
 		// should be not breaking
 		log.Error().Err(err).Msg("Failed to cancel previous pipelines")
 	}
@@ -46,9 +46,7 @@ func start(ctx context.Context, store store.Store, activePipeline *model.Pipelin
 		return nil, err
 	}
 
-	if err := updatePipelineStatus(ctx, activePipeline, repo, user); err != nil {
+	updatePipelineStatus(ctx, activePipeline, repo, user)
-		log.Error().Err(err).Msg("updateBuildStatus")
-	}
 
 	return activePipeline, nil
 }