From 78927af032f525a04bc43536bc53905301c80bab Mon Sep 17 00:00:00 2001 From: chlins Date: Fri, 17 Jul 2020 12:54:25 +0800 Subject: [PATCH] fix(preheat): fix preheat handler PingInstance and UpdateInstance Signed-off-by: chlins --- api/v2.0/swagger.yaml | 12 +++--- .../postgresql/0040_2.1.0_schema.up.sql | 2 +- src/controller/p2p/preheat/controller.go | 13 +++--- src/controller/p2p/preheat/controllor_test.go | 41 +++++++++++++------ .../distribution-instances.component.ts | 16 ++++---- .../distribution-setup-modal.component.ts | 15 ++++--- src/server/v2.0/handler/preheat.go | 34 ++++++++++++--- 7 files changed, 89 insertions(+), 44 deletions(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index bc41ce028..f512ec0c6 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -817,15 +817,12 @@ paths: parameters: - $ref: '#/parameters/requestId' - $ref: '#/parameters/instanceName' - - name: propertySet + - name: instance in: body - description: The property set to update + description: The instance to update required: true schema: - type: object - additionalProperties: - type: object - additionalProperties: true + $ref: '#/definitions/Instance' responses: '200': description: Success @@ -1767,6 +1764,9 @@ definitions: provider_id: type: integer description: The ID of preheat policy provider + provider_name: + type: string + description: The Name of preheat policy provider filters: type: string description: The Filters of preheat policy diff --git a/make/migrations/postgresql/0040_2.1.0_schema.up.sql b/make/migrations/postgresql/0040_2.1.0_schema.up.sql index 60a7e482f..0cc96a0e0 100644 --- a/make/migrations/postgresql/0040_2.1.0_schema.up.sql +++ b/make/migrations/postgresql/0040_2.1.0_schema.up.sql @@ -61,7 +61,7 @@ CREATE TABLE IF NOT EXISTS p2p_preheat_policy ( project_id int NOT NULL, provider_id int NOT NULL, filters varchar(1024), - trigger varchar(16), + trigger varchar(255), enabled boolean, creation_time timestamp, update_time timestamp, diff --git a/src/controller/p2p/preheat/controller.go b/src/controller/p2p/preheat/controller.go index 2398b84bb..21d44c483 100644 --- a/src/controller/p2p/preheat/controller.go +++ b/src/controller/p2p/preheat/controller.go @@ -26,7 +26,6 @@ var ErrorUnhealthy = errors.New("instance unhealthy") // Controller defines related top interfaces to handle the workflow of // the image distribution. -// TODO: Add health check API type Controller interface { // Get all the supported distribution providers // @@ -183,10 +182,6 @@ func (c *controller) DeleteInstance(ctx context.Context, id int64) error { // UpdateInstance implements @Controller.Update func (c *controller) UpdateInstance(ctx context.Context, instance *providerModels.Instance, properties ...string) error { - if len(properties) == 0 { - return errors.New("no properties provided to update") - } - return c.iManager.Update(ctx, instance, properties...) } @@ -206,6 +201,11 @@ func (c *controller) CountPolicy(ctx context.Context, query *q.Query) (int64, er // CreatePolicy creates the policy. func (c *controller) CreatePolicy(ctx context.Context, schema *policyModels.Schema) (int64, error) { + if schema != nil { + now := time.Now() + schema.CreatedAt = now + schema.UpdatedTime = now + } return c.pManager.Create(ctx, schema) } @@ -221,6 +221,9 @@ func (c *controller) GetPolicyByName(ctx context.Context, projectID int64, name // UpdatePolicy updates the policy. func (c *controller) UpdatePolicy(ctx context.Context, schema *policyModels.Schema, props ...string) error { + if schema != nil { + schema.UpdatedTime = time.Now() + } return c.pManager.Update(ctx, schema, props...) } diff --git a/src/controller/p2p/preheat/controllor_test.go b/src/controller/p2p/preheat/controllor_test.go index 54f907ffc..4dd1886a2 100644 --- a/src/controller/p2p/preheat/controllor_test.go +++ b/src/controller/p2p/preheat/controllor_test.go @@ -157,22 +157,18 @@ func (s *preheatSuite) TestCreateInstance() { } func (s *preheatSuite) TestDeleteInstance() { - // err := s.controller.DeleteInstance(s.ctx, 0) - // s.Error(err) + err := s.controller.DeleteInstance(s.ctx, 0) + s.Error(err) - err := s.controller.DeleteInstance(s.ctx, int64(1)) + err = s.controller.DeleteInstance(s.ctx, int64(1)) s.NoError(err) } func (s *preheatSuite) TestUpdateInstance() { - // TODO: test update more - s.fakeInstanceMgr.On("Update", s.ctx, nil).Return(errors.New("no properties provided to update")) + s.fakeInstanceMgr.On("Update", s.ctx, mock.Anything).Return(errors.New("no properties provided to update")) err := s.controller.UpdateInstance(s.ctx, nil) s.Error(err) - err = s.controller.UpdateInstance(s.ctx, &providerModel.Instance{ID: 0}) - s.Error(err) - s.fakeInstanceMgr.On("Update", mock.Anything, mock.Anything, mock.Anything).Return(nil) err = s.controller.UpdateInstance(s.ctx, &providerModel.Instance{ID: 1}, "enabled") s.NoError(err) @@ -192,10 +188,13 @@ func (s *preheatSuite) TestCountPolicy() { } func (s *preheatSuite) TestCreatePolicy() { - s.fakePolicyMgr.On("Create", s.ctx, mock.Anything).Return(int64(1), nil) - id, err := s.controller.CreatePolicy(s.ctx, nil) + policy := &policy.Schema{Name: "test"} + s.fakePolicyMgr.On("Create", s.ctx, policy).Return(int64(1), nil) + id, err := s.controller.CreatePolicy(s.ctx, policy) s.NoError(err) s.Equal(int64(1), id) + s.False(policy.CreatedAt.IsZero()) + s.False(policy.UpdatedTime.IsZero()) } func (s *preheatSuite) TestGetPolicy() { @@ -213,9 +212,11 @@ func (s *preheatSuite) TestGetPolicyByName() { } func (s *preheatSuite) TestUpdatePolicy() { - s.fakePolicyMgr.On("Update", s.ctx, mock.Anything, mock.Anything).Return(nil) - err := s.controller.UpdateInstance(s.ctx, nil, "") + policy := &policy.Schema{Name: "test"} + s.fakePolicyMgr.On("Update", s.ctx, policy, mock.Anything).Return(nil) + err := s.controller.UpdatePolicy(s.ctx, policy, "") s.NoError(err) + s.False(policy.UpdatedTime.IsZero()) } func (s *preheatSuite) TestDeletePolicy() { @@ -259,6 +260,22 @@ func (s *preheatSuite) TestCheckHealth() { err = s.controller.CheckHealth(s.ctx, instance) s.Error(err) + // not health + // health + instance = &providerModel.Instance{ + ID: 1, + Name: "test-instance", + Vendor: provider.DriverDragonfly, + Endpoint: "http://127.0.0.1", + AuthMode: auth.AuthModeNone, + Enabled: true, + Default: true, + Insecure: true, + Status: "Unknown", + } + err = s.controller.CheckHealth(s.ctx, instance) + s.Error(err) + // health instance = &providerModel.Instance{ ID: 1, diff --git a/src/portal/src/app/distribution/distribution-instances/distribution-instances.component.ts b/src/portal/src/app/distribution/distribution-instances/distribution-instances.component.ts index 46e6f8174..db05e8d0e 100644 --- a/src/portal/src/app/distribution/distribution-instances/distribution-instances.component.ts +++ b/src/portal/src/app/distribution/distribution-instances/distribution-instances.component.ts @@ -167,8 +167,10 @@ export class DistributionInstancesComponent implements OnInit, OnDestroy { operMessage.state = OperationState.progressing; operMessage.data.name = this.selectedRow[0].name; this.operationService.publishInfo(operMessage); + const instance: Instance = clone(this.selectedRow[0]); + instance.default = true; this.disService.UpdateInstance({ - propertySet: {default: true}, + instance: instance, preheatInstanceName: this.selectedRow[0].name }) .subscribe( @@ -300,11 +302,11 @@ export class DistributionInstancesComponent implements OnInit, OnDestroy { operMessage.state = OperationState.progressing; operMessage.data.name = instance.name; this.operationService.publishInfo(operMessage); - - instance.enabled = true; + const copiedInstance: Instance = clone(instance); + copiedInstance.enabled = true; return this.disService .UpdateInstance({ - propertySet: {enabled: true}, + instance: copiedInstance, preheatInstanceName: instance.name }) .pipe( @@ -334,11 +336,11 @@ export class DistributionInstancesComponent implements OnInit, OnDestroy { operMessage.state = OperationState.progressing; operMessage.data.name = instance.name; this.operationService.publishInfo(operMessage); - - instance.enabled = false; + const copiedInstance: Instance = clone(instance); + copiedInstance.enabled = false; return this.disService .UpdateInstance({ - propertySet: {enabled: false}, + instance: copiedInstance, preheatInstanceName: instance.name }) .pipe( diff --git a/src/portal/src/app/distribution/distribution-setup-modal/distribution-setup-modal.component.ts b/src/portal/src/app/distribution/distribution-setup-modal/distribution-setup-modal.component.ts index 99c1d7f48..f099ccc27 100644 --- a/src/portal/src/app/distribution/distribution-setup-modal/distribution-setup-modal.component.ts +++ b/src/portal/src/app/distribution/distribution-setup-modal/distribution-setup-modal.component.ts @@ -105,13 +105,6 @@ export class DistributionSetupModalComponent implements OnInit { submit() { if (this.editingMode) { - const data: Instance = { - endpoint: this.model.endpoint, - enabled: this.model.enabled, - description: this.model.description, - auth_mode: this.model.auth_mode, - auth_info: this.model.auth_info - }; const operMessageForEdit = new OperateInfo(); operMessageForEdit.name = 'DISTRIBUTION.UPDATE_INSTANCE'; operMessageForEdit.data.id = this.model.id; @@ -119,7 +112,13 @@ export class DistributionSetupModalComponent implements OnInit { operMessageForEdit.data.name = this.model.name; this.operationService.publishInfo(operMessageForEdit); this.saveBtnState = ClrLoadingState.LOADING; - this.distributionService.UpdateInstance({preheatInstanceName: this.model.name, propertySet: data + const instance: Instance = clone(this.originModelForEdit); + instance.endpoint = this.model.endpoint; + instance.enabled = this.model.enabled; + instance.description = this.model.description; + instance.auth_mode = this.model.auth_mode; + instance.auth_info = this.model.auth_info; + this.distributionService.UpdateInstance({preheatInstanceName: this.model.name, instance: instance }).subscribe( response => { this.translate.get('DISTRIBUTION.UPDATE_SUCCESS').subscribe(msg => { diff --git a/src/server/v2.0/handler/preheat.go b/src/server/v2.0/handler/preheat.go index ca88290cb..5efc1d3ff 100644 --- a/src/server/v2.0/handler/preheat.go +++ b/src/server/v2.0/handler/preheat.go @@ -163,8 +163,17 @@ func (api *preheatAPI) UpdateInstance(ctx context.Context, params operation.Upda return api.SendError(ctx, err) } - var payload *models.InstanceUpdateResp - return operation.NewUpdateInstanceOK().WithPayload(payload) + instance, err := convertParamInstanceToModelInstance(params.Instance) + if err != nil { + return api.SendError(ctx, err) + } + + err = api.preheatCtl.UpdateInstance(ctx, instance) + if err != nil { + return api.SendError(ctx, err) + } + + return operation.NewUpdateInstanceOK() } func convertProvidersToFrontend(backend []*provider.Metadata) (frontend []*models.Metadata) { @@ -199,10 +208,18 @@ func (api *preheatAPI) GetPolicy(ctx context.Context, params operation.GetPolicy return api.SendError(ctx, err) } + // get provider + provider, err := api.preheatCtl.GetInstance(ctx, policy.ProviderID) + if err != nil { + return api.SendError(ctx, err) + } + payload, err = convertPolicyToPayload(policy) if err != nil { return api.SendError(ctx, err) } + payload.ProviderName = provider.Name + return operation.NewGetPolicyOK().WithPayload(payload) } @@ -298,10 +315,17 @@ func (api *preheatAPI) ListPolicies(ctx context.Context, params operation.ListPo var payload []*models.PreheatPolicy for _, policy := range policies { + // get provider + provider, err := api.preheatCtl.GetInstance(ctx, policy.ProviderID) + if err != nil { + return api.SendError(ctx, err) + } + p, err := convertPolicyToPayload(policy) if err != nil { return api.SendError(ctx, err) } + p.ProviderName = provider.Name payload = append(payload, p) } return operation.NewListPoliciesOK().WithPayload(payload).WithXTotalCount(total). @@ -350,7 +374,7 @@ func (api *preheatAPI) PingInstances(ctx context.Context, params operation.PingI return operation.NewPingInstancesNotFound() } if err != nil { - api.SendError(ctx, err) + return api.SendError(ctx, err) } } else { // by endpoint URL @@ -360,13 +384,13 @@ func (api *preheatAPI) PingInstances(ctx context.Context, params operation.PingI instance, err = convertParamInstanceToModelInstance(params.Instance) if err != nil { - api.SendError(ctx, err) + return api.SendError(ctx, err) } } err = api.preheatCtl.CheckHealth(ctx, instance) if err != nil { - api.SendError(ctx, err) + return api.SendError(ctx, err) } return operation.NewPingInstancesOK()