Merge pull request #190 from xuzhu-591/fix-bugs

Fix: creating instance fails and fix image url

core/controller/cluster/controller.go

@@ -17,6 +17,7 @@ package cluster
 import (
 	"context"
 
+	templatemanager "github.com/horizoncd/horizon/pkg/template/manager"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/horizoncd/horizon/core/config"
@@ -133,6 +134,7 @@ type controller struct {
 	applicationMgr        appmanager.Manager
 	autoFreeSvc           *service.AutoFreeSVC
 	applicationSvc        applicationservice.Service
+	templateMgr           templatemanager.Manager
 	templateReleaseMgr    trmanager.Manager
 	templateSchemaGetter  templateschema.Getter
 	outputGetter          output.Getter
@@ -172,6 +174,7 @@ func NewController(config *config.Config, param *param.Param) Controller {
 		k8sutil:               param.K8sUtil,
 		applicationMgr:        param.ApplicationMgr,
 		applicationSvc:        param.ApplicationSvc,
+		templateMgr:           param.TemplateMgr,
 		templateReleaseMgr:    param.TemplateReleaseMgr,
 		templateSchemaGetter:  param.TemplateSchemaGetter,
 		autoFreeSvc:           param.AutoFreeSvc,

core/controller/cluster/controller_basic_v2.go

@@ -115,11 +115,9 @@ func (c *controller) CreateClusterV2(ctx context.Context,
 	if err := validateClusterName(params.Name); err != nil {
 		return nil, err
 	}
-	if params.Git != nil {
-		if params.Git.URL != "" {
-			if err := validate.CheckGitURL(params.Git.URL); err != nil {
-				return nil, err
-			}
+	if params.Git != nil && params.Git.URL != "" {
+		if err := validate.CheckGitURL(params.Git.URL); err != nil {
+			return nil, err
 		}
 	}
 	if params.Image != nil {
@@ -165,7 +163,11 @@ func (c *controller) CreateClusterV2(ctx context.Context,
 		return nil, err
 	}
 
-	// 7. get templateRelease
+	// 7. get template and templateRelease
+	template, err := c.templateMgr.GetByName(ctx, buildTemplateInfo.TemplateInfo.Name)
+	if err != nil {
+		return nil, err
+	}
 	tr, err := c.templateReleaseMgr.GetByTemplateNameAndRelease(ctx,
 		buildTemplateInfo.TemplateInfo.Name, buildTemplateInfo.TemplateInfo.Release)
 	if err != nil {
@@ -174,7 +176,7 @@ func (c *controller) CreateClusterV2(ctx context.Context,
 
 	// 8. customize db infos
 	cluster, tags := params.toClusterModel(application,
-		envEntity, buildTemplateInfo, expireSeconds)
+		envEntity, buildTemplateInfo, template, expireSeconds)
 
 	// 9. update db and tags
 	clusterResp, err := c.clusterMgr.Create(ctx, cluster, tags, params.ExtraMembers)

core/controller/cluster/controller_build_deploy.go

@@ -203,12 +203,15 @@ func assembleImageURL(regionEntity *regionmodels.RegionEntity,
 	normalizedBranch := strings.Join(pinyin.LazyPinyin(branch, args), "")
 	normalizedBranch = regexp.MustCompile(`[^a-zA-Z0-9_.-]`).ReplaceAllString(normalizedBranch, "_")
 
-	if len(commit) > 8 {
-		commit = commit[:8]
-	}
-
+	normalizedCommit := func(commit string) string {
+		res := commit
+		if len(res) > 8 {
+			res = res[:8]
+		}
+		return regexp.MustCompile(`[^a-zA-Z0-9_.-]`).ReplaceAllString(res, "_")
+	}(commit)
 	return path.Join(domain, regionEntity.Registry.Path, application,
-		fmt.Sprintf("%v:%v-%v-%v", cluster, normalizedBranch, commit, timeStr))
+		fmt.Sprintf("%v:%v-%v-%v", cluster, normalizedBranch, normalizedCommit, timeStr))
 }
 
 func (c *controller) GetDiff(ctx context.Context, clusterID uint, refType, ref string) (_ *GetDiffResponse, err error) {

core/controller/cluster/controller_build_deploy_test.go

@@ -101,6 +101,23 @@ func testImageURL(t *testing.T) {
 			},
 			want: "harbor.com/path/app/cluster:ceshi_zhongguohello_-117651f0",
 		},
+		{
+			name: "normal5",
+			args: args{
+				regionEntity: &regionmodels.RegionEntity{
+					Registry: &registrymodels.Registry{
+						Path:   "path",
+						Server: "https://harbor.com",
+					},
+				},
+				application: "app",
+				cluster:     "cluster",
+				branch:      "fix/bug",
+				// commit will be branch name if the repo could not be fetched
+				commit: "fix/bug",
+			},
+			want: "harbor.com/path/app/cluster:fix_bug-fix_bug",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

core/controller/cluster/controller_test.go

@@ -555,6 +555,7 @@ func test(t *testing.T) {
 		}, nil).AnyTimes()
 
 	appMgr := manager.ApplicationMgr
+	templateMgr := manager.TemplateMgr
 	trMgr := manager.TemplateReleaseMgr
 	envMgr := manager.EnvMgr
 	regionMgr := manager.RegionMgr
@@ -636,6 +637,7 @@ func test(t *testing.T) {
 		cd:                   cd,
 		k8sutil:              k8sutil,
 		applicationMgr:       appMgr,
+		templateMgr:          templateMgr,
 		templateReleaseMgr:   trMgr,
 		templateSchemaGetter: templateSchemaGetter,
 		envMgr:               envMgr,
@@ -1302,6 +1304,7 @@ func testV2(t *testing.T) {
 		}, nil).Times(1)
 
 	appMgr := manager.ApplicationMgr
+	templateMgr := manager.TemplateMgr
 	trMgr := manager.TemplateReleaseMgr
 	envMgr := manager.EnvMgr
 	regionMgr := manager.RegionMgr
@@ -1378,6 +1381,16 @@ func testV2(t *testing.T) {
 	}, nil)
 	assert.Nil(t, err)
 
+	_, err = templateMgr.Create(ctx, &templatemodels.Template{
+		Name:      "rollout",
+		ChartName: "rollout",
+		GroupID:   0,
+		OnlyOwner: nil,
+		WithoutCI: true,
+		Type:      templatemodels.TemplateTypeWorkload,
+	})
+	assert.Nil(t, err)
+
 	tr, err := trMgr.Create(ctx, &trmodels.TemplateRelease{
 		TemplateName: templateName,
 		Name:         "v1.0.0",
@@ -1391,6 +1404,7 @@ func testV2(t *testing.T) {
 		clusterMgr:           manager.ClusterMgr,
 		clusterGitRepo:       clusterGitRepo,
 		applicationMgr:       appMgr,
+		templateMgr:          templateMgr,
 		templateReleaseMgr:   trMgr,
 		templateSchemaGetter: templateSchemaGetter,
 		envMgr:               envMgr,
@@ -1615,6 +1629,7 @@ func testUpgrade(t *testing.T) {
 		clusterMgr:            manager.ClusterMgr,
 		clusterGitRepo:        clusterGitRepo,
 		applicationMgr:        appMgr,
+		templateMgr:           manager.TemplateMgr,
 		templateReleaseMgr:    trMgr,
 		templateSchemaGetter:  templateSchemaGetter,
 		envMgr:                envMgr,

core/controller/cluster/models_basic_v2.go

@@ -23,6 +23,7 @@ import (
 	"github.com/horizoncd/horizon/pkg/cluster/models"
 	envregionmodels "github.com/horizoncd/horizon/pkg/environmentregion/models"
 	tagmodels "github.com/horizoncd/horizon/pkg/tag/models"
+	templatemodels "github.com/horizoncd/horizon/pkg/template/models"
 )
 
 type CreateClusterRequestV2 struct {
@@ -53,7 +54,7 @@ type CreateClusterParamsV2 struct {
 
 func (r *CreateClusterParamsV2) toClusterModel(application *appmodels.Application,
 	er *envregionmodels.EnvironmentRegion, info *BuildTemplateInfo,
-	expireSeconds uint) (*models.Cluster, []*tagmodels.Tag) {
+	template *templatemodels.Template, expireSeconds uint) (*models.Cluster, []*tagmodels.Tag) {
 	cluster := &models.Cluster{
 		ApplicationID:   application.ID,
 		Name:            r.Name,
@@ -65,41 +66,58 @@ func (r *CreateClusterParamsV2) toClusterModel(application *appmodels.Applicatio
 		TemplateRelease: info.TemplateInfo.Release,
 		Status:          common.ClusterStatusCreating,
 	}
-	if cluster.Template == application.Template {
-		cluster.GitURL = func() string {
-			if r.Git == nil {
-				return application.GitURL
+	if template.Type == templatemodels.TemplateTypeWorkload {
+		// only workload template need git info and image info
+		if application.GitURL == "" && r.Git != nil {
+			// application's git info is miss, use git info from request directly
+			cluster.GitURL = r.Git.URL
+			cluster.GitSubfolder = r.Git.Subfolder
+			cluster.GitRef = r.Git.Ref()
+			cluster.GitRefType = r.Git.RefType()
+		} else if application.GitURL != "" && r.Git != nil {
+			if r.Git.URL != application.GitURL && r.Git.URL != "" {
+				// git url is not equal, do not inherit git info from application
+				cluster.GitURL = r.Git.URL
+				cluster.GitSubfolder = r.Git.Subfolder
+				cluster.GitRef = r.Git.Ref()
+				cluster.GitRefType = r.Git.RefType()
+			} else {
+				// use git info from cluster, default to application
+				cluster.GitURL = application.GitURL
+				cluster.GitSubfolder = func() string {
+					if r.Git.Subfolder != "" {
+						return r.Git.Subfolder
+					}
+					return application.GitSubfolder
+				}()
+				cluster.GitRef = func() string {
+					if r.Git.Ref() != "" {
+						return r.Git.Ref()
+					}
+					return application.GitRef
+				}()
+				cluster.GitRefType = func() string {
+					if r.Git.RefType() != "" {
+						return r.Git.RefType()
+					}
+					return application.GitRefType
+				}()
 			}
-			if r.Git.URL == "" && application.GitURL != "" {
-				return application.GitURL
-			}
-			// if URL is empty string, this means this cluster not depends on build from git
-			return r.Git.URL
-		}()
-		cluster.GitSubfolder = func() string {
-			if r.Git == nil || r.Git.Subfolder == "" {
-				return application.GitSubfolder
-			}
-			return r.Git.Subfolder
-		}()
-		cluster.GitRef = func() string {
-			if r.Git == nil {
-				return application.GitRef
-			}
-			return r.Git.Ref()
-		}()
-		cluster.GitRefType = func() string {
-			if r.Git == nil {
-				return application.GitRefType
-			}
-			return r.Git.RefType()
-		}()
-		cluster.Image = func() string {
-			if r.Image == nil {
+		} else if r.Image != nil {
+			cluster.Image = func() string {
+				if *r.Image != "" {
+					return *r.Image
+				}
 				return application.Image
-			}
-			return *r.Image
-		}()
+			}()
+		} else {
+			// git info and image info are both empty, use them from application
+			cluster.GitURL = application.GitURL
+			cluster.GitSubfolder = application.GitSubfolder
+			cluster.GitRef = application.GitRef
+			cluster.GitRefType = application.GitRefType
+			cluster.Image = application.Image
+		}
 	}
 	tags := make([]*tagmodels.Tag, 0)
 	for _, tag := range r.Tags {