refactor(quota,middleware): skip overflow error when subtract resources

1. Skip overflow error when subtract resources
2. Take up resources before handle request and put it back when handle
failed for add action in quota interceptor
3. Free resources only after handle success for subtract action in quota
interceptor

Closes #8681

Signed-off-by: He Weiwei <hweiwei@vmware.com>

src/common/quota/manager.go

@@ -110,7 +110,8 @@ func (m *Manager) getUsageForUpdate(o orm.Ormer) (*models.QuotaUsage, error) {
 }
 
 func (m *Manager) updateUsage(o orm.Ormer, resources types.ResourceList,
-	calculate func(types.ResourceList, types.ResourceList) types.ResourceList) error {
+	calculate func(types.ResourceList, types.ResourceList) types.ResourceList,
+	skipOverflow bool) error {
 
 	quota, err := m.getQuotaForUpdate(o)
 	if err != nil {
@@ -137,7 +138,7 @@ func (m *Manager) updateUsage(o orm.Ormer, resources types.ResourceList,
 		return fmt.Errorf("quota usage is negative for resource(s): %s", prettyPrintResourceNames(negativeUsed))
 	}
 
-	if err := isSafe(hardLimits, used, newUsed); err != nil {
+	if err := isSafe(hardLimits, used, newUsed, skipOverflow); err != nil {
 		return err
 	}
 
@@ -245,14 +246,14 @@ func (m *Manager) EnsureQuota(usages types.ResourceList) error {
 // AddResources add resources to usage
 func (m *Manager) AddResources(resources types.ResourceList) error {
 	return dao.WithTransaction(func(o orm.Ormer) error {
-		return m.updateUsage(o, resources, types.Add)
+		return m.updateUsage(o, resources, types.Add, false)
 	})
 }
 
 // SubtractResources subtract resources from usage
 func (m *Manager) SubtractResources(resources types.ResourceList) error {
 	return dao.WithTransaction(func(o orm.Ormer) error {
-		return m.updateUsage(o, resources, types.Subtract)
+		return m.updateUsage(o, resources, types.Subtract, true)
 	})
 }
 

src/common/quota/util.go

@@ -21,7 +21,7 @@ import (
 	"github.com/goharbor/harbor/src/pkg/types"
 )
 
-func isSafe(hardLimits types.ResourceList, currentUsed types.ResourceList, newUsed types.ResourceList) error {
+func isSafe(hardLimits types.ResourceList, currentUsed types.ResourceList, newUsed types.ResourceList, skipOverflow bool) error {
 	var errs Errors
 
 	for resource, value := range newUsed {
@@ -35,7 +35,7 @@ func isSafe(hardLimits types.ResourceList, currentUsed types.ResourceList, newUs
 			continue
 		}
 
-		if value > hardLimit {
+		if value > hardLimit && !skipOverflow {
 			errs = errs.Add(NewResourceOverflowError(resource, hardLimit, currentUsed[resource], value))
 		}
 	}

src/common/quota/util_test.go

@@ -25,6 +25,7 @@ func Test_isSafe(t *testing.T) {
 		hardLimits   types.ResourceList
 		currentUsed  types.ResourceList
 		newUsed      types.ResourceList
+		skipOverflow bool
 	}
 	tests := []struct {
 		name    string
@@ -37,6 +38,7 @@ func Test_isSafe(t *testing.T) {
 				types.ResourceList{types.ResourceStorage: types.UNLIMITED},
 				types.ResourceList{types.ResourceStorage: 1000},
 				types.ResourceList{types.ResourceStorage: 1000},
+				false,
 			},
 			false,
 		},
@@ -46,6 +48,7 @@ func Test_isSafe(t *testing.T) {
 				types.ResourceList{types.ResourceStorage: 100},
 				types.ResourceList{types.ResourceStorage: 10},
 				types.ResourceList{types.ResourceStorage: 1},
+				false,
 			},
 			false,
 		},
@@ -55,22 +58,34 @@ func Test_isSafe(t *testing.T) {
 				types.ResourceList{types.ResourceStorage: 100},
 				types.ResourceList{types.ResourceStorage: 0},
 				types.ResourceList{types.ResourceStorage: 200},
+				false,
 			},
 			true,
 		},
+		{
+			"skip overflow",
+			args{
+				types.ResourceList{types.ResourceStorage: 100},
+				types.ResourceList{types.ResourceStorage: 0},
+				types.ResourceList{types.ResourceStorage: 200},
+				true,
+			},
+			false,
+		},
 		{
 			"hard limit not found",
 			args{
 				types.ResourceList{types.ResourceStorage: 100},
 				types.ResourceList{types.ResourceCount: 0},
 				types.ResourceList{types.ResourceCount: 1},
+				false,
 			},
 			true,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := isSafe(tt.args.hardLimits, tt.args.currentUsed, tt.args.newUsed); (err != nil) != tt.wantErr {
+			if err := isSafe(tt.args.hardLimits, tt.args.currentUsed, tt.args.newUsed, tt.args.skipOverflow); (err != nil) != tt.wantErr {
 				t.Errorf("isSafe() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})

src/core/middlewares/interceptor/quota/quota.go

@@ -16,7 +16,9 @@ package quota
 
 import (
 	"fmt"
+	"math/rand"
 	"net/http"
+	"time"
 
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/common/utils/redis"
@@ -24,6 +26,10 @@ import (
 	"github.com/goharbor/harbor/src/pkg/types"
 )
 
+func init() {
+	rand.Seed(time.Now().UnixNano())
+}
+
 // New ....
 func New(opts ...Option) interceptor.Interceptor {
 	options := newOptions(opts...)
@@ -59,7 +65,7 @@ func (qi *quotaInterceptor) HandleRequest(req *http.Request) (err error) {
 		return
 	}
 
-	err = qi.reserve()
+	err = qi.doTry()
 	if err != nil {
 		log.Errorf("Failed to %s resources, error: %v", qi.opts.Action, err)
 	}
@@ -80,14 +86,18 @@ func (qi *quotaInterceptor) HandleResponse(w http.ResponseWriter, req *http.Requ
 
 	switch sr.Status() {
 	case opts.StatusCode:
+		if err := qi.doConfirm(); err != nil {
+			log.Errorf("Failed to confirm for resource, error: %v", err)
+		}
+
 		if opts.OnFulfilled != nil {
 			if err := opts.OnFulfilled(w, req); err != nil {
 				log.Errorf("Failed to handle on fulfilled, error: %v", err)
 			}
 		}
 	default:
-		if err := qi.unreserve(); err != nil {
+		if err := qi.doCancel(); err != nil {
-			log.Errorf("Failed to %s resources, error: %v", opts.Action, err)
+			log.Errorf("Failed to cancel for resource, error: %v", err)
 		}
 
 		if opts.OnRejected != nil {
@@ -148,42 +158,65 @@ func (qi *quotaInterceptor) computeResources(req *http.Request) error {
 	return nil
 }
 
-func (qi *quotaInterceptor) reserve() error {
+func (qi *quotaInterceptor) doTry() error {
 	if !qi.opts.EnforceResources() {
-		// Do nothing in reserve resources when quota interceptor not enforce resources
+		// Do nothing in try stage when quota interceptor not enforce resources
 		return nil
 	}
 
-	if len(qi.resources) == 0 {
+	// Add resources in try stage when it is add action
-		return nil
+	// And do nothing in confirm stage for add action
-	}
+	if len(qi.resources) != 0 && qi.opts.Action == AddAction {
-
-	switch qi.opts.Action {
-	case AddAction:
-		return qi.opts.Manager.AddResources(qi.resources)
-	case SubtractAction:
-		return qi.opts.Manager.SubtractResources(qi.resources)
-	}
-
-	return nil
-}
-
-func (qi *quotaInterceptor) unreserve() error {
-	if !qi.opts.EnforceResources() {
-		// Do nothing in unreserve resources when quota interceptor not enforce resources
-		return nil
-	}
-
-	if len(qi.resources) == 0 {
-		return nil
-	}
-
-	switch qi.opts.Action {
-	case AddAction:
-		return qi.opts.Manager.SubtractResources(qi.resources)
-	case SubtractAction:
 		return qi.opts.Manager.AddResources(qi.resources)
 	}
 
 	return nil
 }
+
+func (qi *quotaInterceptor) doConfirm() error {
+	if !qi.opts.EnforceResources() {
+		// Do nothing in confirm stage when quota interceptor not enforce resources
+		return nil
+	}
+
+	// Subtract resources in confirm stage when it is subtract action
+	// And do nothing in try stage for subtract action
+	if len(qi.resources) != 0 && qi.opts.Action == SubtractAction {
+		return retry(3, 100*time.Millisecond, func() error {
+			return qi.opts.Manager.SubtractResources(qi.resources)
+		})
+	}
+
+	return nil
+}
+
+func (qi *quotaInterceptor) doCancel() error {
+	if !qi.opts.EnforceResources() {
+		// Do nothing in cancel stage when quota interceptor not enforce resources
+		return nil
+	}
+
+	// Subtract resources back when process failed for add action
+	if len(qi.resources) != 0 && qi.opts.Action == AddAction {
+		return retry(3, 100*time.Millisecond, func() error {
+			return qi.opts.Manager.SubtractResources(qi.resources)
+		})
+	}
+
+	return nil
+}
+
+func retry(attempts int, sleep time.Duration, f func() error) error {
+	if err := f(); err != nil {
+		if attempts--; attempts > 0 {
+			r := time.Duration(rand.Int63n(int64(sleep)))
+			sleep = sleep + r/2
+
+			time.Sleep(sleep)
+			return retry(attempts, 2*sleep, f)
+		}
+		return err
+	}
+
+	return nil
+}