Merge pull request #12875 from chlins/fix/disable-change-p2p-provider-vendor

fix(p2p): disable change provider vendor type
This commit is contained in:
Steven Zou 2020-08-26 10:59:50 +08:00 committed by GitHub
commit 79665ed997
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 2 deletions

View File

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

View File

@ -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() {