From c279b7f3e95f154b4cb5f861b797b8317a88cb4b Mon Sep 17 00:00:00 2001 From: Ziming Date: Thu, 15 Aug 2019 20:12:59 +0800 Subject: [PATCH] fix retention rule compute error (#8664) Change-Id: I16d7284b17508885e136f2d9ea5651978ba4a6d8 Signed-off-by: Ziming Zhang --- src/core/api/retention.go | 4 ++-- src/pkg/retention/policy/rule/dayspl/evaluator.go | 7 ++++--- src/pkg/retention/policy/rule/dayspl/evaluator_test.go | 10 +++++----- src/pkg/retention/policy/rule/daysps/evaluator.go | 7 ++++--- src/pkg/retention/policy/rule/daysps/evaluator_test.go | 10 +++++----- src/pkg/retention/policy/rule/lastx/evaluator.go | 6 +++--- src/pkg/retention/policy/rule/lastx/evaluator_test.go | 8 ++++---- src/pkg/retention/policy/rule/latestk/evaluator.go | 4 ++-- .../retention/policy/rule/latestk/evaluator_test.go | 8 ++++---- src/pkg/retention/policy/rule/latestpl/evaluator.go | 6 +++--- .../retention/policy/rule/latestpl/evaluator_test.go | 10 +++++----- src/pkg/retention/policy/rule/latestps/evaluator.go | 6 +++--- .../retention/policy/rule/latestps/evaluator_test.go | 10 +++++----- 13 files changed, 49 insertions(+), 47 deletions(-) diff --git a/src/core/api/retention.go b/src/core/api/retention.go index 76fea34b3..1a14bfdaa 100644 --- a/src/core/api/retention.go +++ b/src/core/api/retention.go @@ -67,7 +67,7 @@ func (r *RetentionAPI) GetMetadatas() { ] }, { - "rule_template": "dayspl", + "rule_template": "nDaysSinceLastPull", "display_text": "pulled within the last # days", "action": "retain", "params": [ @@ -79,7 +79,7 @@ func (r *RetentionAPI) GetMetadatas() { ] }, { - "rule_template": "daysps", + "rule_template": "nDaysSinceLastPush", "display_text": "pushed within the last # days", "action": "retain", "params": [ diff --git a/src/pkg/retention/policy/rule/dayspl/evaluator.go b/src/pkg/retention/policy/rule/dayspl/evaluator.go index 9b2fb34e9..c0fd76256 100644 --- a/src/pkg/retention/policy/rule/dayspl/evaluator.go +++ b/src/pkg/retention/policy/rule/dayspl/evaluator.go @@ -58,12 +58,13 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if p, ok := params[ParameterN]; ok { - if v, ok := p.(int); ok && v >= 0 { - return &evaluator{n: v} + if v, ok := p.(float64); ok && v >= 0 { + return &evaluator{n: int(v)} } } } - log.Debugf("default parameter %d used for rule %s", DefaultN, TemplateID) + log.Warningf("default parameter %d used for rule %s", DefaultN, TemplateID) + return &evaluator{n: DefaultN} } diff --git a/src/pkg/retention/policy/rule/dayspl/evaluator_test.go b/src/pkg/retention/policy/rule/dayspl/evaluator_test.go index 0c8ba1ec1..a8587ccd8 100644 --- a/src/pkg/retention/policy/rule/dayspl/evaluator_test.go +++ b/src/pkg/retention/policy/rule/dayspl/evaluator_test.go @@ -15,7 +15,7 @@ package dayspl import ( - "strconv" + "fmt" "testing" "time" @@ -36,8 +36,8 @@ func (e *EvaluatorTestSuite) TestNew() { args rule.Parameters expectedN int }{ - {Name: "Valid", args: map[string]rule.Parameter{ParameterN: 5}, expectedN: 5}, - {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterN: -1}, expectedN: DefaultN}, + {Name: "Valid", args: map[string]rule.Parameter{ParameterN: float64(5)}, expectedN: 5}, + {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterN: float64(-1)}, expectedN: DefaultN}, {Name: "Default If Not Set", args: map[string]rule.Parameter{}, expectedN: DefaultN}, {Name: "Default If Wrong Type", args: map[string]rule.Parameter{ParameterN: "foo"}, expectedN: DefaultN}, } @@ -65,7 +65,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } tests := []struct { - n int + n float64 expected int minPullTime int64 }{ @@ -80,7 +80,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } for _, tt := range tests { - e.T().Run(strconv.Itoa(tt.n), func(t *testing.T) { + e.T().Run(fmt.Sprintf("%v", tt.n), func(t *testing.T) { sut := New(map[string]rule.Parameter{ParameterN: tt.n}) result, err := sut.Process(data) diff --git a/src/pkg/retention/policy/rule/daysps/evaluator.go b/src/pkg/retention/policy/rule/daysps/evaluator.go index 2c121dde7..ee4dd436d 100644 --- a/src/pkg/retention/policy/rule/daysps/evaluator.go +++ b/src/pkg/retention/policy/rule/daysps/evaluator.go @@ -58,12 +58,13 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if p, ok := params[ParameterN]; ok { - if v, ok := p.(int); ok && v >= 0 { - return &evaluator{n: v} + if v, ok := p.(float64); ok && v >= 0 { + return &evaluator{n: int(v)} } } } - log.Debugf("default parameter %d used for rule %s", DefaultN, TemplateID) + log.Warningf("default parameter %d used for rule %s", DefaultN, TemplateID) + return &evaluator{n: DefaultN} } diff --git a/src/pkg/retention/policy/rule/daysps/evaluator_test.go b/src/pkg/retention/policy/rule/daysps/evaluator_test.go index 07c9f9bdd..75287ce4f 100644 --- a/src/pkg/retention/policy/rule/daysps/evaluator_test.go +++ b/src/pkg/retention/policy/rule/daysps/evaluator_test.go @@ -15,7 +15,7 @@ package daysps import ( - "strconv" + "fmt" "testing" "time" @@ -36,8 +36,8 @@ func (e *EvaluatorTestSuite) TestNew() { args rule.Parameters expectedN int }{ - {Name: "Valid", args: map[string]rule.Parameter{ParameterN: 5}, expectedN: 5}, - {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterN: -1}, expectedN: DefaultN}, + {Name: "Valid", args: map[string]rule.Parameter{ParameterN: float64(5)}, expectedN: 5}, + {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterN: float64(-1)}, expectedN: DefaultN}, {Name: "Default If Not Set", args: map[string]rule.Parameter{}, expectedN: DefaultN}, {Name: "Default If Wrong Type", args: map[string]rule.Parameter{ParameterN: "foo"}, expectedN: DefaultN}, } @@ -65,7 +65,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } tests := []struct { - n int + n float64 expected int minPushTime int64 }{ @@ -80,7 +80,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } for _, tt := range tests { - e.T().Run(strconv.Itoa(tt.n), func(t *testing.T) { + e.T().Run(fmt.Sprintf("%v", tt.n), func(t *testing.T) { sut := New(map[string]rule.Parameter{ParameterN: tt.n}) result, err := sut.Process(data) diff --git a/src/pkg/retention/policy/rule/lastx/evaluator.go b/src/pkg/retention/policy/rule/lastx/evaluator.go index 6d98c5c2d..b466f5eda 100644 --- a/src/pkg/retention/policy/rule/lastx/evaluator.go +++ b/src/pkg/retention/policy/rule/lastx/evaluator.go @@ -59,15 +59,15 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if param, ok := params[ParameterX]; ok { - if v, ok := param.(int); ok && v >= 0 { + if v, ok := param.(float64); ok && v >= 0 { return &evaluator{ - x: v, + x: int(v), } } } } - log.Debugf("default parameter %d used for rule %s", DefaultX, TemplateID) + log.Warningf("default parameter %d used for rule %s", DefaultX, TemplateID) return &evaluator{ x: DefaultX, diff --git a/src/pkg/retention/policy/rule/lastx/evaluator_test.go b/src/pkg/retention/policy/rule/lastx/evaluator_test.go index eafc30f2f..becd79234 100644 --- a/src/pkg/retention/policy/rule/lastx/evaluator_test.go +++ b/src/pkg/retention/policy/rule/lastx/evaluator_test.go @@ -21,8 +21,8 @@ func (e *EvaluatorTestSuite) TestNew() { args rule.Parameters expectedX int }{ - {Name: "Valid", args: map[string]rule.Parameter{ParameterX: 3}, expectedX: 3}, - {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterX: -3}, expectedX: DefaultX}, + {Name: "Valid", args: map[string]rule.Parameter{ParameterX: float64(3)}, expectedX: 3}, + {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterX: float64(-3)}, expectedX: DefaultX}, {Name: "Default If Not Set", args: map[string]rule.Parameter{}, expectedX: DefaultX}, {Name: "Default If Wrong Type", args: map[string]rule.Parameter{}, expectedX: DefaultX}, } @@ -48,7 +48,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } tests := []struct { - days int + days float64 expected int }{ {days: 0, expected: 0}, @@ -62,7 +62,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } for _, tt := range tests { - e.T().Run(fmt.Sprintf("%d days - should keep %d", tt.days, tt.expected), func(t *testing.T) { + e.T().Run(fmt.Sprintf("%v days - should keep %d", tt.days, tt.expected), func(t *testing.T) { e := New(rule.Parameters{ParameterX: tt.days}) result, err := e.Process(data) diff --git a/src/pkg/retention/policy/rule/latestk/evaluator.go b/src/pkg/retention/policy/rule/latestk/evaluator.go index bb1d246de..f6d73599a 100644 --- a/src/pkg/retention/policy/rule/latestk/evaluator.go +++ b/src/pkg/retention/policy/rule/latestk/evaluator.go @@ -65,9 +65,9 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if param, ok := params[ParameterK]; ok { - if v, ok := param.(int); ok && v >= 0 { + if v, ok := param.(float64); ok && v >= 0 { return &evaluator{ - k: v, + k: int(v), } } } diff --git a/src/pkg/retention/policy/rule/latestk/evaluator_test.go b/src/pkg/retention/policy/rule/latestk/evaluator_test.go index ab2967f51..24b04fb9e 100644 --- a/src/pkg/retention/policy/rule/latestk/evaluator_test.go +++ b/src/pkg/retention/policy/rule/latestk/evaluator_test.go @@ -15,7 +15,7 @@ package latestk import ( - "strconv" + "fmt" "testing" "github.com/goharbor/harbor/src/pkg/retention/policy/rule" @@ -58,7 +58,7 @@ func (e *EvaluatorTestSuite) TestProcess() { {k: 99, expected: len(e.artifacts)}, } for _, tt := range tests { - e.T().Run(strconv.Itoa(tt.k), func(t *testing.T) { + e.T().Run(fmt.Sprintf("%v", tt.k), func(t *testing.T) { sut := &evaluator{k: tt.k} result, err := sut.Process(e.artifacts) @@ -79,8 +79,8 @@ func (e *EvaluatorTestSuite) TestNew() { params rule.Parameters expectedK int }{ - {name: "Valid", params: rule.Parameters{ParameterK: 5}, expectedK: 5}, - {name: "Default If Negative", params: rule.Parameters{ParameterK: -5}, expectedK: DefaultK}, + {name: "Valid", params: rule.Parameters{ParameterK: float64(5)}, expectedK: 5}, + {name: "Default If Negative", params: rule.Parameters{ParameterK: float64(-5)}, expectedK: DefaultK}, {name: "Default If Wrong Type", params: rule.Parameters{ParameterK: "5"}, expectedK: DefaultK}, {name: "Default If Wrong Key", params: rule.Parameters{"n": 5}, expectedK: DefaultK}, {name: "Default If Empty", params: rule.Parameters{}, expectedK: DefaultK}, diff --git a/src/pkg/retention/policy/rule/latestpl/evaluator.go b/src/pkg/retention/policy/rule/latestpl/evaluator.go index 620790a73..bed7b6e4e 100644 --- a/src/pkg/retention/policy/rule/latestpl/evaluator.go +++ b/src/pkg/retention/policy/rule/latestpl/evaluator.go @@ -59,13 +59,13 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if p, ok := params[ParameterN]; ok { - if v, ok := p.(int); ok && v >= 0 { - return &evaluator{n: v} + if v, ok := p.(float64); ok && v >= 0 { + return &evaluator{n: int(v)} } } } - log.Debugf("default parameter %d used for rule %s", DefaultN, TemplateID) + log.Warningf("default parameter %d used for rule %s", DefaultN, TemplateID) return &evaluator{n: DefaultN} } diff --git a/src/pkg/retention/policy/rule/latestpl/evaluator_test.go b/src/pkg/retention/policy/rule/latestpl/evaluator_test.go index 443481649..69b0605f5 100644 --- a/src/pkg/retention/policy/rule/latestpl/evaluator_test.go +++ b/src/pkg/retention/policy/rule/latestpl/evaluator_test.go @@ -15,8 +15,8 @@ package latestpl import ( + "fmt" "math/rand" - "strconv" "testing" "github.com/goharbor/harbor/src/pkg/retention/policy/rule" @@ -35,8 +35,8 @@ func (e *EvaluatorTestSuite) TestNew() { args rule.Parameters expectedK int }{ - {Name: "Valid", args: map[string]rule.Parameter{ParameterN: 5}, expectedK: 5}, - {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterN: -1}, expectedK: DefaultN}, + {Name: "Valid", args: map[string]rule.Parameter{ParameterN: float64(5)}, expectedK: 5}, + {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterN: float64(-1)}, expectedK: DefaultN}, {Name: "Default If Not Set", args: map[string]rule.Parameter{}, expectedK: DefaultN}, {Name: "Default If Wrong Type", args: map[string]rule.Parameter{ParameterN: "foo"}, expectedK: DefaultN}, } @@ -57,7 +57,7 @@ func (e *EvaluatorTestSuite) TestProcess() { }) tests := []struct { - n int + n float64 expected int minPullTime int64 }{ @@ -69,7 +69,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } for _, tt := range tests { - e.T().Run(strconv.Itoa(tt.n), func(t *testing.T) { + e.T().Run(fmt.Sprintf("%v", tt.n), func(t *testing.T) { ev := New(map[string]rule.Parameter{ParameterN: tt.n}) result, err := ev.Process(data) diff --git a/src/pkg/retention/policy/rule/latestps/evaluator.go b/src/pkg/retention/policy/rule/latestps/evaluator.go index 8ac090b3f..ac000a302 100644 --- a/src/pkg/retention/policy/rule/latestps/evaluator.go +++ b/src/pkg/retention/policy/rule/latestps/evaluator.go @@ -62,15 +62,15 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if param, ok := params[ParameterK]; ok { - if v, ok := param.(int); ok && v >= 0 { + if v, ok := param.(float64); ok && v >= 0 { return &evaluator{ - k: v, + k: int(v), } } } } - log.Debugf("default parameter %d used for rule %s", DefaultK, TemplateID) + log.Warningf("default parameter %d used for rule %s", DefaultK, TemplateID) return &evaluator{ k: DefaultK, diff --git a/src/pkg/retention/policy/rule/latestps/evaluator_test.go b/src/pkg/retention/policy/rule/latestps/evaluator_test.go index 7136b69d6..6e303c3c4 100644 --- a/src/pkg/retention/policy/rule/latestps/evaluator_test.go +++ b/src/pkg/retention/policy/rule/latestps/evaluator_test.go @@ -1,8 +1,8 @@ package latestps import ( + "fmt" "math/rand" - "strconv" "testing" "github.com/stretchr/testify/suite" @@ -22,8 +22,8 @@ func (e *EvaluatorTestSuite) TestNew() { args rule.Parameters expectedK int }{ - {Name: "Valid", args: map[string]rule.Parameter{ParameterK: 5}, expectedK: 5}, - {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterK: -1}, expectedK: DefaultK}, + {Name: "Valid", args: map[string]rule.Parameter{ParameterK: float64(5)}, expectedK: 5}, + {Name: "Default If Negative", args: map[string]rule.Parameter{ParameterK: float64(-1)}, expectedK: DefaultK}, {Name: "Default If Not Set", args: map[string]rule.Parameter{}, expectedK: DefaultK}, {Name: "Default If Wrong Type", args: map[string]rule.Parameter{ParameterK: "foo"}, expectedK: DefaultK}, } @@ -44,7 +44,7 @@ func (e *EvaluatorTestSuite) TestProcess() { }) tests := []struct { - k int + k float64 expected int }{ {k: 0, expected: 0}, @@ -55,7 +55,7 @@ func (e *EvaluatorTestSuite) TestProcess() { } for _, tt := range tests { - e.T().Run(strconv.Itoa(tt.k), func(t *testing.T) { + e.T().Run(fmt.Sprintf("%v", tt.k), func(t *testing.T) { e := New(map[string]rule.Parameter{ParameterK: tt.k}) result, err := e.Process(data)