Merge pull request #12507 from chlins/fix/preheat-update-instance

fix(preheat): fix preheat handler PingInstance and UpdateInstance

api/v2.0/swagger.yaml

@@ -819,15 +819,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
+            $ref: '#/definitions/Instance'
-            additionalProperties:
-              type: object
-              additionalProperties: true
       responses:
         '200':
           description: Success
@@ -1777,6 +1774,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

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,

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...)
 }
 

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)
+	err := s.controller.DeleteInstance(s.ctx, 0)
-	// s.Error(err)
+	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, mock.Anything).Return(errors.New("no properties provided to update"))
-	s.fakeInstanceMgr.On("Update", s.ctx, nil).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)
+	policy := &policy.Schema{Name: "test"}
-	id, err := s.controller.CreatePolicy(s.ctx, nil)
+	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)
+	policy := &policy.Schema{Name: "test"}
-	err := s.controller.UpdateInstance(s.ctx, nil, "")
+	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,

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(
@@ -303,11 +305,11 @@ export class DistributionInstancesComponent implements OnInit, OnDestroy {
     operMessage.state = OperationState.progressing;
     operMessage.data.name = instance.name;
     this.operationService.publishInfo(operMessage);
-
+    const copiedInstance: Instance = clone(instance);
-    instance.enabled = true;
+    copiedInstance.enabled = true;
     return this.disService
       .UpdateInstance({
-        propertySet: {enabled: true},
+        instance: copiedInstance,
         preheatInstanceName: instance.name
       })
       .pipe(
@@ -337,11 +339,11 @@ export class DistributionInstancesComponent implements OnInit, OnDestroy {
     operMessage.state = OperationState.progressing;
     operMessage.data.name = instance.name;
     this.operationService.publishInfo(operMessage);
-
+    const copiedInstance: Instance = clone(instance);
-    instance.enabled = false;
+    copiedInstance.enabled = false;
     return this.disService
       .UpdateInstance({
-        propertySet: {enabled: false},
+        instance: copiedInstance,
         preheatInstanceName: instance.name
       })
       .pipe(

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 => {

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
+	instance, err := convertParamInstanceToModelInstance(params.Instance)
-	return operation.NewUpdateInstanceOK().WithPayload(payload)
+	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()