Merge pull request #12698 from kofj/fix_p2p_policy_delete

[FIX] Delete executions of the P2P policy.

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
+	pManager     policy.Manager
-	scheduler scheduler.Scheduler
+	scheduler    scheduler.Scheduler
+	executionMgr task.ExecutionManager
 }
 
 // NewController is constructor of controller
 func NewController() Controller {
 	return &controller{
-		iManager:  instance.Mgr,
+		iManager:     instance.Mgr,
-		pManager:  policy.Mgr,
+		pManager:     policy.Mgr,
-		scheduler: scheduler.Sched,
+		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 &&
+		if schema.Trigger.Type == policyModels.TriggerTypeScheduled && oldCron != cron {
-			s0.Trigger.Settings.Cron != 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)

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,
+		iManager:     fakeInstanceMgr,
-		pManager:  fakePolicyMgr,
+		pManager:     fakePolicyMgr,
-		scheduler: fakeScheduler,
+		scheduler:    fakeScheduler,
+		executionMgr: fakeExecutionMgr,
 	}
 	assert.NotNil(t, c)
 
 	ctx := orm.NewContext(context.TODO(), &ormtesting.FakeOrmer{})
 	suite.Run(t, &preheatSuite{
-		ctx:             ctx,
+		ctx:              ctx,
-		controller:      c,
+		controller:       c,
-		fakeInstanceMgr: fakeInstanceMgr,
+		fakeInstanceMgr:  fakeInstanceMgr,
-		fakePolicyMgr:   fakePolicyMgr,
+		fakePolicyMgr:    fakePolicyMgr,
-		fakeScheduler:   fakeScheduler,
+		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)

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)