diff --git a/pkg/compute/models/secgroupcache.go b/pkg/compute/models/secgroupcache.go index e353752935..4289fff99c 100644 --- a/pkg/compute/models/secgroupcache.go +++ b/pkg/compute/models/secgroupcache.go @@ -771,18 +771,32 @@ func (self *SSecurityGroupCache) CreateISecurityGroup(ctx context.Context) (clou } func (self *SSecurityGroupCache) GetSecuritRuleSet(ctx context.Context) (cloudprovider.SecurityRuleSet, []SSecurityGroupCache, error) { - ruleSet := cloudprovider.SecurityRuleSet{} secgroup, err := self.GetSecgroup() if err != nil { - return ruleSet, nil, errors.Wrapf(err, "GetSecgroup") + return nil, nil, errors.Wrapf(err, "GetSecgroup") } rules, err := secgroup.getSecurityRules() if err != nil { - return ruleSet, nil, errors.Wrapf(err, "getSecurityRules") + return nil, nil, errors.Wrapf(err, "getSecurityRules") } + return self.convertRules(ctx, rules) +} +func (self *SSecurityGroupCache) GetOldSecuritRuleSet(ctx context.Context) (cloudprovider.SecurityRuleSet, []SSecurityGroupCache, error) { + secgroup, err := self.GetSecgroup() + if err != nil { + return nil, nil, errors.Wrapf(err, "GetSecgroup") + } + rules, err := secgroup.GetOldRules() + if err != nil { + return nil, nil, errors.Wrapf(err, "GetOldRules") + } + return self.convertRules(ctx, rules) +} + +func (self *SSecurityGroupCache) convertRules(ctx context.Context, rules []SSecurityGroupRule) (cloudprovider.SecurityRuleSet, []SSecurityGroupCache, error) { + ruleSet := cloudprovider.SecurityRuleSet{} caches := []SSecurityGroupCache{} - driver := GetRegionDriver(self.GetProviderName()) for i := range rules { if !driver.IsSupportPeerSecgroup() && len(rules[i].PeerSecgroupId) > 0 { @@ -835,11 +849,11 @@ func (self *SSecurityGroupCache) SyncRules(ctx context.Context, skipSyncRule boo if err != nil { return err } - iSecgroup, isNew, err := self.GetOrCreateISecurityGroup(ctx) + iSecgroup, err := self.GetISecurityGroup(ctx) if err != nil { return errors.Wrapf(err, "GetOrCreateISecurityGroup") } - if !isNew && skipSyncRule { + if skipSyncRule { return nil } diff --git a/pkg/compute/models/secgrouprules.go b/pkg/compute/models/secgrouprules.go index ce6f7f52e7..c18a725fa5 100644 --- a/pkg/compute/models/secgrouprules.go +++ b/pkg/compute/models/secgrouprules.go @@ -16,6 +16,7 @@ package models import ( "context" + "fmt" "net" "yunion.io/x/jsonutils" @@ -70,6 +71,8 @@ type SSecurityGroupRule struct { Action string `width:"5" charset:"ascii" nullable:"false" list:"user" update:"user" create:"required"` Description string `width:"256" charset:"utf8" list:"user" update:"user" create:"optional"` PeerSecgroupId string `width:"128" charset:"ascii" create:"optional" list:"user" update:"user"` + + IsDirty bool `nullable:"false" default:"false"` } func (self *SSecurityGroupRule) GetId() string { @@ -388,12 +391,13 @@ func (self *SSecurityGroupRule) toRule() (*secrules.SecurityRule, error) { func (self *SSecurityGroupRule) PostCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data jsonutils.JSONObject) { self.SResourceBase.PostCreate(ctx, userCred, ownerId, query, data) - if len(self.PeerSecgroupId) > 0 { - db.Update(self, func() error { + db.Update(self, func() error { + if len(self.PeerSecgroupId) > 0 { self.CIDR = "" - return nil - }) - } + } + self.IsDirty = true + return nil + }) log.Debugf("POST Create %s", data) if secgroup := self.GetSecGroup(); secgroup != nil { @@ -405,12 +409,30 @@ func (self *SSecurityGroupRule) PostCreate(ctx context.Context, userCred mcclien func (self *SSecurityGroupRule) PreDelete(ctx context.Context, userCred mcclient.TokenCredential) { self.SResourceBase.PreDelete(ctx, userCred) + db.Update(self, func() error { + self.IsDirty = true + return nil + }) + if secgroup := self.GetSecGroup(); secgroup != nil { logclient.AddSimpleActionLog(secgroup, logclient.ACT_DELETE, jsonutils.Marshal(self), userCred, true) secgroup.DoSync(ctx, userCred) } } +func (self *SSecurityGroupRule) PreUpdate(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) { + old := &SSecurityGroupRule{} + old.SetModelManager(SecurityGroupRuleManager, old) + jsonutils.Update(old, self) + old.IsDirty = true + old.Deleted = true + old.Id = fmt.Sprintf("%s-back", self.Id) + err := SecurityGroupRuleManager.TableSpec().InsertOrUpdate(ctx, old) + if err != nil { + log.Errorf("insert old rule error: %v", err) + } +} + func (self *SSecurityGroupRule) PostUpdate(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) { self.SResourceBase.PostUpdate(ctx, userCred, query, data) diff --git a/pkg/compute/models/secgroups.go b/pkg/compute/models/secgroups.go index 1c97c426f2..b88eaf87c6 100644 --- a/pkg/compute/models/secgroups.go +++ b/pkg/compute/models/secgroups.go @@ -17,6 +17,7 @@ package models import ( "context" "database/sql" + "fmt" "strings" "time" @@ -369,6 +370,33 @@ func (self *SSecurityGroup) getDesc() *api.SecgroupJsonDesc { } } +func (self *SSecurityGroup) ClearRuleDirty() error { + _, err := sqlchemy.GetDB().Exec( + fmt.Sprintf( + "update %s set is_dirty = false where secgroup_id = ?", + SecurityGroupRuleManager.TableSpec().Name(), + ), self.Id, + ) + return err +} + +func (self *SSecurityGroup) GetOldRules() ([]SSecurityGroupRule, error) { + q := SecurityGroupRuleManager.Query().Equals("secgroup_id", self.Id).IsFalse("is_dirty") + rules := []SSecurityGroupRule{} + err := db.FetchModelObjects(SecurityGroupRuleManager, q, &rules) + if err != nil { + return nil, err + } + q = SecurityGroupRuleManager.RawQuery().Equals("secgroup_id", self.Id).IsTrue("is_dirty").IsTrue("deleted") + part := []SSecurityGroupRule{} + err = db.FetchModelObjects(SecurityGroupRuleManager, q, &part) + if err != nil { + return nil, err + } + rules = append(rules, part...) + return rules, nil +} + func (manager *SSecurityGroupManager) FetchCustomizeColumns( ctx context.Context, userCred mcclient.TokenCredential, diff --git a/pkg/compute/tasks/security_group_sync_rules_task.go b/pkg/compute/tasks/security_group_sync_rules_task.go index 6d1fb40393..70e61ff255 100644 --- a/pkg/compute/tasks/security_group_sync_rules_task.go +++ b/pkg/compute/tasks/security_group_sync_rules_task.go @@ -16,6 +16,7 @@ package tasks import ( "context" + "fmt" "yunion.io/x/jsonutils" "yunion.io/x/pkg/errors" @@ -23,6 +24,7 @@ import ( api "yunion.io/x/onecloud/pkg/apis/compute" "yunion.io/x/onecloud/pkg/cloudcommon/db" "yunion.io/x/onecloud/pkg/cloudcommon/db/taskman" + "yunion.io/x/onecloud/pkg/cloudprovider" "yunion.io/x/onecloud/pkg/compute/models" "yunion.io/x/onecloud/pkg/util/logclient" ) @@ -36,12 +38,14 @@ func init() { } func (self *SecurityGroupSyncRulesTask) taskFailed(ctx context.Context, secgroup *models.SSecurityGroup, err error) { + secgroup.ClearRuleDirty() secgroup.SetStatus(self.UserCred, api.SECGROUP_STATUS_READY, "") logclient.AddActionLogWithContext(ctx, secgroup, logclient.ACT_SYNC_CONF, err, self.UserCred, false) self.SetStageFailed(ctx, jsonutils.NewString(err.Error())) } func (self *SecurityGroupSyncRulesTask) taskComplete(ctx context.Context, secgroup *models.SSecurityGroup) { + secgroup.ClearRuleDirty() secgroup.SetStatus(self.UserCred, api.SECGROUP_STATUS_READY, "") self.SetStageComplete(ctx, nil) } @@ -54,6 +58,39 @@ func (self *SecurityGroupSyncRulesTask) OnInit(ctx context.Context, obj db.IStan return } + for i := range caches { + iSecgroup, err := caches[i].GetISecurityGroup(ctx) + if err != nil { + self.taskFailed(ctx, secgroup, errors.Wrapf(err, "GetISecurityGroup")) + return + } + oldRules, _, err := caches[i].GetOldSecuritRuleSet(ctx) + if err != nil { + self.taskFailed(ctx, secgroup, errors.Wrapf(err, "GetOldSecuritRuleSet")) + return + } + rules, err := iSecgroup.GetRules() + if err != nil { + self.taskFailed(ctx, secgroup, errors.Wrapf(err, "GetRules")) + return + } + + src := cloudprovider.NewSecRuleInfo(models.GetRegionDriver(api.CLOUD_PROVIDER_ONECLOUD)) + src.Rules = oldRules + + region, err := caches[i].GetRegion() + if err != nil { + self.taskFailed(ctx, secgroup, errors.Wrapf(err, "GetRegion")) + return + } + dest := cloudprovider.NewSecRuleInfo(models.GetRegionDriver(region.Provider)) + dest.Rules = rules + _, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(src, dest, false) + if len(inDels)+len(outDels)+len(inAdds)+len(outAdds) > 0 { + self.taskFailed(ctx, secgroup, fmt.Errorf("secgroup rule not equal with cache %s(%s)", caches[i].Name, region.Provider)) + return + } + } for i := range caches { err := caches[i].SyncRules(ctx, false) if err != nil {