Skip to content

Commit

Permalink
fix: bugs
Browse files Browse the repository at this point in the history 
Signed-off-by: xu.zhu <[email protected]>
  • Loading branch information
@xuzhu-591
xuzhu-591 committed Dec 7, 2023
1 parent d8d560d commit 036c679
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 101 deletions.
68 changes: 37 additions & 31 deletions core/controller/application/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
trschemamock "github.com/horizoncd/horizon/mock/pkg/templaterelease/schema"
"github.com/horizoncd/horizon/pkg/application/gitrepo"
"github.com/horizoncd/horizon/pkg/application/models"
appregionmodels "github.com/horizoncd/horizon/pkg/applicationregion/models"
userauth "github.com/horizoncd/horizon/pkg/authentication/user"
codemodels "github.com/horizoncd/horizon/pkg/cluster/code"
clustermodels "github.com/horizoncd/horizon/pkg/cluster/models"
Expand All @@ -36,6 +37,7 @@ import (
groupmodels "github.com/horizoncd/horizon/pkg/group/models"
groupservice "github.com/horizoncd/horizon/pkg/group/service"
membermodels "github.com/horizoncd/horizon/pkg/member/models"
"github.com/horizoncd/horizon/pkg/param"
"github.com/horizoncd/horizon/pkg/param/managerparam"
regionmodels "github.com/horizoncd/horizon/pkg/region/models"
tagmodels "github.com/horizoncd/horizon/pkg/tag/models"
Expand Down Expand Up @@ -298,6 +300,9 @@ func TestMain(m *testing.M) {
if err := db.AutoMigrate(&usermodel.User{}); err != nil {
panic(err)
}
if err := db.AutoMigrate(&appregionmodels.ApplicationRegion{}); err != nil {
panic(err)
}
ctx = context.TODO()
ctx = context.WithValue(ctx, common.UserContextKey(), &userauth.DefaultInfo{
Name: "Tony",
Expand Down Expand Up @@ -357,26 +362,28 @@ func Test(t *testing.T) {
_, err := manager.TemplateReleaseMgr.Create(ctx, tr)
assert.Nil(t, err)

c = &controller{
applicationGitRepo: applicationGitRepo,
templateSchemaGetter: templateSchemaGetter,
tagMgr: manager.TagMgr,
applicationMgr: manager.ApplicationMgr,
groupMgr: manager.GroupMgr,
groupSvc: groupservice.NewService(manager),
templateReleaseMgr: manager.TemplateReleaseMgr,
clusterMgr: manager.ClusterMgr,
userSvc: userservice.NewService(manager),
eventSvc: eventservice.New(manager),
memberManager: manager.MemberMgr,
params := &param.Param{
Manager: manager,
UserSvc: userservice.NewService(manager),
EventSvc: eventservice.New(manager),
GroupSvc: groupservice.NewService(manager),
ApplicationGitRepo: applicationGitRepo,
TemplateSchemaGetter: templateSchemaGetter,
}
c = NewController(params)

group, err := manager.GroupMgr.Create(ctx, &groupmodels.Group{
Name: "ABC",
Path: "abc",
})
assert.Nil(t, err)

groupToTransfer, err := manager.GroupMgr.Create(ctx, &groupmodels.Group{
Name: "ABC-transfer",
Path: "abc/transfer",
})
assert.Nil(t, err)

createRequest := &CreateApplicationRequest{
Base: Base{
Description: "this is a description",
Expand Down Expand Up @@ -445,6 +452,12 @@ func Test(t *testing.T) {

assert.Equal(t, resp.Description, updatedDescription)

err = c.Transfer(ctx, resp.ID, groupToTransfer.ID)
assert.Nil(t, err)
resp, err = c.GetApplication(ctx, resp.ID)
assert.Nil(t, err)
assert.Equal(t, resp.GroupID, groupToTransfer.ID)

err = c.DeleteApplication(ctx, resp.ID, false)
assert.Nil(t, err)

Expand Down Expand Up @@ -513,20 +526,15 @@ func TestV2(t *testing.T) {
}
_, err := manager.TemplateReleaseMgr.Create(ctx, tr)
assert.Nil(t, err)
c := &controller{
applicationGitRepo: applicationGitRepo,
templateSchemaGetter: templateSchemaGetter,
applicationMgr: manager.ApplicationMgr,
tagMgr: manager.TagMgr,
groupMgr: manager.GroupMgr,
groupSvc: groupservice.NewService(manager),
templateReleaseMgr: manager.TemplateReleaseMgr,
clusterMgr: manager.ClusterMgr,
userSvc: userservice.NewService(manager),
eventSvc: eventservice.New(manager),
memberManager: manager.MemberMgr,
params := &param.Param{
Manager: manager,
UserSvc: userservice.NewService(manager),
EventSvc: eventservice.New(manager),
GroupSvc: groupservice.NewService(manager),
ApplicationGitRepo: applicationGitRepo,
TemplateSchemaGetter: templateSchemaGetter,
}

c := NewController(params)
group, err := manager.GroupMgr.Create(ctx, &groupmodels.Group{
Name: "cde",
Path: "cde",
Expand Down Expand Up @@ -673,12 +681,10 @@ func TestListUserApplication(t *testing.T) {
applications = append(applications, application)
}

c = &controller{
applicationMgr: manager.ApplicationMgr,
groupMgr: manager.GroupMgr,
groupSvc: groupservice.NewService(manager),
memberManager: manager.MemberMgr,
}
c = NewController(&param.Param{
Manager: manager,
GroupSvc: groupservice.NewService(manager),
})

// nolint
ctx = context.WithValue(ctx, common.UserContextKey(), &userauth.DefaultInfo{
Expand Down
10 changes: 5 additions & 5 deletions core/controller/cluster/controller_internal_v2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,12 @@ func (c *controller) InternalDeployV2(ctx context.Context, clusterID uint,
if err != nil {
return nil, perror.WithMessage(err, op)
}
// 4. update config commit and status
if err := c.prMgr.PipelineRun.UpdateConfigCommitByID(ctx, pr.ID, commit); err != nil {
return nil, err
}
if err := updatePRStatus(prmodels.StatusCommitted, commit); err != nil {
return nil, err
}
}

// 5. merge branch from gitops to master if diff is not empty and update status
// 4. merge branch from gitops to master if diff is not empty and update status
configCommit, err := c.clusterGitRepo.GetConfigCommit(ctx, application.Name, cluster.Name)
if err != nil {
return nil, err
Expand All @@ -139,6 +135,10 @@ func (c *controller) InternalDeployV2(ctx context.Context, clusterID uint,
if err != nil {
return nil, perror.WithMessage(err, op)
}
// 5. update config commit and status
if err := c.prMgr.PipelineRun.UpdateConfigCommitByID(ctx, pr.ID, masterRevision); err != nil {
return nil, err
}
if err := updatePRStatus(prmodels.StatusMerged, masterRevision); err != nil {
return nil, err
}
Expand Down
33 changes: 12 additions & 21 deletions core/controller/pipelinerun/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ type Controller interface {
GetCheckRunByID(ctx context.Context, checkRunID uint) (*prmodels.CheckRun, error)
UpdateCheckRunByID(ctx context.Context, checkRunID uint, request *CreateOrUpdateCheckRunRequest) error

ListMessagesByPipelinerun(ctx context.Context, pipelinerunID uint, query *q.Query) (int, []*prmodels.PRMessage, error)
// Execute runs a pipelineRun only if its state is ready.
Execute(ctx context.Context, pipelinerunID uint, force bool) error
// Cancel withdraws a pipelineRun only if its state is pending.
Expand Down Expand Up @@ -367,14 +366,6 @@ func (c *controller) UpdateCheckRunByID(ctx context.Context, checkRunID uint,
return c.updatePrStatusByCheckrunID(ctx, checkRunID)
}

func (c *controller) ListMessagesByPipelinerun(ctx context.Context,
pipelinerunID uint, query *q.Query) (int, []*prmodels.PRMessage, error) {
const op = "pipelinerun controller: list pr message"
defer wlog.Start(ctx, op).StopPrint()

return c.prMgr.Message.List(ctx, pipelinerunID, query)
}

func (c *controller) Execute(ctx context.Context, pipelinerunID uint, force bool) error {
const op = "pipelinerun controller: execute pipelinerun"
defer wlog.Start(ctx, op).StopPrint()
Expand Down Expand Up @@ -626,7 +617,7 @@ func (c *controller) executeRollback(ctx context.Context, application *appmodels
// 5. update template and tags in db
cluster, err = c.clusterSvc.SyncDBWithGitRepo(ctx, application, cluster)
if err != nil {
return perror.Wrapf(err, "failed to sync db with git repo, cluster = %s", cluster.Name)
return perror.Wrapf(err, "failed to sync db with git repo")
}

// 6. create cluster in cd system
Expand Down Expand Up @@ -765,7 +756,7 @@ func (c *controller) ListPRMessages(ctx context.Context,
return 0, nil, err
}
userIDs := make([]uint, 0, len(messages))
m := make(map[uint]struct{}, 0)
m := make(map[uint]struct{})
for _, message := range messages {
if _, ok := m[message.CreatedBy]; !ok {
userIDs = append(userIDs, message.CreatedBy)
Expand All @@ -785,30 +776,30 @@ func (c *controller) ListPRMessages(ctx context.Context,
return 0, nil, err
}
userMap := make(map[uint]*usermodels.User, 0)
for _, user := range users {
userMap[user.ID] = user
for _, u := range users {
userMap[u.ID] = u
}
result := make([]*PrMessage, 0, len(messages))
for _, message := range messages {
resultMsg := &PrMessage{
Content: message.Content,
CreatedAt: message.CreatedAt,
}
if user, ok := userMap[message.CreatedBy]; ok {
if u, ok := userMap[message.CreatedBy]; ok {
resultMsg.CreatedBy = User{
ID: user.ID,
Name: user.FullName,
ID: u.ID,
Name: u.FullName,
}
if user.UserType == usermodels.UserTypeRobot {
if u.UserType == usermodels.UserTypeRobot {
resultMsg.CreatedBy.UserType = "bot"
}
}
if user, ok := userMap[message.UpdatedBy]; ok {
if u, ok := userMap[message.UpdatedBy]; ok {
resultMsg.UpdatedBy = User{
ID: user.ID,
Name: user.FullName,
ID: u.ID,
Name: u.FullName,
}
if user.UserType == usermodels.UserTypeRobot {
if u.UserType == usermodels.UserTypeRobot {
resultMsg.CreatedBy.UserType = "bot"
}
}
Expand Down
52 changes: 26 additions & 26 deletions core/controller/pipelinerun/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func TestExecutePipelineRun(t *testing.T) {
&usermodel.User{}, &trmodels.TemplateRelease{}, &eventmodels.Event{}); err != nil {
panic(err)
}
param := managerparam.InitManager(db)
mgr := managerparam.InitManager(db)
ctx := context.Background()
// nolint
ctx = context.WithValue(ctx, common.UserContextKey(), &userauth.DefaultInfo{
Expand All @@ -428,62 +428,62 @@ func TestExecutePipelineRun(t *testing.T) {
mockClusterGitRepo := clustergitrepomock.NewMockClusterGitRepo(mockCtl)
mockCD := cdmock.NewMockCD(mockCtl)

groupSvc := groupservice.NewService(manager)
eventSvc := eventservice.New(manager)
applicationSvc := applicationservice.NewService(groupSvc, manager)
clusterSvc := clusterservice.NewService(applicationSvc, mockClusterGitRepo, manager)
groupSvc := groupservice.NewService(mgr)
eventSvc := eventservice.New(mgr)
applicationSvc := applicationservice.NewService(groupSvc, mgr)
clusterSvc := clusterservice.NewService(applicationSvc, mockClusterGitRepo, mgr)

ctrl := controller{
prMgr: param.PRMgr,
appMgr: param.ApplicationMgr,
clusterMgr: param.ClusterMgr,
envMgr: param.EnvMgr,
regionMgr: param.RegionMgr,
prMgr: mgr.PRMgr,
appMgr: mgr.ApplicationMgr,
clusterMgr: mgr.ClusterMgr,
envMgr: mgr.EnvMgr,
regionMgr: mgr.RegionMgr,
tektonFty: mockFactory,
tokenSvc: tokenservice.NewService(param, tokenConfig),
tokenSvc: tokenservice.NewService(mgr, tokenConfig),
tokenConfig: tokenConfig,
clusterGitRepo: mockClusterGitRepo,
templateReleaseMgr: param.TemplateReleaseMgr,
templateReleaseMgr: mgr.TemplateReleaseMgr,
cd: mockCD,
clusterSvc: clusterSvc,
eventSvc: eventSvc,
}

_, err1 := param.EventMgr.CreateEvent(ctx, &eventmodels.Event{
_, err1 := mgr.EventMgr.CreateEvent(ctx, &eventmodels.Event{
EventSummary: eventmodels.EventSummary{
ResourceID: 1,
},
})
assert.NoError(t, err1)

_, err := param.UserMgr.Create(ctx, &usermodel.User{
_, err := mgr.UserMgr.Create(ctx, &usermodel.User{
Name: "Tony",
})
assert.NoError(t, err)

group, err := param.GroupMgr.Create(ctx, &groupmodels.Group{
group, err := mgr.GroupMgr.Create(ctx, &groupmodels.Group{
Name: "test",
})
assert.NoError(t, err)

app, err := param.ApplicationMgr.Create(ctx, &applicationmodel.Application{
app, err := mgr.ApplicationMgr.Create(ctx, &applicationmodel.Application{
Name: "test",
GroupID: group.ID,
}, nil)
assert.NoError(t, err)

registryID, err := param.RegistryMgr.Create(ctx, &registrymodels.Registry{
registryID, err := mgr.RegistryMgr.Create(ctx, &registrymodels.Registry{
Name: "test",
})
assert.NoError(t, err)

region, err := param.RegionMgr.Create(ctx, &regionmodels.Region{
region, err := mgr.RegionMgr.Create(ctx, &regionmodels.Region{
Name: "test",
RegistryID: registryID,
})
assert.NoError(t, err)

cluster, err := param.ClusterMgr.Create(ctx, &clustermodel.Cluster{
cluster, err := mgr.ClusterMgr.Create(ctx, &clustermodel.Cluster{
Name: "clusterGit",
ApplicationID: app.ID,
GitURL: "hello",
Expand All @@ -493,7 +493,7 @@ func TestExecutePipelineRun(t *testing.T) {
}, nil, nil)
assert.NoError(t, err)

_, err = param.TemplateReleaseMgr.Create(ctx, &trmodels.TemplateRelease{
_, err = mgr.TemplateReleaseMgr.Create(ctx, &trmodels.TemplateRelease{
TemplateName: "javaapp",
Name: "v1.0.0",
})
Expand Down Expand Up @@ -541,31 +541,31 @@ func TestExecutePipelineRun(t *testing.T) {
mockCD.EXPECT().CreateCluster(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockCD.EXPECT().DeployCluster(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()

prPending, err := param.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
prPending, err := mgr.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
ClusterID: cluster.ID,
Action: prmodels.ActionBuildDeploy,
Status: string(pipelinemodel.StatusPending),
})
assert.NoError(t, err)
assert.Equal(t, string(pipelinemodel.StatusPending), prPending.Status)

prDeployReady, err := param.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
prDeployReady, err := mgr.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
ClusterID: cluster.ID,
Action: prmodels.ActionDeploy,
Status: string(pipelinemodel.StatusReady),
})
assert.NoError(t, err)
assert.Equal(t, string(pipelinemodel.StatusReady), prDeployReady.Status)

prRestartReady, err := param.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
prRestartReady, err := mgr.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
ClusterID: cluster.ID,
Action: prmodels.ActionRestart,
Status: string(pipelinemodel.StatusReady),
})
assert.NoError(t, err)
assert.Equal(t, string(pipelinemodel.StatusReady), prRestartReady.Status)

prOK, err := param.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
prOK, err := mgr.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
ClusterID: cluster.ID,
Action: prmodels.ActionBuildDeploy,
Status: string(pipelinemodel.StatusOK),
Expand All @@ -575,7 +575,7 @@ func TestExecutePipelineRun(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, string(pipelinemodel.StatusOK), prOK.Status)

prRollbackReady, err := param.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
prRollbackReady, err := mgr.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
ClusterID: cluster.ID,
Action: prmodels.ActionRollback,
Status: string(pipelinemodel.StatusReady),
Expand All @@ -599,7 +599,7 @@ func TestExecutePipelineRun(t *testing.T) {
err = ctrl.Execute(ctx, prPending.ID, true)
assert.NoError(t, err)

PRCancel, err := param.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
PRCancel, err := mgr.PRMgr.PipelineRun.Create(ctx, &prmodels.Pipelinerun{
ClusterID: cluster.ID,
Status: string(pipelinemodel.StatusPending),
})
Expand Down
Loading

0 comments on commit 036c679

Please sign in to comment.