From 13c0b0808168b3dbb0fbb489fd0d0f20cba2c88e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=B1=E6=99=93=E5=85=B5?= <596908030@qq.com> Date: Tue, 30 Nov 2021 17:24:29 +0800 Subject: [PATCH] Fix: update envBinding related workflow bug (#2831) * Fix: fix update envBinding related workflow bug * Fix: support cloudresource step * Fix: add unit test * Fix: add unit test * Fix: add unit test * unit test Co-authored-by: zhuxiaobing --- pkg/apiserver/rest/usecase/application.go | 7 +- .../rest/usecase/application_test.go | 8 +++ pkg/apiserver/rest/usecase/envbinding.go | 67 ++++++++++++++++++- pkg/apiserver/rest/usecase/envbinding_test.go | 24 +++++-- pkg/apiserver/rest/usecase/workflow.go | 32 +++++---- pkg/apiserver/rest/webservice/application.go | 4 +- 6 files changed, 117 insertions(+), 25 deletions(-) diff --git a/pkg/apiserver/rest/usecase/application.go b/pkg/apiserver/rest/usecase/application.go index db54b85b3..24827b01f 100644 --- a/pkg/apiserver/rest/usecase/application.go +++ b/pkg/apiserver/rest/usecase/application.go @@ -126,7 +126,8 @@ func (c *applicationUsecaseImpl) ListApplications(ctx context.Context, listOptio } var list []*apisv1.ApplicationBase for _, entity := range entitys { - appBase := c.converAppModelToBase(entity.(*model.Application)) + appModel := entity.(*model.Application) + appBase := c.converAppModelToBase(appModel) if listOptions.Query != "" && !(strings.Contains(appBase.Alias, listOptions.Query) || strings.Contains(appBase.Name, listOptions.Query) || @@ -134,8 +135,8 @@ func (c *applicationUsecaseImpl) ListApplications(ctx context.Context, listOptio continue } if listOptions.TargetName != "" { - targetIsContain, _ := c.envBindingUsecase.CheckAppEnvBindingsContainTarget(ctx, &app, listOptions.TargetName) - if targetIsContain { + targetIsContain, _ := c.envBindingUsecase.CheckAppEnvBindingsContainTarget(ctx, appModel, listOptions.TargetName) + if !targetIsContain { continue } } diff --git a/pkg/apiserver/rest/usecase/application_test.go b/pkg/apiserver/rest/usecase/application_test.go index af16fa2e9..87120f5d0 100644 --- a/pkg/apiserver/rest/usecase/application_test.go +++ b/pkg/apiserver/rest/usecase/application_test.go @@ -163,6 +163,14 @@ var _ = Describe("Test application usecase function", func() { Expect(err).Should(BeNil()) }) + It("Test ListApplications and filter by targetName function", func() { + list, err := appUsecase.ListApplications(context.TODO(), v1.ListApplicatioOptions{ + Namespace: "test-app-namespace", + TargetName: "dev-target"}) + Expect(err).Should(BeNil()) + Expect(cmp.Diff(len(list), 2)).Should(BeEmpty()) + }) + It("Test DetailApplication function", func() { appModel, err := appUsecase.GetApplication(context.TODO(), "test-app-sadasd") Expect(err).Should(BeNil()) diff --git a/pkg/apiserver/rest/usecase/envbinding.go b/pkg/apiserver/rest/usecase/envbinding.go index 3c615726f..7d133caab 100644 --- a/pkg/apiserver/rest/usecase/envbinding.go +++ b/pkg/apiserver/rest/usecase/envbinding.go @@ -255,8 +255,44 @@ func (e *envBindingUsecaseImpl) createEnvWorkflow(ctx context.Context, app *mode } func (e *envBindingUsecaseImpl) updateEnvWorkflow(ctx context.Context, app *model.Application, env *model.EnvBinding) error { - // TODO: update env workflow // The existing step configuration should be maintained and the delivery target steps should be automatically updated. + envSteps := e.genEnvWorkflowSteps(ctx, env, app) + workflow, err := e.workflowUsecase.GetWorkflow(ctx, app, convertWorkflowName(env.Name)) + if err != nil { + return err + } + + var envStepNames = env.TargetNames + var workflowStepNames []string + for _, step := range workflow.Steps { + if isEnvStepType(step.Type) { + workflowStepNames = append(workflowStepNames, step.Name) + } + } + + var filteredSteps []apisv1.WorkflowStep + _, readyToDeleteSteps, readyToAddSteps := compareSlices(workflowStepNames, envStepNames) + + for _, step := range workflow.Steps { + if isEnvStepType(step.Type) && utils.StringsContain(readyToDeleteSteps, step.Name) { + continue + } + filteredSteps = append(filteredSteps, convertFromWorkflowStepModel(step)) + } + + for _, step := range envSteps { + if isEnvStepType(step.Type) && utils.StringsContain(readyToAddSteps, step.Name) { + filteredSteps = append(filteredSteps, step) + } + } + + _, err = e.workflowUsecase.UpdateWorkflow(ctx, workflow, apisv1.UpdateWorkflowRequest{ + Steps: filteredSteps, + Description: workflow.Description, + }) + if err != nil { + return err + } return nil } @@ -417,3 +453,32 @@ func (e *envBindingUsecaseImpl) genEnvWorkflowSteps(ctx context.Context, env *mo func convertWorkflowName(envName string) string { return fmt.Sprintf("workflow-%s", envName) } + +func compareSlices(a []string, b []string) ([]string, []string, []string) { + m := make(map[string]uint8) + for _, k := range a { + m[k] |= 1 << 0 + } + for _, k := range b { + m[k] |= 1 << 1 + } + + var inAAndB, inAButNotB, inBButNotA []string + for k, v := range m { + a := v&(1<<0) != 0 + b := v&(1<<1) != 0 + switch { + case a && b: + inAAndB = append(inAAndB, k) + case a && !b: + inAButNotB = append(inAButNotB, k) + case !a && b: + inBButNotA = append(inBButNotA, k) + } + } + return inAAndB, inAButNotB, inBButNotA +} + +func isEnvStepType(stepType string) bool { + return stepType == Deploy2Env || stepType == DeployCloudResource +} diff --git a/pkg/apiserver/rest/usecase/envbinding_test.go b/pkg/apiserver/rest/usecase/envbinding_test.go index 7a32ea233..1598c4fb6 100644 --- a/pkg/apiserver/rest/usecase/envbinding_test.go +++ b/pkg/apiserver/rest/usecase/envbinding_test.go @@ -23,6 +23,8 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/oam-dev/kubevela/pkg/apiserver/rest/utils" + "github.com/oam-dev/kubevela/pkg/apiserver/model" apisv1 "github.com/oam-dev/kubevela/pkg/apiserver/rest/apis/v1" ) @@ -31,6 +33,7 @@ var _ = Describe("Test envBindingUsecase functions", func() { var ( envBindingUsecase *envBindingUsecaseImpl workflowUsecase *workflowUsecaseImpl + definitionUsecase DefinitionUsecase envBindingDemo1 apisv1.EnvBinding envBindingDemo2 apisv1.EnvBinding testApp *model.Application @@ -41,7 +44,8 @@ var _ = Describe("Test envBindingUsecase functions", func() { Namespace: "default", } workflowUsecase = &workflowUsecaseImpl{ds: ds, kubeClient: k8sClient} - envBindingUsecase = &envBindingUsecaseImpl{ds: ds, workflowUsecase: workflowUsecase, kubeClient: k8sClient} + definitionUsecase = &definitionUsecaseImpl{kubeClient: k8sClient, caches: make(map[string]*utils.MemoryCache)} + envBindingUsecase = &envBindingUsecaseImpl{ds: ds, workflowUsecase: workflowUsecase, definitionUsecase: definitionUsecase, kubeClient: k8sClient} envBindingDemo1 = apisv1.EnvBinding{ Name: "dev", Alias: "dev alias", @@ -99,14 +103,24 @@ var _ = Describe("Test envBindingUsecase functions", func() { It("Test Application UpdateEnv function", func() { envBinding, err := envBindingUsecase.UpdateEnvBinding(context.TODO(), testApp, "prod", apisv1.PutApplicationEnvRequest{ - TargetNames: []string{"prod-target-new1"}, + TargetNames: []string{"prod-target-new1", "prod-target-new2"}, }) Expect(err).Should(BeNil()) Expect(envBinding).ShouldNot(BeNil()) Expect(cmp.Diff(envBinding.TargetNames[0], "prod-target-new1")).Should(BeEmpty()) - // workflow, err := workflowUsecase.GetWorkflow(context.TODO(), testApp, "prod") - // Expect(err).Should(BeNil()) - // Expect(cmp.Diff(workflow.Steps[0].Name, "prod-target-new1")).Should(BeEmpty()) + workflow, err := workflowUsecase.GetWorkflow(context.TODO(), testApp, "workflow-prod") + Expect(err).Should(BeNil()) + Expect(cmp.Diff(workflow.Steps[0].Name, "prod-target-new1")).Should(BeEmpty()) + + envBinding, err = envBindingUsecase.UpdateEnvBinding(context.TODO(), testApp, "prod", apisv1.PutApplicationEnvRequest{ + TargetNames: []string{"prod-target-new3", "prod-target-new2"}, + }) + Expect(err).Should(BeNil()) + Expect(envBinding).ShouldNot(BeNil()) + Expect(cmp.Diff(envBinding.TargetNames[0], "prod-target-new3")).Should(BeEmpty()) + workflow, err = workflowUsecase.GetWorkflow(context.TODO(), testApp, "workflow-prod") + Expect(err).Should(BeNil()) + Expect(cmp.Diff(workflow.Steps[1].Name, "prod-target-new3")).Should(BeEmpty()) }) It("Test Application DeleteEnv function", func() { diff --git a/pkg/apiserver/rest/usecase/workflow.go b/pkg/apiserver/rest/usecase/workflow.go index b3d36506c..c4b3213ff 100644 --- a/pkg/apiserver/rest/usecase/workflow.go +++ b/pkg/apiserver/rest/usecase/workflow.go @@ -229,20 +229,7 @@ func (w *workflowUsecaseImpl) UpdateWorkflow(ctx context.Context, workflow *mode func converWorkflowBase(workflow *model.Workflow) apisv1.WorkflowBase { var steps []apisv1.WorkflowStep for _, step := range workflow.Steps { - apiStep := apisv1.WorkflowStep{ - Name: step.Name, - Type: step.Type, - Alias: step.Alias, - Description: step.Description, - Inputs: step.Inputs, - Outputs: step.Outputs, - Properties: step.Properties.JSON(), - DependsOn: step.DependsOn, - } - if step.Properties != nil { - apiStep.Properties = step.Properties.JSON() - } - steps = append(steps, apiStep) + steps = append(steps, convertFromWorkflowStepModel(step)) } return apisv1.WorkflowBase{ Name: workflow.Name, @@ -674,6 +661,23 @@ func convertFromRecordModel(record *model.WorkflowRecord) *apisv1.WorkflowRecord } } +func convertFromWorkflowStepModel(step model.WorkflowStep) apisv1.WorkflowStep { + apiStep := apisv1.WorkflowStep{ + Name: step.Name, + Type: step.Type, + Alias: step.Alias, + Description: step.Description, + Inputs: step.Inputs, + Outputs: step.Outputs, + Properties: step.Properties.JSON(), + DependsOn: step.DependsOn, + } + if step.Properties != nil { + apiStep.Properties = step.Properties.JSON() + } + return apiStep +} + func convertBool(b *bool) bool { if b == nil { return false diff --git a/pkg/apiserver/rest/webservice/application.go b/pkg/apiserver/rest/webservice/application.go index 08bd3c5dd..f47e7f515 100644 --- a/pkg/apiserver/rest/webservice/application.go +++ b/pkg/apiserver/rest/webservice/application.go @@ -63,7 +63,7 @@ func (c *applicationWebService) GetWebService() *restful.WebService { Metadata(restfulspec.KeyOpenAPITags, tags). Param(ws.QueryParameter("query", "Fuzzy search based on name or description").DataType("string")). Param(ws.QueryParameter("namespace", "The namespace of the managed cluster").DataType("string")). - Param(ws.QueryParameter("target", "Name of the application delivery target").DataType("string")). + Param(ws.QueryParameter("targetName", "Name of the application delivery target").DataType("string")). Returns(200, "", apis.ListApplicationResponse{}). Returns(400, "", bcode.Bcode{}). Writes(apis.ListApplicationResponse{})) @@ -506,7 +506,7 @@ func (c *applicationWebService) createApplication(req *restful.Request, res *res func (c *applicationWebService) listApplications(req *restful.Request, res *restful.Response) { apps, err := c.applicationUsecase.ListApplications(req.Request.Context(), apis.ListApplicatioOptions{ Namespace: req.QueryParameter("namespace"), - TargetName: req.QueryParameter("target"), + TargetName: req.QueryParameter("targetName"), Query: req.QueryParameter("query"), }) if err != nil {