diff --git a/src/controller/p2p/preheat/controller.go b/src/controller/p2p/preheat/controller.go index 2d648149e..14e7b1d60 100644 --- a/src/controller/p2p/preheat/controller.go +++ b/src/controller/p2p/preheat/controller.go @@ -187,6 +187,10 @@ func (c *controller) CreateInstance(ctx context.Context, instance *providerModel // DeleteInstance implements @Controller.Delete func (c *controller) DeleteInstance(ctx context.Context, id int64) error { + ins, err := c.GetInstance(ctx, id) + if err != nil { + return err + } // delete instance should check the instance whether be used by policies policies, err := c.ListPolicies(ctx, &q.Query{ Keywords: map[string]interface{}{ @@ -200,7 +204,7 @@ func (c *controller) DeleteInstance(ctx context.Context, id int64) error { if len(policies) > 0 { return errors.New(nil). WithCode(errors.PreconditionCode). - WithMessage("Can't delete instance %d, %d preheat policies use it as provider", id, len(policies)) + WithMessage("Provider [%s] cannot be deleted as some preheat policies are using it", ins.Name) } return c.iManager.Delete(ctx, id) @@ -208,6 +212,11 @@ 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 { + oldIns, err := c.GetInstance(ctx, instance.ID) + if err != nil { + return err + } + if !instance.Enabled { // update instance should check the instance whether be used by policies policies, err := c.ListPolicies(ctx, &q.Query{ @@ -222,10 +231,15 @@ func (c *controller) UpdateInstance(ctx context.Context, instance *providerModel if len(policies) > 0 { return errors.New(nil). WithCode(errors.PreconditionCode). - WithMessage("Can't disable instance %d, %d preheat policies use it as provider", instance.ID, len(policies)) + WithMessage("Provider [%s] cannot be disabled as some preheat policies are using it", oldIns.Name) } } + // vendor type does not support change + if oldIns.Vendor != instance.Vendor { + return errors.Errorf("provider [%s] vendor cannot be changed", oldIns.Name) + } + return c.iManager.Update(ctx, instance, properties...) } diff --git a/src/controller/p2p/preheat/controllor_test.go b/src/controller/p2p/preheat/controllor_test.go index 4413912c4..e1c290822 100644 --- a/src/controller/p2p/preheat/controllor_test.go +++ b/src/controller/p2p/preheat/controllor_test.go @@ -175,6 +175,7 @@ func (s *preheatSuite) TestCreateInstance() { func (s *preheatSuite) TestDeleteInstance() { // instance be used should not be deleted + s.fakeInstanceMgr.On("Get", s.ctx, int64(1)).Return(&providerModel.Instance{ID: 1}, nil) s.fakePolicyMgr.On("ListPolicies", s.ctx, &q.Query{Keywords: map[string]interface{}{"provider_id": int64(1)}}).Return([]*policy.Schema{ { ProviderID: 1, @@ -183,6 +184,7 @@ func (s *preheatSuite) TestDeleteInstance() { err := s.controller.DeleteInstance(s.ctx, int64(1)) s.Error(err, "instance should not be deleted") + s.fakeInstanceMgr.On("Get", s.ctx, int64(2)).Return(&providerModel.Instance{ID: 2}, nil) s.fakePolicyMgr.On("ListPolicies", s.ctx, &q.Query{Keywords: map[string]interface{}{"provider_id": int64(2)}}).Return([]*policy.Schema{}, nil) s.fakeInstanceMgr.On("Delete", s.ctx, int64(2)).Return(nil) err = s.controller.DeleteInstance(s.ctx, int64(2)) @@ -191,11 +193,13 @@ func (s *preheatSuite) TestDeleteInstance() { func (s *preheatSuite) TestUpdateInstance() { // normal update + s.fakeInstanceMgr.On("Get", s.ctx, int64(1000)).Return(&providerModel.Instance{ID: 1000}, nil) s.fakeInstanceMgr.On("Update", s.ctx, &providerModel.Instance{ID: 1000, Enabled: true}).Return(nil) err := s.controller.UpdateInstance(s.ctx, &providerModel.Instance{ID: 1000, Enabled: true}) s.NoError(err, "instance can be updated") // disable instance should error due to with policy used + s.fakeInstanceMgr.On("Get", s.ctx, int64(1001)).Return(&providerModel.Instance{ID: 1001}, nil) s.fakeInstanceMgr.On("Update", s.ctx, &providerModel.Instance{ID: 1001}).Return(nil) s.fakePolicyMgr.On("ListPolicies", s.ctx, &q.Query{Keywords: map[string]interface{}{"provider_id": int64(1001)}}).Return([]*policy.Schema{ {ProviderID: 1001}, @@ -204,10 +208,18 @@ func (s *preheatSuite) TestUpdateInstance() { s.Error(err, "instance should not be disabled") // disable instance can be deleted if no policy used + s.fakeInstanceMgr.On("Get", s.ctx, int64(1002)).Return(&providerModel.Instance{ID: 1002}, nil) s.fakeInstanceMgr.On("Update", s.ctx, &providerModel.Instance{ID: 1002}).Return(nil) s.fakePolicyMgr.On("ListPolicies", s.ctx, &q.Query{Keywords: map[string]interface{}{"provider_id": int64(1002)}}).Return([]*policy.Schema{}, nil) err = s.controller.UpdateInstance(s.ctx, &providerModel.Instance{ID: 1002}) s.NoError(err, "instance can be disabled") + + // not support change vendor type + s.fakeInstanceMgr.On("Get", s.ctx, int64(1003)).Return(&providerModel.Instance{ID: 1003, Vendor: "dragonfly"}, nil) + s.fakeInstanceMgr.On("Update", s.ctx, &providerModel.Instance{ID: 1003, Vendor: "kraken"}).Return(nil) + s.fakePolicyMgr.On("ListPolicies", s.ctx, &q.Query{Keywords: map[string]interface{}{"provider_id": int64(1003)}}).Return([]*policy.Schema{}, nil) + err = s.controller.UpdateInstance(s.ctx, &providerModel.Instance{ID: 1003, Vendor: "kraken"}) + s.Error(err, "provider vendor cannot be changed") } func (s *preheatSuite) TestGetInstance() {