Skip to content

Commit 2ae03e4

Browse files
duy0611duy0611
authored
fix: auto_merge not working properly with orchestrator backend (#1871)
* fix: auto_merge not working properly iwith orchestrator backend When running with Digger backend mode, PR is merged when `auto_merge` is enabled even though not all impacted projects have been applied. This is caused because Digger batch does not have context whether all the needed jobs have been triggered when using `digger apply` with `-p` flag. Unlike in backendless mode, where all the jobs are run sequentially and the parent one does have context if all the impacted projects have been applied, in backend mode jobs are triggered via the Batch one, which does have this context. This PR addresses the issue by adding one more column to store this context, which will then be used in the subsequent run.
1 parent 349de90 commit 2ae03e4

File tree

15 files changed

+110
-77
lines changed

15 files changed

+110
-77
lines changed

‎backend/controllers/github.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR
513513
return fmt.Errorf("error processing event")
514514
}
515515

516-
jobsForImpactedProjects, _, err := dg_github.ConvertGithubPullRequestEventToJobs(payload, impactedProjects, nil, *config, false)
516+
jobsForImpactedProjects, coverAllImpactedProjects, err := dg_github.ConvertGithubPullRequestEventToJobs(payload, impactedProjects, nil, *config, false)
517517
if err != nil {
518518
slog.Error("Error converting event to jobs",
519519
"prNumber", prNumber,
@@ -727,6 +727,7 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR
727727
0,
728728
aiSummaryCommentId,
729729
config.ReportTerraformOutputs,
730+
coverAllImpactedProjects,
730731
nil,
731732
)
732733
if err != nil {
@@ -1350,7 +1351,7 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu
13501351
"requestedProject", requestedProject,
13511352
)
13521353

1353-
jobs, _, err := generic.ConvertIssueCommentEventToJobs(repoFullName, actor, issueNumber, commentBody, impactedProjects, requestedProject, config.Workflows, prBranchName, defaultBranch)
1354+
jobs, coverAllImpactedProjects, err := generic.ConvertIssueCommentEventToJobs(repoFullName, actor, issueNumber, commentBody, impactedProjects, requestedProject, config.Workflows, prBranchName, defaultBranch)
13541355
if err != nil {
13551356
slog.Error("Error converting event to jobs",
13561357
"issueNumber", issueNumber,
@@ -1532,6 +1533,7 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu
15321533
0,
15331534
aiSummaryCommentId,
15341535
config.ReportTerraformOutputs,
1536+
coverAllImpactedProjects,
15351537
nil,
15361538
)
15371539
if err != nil {

‎backend/controllers/github_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,8 @@ func TestJobsTreeWithOneJobsAndTwoProjects(t *testing.T) {
724724
graph, err := configuration.CreateProjectDependencyGraph(projects)
725725
assert.NoError(t, err)
726726

727-
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 41584295, "", 2, "diggerhq", "parallel_jobs_demo", "diggerhq/parallel_jobs_demo", "", 123, "test", 0, "", false, nil)
727+
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 41584295, "", 2, "diggerhq", "parallel_jobs_demo", "diggerhq/parallel_jobs_demo", "", 123, "test", 0, "", false, true, nil)
728+
728729
assert.NoError(t, err)
729730
assert.Equal(t, 1, len(result))
730731
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobID)
@@ -753,7 +754,8 @@ func TestJobsTreeWithTwoDependantJobs(t *testing.T) {
753754
projectMap["dev"] = project1
754755
projectMap["prod"] = project2
755756

756-
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 123, "", 2, "", "", "test", "", 123, "test", 0, "", false, nil)
757+
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 123, "", 2, "", "", "test", "", 123, "test", 0, "", false, true, nil)
758+
757759
assert.NoError(t, err)
758760
assert.Equal(t, 2, len(result))
759761

