diff --git a/src/controller/p2p/preheat/controller.go b/src/controller/p2p/preheat/controller.go index cc7f9c5b4..2d648149e 100644 --- a/src/controller/p2p/preheat/controller.go +++ b/src/controller/p2p/preheat/controller.go @@ -13,6 +13,7 @@ import ( "github.com/goharbor/harbor/src/pkg/p2p/preheat/policy" "github.com/goharbor/harbor/src/pkg/p2p/preheat/provider" "github.com/goharbor/harbor/src/pkg/scheduler" + "github.com/goharbor/harbor/src/pkg/task" ) const ( @@ -126,16 +127,18 @@ type controller struct { // For instance iManager instance.Manager // For policy - pManager policy.Manager - scheduler scheduler.Scheduler + pManager policy.Manager + scheduler scheduler.Scheduler + executionMgr task.ExecutionManager } // NewController is constructor of controller func NewController() Controller { return &controller{ - iManager: instance.Mgr, - pManager: policy.Mgr, - scheduler: scheduler.Sched, + iManager: instance.Mgr, + pManager: policy.Mgr, + scheduler: scheduler.Sched, + executionMgr: task.NewExecutionManager(), } } @@ -341,8 +344,7 @@ func (c *controller) UpdatePolicy(ctx context.Context, schema *policyModels.Sche } } else { // not change trigger type - if schema.Trigger.Type == policyModels.TriggerTypeScheduled && - s0.Trigger.Settings.Cron != cron { + if schema.Trigger.Type == policyModels.TriggerTypeScheduled && oldCron != cron { // unschedule old if len(oldCron) > 0 { needUn = true @@ -370,11 +372,6 @@ func (c *controller) UpdatePolicy(ctx context.Context, schema *policyModels.Sche TriggerParam{PolicyID: schema.ID}); err != nil { return err } - if err := schema.Encode(); err != nil { - // Possible - // TODO: Refactor - return err // whether update or not has been not important as the jon reference will be lost - } } // Update timestamp @@ -396,9 +393,35 @@ func (c *controller) DeletePolicy(ctx context.Context, id int64) error { } } + if err = c.deleteExecs(ctx, id); err != nil { + return err + } + return c.pManager.Delete(ctx, id) } +// deleteExecs delete executions +func (c *controller) deleteExecs(ctx context.Context, vendorID int64) error { + executions, err := c.executionMgr.List(ctx, &q.Query{ + Keywords: map[string]interface{}{ + "VendorType": job.P2PPreheat, + "VendorID": vendorID, + }, + }) + + if err != nil { + return err + } + + for _, execution := range executions { + if err = c.executionMgr.Delete(ctx, execution.ID); err != nil { + return err + } + } + + return nil +} + // ListPolicies lists policies by query. func (c *controller) ListPolicies(ctx context.Context, query *q.Query) ([]*policyModels.Schema, error) { return c.pManager.ListPolicies(ctx, query) diff --git a/src/controller/p2p/preheat/controllor_test.go b/src/controller/p2p/preheat/controllor_test.go index 3fd554f16..4413912c4 100644 --- a/src/controller/p2p/preheat/controllor_test.go +++ b/src/controller/p2p/preheat/controllor_test.go @@ -15,10 +15,12 @@ import ( providerModel "github.com/goharbor/harbor/src/pkg/p2p/preheat/models/provider" "github.com/goharbor/harbor/src/pkg/p2p/preheat/provider" "github.com/goharbor/harbor/src/pkg/p2p/preheat/provider/auth" + taskModel "github.com/goharbor/harbor/src/pkg/task" ormtesting "github.com/goharbor/harbor/src/testing/lib/orm" "github.com/goharbor/harbor/src/testing/pkg/p2p/preheat/instance" pmocks "github.com/goharbor/harbor/src/testing/pkg/p2p/preheat/policy" smocks "github.com/goharbor/harbor/src/testing/pkg/scheduler" + tmocks "github.com/goharbor/harbor/src/testing/pkg/task" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" @@ -32,6 +34,7 @@ type preheatSuite struct { fakePolicyMgr *pmocks.FakeManager fakeScheduler *smocks.Scheduler mockInstanceServer *httptest.Server + fakeExecutionMgr *tmocks.FakeExecutionManager } func TestPreheatSuite(t *testing.T) { @@ -39,21 +42,24 @@ func TestPreheatSuite(t *testing.T) { fakeInstanceMgr := &instance.FakeManager{} fakePolicyMgr := &pmocks.FakeManager{} fakeScheduler := &smocks.Scheduler{} + fakeExecutionMgr := &tmocks.FakeExecutionManager{} var c = &controller{ - iManager: fakeInstanceMgr, - pManager: fakePolicyMgr, - scheduler: fakeScheduler, + iManager: fakeInstanceMgr, + pManager: fakePolicyMgr, + scheduler: fakeScheduler, + executionMgr: fakeExecutionMgr, } assert.NotNil(t, c) ctx := orm.NewContext(context.TODO(), &ormtesting.FakeOrmer{}) suite.Run(t, &preheatSuite{ - ctx: ctx, - controller: c, - fakeInstanceMgr: fakeInstanceMgr, - fakePolicyMgr: fakePolicyMgr, - fakeScheduler: fakeScheduler, + ctx: ctx, + controller: c, + fakeInstanceMgr: fakeInstanceMgr, + fakePolicyMgr: fakePolicyMgr, + fakeScheduler: fakeScheduler, + fakeExecutionMgr: fakeExecutionMgr, }) } @@ -291,6 +297,13 @@ func (s *preheatSuite) TestUpdatePolicy() { func (s *preheatSuite) TestDeletePolicy() { var p0 = &policy.Schema{Name: "test", Trigger: &policy.Trigger{Type: policy.TriggerTypeScheduled}} s.fakePolicyMgr.On("Get", s.ctx, int64(1)).Return(p0, nil) + s.fakeExecutionMgr.On("List", s.ctx, mock.AnythingOfType("*q.Query")).Return( + []*taskModel.Execution{ + {ID: 1}, + {ID: 2}, + }, nil, + ) + s.fakeExecutionMgr.On("Delete", mock.Anything, mock.Anything).Return(nil) s.fakePolicyMgr.On("Delete", s.ctx, int64(1)).Return(nil) err := s.controller.DeletePolicy(s.ctx, 1) s.NoError(err) diff --git a/src/pkg/p2p/preheat/policy/manager.go b/src/pkg/p2p/preheat/policy/manager.go index 756fc1596..b8f84588e 100644 --- a/src/pkg/p2p/preheat/policy/manager.go +++ b/src/pkg/p2p/preheat/policy/manager.go @@ -35,7 +35,7 @@ type Manager interface { Update(ctx context.Context, schema *policy.Schema, props ...string) (err error) // Get the policy schema by id Get(ctx context.Context, id int64) (schema *policy.Schema, err error) - // GetByName the policy schema by id + // GetByName the policy schema by project ID and name GetByName(ctx context.Context, projectID int64, name string) (schema *policy.Schema, err error) // Delete the policy schema by id Delete(ctx context.Context, id int64) (err error)