Merge pull request #2795 from Mjoycarry/bugfix/nat

Bugfix and feature add: nat

cmd/climc/shell/natgateways.go

@@ -43,4 +43,30 @@ func init() {
 		printObject(results)
 		return nil
 	})
+
+	type NatGatewayListEipOptions struct {
+		ID string `help:"ID"`
+	}
+
+	R(&NatGatewayListEipOptions{}, "natgateway-dnat-resources", "list resources in dnats of natgateway",
+		func(s *mcclient.ClientSession, opts *NatGatewayListEipOptions) error {
+
+		ret, err := modules.NatGateways.PerformAction(s, opts.ID, "dnat-resources", nil)
+		if err != nil {
+			return err
+		}
+		printObject(ret)
+		return nil
+	})
+
+	R(&NatGatewayListEipOptions{}, "natgateway-snat-resources", "list resources in snats of natgateway",
+		func(s *mcclient.ClientSession, opts *NatGatewayListEipOptions) error {
+
+			ret, err := modules.NatGateways.PerformAction(s, opts.ID, "snat-resources", nil)
+			if err != nil {
+				return err
+			}
+			printObject(ret)
+			return nil
+		})
 }

docs/index.yaml

@@ -245,6 +245,10 @@ paths:
     $ref: "./natgateway/natgateways.yaml"
   /natgateways/{natgatewayId}:
     $ref: "./natgateway/natgateway.yaml"
+  /natgateways/{natgatewayId}/dnat-eips:
+    $ref: "./natgateway/dnat-eips.yaml"
+  /natgateways/{natgatewayId}/snat-eips:
+    $ref: "./natgateway/snat-eips.yaml"
   /natdentries:
     $ref: "./natgateway/dnatentries.yaml"
   /natdentries/{dnatentryId}:

docs/natgateway/dnat-eips.yaml

@@ -0,0 +1,13 @@
+post:
+  summary: NAT网关下DNAT规则EIP的集合
+  parameters:
+    - $ref: "../parameters/natgateway.yaml#/natgatewayId"
+  responses:
+    200:
+      description: DNAT规则EIP集合
+      schema:
+        - $ref: "../schemas/natgateway.yaml#/EIPAddrs"
+  tags:
+    - natgateway
+
+

docs/natgateway/snat-eips.yaml

@@ -0,0 +1,11 @@
+post:
+  summary: NAT网关下SNAT规则EIP的集合
+  parameters:
+    - $ref: "../parameters/natgateway.yaml#/natgatewayId"
+  responses:
+    200:
+      description: SNAT规则EIP集合
+      schema:
+        - $ref: "../schemas/natgateway.yaml#/EIPAddrs"
+  tags:
+    - natgateway

docs/schemas/natgateway.yaml

@@ -225,9 +225,7 @@ SNatEntry:
       example: 4daf9886-ebe6-4a8a-8082-3bd623d1c857
       description: 所属NAT网关ID
     network:
-      type: string
-      example: test-network
-      description: 所属子网名称
+      $ref: 'network.yaml#/Network'
     network_id:
       type: string
       example: 7caf732e-93fc-42c0-89d2-86aac2970c38
