Merge pull request opensandbox-group#586 from fengcone/feature/public-delRecycle

fix(controller): delete dirty pods in pool allocation

Author: Spground

kubernetes/internal/controller/allocator.go

@@ -183,6 +183,8 @@ type AllocStatus struct {
 	PodAllocation map[string]string
 	// pod request count
 	PodSupplement int32
+	// DirtyPods are pods that should be recycled (deleted) after release
+	DirtyPods []string
 }
 
 type SandboxSyncInfo struct {
@@ -387,6 +389,7 @@ func (allocator *defaultAllocator) deallocate(ctx context.Context, status *Alloc
 		for _, pod := range pods {
 			delete(status.PodAllocation, pod)
 			poolDeallocate = true
+			status.DirtyPods = append(status.DirtyPods, pod)
 		}
 		delete(sandboxToPods, name)
 	}
@@ -408,6 +411,7 @@ func (allocator *defaultAllocator) doDeallocate(ctx context.Context, status *All
 	for _, pod := range toRelease.Pods {
 		delete(status.PodAllocation, pod)
 		deallocate = true
+		status.DirtyPods = append(status.DirtyPods, pod)
 		log.V(1).Info("Pod released from sandbox", "pod", pod, "sandbox", name)
 	}
 	pods := make([]string, 0)

kubernetes/internal/controller/allocator_test.go

@@ -271,21 +271,20 @@ func TestAllocatorSchedule(t *testing.T) {
 				},
 			},
 			poolAlloc: &PoolAllocation{
-				PodAllocation: map[string]string{},
+				PodAllocation: map[string]string{
+					"pod1": "sbx1",
+				},
 			},
 			sandboxAlloc: &SandboxAllocation{
-				Pods: []string{
-					"pod1",
-				},
+				Pods: []string{"pod1"},
 			},
 			release: &AllocationRelease{
-				Pods: []string{
-					"pod1", "sbx1",
-				},
+				Pods: []string{"pod1"},
 			},
 			wantStatus: &AllocStatus{
 				PodAllocation: map[string]string{},
 				PodSupplement: 0,
+				DirtyPods:     []string{"pod1"},
 			},
 		},
 	}
@@ -298,6 +297,10 @@ func TestAllocatorSchedule(t *testing.T) {
 			syncer.EXPECT().GetRelease(gomock.Any(), gomock.Any()).Return(c.release, nil).Times(len(c.spec.Sandboxes))
 			status, _, _, err := allocator.Schedule(context.Background(), c.spec)
 			assert.NoError(t, err)
+			if !reflect.DeepEqual(c.wantStatus, status) {
+				t.Logf("want: %+v", c.wantStatus)
+				t.Logf("got: %+v", status)
+			}
 			assert.True(t, reflect.DeepEqual(c.wantStatus, status))
 		})
 	}

kubernetes/internal/controller/pool_controller.go

@@ -166,7 +166,7 @@ func (r *PoolReconciler) reconcilePool(ctx context.Context, pool *sandboxv1alpha
 		schedulePods := r.filterEvictingPods(ctx, latestPool, pods, allocBeforeSchedule)
 
 		// 4. Schedule and allocate
-		podAllocation, pendingSyncs, idlePods, supplySandbox, poolDirty, err := r.scheduleSandbox(ctx, latestPool, batchSandboxes, schedulePods)
+		podAllocation, pendingSyncs, idlePods, dirtyPods, supplySandbox, poolDirty, err := r.scheduleSandbox(ctx, latestPool, batchSandboxes, schedulePods)
 		if err != nil {
 			return err
 		}
@@ -207,7 +207,7 @@ func (r *PoolReconciler) reconcilePool(ctx context.Context, pool *sandboxv1alpha
 		if err != nil {
 			return err
 		}
-		latestIdlePods, deleteOld, supplyNew := r.updatePool(ctx, latestRevision, schedulePods, idlePods)
+		latestIdlePods, deleteOld, supplyNew := r.updatePool(ctx, latestRevision, schedulePods, idlePods, dirtyPods)
 
 		args := &scaleArgs{
 			latestRevision: latestRevision,
@@ -327,7 +327,7 @@ func (r *PoolReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *PoolReconciler) scheduleSandbox(ctx context.Context, pool *sandboxv1alpha1.Pool, batchSandboxes []*sandboxv1alpha1.BatchSandbox, pods []*corev1.Pod) (map[string]string, []SandboxSyncInfo, []string, int32, bool, error) {
+func (r *PoolReconciler) scheduleSandbox(ctx context.Context, pool *sandboxv1alpha1.Pool, batchSandboxes []*sandboxv1alpha1.BatchSandbox, pods []*corev1.Pod) (map[string]string, []SandboxSyncInfo, []string, []string, int32, bool, error) {
 	log := logf.FromContext(ctx)
 	spec := &AllocSpec{
 		Sandboxes: batchSandboxes,
@@ -336,7 +336,7 @@ func (r *PoolReconciler) scheduleSandbox(ctx context.Context, pool *sandboxv1alp
 	}
 	status, pendingSyncs, poolDirty, err := r.Allocator.Schedule(ctx, spec)
 	if err != nil {
-		return nil, nil, nil, 0, false, err
+		return nil, nil, nil, nil, 0, false, err
 	}
 	idlePods := make([]string, 0)
 	for _, pod := range pods {
@@ -346,10 +346,10 @@ func (r *PoolReconciler) scheduleSandbox(ctx context.Context, pool *sandboxv1alp
 	}
 	log.Info("Schedule result", "pool", pool.Name, "allocated", len(status.PodAllocation),
 		"idlePods", len(idlePods), "supplement", status.PodSupplement, "pendingSyncs", len(pendingSyncs), "poolDirty", poolDirty)
-	return status.PodAllocation, pendingSyncs, idlePods, status.PodSupplement, poolDirty, nil
+	return status.PodAllocation, pendingSyncs, idlePods, status.DirtyPods, status.PodSupplement, poolDirty, nil
 }
 
-func (r *PoolReconciler) updatePool(ctx context.Context, latestRevision string, pods []*corev1.Pod, idlePods []string) ([]string, []string, int32) {
+func (r *PoolReconciler) updatePool(ctx context.Context, latestRevision string, pods []*corev1.Pod, idlePods []string, dirtyPods []string) ([]string, []string, int32) {
 	podMap := make(map[string]*corev1.Pod)
 	for _, pod := range pods {
 		podMap[pod.Name] = pod
@@ -358,7 +358,18 @@ func (r *PoolReconciler) updatePool(ctx context.Context, latestRevision string,
 	deleteOld := make([]string, 0)
 	supplyNew := int32(0)
 
+	dirtySet := make(map[string]bool)
+	for _, p := range dirtyPods {
+		dirtySet[p] = true
+	}
+
 	for _, name := range idlePods {
+		if dirtySet[name] {
+			deleteOld = append(deleteOld, name)
+			// no need to supply, next reconcile will do this job
+			continue
+		}
+
 		pod, ok := podMap[name]
 		if !ok {
 			continue