@@ -786,7 +788,8 @@ func TestJobsTreeWithTwoIndependentJobs(t *testing.T) {
786788
projectMap["dev"] = project1
787789
projectMap["prod"] = project2
788790

789-
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 123, "", 2, "", "", "test", "", 123, "test", 0, "", false, nil)
791+
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 123, "", 2, "", "", "test", "", 123, "test", 0, "", false, true, nil)
792+
790793
assert.NoError(t, err)
791794
assert.Equal(t, 2, len(result))
792795
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobID)
@@ -831,7 +834,8 @@ func TestJobsTreeWithThreeLevels(t *testing.T) {
831834
projectMap["555"] = project5
832835
projectMap["666"] = project6
833836

834-
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 123, "", 2, "", "", "test", "", 123, "test", 0, "", false, nil)
837+
_, result, err := utils.ConvertJobsToDiggerJobs("", "github", 1, jobs, projectMap, graph, 123, "", 2, "", "", "test", "", 123, "test", 0, "", false, true, nil)
838+
835839
assert.NoError(t, err)
836840
assert.Equal(t, 6, len(result))
837841
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["111"].DiggerJobID)

‎backend/controllers/projects.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,7 @@ func AutomergePRforBatchIfEnabled(gh utils.GithubClientProvider, batch *models.D
13851385

13861386
if batch.Status == orchestrator_scheduler.BatchJobSucceeded &&
13871387
batch.BatchType == orchestrator_scheduler.DiggerCommandApply &&
1388+
batch.CoverAllImpactedProjects == true &&
13881389
automerge == true {
13891390

13901391
slog.Info("Conditions met for auto-merge, proceeding",

‎backend/controllers/projects_test.go‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
package controllers
22

33
import (
4+
"net/http"
5+
"testing"
6+
47
"github.com/diggerhq/digger/backend/models"
58
"github.com/diggerhq/digger/backend/utils"
69
orchestrator_scheduler "github.com/diggerhq/digger/libs/scheduler"
710
"github.com/google/go-github/v61/github"
811
"github.com/google/uuid"
912
"github.com/migueleliasweb/go-github-mock/src/mock"
1013
"github.com/stretchr/testify/assert"
11-
"net/http"
12-
"testing"
1314
)
1415

1516
func TestAutomergeWhenBatchIsSuccessfulStatus(t *testing.T) {
@@ -68,11 +69,12 @@ func TestAutomergeWhenBatchIsSuccessfulStatus(t *testing.T) {
6869
" - name: dev\n" +
6970
" dir: dev\n" +
7071
"auto_merge: false",
71-
GithubInstallationId: int64(41584295),
72-
RepoFullName: "diggerhq/github-job-scheduler",
73-
RepoOwner: "diggerhq",
74-
RepoName: "github-job-scheduler",
75-
BatchType: orchestrator_scheduler.DiggerCommandApply,
72+
GithubInstallationId: int64(41584295),
73+
RepoFullName: "diggerhq/github-job-scheduler",
74+
RepoOwner: "diggerhq",
75+
RepoName: "github-job-scheduler",
76+
BatchType: orchestrator_scheduler.DiggerCommandApply,
77+
CoverAllImpactedProjects: true,
7678
}
7779
err := AutomergePRforBatchIfEnabled(gh, &batch)
7880
assert.NoError(t, err)
@@ -94,8 +96,19 @@ func TestAutomergeWhenBatchIsSuccessfulStatus(t *testing.T) {
9496
" dir: dev\n" +
9597
"auto_merge: true"
9698
batch.BatchType = orchestrator_scheduler.DiggerCommandApply
99+
batch.CoverAllImpactedProjects = false
97100
err = AutomergePRforBatchIfEnabled(gh, &batch)
98101
assert.NoError(t, err)
99-
assert.True(t, isMergeCalled)
102+
assert.False(t, isMergeCalled)
100103

104+
batch.DiggerConfig = "" +
105+
"projects:\n" +
106+
" - name: dev\n" +
107+
" dir: dev\n" +
108+
"auto_merge: true"
109+
batch.BatchType = orchestrator_scheduler.DiggerCommandApply
110+
batch.CoverAllImpactedProjects = true
111+
err = AutomergePRforBatchIfEnabled(gh, &batch)
112+
assert.NoError(t, err)
113+
assert.True(t, isMergeCalled)
101114
}

‎backend/migrations/20250416152705.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-- Modify "digger_batches" table
2+
ALTER TABLE "public"."digger_batches" ADD COLUMN "cover_all_impacted_projects" boolean NULL;

‎backend/migrations/atlas.sum‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
h1:FUyT8bE1jsztutCMQI5+hbieTDYCpac4f1BQk/RksrM=
1+
h1:fdisQq5X2P2f4UyvwdQ2k7q9iqRSi02mm1ydhG0V0G8=
22
20231227132525.sql h1:43xn7XC0GoJsCnXIMczGXWis9d504FAWi4F1gViTIcw=
33
20240115170600.sql h1:IW8fF/8vc40+eWqP/xDK+R4K9jHJ9QBSGO6rN9LtfSA=
44
20240116123649.sql h1:R1JlUIgxxF6Cyob9HdtMqiKmx/BfnsctTl5rvOqssQw=
@@ -46,3 +46,4 @@ h1:FUyT8bE1jsztutCMQI5+hbieTDYCpac4f1BQk/RksrM=
4646
20250302190926.sql h1:F3FnaGnZv1Hwmg6W9Nacg5fbdiYbZGgS/mkuogtCso0=
4747
20250325115901.sql h1:yrha7g515WPkFRHfidvtLVWMeQmRD8rzVyWtPbuk0ws=
4848
20250325134924.sql h1:5vywDVuT0FPmQKP75AvxopxOeuKXsTEN00rgQjnA+ss=
49+
20250416152705.sql h1:yeszz4c/BlyVgUUIk7aTUz/DUNMC40+9fe1Q8Ya4TVI=

‎backend/models/scheduler.go‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,22 @@ const DiggerVCSGitlab DiggerVCSType = "gitlab"
2424
const DiggerVCSBitbucket DiggerVCSType = "bitbucket"
2525

2626
type DiggerBatch struct {
27-
ID uuid.UUID `gorm:"primary_key"`
28-
VCS DiggerVCSType
29-
PrNumber int
30-
CommentId *int64
31-
AiSummaryCommentId string
32-
Status orchestrator_scheduler.DiggerBatchStatus
33-
BranchName string
34-
DiggerConfig string
35-
GithubInstallationId int64
36-
GitlabProjectId int
37-
RepoFullName string
38-
RepoOwner string
39-
RepoName string
40-
BatchType orchestrator_scheduler.DiggerCommand
41-
ReportTerraformOutputs bool
27+
ID uuid.UUID `gorm:"primary_key"`
28+
VCS DiggerVCSType
29+
PrNumber int
30+
CommentId *int64
31+
AiSummaryCommentId string
32+
Status orchestrator_scheduler.DiggerBatchStatus
33+
BranchName string
34+
DiggerConfig string
35+
GithubInstallationId int64
36+
GitlabProjectId int
37+
RepoFullName string
38+
RepoOwner string
39+
RepoName string
40+
BatchType orchestrator_scheduler.DiggerCommand
41+
ReportTerraformOutputs bool
42+
CoverAllImpactedProjects bool
4243
// used for module source grouping comments
4344
SourceDetails []byte
4445
VCSConnectionId *uint ``

‎backend/models/storage.go‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -727,25 +727,26 @@ func (db *Database) GetDiggerBatch(batchId *uuid.UUID) (*DiggerBatch, error) {
727727
return batch, nil
728728
}
729729

730-
func (db *Database) CreateDiggerBatch(vcsType DiggerVCSType, githubInstallationId int64, repoOwner string, repoName string, repoFullname string, PRNumber int, diggerConfig string, branchName string, batchType scheduler.DiggerCommand, commentId *int64, gitlabProjectId int, aiSummaryCommentId string, reportTerraformOutputs bool, VCSConnectionId *uint) (*DiggerBatch, error) {
730+
func (db *Database) CreateDiggerBatch(vcsType DiggerVCSType, githubInstallationId int64, repoOwner string, repoName string, repoFullname string, PRNumber int, diggerConfig string, branchName string, batchType scheduler.DiggerCommand, commentId *int64, gitlabProjectId int, aiSummaryCommentId string, reportTerraformOutputs bool, coverAllImpactedProjects bool, VCSConnectionId *uint) (*DiggerBatch, error) {
731731
uid := uuid.New()
732732
batch := &DiggerBatch{
733-
ID: uid,
734-
VCS: vcsType,
735-
VCSConnectionId: VCSConnectionId,
736-
GithubInstallationId: githubInstallationId,
737-
RepoOwner: repoOwner,
738-
RepoName: repoName,
739-
RepoFullName: repoFullname,
740-
PrNumber: PRNumber,
741-
CommentId: commentId,
742-
Status: scheduler.BatchJobCreated,
743-
BranchName: branchName,
744-
DiggerConfig: diggerConfig,
745-
BatchType: batchType,
746-
GitlabProjectId: gitlabProjectId,
747-
AiSummaryCommentId: aiSummaryCommentId,
748-
ReportTerraformOutputs: reportTerraformOutputs,
733+
ID: uid,
734+
VCS: vcsType,
735+
VCSConnectionId: VCSConnectionId,
736+
GithubInstallationId: githubInstallationId,
737+
RepoOwner: repoOwner,
738+
RepoName: repoName,
739+
RepoFullName: repoFullname,
740+
PrNumber: PRNumber,
741+
CommentId: commentId,
742+
Status: scheduler.BatchJobCreated,
743+
BranchName: branchName,
744+
DiggerConfig: diggerConfig,
745+
BatchType: batchType,
746+
GitlabProjectId: gitlabProjectId,
747+
AiSummaryCommentId: aiSummaryCommentId,
748+
ReportTerraformOutputs: reportTerraformOutputs,
749+
CoverAllImpactedProjects: coverAllImpactedProjects,
749750
}
750751
result := db.GormDB.Save(batch)
751752
if result.Error != nil {

‎backend/models/storage_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func TestGetDiggerJobsForBatchPreloadsSummary(t *testing.T) {
143143
resourcesUpdated := uint(2)
144144
resourcesDeleted := uint(3)
145145

146-
batch, err := DB.CreateDiggerBatch(DiggerVCSGithub, 123, repoOwner, repoName, repoFullName, prNumber, diggerconfig, branchName, batchType, &commentId, 0, "", false, nil)
146+
batch, err := DB.CreateDiggerBatch(DiggerVCSGithub, 123, repoOwner, repoName, repoFullName, prNumber, diggerconfig, branchName, batchType, &commentId, 0, "", false, true, nil)
147147
assert.NoError(t, err)
148148

149149
job, err := DB.CreateDiggerJob(batch.ID, []byte(jobSpec), "workflow_file.yml")

‎backend/tasks/runs_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func TestThatRunQueueItemMovesFromQueuedToPlanningAfterPickup(t *testing.T) {
139139

140140
for i, testParam := range testParameters {
141141
ciService := github2.MockCiService{}
142-
batch, _ := models.DB.CreateDiggerBatch(models.DiggerVCSGithub, 123, "", "", "", 22, "", "", "", nil, 0, "", false, nil)
142+
batch, _ := models.DB.CreateDiggerBatch(models.DiggerVCSGithub, 123, "", "", "", 22, "", "", "", nil, 0, "", false, true, nil)
143143
project, _ := models.DB.CreateProject(fmt.Sprintf("test%v", i), nil, nil, false, false)
144144
planStage, _ := models.DB.CreateDiggerRunStage(batch.ID.String())
145145
applyStage, _ := models.DB.CreateDiggerRunStage(batch.ID.String())

0 commit comments

Comments
 (0)