diff --git a/src/common/quota/manager.go b/src/common/quota/manager.go index a70199ed3..cfcbe4741 100644 --- a/src/common/quota/manager.go +++ b/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) }) } diff --git a/src/common/quota/util.go b/src/common/quota/util.go index 5f8687cc7..e57e05170 100644 --- a/src/common/quota/util.go +++ b/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)) } } diff --git a/src/common/quota/util_test.go b/src/common/quota/util_test.go index d0db1166a..44432348d 100644 --- a/src/common/quota/util_test.go +++ b/src/common/quota/util_test.go @@ -22,9 +22,10 @@ import ( func Test_isSafe(t *testing.T) { type args struct { - hardLimits types.ResourceList - currentUsed types.ResourceList - newUsed types.ResourceList + 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) } }) diff --git a/src/core/middlewares/interceptor/quota/quota.go b/src/core/middlewares/interceptor/quota/quota.go index 607f58dde..1ae530078 100644 --- a/src/core/middlewares/interceptor/quota/quota.go +++ b/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 { - log.Errorf("Failed to %s resources, error: %v", opts.Action, err) + if err := qi.doCancel(); err != nil { + 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 { - return nil - } - - 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: + // Add resources in try stage when it is add action + // And do nothing in confirm stage for add action + if len(qi.resources) != 0 && qi.opts.Action == AddAction { 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 +}