@@ -348,3 +346,12 @@ SNatEntryCreate:
           type: string
           example: 192.168.1.0/24
           description: 映射到公网IP的内网网段(和network_id互斥，只需要传入一个）
+
+EIPAddrs:
+  type: object
+  properties:
+    eips:
+      type: array
+      items:
+        type: string
+      example: [34.23.125.45, 23.45.123.56]

pkg/cloudcommon/policy/resources.go

@@ -37,6 +37,9 @@ var (
 		"loadbalancerclusters",
 		"loadbalanceragents",
 		"netowkinterfaces",
+		"natgateways",
+		"natsentries",
+		"natdentries",
 	}
 	computeDomainResources = []string{
 		"cloudaccounts",

pkg/compute/models/natdtable.go

@@ -16,6 +16,7 @@ package models
 
 import (
 	"context"
+	"fmt"
 
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
@@ -140,6 +141,36 @@ func (man *SNatDEntryManager) ValidateCreateData(ctx context.Context, userCred m
 		return nil, httperrors.NewInputParameterError("invalid internal ip address: %s", input.InternalIp)
 	}
 
+	// check ip + port
+	eip, err := man.checkIPPort(input)
+	if err != nil {
+		return nil, httperrors.NewInputParameterError(err.Error())
+	}
+
+	// check that eip is suitable
+	if len(eip.AssociateId) != 0 {
+		if eip.AssociateId != input.NatgatewayId {
+			return nil, httperrors.NewInputParameterError("eip has been binding to another instance")
+		} else if !man.canBindIP(eip.IpAddr) {
+			return nil, httperrors.NewInputParameterError("eip has been binding to snat rules")
+		}
+	} else {
+		data.Add(jsonutils.NewBool(true), "need_bind")
+	}
+	data.Remove("external_ip_id")
+	data.Add(jsonutils.NewString(eip.ExternalId), "external_ip_id")
+	return data, nil
+}
+
+func (manager *SNatDEntryManager) checkIPPort(input *api.SNatDCreateInput) (*SElasticip, error) {
+	q := manager.Query().Equals("external_ip", input.ExternalIp).Equals("external_port", input.ExternalPort)
+	count, err := q.CountWithError()
+	if err != nil {
+		return nil, errors.Wrap(err, "fetch dnat with same external_ip and external_port")
+	}
+	if count > 0 {
+		return nil, fmt.Errorf("there are dnat rules with same external ip and external port")
+	}
 	model, err := ElasticipManager.FetchById(input.ExternalIpId)
 	if err != nil {
 		return nil, err
@@ -149,22 +180,9 @@ func (man *SNatDEntryManager) ValidateCreateData(ctx context.Context, userCred m
 	}
 	eip := model.(*SElasticip)
 	if eip.IpAddr != input.ExternalIp {
-		return nil, errors.Error("No such eip")
+		return nil, fmt.Errorf("No such eip")
 	}
-
-	// check that eip is suitable
-	if len(eip.AssociateId) != 0 {
-		if eip.AssociateId != input.NatgatewayId {
-			return nil, httperrors.NewInputParameterError("eip has been binding to another instance")
-		} else if !man.canBindIP(eip.IpAddr) {
-			return nil, httperrors.NewInputParameterError("eip has been binding to dnat rules")
-		}
-	} else {
-		data.Add(jsonutils.NewBool(true), "need_bind")
-	}
-	data.Remove("external_ip_id")
-	data.Add(jsonutils.NewString(eip.ExternalId), "external_ip_id")
-	return data, nil
+	return eip, nil
 }
 
 func (manager *SNatDEntryManager) SyncNatDTable(ctx context.Context, userCred mcclient.TokenCredential, syncOwnerId mcclient.IIdentityProvider, provider *SCloudprovider, nat *SNatGateway, extDTable []cloudprovider.ICloudNatDEntry) compare.SyncResult {

pkg/compute/models/natgateways.go

@@ -112,6 +112,86 @@ func (man *SNatGetewayManager) ValidateCreateData(ctx context.Context, userCred
 	return nil, httperrors.NewNotImplementedError("Not Implemented")
 }
 
+func (self *SNatGateway) AllowPerformSnatResources(ctx context.Context, userCred mcclient.TokenCredential,
+	qurey jsonutils.JSONObject) bool {
+
+	return true
+}
+func (self *SNatGateway) PerformSnatResources(ctx context.Context, userCred mcclient.TokenCredential,
+	query jsonutils.JSONObject, data jsonutils.JSONObject) (jsonutils.JSONObject, error) {
+
+	q := NatSEntryManager.Query("ip", "network_id").Equals("natgateway_id", self.Id)
+
+	rows, err := q.Rows()
+	if err != nil {
+		return nil, errors.Wrapf(err, "fetch resource with natgateway_id %s error", self.Id)
+	}
+	ipset, ip := make(map[string]struct{}), ""
+	networks, network := make([]string, 0), ""
+	for rows.Next() {
+		err := rows.Scan(&ip, &network)
+		if err != nil {
+			return nil, err
+		}
+		if _, ok := ipset[ip]; !ok {
+			ipset[ip] = struct{}{}
+		}
+		networks = append(networks, network)
+	}
+	ips := make([]string, 0, len(ipset))
+	for ip := range ipset {
+		ips = append(ips, ip)
+	}
+
+	ret := jsonutils.NewDict()
+	ret.Add(jsonutils.Marshal(ips), "eips")
+	ret.Add(jsonutils.Marshal(networks), "networks")
+
+	return ret, nil
+}
+
+func (self *SNatGateway) AllowPerformDnatResources(ctx context.Context, userCred mcclient.TokenCredential,
+	qurey jsonutils.JSONObject) bool {
+
+	return true
+}
+func (self *SNatGateway) PerformDnatResources(ctx context.Context, userCred mcclient.TokenCredential,
+	query jsonutils.JSONObject, data jsonutils.JSONObject) (jsonutils.JSONObject, error) {
+
+	q := NatDEntryManager.Query("external_ip").Equals("natgateway_id", self.Id)
+
+	ips, err := self.extractEipAddr(q)
+	if err != nil {
+		return nil, err
+	}
+	ret := jsonutils.NewDict()
+	ret.Add(jsonutils.Marshal(ips), "eips")
+	return ret, nil
+}
+
+func (self *SNatGateway) extractEipAddr(q *sqlchemy.SQuery) ([]string, error) {
+	rows, err := q.Rows()
+	if err != nil {
+		return nil, errors.Wrapf(err, "fetch resource with natgateway_id %s error", self.Id)
+	}
+	ipset, ip := make(map[string]struct{}), ""
+	for rows.Next() {
+		err := rows.Scan(&ip)
+		if err != nil {
+			return nil, err
+		}
+		if _, ok := ipset[ip]; !ok {
+			ipset[ip] = struct{}{}
+		}
+	}
+	ips := make([]string, 0, len(ipset))
+	for ip := range ipset {
+		ips = append(ips, ip)
+	}
+
+	return ips, nil
+}
+
 func (self *SNatGateway) GetVpc() (*SVpc, error) {
 	_vpc, err := VpcManager.FetchById(self.VpcId)
 	if err != nil {

pkg/compute/models/natstable.go

@@ -16,12 +16,14 @@ package models
 
 import (
 	"context"
+	"fmt"
+	"net"
 
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
 	"yunion.io/x/pkg/errors"
 	"yunion.io/x/pkg/util/compare"
-	"yunion.io/x/pkg/util/regutils"
+	"yunion.io/x/pkg/util/netutils"
 	"yunion.io/x/sqlchemy"
 
 	api "yunion.io/x/onecloud/pkg/apis/compute"
@@ -98,7 +100,10 @@ func (self *SNatSEntry) GetNatgateway() (*SNatGateway, error) {
 }
 
 func (self *SNatSEntry) GetNetwork() (*SNetwork, error) {
-	_network, err := NetworkManager.FetchById(self.NatgatewayId)
+	if len(self.NetworkId) == 0 {
+		return nil, nil
+	}
+	_network, err := NetworkManager.FetchById(self.NetworkId)
 	if err != nil {
 		return nil, err
 	}
@@ -142,20 +147,38 @@ func (man *SNatSEntryManager) ValidateCreateData(ctx context.Context, userCred m
 	if len(input.SourceCidr) != 0 && len(input.NetworkId) != 0 {
 		return nil, httperrors.NewInputParameterError("Only one of that sourceCIDR and netword_id is needed")
 	}
+
 	if len(input.SourceCidr) != 0 {
-		if !regutils.MatchCIDR(input.SourceCidr) {
-			return nil, httperrors.NewInputParameterError("invalid sourcecidr: %s", input.SourceCidr)
-		}
-		//todo check cidr is in range vpc
-	} else {
-		model, err := NetworkManager.FetchById(input.NetworkId)
+		//check sourceCidr and convert to netutils.IPV4Range
+		sourceIPV4Range, err := newIPv4RangeFromCIDR(input.SourceCidr)
 		if err != nil {
-			return nil, errors.Wrap(err, "fetch network error")
+			return nil, httperrors.NewInputParameterError(err.Error())
 		}
-		if model == nil {
-			return nil, httperrors.NewInputParameterError("no such network")
+		// get natgateway
+		model, err := man.FetchById(input.NatgatewayId)
+		if err != nil {
+			return nil, err
+		}
+		natgateway := model.(*SNatGateway)
+		// get vpc
+		vpc, err := natgateway.GetVpc()
+		if err != nil {
+			return nil, err
+		}
+		vpcIPV4Range, err := newIPv4RangeFromCIDR(vpc.CidrBlock)
+		if err != nil {
+			return nil, errors.Wrap(err, "convert vpc cidr to ipv4range error")
+		}
+		if !vpcIPV4Range.ContainsRange(sourceIPV4Range) {
+			return nil, httperrors.NewInputParameterError("cidr %s is not in range vpc %s", input.SourceCidr,
+				vpc.CidrBlock)
+		}
+
+	} else {
+		network, err := man.checkNetWorkId(input.NetworkId)
+		if err != nil {
+			return nil, httperrors.NewInputParameterError(err.Error())
 		}
-		network := model.(*SNetwork)
 		data.Add(jsonutils.NewString(network.GetExternalId()), "network_ext_id")
 	}
 
@@ -305,12 +328,32 @@ func (manager *SNatSEntryManager) newFromCloudNatSTable(ctx context.Context, use
 	return &table, nil
 }
 
+func (manager *SNatSEntryManager) checkNetWorkId(networkId string) (*SNetwork, error) {
+	// check that is these snat rule has neworkid
+	q := manager.Query().Equals("network_id", networkId)
+	count, err := q.CountWithError()
+	if err != nil {
+		return nil, errors.Wrap(err, "count snat with networkId failed")
+	}
+	if count > 0 {
+		return nil, fmt.Errorf("a network has only one snat rule")
+	}
+	model, err := NetworkManager.FetchById(networkId)
+	if err != nil {
+		return nil, errors.Wrap(err, "fetch network error")
+	}
+	if model == nil {
+		return nil, httperrors.NewInputParameterError("no such network")
+	}
+	return model.(*SNetwork), nil
+}
+
 func (self *SNatSEntry) GetExtraDetails(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject) (*jsonutils.JSONDict, error) {
 	extra, err := self.SStatusStandaloneResourceBase.GetExtraDetails(ctx, userCred, query)
 	if err != nil {
 		return nil, err
 	}
-	return extra, nil
+	return self.getMoreDetails(ctx, userCred, extra)
 }
 
 func (self *SNatSEntry) GetCustomizeColumns(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject) *jsonutils.JSONDict {
@@ -321,13 +364,23 @@ func (self *SNatSEntry) GetCustomizeColumns(ctx context.Context, userCred mcclie
 		return extra
 	}
 	extra.Add(jsonutils.NewString(natgateway.Name), "natgateway")
+
+	extra, _ = self.getMoreDetails(ctx, userCred, extra)
+	return extra
+}
+
+func (self *SNatSEntry) getMoreDetails(ctx context.Context, userCred mcclient.TokenCredential,
+	query *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
+
 	network, err := self.GetNetwork()
 	if err != nil {
-		log.Errorf("failed to get network %s for stable %s(%s) error: %v", self.NetworkId, self.Name, self.Id, err)
-		return extra
+		return query, nil
 	}
-	extra.Add(jsonutils.NewString(network.Name), "network")
-	return extra
+	if network == nil {
+		return query, nil
+	}
+	query.Add(jsonutils.Marshal(network), "network")
+	return query, nil
 }
 
 func (self *SNatSEntry) PostCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data jsonutils.JSONObject) {
@@ -394,3 +447,11 @@ func (self *SNatSEntryManager) canBindIP(ipAddr string) bool {
 	}
 	return true
 }
+
+func newIPv4RangeFromCIDR(cidr string) (netutils.IPV4AddrRange, error) {
+	_, ipNet, err := net.ParseCIDR(cidr)
+	if err != nil {
+		return netutils.IPV4AddrRange{}, errors.Wrapf(err, "invalid cidr: %s", cidr)
+	}
+	return netutils.NewIPV4AddrRangeFromIPNet(ipNet), nil
+}

pkg/compute/models/snapshotpolicy.go

@@ -191,14 +191,14 @@ func (manager *SSnapshotPolicyManager) ValidateCreateData(ctx context.Context, u
 
 // ==================================================== update =========================================================
 
-func (self *SSnapshotPolicy) AllowPerformUpdate(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) AllowPerformUpdate(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject, data jsonutils.JSONObject) bool {
 	// no fo now
 	return false
-	//return self.IsOwner(userCred) || db.IsAdminAllowPerform(userCred, self, "update")
+	//return sp.IsOwner(userCred) || db.IsAdminAllowPerform(userCred, sp, "update")
 }
 
-func (self *SSnapshotPolicy) PerformUpdate(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) PerformUpdate(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject, data jsonutils.JSONObject) (jsonutils.JSONObject, error) {
 	//check param
 	input := &api.SSnapshotPolicyCreateInput{}
@@ -206,20 +206,20 @@ func (self *SSnapshotPolicy) PerformUpdate(ctx context.Context, userCred mcclien
 	if err != nil {
 		return nil, httperrors.NewInputParameterError("Unmarshel input failed %s", err)
 	}
-	err = self.UpdateParamCheck(input)
+	err = sp.UpdateParamCheck(input)
 	if err != nil {
 		return nil, err
 	}
-	return nil, self.StartSnapshotPolicyUpdateTask(ctx, userCred, input)
+	return nil, sp.StartSnapshotPolicyUpdateTask(ctx, userCred, input)
 }
 
-func (self *SSnapshotPolicy) StartSnapshotPolicyUpdateTask(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) StartSnapshotPolicyUpdateTask(ctx context.Context, userCred mcclient.TokenCredential,
 	input *api.SSnapshotPolicyCreateInput) error {
 
 	params := jsonutils.NewDict()
 	params.Add(jsonutils.Marshal(input), "input")
-	self.SetStatus(userCred, api.SNAPSHOT_POLICY_UPDATING, "")
-	if task, err := taskman.TaskManager.NewTask(ctx, "SnapshotPolicyUpdateTask", self, userCred, params,
+	sp.SetStatus(userCred, api.SNAPSHOT_POLICY_UPDATING, "")
+	if task, err := taskman.TaskManager.NewTask(ctx, "SnapshotPolicyUpdateTask", sp, userCred, params,
 		"", "", nil); err == nil {
 		return err
 	} else {
@@ -229,7 +229,7 @@ func (self *SSnapshotPolicy) StartSnapshotPolicyUpdateTask(ctx context.Context,
 }
 
 // UpdateParamCheck check if update parameters are correct and need to update
-func (self *SSnapshotPolicy) UpdateParamCheck(input *api.SSnapshotPolicyCreateInput) error {
+func (sp *SSnapshotPolicy) UpdateParamCheck(input *api.SSnapshotPolicyCreateInput) error {
 	var err error
 	updateNum := 0
 
@@ -237,7 +237,7 @@ func (self *SSnapshotPolicy) UpdateParamCheck(input *api.SSnapshotPolicyCreateIn
 		if input.RetentionDays < -1 || input.RetentionDays > 65535 {
 			return httperrors.NewInputParameterError("Retention days must in 1~65535 or -1")
 		}
-		if input.RetentionDays != self.RetentionDays {
+		if input.RetentionDays != sp.RetentionDays {
 			updateNum++
 		}
 	}
@@ -247,7 +247,7 @@ func (self *SSnapshotPolicy) UpdateParamCheck(input *api.SSnapshotPolicyCreateIn
 		if err != nil {
 			return httperrors.NewInputParameterError(err.Error())
 		}
-		if self.RepeatWeekdays != SnapshotPolicyManager.RepeatWeekdaysParseIntArray(input.RepeatWeekdays) {
+		if sp.RepeatWeekdays != SnapshotPolicyManager.RepeatWeekdaysParseIntArray(input.RepeatWeekdays) {
 			updateNum++
 		}
 	}
@@ -257,7 +257,7 @@ func (self *SSnapshotPolicy) UpdateParamCheck(input *api.SSnapshotPolicyCreateIn
 		if err != nil {
 			return httperrors.NewInputParameterError(err.Error())
 		}
-		if self.TimePoints != SnapshotPolicyManager.TimePointsParseIntArray(input.TimePoints) {
+		if sp.TimePoints != SnapshotPolicyManager.TimePointsParseIntArray(input.TimePoints) {
 			updateNum++
 		}
 	}
@@ -270,33 +270,33 @@ func (self *SSnapshotPolicy) UpdateParamCheck(input *api.SSnapshotPolicyCreateIn
 
 // ==================================================== delete =========================================================
 
-func (self *SSnapshotPolicy) DetachAfterDelete(ctx context.Context, userCred mcclient.TokenCredential) error {
-	err := SnapshotPolicyDiskManager.SyncDetachBySnapshotpolicy(ctx, userCred, nil, self)
+func (sp *SSnapshotPolicy) DetachAfterDelete(ctx context.Context, userCred mcclient.TokenCredential) error {
+	err := SnapshotPolicyDiskManager.SyncDetachBySnapshotpolicy(ctx, userCred, nil, sp)
 	if err != nil {
 		return errors.Wrap(err, "detach after delete failed")
 	}
 	return nil
 }
 
-func (self *SSnapshotPolicy) CustomizeDelete(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.
+func (sp *SSnapshotPolicy) CustomizeDelete(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.
 	JSONObject, data jsonutils.JSONObject) error {
 
-	// check if self bind to some disks
-	sds, err := SnapshotPolicyDiskManager.FetchAllBySnapshotpolicyID(ctx, userCred, self.GetId())
+	// check if sp bind to some disks
+	sds, err := SnapshotPolicyDiskManager.FetchAllBySnapshotpolicyID(ctx, userCred, sp.GetId())
 	if err != nil {
 		return errors.Wrap(err, "fetch bind info failed")
 	}
 	if len(sds) != 0 {
 		return httperrors.NewBadRequestError("Couldn't delete snapshot policy binding to disks")
 	}
-	self.SetStatus(userCred, api.SNAPSHOT_POLICY_DELETING, "")
-	return self.StartSnapshotPolicyDeleteTask(ctx, userCred, jsonutils.NewDict(), "")
+	sp.SetStatus(userCred, api.SNAPSHOT_POLICY_DELETING, "")
+	return sp.StartSnapshotPolicyDeleteTask(ctx, userCred, jsonutils.NewDict(), "")
 }
 
-func (self *SSnapshotPolicy) StartSnapshotPolicyDeleteTask(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) StartSnapshotPolicyDeleteTask(ctx context.Context, userCred mcclient.TokenCredential,
 	params *jsonutils.JSONDict, parentTaskId string) error {
 
-	task, err := taskman.TaskManager.NewTask(ctx, "SnapshotPolicyDeleteTask", self, userCred, params,
+	task, err := taskman.TaskManager.NewTask(ctx, "SnapshotPolicyDeleteTask", sp, userCred, params,
 		parentTaskId, "", nil)
 	if err != nil {
 		return err
@@ -305,29 +305,29 @@ func (self *SSnapshotPolicy) StartSnapshotPolicyDeleteTask(ctx context.Context,
 	return nil
 }
 
-func (self *SSnapshotPolicy) GetCustomizeColumns(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) GetCustomizeColumns(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject) *jsonutils.JSONDict {
 
-	ret, _ := self.getMoreDetails(ctx, userCred, query)
+	ret, _ := sp.getMoreDetails(ctx, userCred, query)
 	return ret
 }
 
-func (self *SSnapshotPolicy) GetExtraDetails(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) GetExtraDetails(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject) (*jsonutils.JSONDict, error) {
 
-	return self.getMoreDetails(ctx, userCred, query)
+	return sp.getMoreDetails(ctx, userCred, query)
 }
 
-func (self *SSnapshotPolicy) getMoreDetails(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) getMoreDetails(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject) (*jsonutils.JSONDict, error) {
 
 	ret := query.(*jsonutils.JSONDict)
 	// more
-	weekdays := SnapshotPolicyManager.RepeatWeekdaysToIntArray(self.RepeatWeekdays)
-	timePoints := SnapshotPolicyManager.TimePointsToIntArray(self.TimePoints)
+	weekdays := SnapshotPolicyManager.RepeatWeekdaysToIntArray(sp.RepeatWeekdays)
+	timePoints := SnapshotPolicyManager.TimePointsToIntArray(sp.TimePoints)
 	ret.Add(jsonutils.Marshal(weekdays), "repeat_weekdays")
 	ret.Add(jsonutils.Marshal(timePoints), "time_points")
-	count, err := SnapshotPolicyDiskManager.FetchDiskCountBySPID(self.Id)
+	count, err := SnapshotPolicyDiskManager.FetchDiskCountBySPID(sp.Id)
 	if err != nil {
 		return ret, err
 	}
@@ -467,7 +467,7 @@ func (manager *SSnapshotPolicyManager) newFromCloudSnapshotPolicy(
 	return &snapshotPolicy, nil
 }
 
-func (self *SSnapshotPolicy) Equals(cloudSP cloudprovider.ICloudSnapshotPolicy) bool {
+func (sp *SSnapshotPolicy) Equals(cloudSP cloudprovider.ICloudSnapshotPolicy) bool {
 	rws, err := cloudSP.GetRepeatWeekdays()
 	if err != nil {
 		return false
@@ -479,8 +479,8 @@ func (self *SSnapshotPolicy) Equals(cloudSP cloudprovider.ICloudSnapshotPolicy)
 	repeatWeekdays := SnapshotPolicyManager.RepeatWeekdaysParseIntArray(rws)
 	timePoints := SnapshotPolicyManager.TimePointsParseIntArray(tps)
 
-	return self.RetentionDays == cloudSP.GetRetentionDays() && self.RepeatWeekdays == repeatWeekdays && self.
-		TimePoints == timePoints && self.IsActivated.Bool() == cloudSP.IsActivated()
+	return sp.RetentionDays == cloudSP.GetRetentionDays() && sp.RepeatWeekdays == repeatWeekdays && sp.
+		TimePoints == timePoints && sp.IsActivated.Bool() == cloudSP.IsActivated()
 }
 
 func (manager *SSnapshotPolicyManager) getProviderSnapshotPolicies(region *SCloudregion, provider *SCloudprovider) ([]SSnapshotPolicy, error) {
@@ -496,25 +496,25 @@ func (manager *SSnapshotPolicyManager) getProviderSnapshotPolicies(region *SClou
 	return snapshotPolicies, nil
 }
 
-func (self *SSnapshotPolicy) syncRemoveCloudSnapshot(ctx context.Context, userCred mcclient.TokenCredential) error {
-	lockman.LockObject(ctx, self)
-	defer lockman.ReleaseObject(ctx, self)
+func (sp *SSnapshotPolicy) syncRemoveCloudSnapshot(ctx context.Context, userCred mcclient.TokenCredential) error {
+	lockman.LockObject(ctx, sp)
+	defer lockman.ReleaseObject(ctx, sp)
 
-	err := self.ValidateDeleteCondition(ctx)
+	err := sp.ValidateDeleteCondition(ctx)
 	if err != nil {
-		err = self.SetStatus(userCred, api.SNAPSHOT_POLICY_UNKNOWN, "sync to delete")
+		err = sp.SetStatus(userCred, api.SNAPSHOT_POLICY_UNKNOWN, "sync to delete")
 	} else {
-		err = self.RealDelete(ctx, userCred)
+		err = sp.RealDelete(ctx, userCred)
 	}
 	return err
 }
 
-func (self *SSnapshotPolicy) Delete(ctx context.Context, userCred mcclient.TokenCredential) error {
+func (sp *SSnapshotPolicy) Delete(ctx context.Context, userCred mcclient.TokenCredential) error {
 	return nil
 }
 
-func (self *SSnapshotPolicy) RealDelete(ctx context.Context, userCred mcclient.TokenCredential) error {
-	return db.DeleteModel(ctx, userCred, self)
+func (sp *SSnapshotPolicy) RealDelete(ctx context.Context, userCred mcclient.TokenCredential) error {
+	return db.DeleteModel(ctx, userCred, sp)
 }
 
 // ==================================================== utils ==========================================================
@@ -555,23 +555,23 @@ func (self *SSnapshotPolicyManager) TimePointsToIntArray(n uint32) []int {
 	return bitmap.Uint2IntArray(n)
 }
 
-func (self *SSnapshotPolicy) GenerateCreateSpParams() *cloudprovider.SnapshotPolicyInput {
-	intWeekdays := SnapshotPolicyManager.RepeatWeekdaysToIntArray(self.RepeatWeekdays)
-	intTimePoints := SnapshotPolicyManager.TimePointsToIntArray(self.TimePoints)
+func (sp *SSnapshotPolicy) GenerateCreateSpParams() *cloudprovider.SnapshotPolicyInput {
+	intWeekdays := SnapshotPolicyManager.RepeatWeekdaysToIntArray(sp.RepeatWeekdays)
+	intTimePoints := SnapshotPolicyManager.TimePointsToIntArray(sp.TimePoints)
 
 	return &cloudprovider.SnapshotPolicyInput{
-		RetentionDays:  self.RetentionDays,
+		RetentionDays:  sp.RetentionDays,
 		RepeatWeekdays: intWeekdays,
 		TimePoints:     intTimePoints,
-		PolicyName:     self.Name,
+		PolicyName:     sp.Name,
 	}
 }
 
 // ==================================================== action =========================================================
-func (manager *SSnapshotPolicy) AllowPerformBindDisks(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) AllowPerformBindDisks(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject) bool {
 
-	return manager.IsOwner(userCred) || db.IsAdminAllowPerform(userCred, manager, "bind-disks")
+	return sp.IsOwner(userCred) || db.IsAdminAllowPerform(userCred, sp, "bind-disks")
 }
 
 func (sp *SSnapshotPolicy) PerformBindDisks(ctx context.Context, userCred mcclient.TokenCredential,
@@ -628,10 +628,10 @@ func (sp *SSnapshotPolicy) PerformBindDisks(ctx context.Context, userCred mcclie
 	return nil, nil
 }
 
-func (manager *SSnapshotPolicy) AllowPerformUnbindDisks(ctx context.Context, userCred mcclient.TokenCredential,
+func (sp *SSnapshotPolicy) AllowPerformUnbindDisks(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject) bool {
 
-	return manager.IsOwner(userCred) || db.IsAdminAllowPerform(userCred, manager, "bind-disks")
+	return sp.IsOwner(userCred) || db.IsAdminAllowPerform(userCred, sp, "bind-disks")
 }
 
 func (sp *SSnapshotPolicy) PerformUnbindDisks(ctx context.Context, userCred mcclient.TokenCredential,

pkg/compute/tasks/natdentry_delete_task.go

@@ -19,9 +19,9 @@ import (
 	"time"
 
 	"yunion.io/x/jsonutils"
-	"yunion.io/x/onecloud/pkg/cloudprovider"
 	"yunion.io/x/pkg/errors"
 
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	api "yunion.io/x/onecloud/pkg/apis/compute"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/taskman"
@@ -52,32 +52,25 @@ func (self *SNatDEntryDeleteTask) OnInit(ctx context.Context, obj db.IStandalone
 		self.taskFailed(ctx, dnatEntry, errors.Wrap(err, "Get NatGateway failed"))
 		return
 	}
+
 	cloudNatDEntry, err := cloudNatGateway.GetINatDEntryByID(dnatEntry.ExternalId)
-	if err != nil {
+	if err == cloudprovider.ErrNotFound {
+		// already delete
+	} else if err != nil {
 		self.taskFailed(ctx, dnatEntry, errors.Wrapf(err, "Get DNat Entry by ID '%s' failed", dnatEntry.ExternalId))
 		return
-	}
-	if cloudNatDEntry == nil {
-		err = dnatEntry.Purge(ctx, self.UserCred)
+	} else if cloudNatDEntry != nil {
+		err = cloudNatDEntry.Delete()
+		if err != nil {
+			self.taskFailed(ctx, dnatEntry, errors.Wrapf(err, "Delete DNat Entry '%s' failed", dnatEntry.ExternalId))
+			return
+		}
+
+		err = cloudprovider.WaitDeleted(cloudNatDEntry, 10*time.Second, 300*time.Second)
 		if err != nil {
 			self.taskFailed(ctx, dnatEntry, err)
 			return
 		}
-		logclient.AddActionLogWithStartable(self, dnatEntry, logclient.ACT_DELETE, nil, self.UserCred, true)
-		self.SetStageComplete(ctx, nil)
-		return
-	}
-
-	err = cloudNatDEntry.Delete()
-	if err != nil {
-		self.taskFailed(ctx, dnatEntry, errors.Wrapf(err, "Delete DNat Entry '%s' failed", dnatEntry.ExternalId))
-		return
-	}
-
-	err = cloudprovider.WaitDeleted(cloudNatDEntry, 10*time.Second, 300*time.Second)
-	if err != nil {
-		self.taskFailed(ctx, dnatEntry, err)
-		return
 	}
 
 	err = dnatEntry.Purge(ctx, self.UserCred)

pkg/compute/tasks/natsentry_delete_task.go

@@ -19,9 +19,9 @@ import (
 	"time"
 
 	"yunion.io/x/jsonutils"
-	"yunion.io/x/onecloud/pkg/cloudprovider"
 	"yunion.io/x/pkg/errors"
 
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	api "yunion.io/x/onecloud/pkg/apis/compute"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/taskman"
@@ -53,31 +53,23 @@ func (self *SNatSEntryDeleteTask) OnInit(ctx context.Context, obj db.IStandalone
 		return
 	}
 	cloudNatSEntry, err := cloudNatGateway.GetINatSEntryByID(snatEntry.ExternalId)
-	if err != nil {
+	if err == cloudprovider.ErrNotFound {
+		//already delete
+	} else if err != nil {
 		self.taskFailed(ctx, snatEntry, errors.Wrapf(err, "Get SNat Entry by ID '%s' failed", snatEntry.ExternalId))
 		return
-	}
-	if cloudNatSEntry == nil {
-		err = snatEntry.Purge(ctx, self.UserCred)
+	} else if cloudNatSEntry != nil {
+		err = cloudNatSEntry.Delete()
+		if err != nil {
+			self.taskFailed(ctx, snatEntry, errors.Wrapf(err, "Delete SNat Entry '%s' failed", snatEntry.ExternalId))
+			return
+		}
+
+		err = cloudprovider.WaitDeleted(cloudNatSEntry, 10*time.Second, 300*time.Second)
 		if err != nil {
 			self.taskFailed(ctx, snatEntry, err)
 			return
 		}
-		logclient.AddActionLogWithStartable(self, snatEntry, logclient.ACT_DELETE, nil, self.UserCred, true)
-		self.SetStageComplete(ctx, nil)
-		return
-	}
-
-	err = cloudNatSEntry.Delete()
-	if err != nil {
-		self.taskFailed(ctx, snatEntry, errors.Wrapf(err, "Delete SNat Entry '%s' failed", snatEntry.ExternalId))
-		return
-	}
-
-	err = cloudprovider.WaitDeleted(cloudNatSEntry, 10*time.Second, 300*time.Second)
-	if err != nil {
-		self.taskFailed(ctx, snatEntry, err)
-		return
 	}
 
 	err = snatEntry.Purge(ctx, self.UserCred)