Skip to content

Commit 684c297

Browse files
Copilotprakarsh-dt
andcommitted
Add database schema and repository methods for auto-abort previous builds feature
Co-authored-by: prakarsh-dt <[email protected]>
1 parent 2d200ba commit 684c297

File tree

6 files changed

+218
-0
lines changed

6 files changed

+218
-0
lines changed

internal/sql/repository/pipelineConfig/CiPipelineRepository.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type CiPipeline struct {
5050
ScanEnabled bool `sql:"scan_enabled,notnull"`
5151
IsDockerConfigOverridden bool `sql:"is_docker_config_overridden, notnull"`
5252
PipelineType string `sql:"ci_pipeline_type"`
53+
AutoAbortPreviousBuilds bool `sql:"auto_abort_previous_builds,notnull"`
5354
sql.AuditLog
5455
CiPipelineMaterials []*CiPipelineMaterial
5556
CiTemplate *CiTemplate

internal/sql/repository/pipelineConfig/CiWorkflowRepository.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ type CiWorkflowRepository interface {
3838
FindCiWorkflowGitTriggersById(id int) (workflow *CiWorkflow, err error)
3939
FindCiWorkflowGitTriggersByIds(ids []int) ([]*CiWorkflow, error)
4040
FindByName(name string) (*CiWorkflow, error)
41+
FindRunningWorkflowsForPipeline(pipelineId int) ([]*CiWorkflow, error)
4142

4243
FindLastTriggeredWorkflowByCiIds(pipelineId []int) (ciWorkflow []*CiWorkflow, err error)
4344
FindWorkflowsByCiWorkflowIds(ciWorkflowIds []int) (ciWorkflow []*CiWorkflow, err error)
@@ -187,6 +188,20 @@ func (impl *CiWorkflowRepositoryImpl) FindByStatusesIn(activeStatuses []string)
187188
return ciWorkFlows, err
188189
}
189190

191+
func (impl *CiWorkflowRepositoryImpl) FindRunningWorkflowsForPipeline(pipelineId int) ([]*CiWorkflow, error) {
192+
var ciWorkFlows []*CiWorkflow
193+
// Status values for running/pending workflows that can be aborted
194+
runningStatuses := []string{"Running", "Starting", "Pending"}
195+
196+
err := impl.dbConnection.Model(&ciWorkFlows).
197+
Column("ci_workflow.*", "CiPipeline").
198+
Where("ci_workflow.ci_pipeline_id = ?", pipelineId).
199+
Where("ci_workflow.status in (?)", pg.In(runningStatuses)).
200+
Order("ci_workflow.started_on DESC").
201+
Select()
202+
return ciWorkFlows, err
203+
}
204+
190205
// FindByPipelineId gets only those workflowWithArtifact whose parent_ci_workflow_id is null, this is done to accommodate multiple ci_artifacts through a single workflow(parent), making child workflows for other ci_artifacts (this has been done due to design understanding and db constraint) single workflow single ci-artifact
191206
func (impl *CiWorkflowRepositoryImpl) FindByPipelineId(pipelineId int, offset int, limit int) ([]WorkflowWithArtifact, error) {
192207
var wfs []WorkflowWithArtifact

pkg/build/trigger/HandlerService.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ type HandlerService interface {
8484
GetRunningWorkflowLogs(workflowId int, followLogs bool) (*bufio.Reader, func() error, error)
8585
GetHistoricBuildLogs(workflowId int, ciWorkflow *pipelineConfig.CiWorkflow) (map[string]string, error)
8686
DownloadCiWorkflowArtifacts(pipelineId int, buildId int) (*os.File, error)
87+
abortPreviousRunningBuilds(pipelineId int, triggeredBy int32) error
8788
}
8889

8990
// CATEGORY=CI_BUILDX
@@ -707,6 +708,13 @@ func (impl *HandlerServiceImpl) triggerCiPipeline(trigger *types.CiTriggerReques
707708
return 0, err
708709
}
709710

711+
// Check if auto-abort is enabled for this pipeline and abort previous builds if needed
712+
err = impl.abortPreviousRunningBuilds(trigger.PipelineId, trigger.TriggeredBy)
713+
if err != nil {
714+
impl.Logger.Errorw("error in aborting previous running builds", "pipelineId", trigger.PipelineId, "err", err)
715+
// Log error but don't fail the trigger - previous builds aborting is a best-effort operation
716+
}
717+
710718
err = impl.executeCiPipeline(workflowRequest)
711719
if err != nil {
712720
impl.Logger.Errorw("error in executing ci pipeline", "err", err)
@@ -2065,3 +2073,84 @@ func (impl *HandlerServiceImpl) DownloadCiWorkflowArtifacts(pipelineId int, buil
20652073
impl.Logger.Infow("Downloaded ", "filename", file.Name(), "bytes", numBytes)
20662074
return file, nil
20672075
}
2076+
2077+
// abortPreviousRunningBuilds checks if auto-abort is enabled for the pipeline and aborts previous running builds
2078+
func (impl *HandlerServiceImpl) abortPreviousRunningBuilds(pipelineId int, triggeredBy int32) error {
2079+
// Get pipeline configuration to check if auto-abort is enabled
2080+
ciPipeline, err := impl.ciPipelineRepository.FindById(pipelineId)
2081+
if err != nil {
2082+
impl.Logger.Errorw("error in finding ci pipeline", "pipelineId", pipelineId, "err", err)
2083+
return err
2084+
}
2085+
2086+
// Check if auto-abort is enabled for this pipeline
2087+
if !ciPipeline.AutoAbortPreviousBuilds {
2088+
impl.Logger.Debugw("auto-abort not enabled for pipeline", "pipelineId", pipelineId)
2089+
return nil
2090+
}
2091+
2092+
// Find all running/pending workflows for this pipeline
2093+
runningWorkflows, err := impl.ciWorkflowRepository.FindRunningWorkflowsForPipeline(pipelineId)
2094+
if err != nil {
2095+
impl.Logger.Errorw("error in finding running workflows for pipeline", "pipelineId", pipelineId, "err", err)
2096+
return err
2097+
}
2098+
2099+
if len(runningWorkflows) == 0 {
2100+
impl.Logger.Debugw("no running workflows found to abort for pipeline", "pipelineId", pipelineId)
2101+
return nil
2102+
}
2103+
2104+
impl.Logger.Infow("found running workflows to abort due to auto-abort configuration",
2105+
"pipelineId", pipelineId, "workflowCount", len(runningWorkflows), "triggeredBy", triggeredBy)
2106+
2107+
// Abort each running workflow
2108+
for _, workflow := range runningWorkflows {
2109+
// Check if the workflow is in a critical phase that should not be aborted
2110+
if impl.isWorkflowInCriticalPhase(workflow) {
2111+
impl.Logger.Infow("skipping abort of workflow in critical phase",
2112+
"workflowId", workflow.Id, "status", workflow.Status, "pipelineId", pipelineId)
2113+
continue
2114+
}
2115+
2116+
// Attempt to cancel the build
2117+
_, err := impl.CancelBuild(workflow.Id, false)
2118+
if err != nil {
2119+
impl.Logger.Errorw("error aborting previous running build",
2120+
"workflowId", workflow.Id, "pipelineId", pipelineId, "err", err)
2121+
// Continue with other workflows even if one fails
2122+
continue
2123+
}
2124+
2125+
impl.Logger.Infow("successfully aborted previous running build due to auto-abort",
2126+
"workflowId", workflow.Id, "pipelineId", pipelineId, "abortedBy", triggeredBy)
2127+
}
2128+
2129+
return nil
2130+
}
2131+
2132+
// isWorkflowInCriticalPhase determines if a workflow is in a critical phase and should not be aborted
2133+
// This protects builds that are in the final stages like pushing cache or artifacts
2134+
func (impl *HandlerServiceImpl) isWorkflowInCriticalPhase(workflow *pipelineConfig.CiWorkflow) bool {
2135+
// For now, we consider "Starting" as safe to abort, but "Running" needs more careful consideration
2136+
// In the future, this could be extended to check actual workflow steps/stages
2137+
2138+
// If workflow has been running for less than 2 minutes, it's likely still in setup phase
2139+
if workflow.Status == "Running" && workflow.StartedOn.IsZero() == false {
2140+
runningDuration := time.Since(workflow.StartedOn)
2141+
if runningDuration < 2*time.Minute {
2142+
impl.Logger.Debugw("workflow is in early running phase, safe to abort",
2143+
"workflowId", workflow.Id, "runningDuration", runningDuration.String())
2144+
return false
2145+
}
2146+
2147+
// For workflows running longer, we should be more cautious
2148+
// This could be extended to check actual workflow phases using workflow service APIs
2149+
impl.Logger.Debugw("workflow has been running for a while, considering as critical phase",
2150+
"workflowId", workflow.Id, "runningDuration", runningDuration.String())
2151+
return true
2152+
}
2153+
2154+
// "Starting" and "Pending" are generally safe to abort
2155+
return false
2156+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright (c) 2024. Devtron Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package trigger
18+
19+
import (
20+
"testing"
21+
"time"
22+
23+
"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig"
24+
"github.com/devtron-labs/devtron/pkg/pipeline/types"
25+
"github.com/stretchr/testify/assert"
26+
)
27+
28+
func TestHandlerServiceImpl_isWorkflowInCriticalPhase(t *testing.T) {
29+
// Create a handler service instance for testing
30+
handlerService := &HandlerServiceImpl{}
31+
32+
t.Run("Starting workflow should not be in critical phase", func(t *testing.T) {
33+
workflow := &pipelineConfig.CiWorkflow{
34+
Id: 1,
35+
Status: "Starting",
36+
StartedOn: time.Now(),
37+
}
38+
result := handlerService.isWorkflowInCriticalPhase(workflow)
39+
assert.False(t, result, "Starting workflow should not be in critical phase")
40+
})
41+
42+
t.Run("Pending workflow should not be in critical phase", func(t *testing.T) {
43+
workflow := &pipelineConfig.CiWorkflow{
44+
Id: 2,
45+
Status: "Pending",
46+
StartedOn: time.Now(),
47+
}
48+
result := handlerService.isWorkflowInCriticalPhase(workflow)
49+
assert.False(t, result, "Pending workflow should not be in critical phase")
50+
})
51+
52+
t.Run("Recently started Running workflow should not be in critical phase", func(t *testing.T) {
53+
workflow := &pipelineConfig.CiWorkflow{
54+
Id: 3,
55+
Status: "Running",
56+
StartedOn: time.Now().Add(-1 * time.Minute), // Started 1 minute ago
57+
}
58+
result := handlerService.isWorkflowInCriticalPhase(workflow)
59+
assert.False(t, result, "Recently started Running workflow should not be in critical phase")
60+
})
61+
62+
t.Run("Long running workflow should be in critical phase", func(t *testing.T) {
63+
workflow := &pipelineConfig.CiWorkflow{
64+
Id: 4,
65+
Status: "Running",
66+
StartedOn: time.Now().Add(-5 * time.Minute), // Started 5 minutes ago
67+
}
68+
result := handlerService.isWorkflowInCriticalPhase(workflow)
69+
assert.True(t, result, "Long running workflow should be in critical phase")
70+
})
71+
72+
t.Run("Running workflow with zero StartedOn should not be in critical phase", func(t *testing.T) {
73+
workflow := &pipelineConfig.CiWorkflow{
74+
Id: 5,
75+
Status: "Running",
76+
StartedOn: time.Time{}, // Zero time
77+
}
78+
result := handlerService.isWorkflowInCriticalPhase(workflow)
79+
assert.False(t, result, "Running workflow with zero StartedOn should not be in critical phase")
80+
})
81+
}
82+
83+
func TestCiTriggerRequest_HasPipelineId(t *testing.T) {
84+
t.Run("CiTriggerRequest should contain PipelineId", func(t *testing.T) {
85+
triggerRequest := &types.CiTriggerRequest{
86+
PipelineId: 123,
87+
TriggeredBy: 1,
88+
}
89+
assert.Equal(t, 123, triggerRequest.PipelineId, "CiTriggerRequest should have PipelineId field")
90+
assert.Equal(t, int32(1), triggerRequest.TriggeredBy, "CiTriggerRequest should have TriggeredBy field")
91+
})
92+
}
93+
94+
func TestCiPipeline_HasAutoAbortField(t *testing.T) {
95+
t.Run("CiPipeline should have AutoAbortPreviousBuilds field", func(t *testing.T) {
96+
pipeline := &pipelineConfig.CiPipeline{
97+
Id: 1,
98+
AutoAbortPreviousBuilds: true,
99+
}
100+
assert.True(t, pipeline.AutoAbortPreviousBuilds, "CiPipeline should have AutoAbortPreviousBuilds field")
101+
})
102+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-- Remove auto-abort configuration from ci_pipeline
2+
DROP INDEX IF EXISTS idx_ci_pipeline_auto_abort;
3+
ALTER TABLE ci_pipeline DROP COLUMN IF EXISTS auto_abort_previous_builds;
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-- Add configuration for auto-abort previous builds feature
2+
ALTER TABLE ci_pipeline
3+
ADD COLUMN IF NOT EXISTS auto_abort_previous_builds BOOLEAN DEFAULT FALSE;
4+
5+
-- Add index for performance when querying by pipeline id and auto abort setting
6+
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_ci_pipeline_auto_abort
7+
ON ci_pipeline (id, auto_abort_previous_builds)
8+
WHERE auto_abort_previous_builds = TRUE;

0 commit comments

Comments
 (0)