Skip to content

Commit 41bab9b

Browse files
BSWANGBSWANG
authored
Merge pull request #946 from l1b0k/feat/requeue
feat: enhance requeue logic and introduce dynamic pool sync period
2 parents 8a50ec8 + 7d9e46f commit 41bab9b

File tree

2 files changed

+261
-11
lines changed

2 files changed

+261
-11
lines changed

‎pkg/controller/multi-ip/node/pool.go‎

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,8 @@ func (n *ReconcileNode) Reconcile(ctx context.Context, request reconcile.Request
345345
return reconcile.Result{}, err
346346
}
347347

348+
requeueAfter := n.requeueAfter(node)
349+
348350
if !reflect.DeepEqual(beforeStatus, afterStatus) {
349351
span.AddEvent("update node cr")
350352

@@ -353,11 +355,14 @@ func (n *ReconcileNode) Reconcile(ctx context.Context, request reconcile.Request
353355
if err != nil && nodeStatus.StatusChanged.CompareAndSwap(true, false) {
354356
nodeStatus.NeedSyncOpenAPI.Store(true)
355357
}
358+
if err != nil {
359+
return reconcile.Result{}, err
360+
}
356361

357-
return reconcile.Result{RequeueAfter: 1 * time.Second}, err
362+
return reconcile.Result{RequeueAfter: requeueAfter}, nil
358363
}
359364

360-
return reconcile.Result{}, utilerrors.NewAggregate(errorList)
365+
return reconcile.Result{RequeueAfter: requeueAfter}, utilerrors.NewAggregate(errorList)
361366
}
362367

363368
// syncWithAPI will sync all eni from openAPI. Need to re-sync with local pods.
@@ -1275,15 +1280,7 @@ func (n *ReconcileNode) handleStatus(ctx context.Context, node *networkv1beta1.N
12751280
func (n *ReconcileNode) adjustPool(ctx context.Context, node *networkv1beta1.Node) error {
12761281
l := logf.FromContext(ctx).WithName("adjustPool")
12771282

1278-
gcPeriod := n.gcPeriod
1279-
if node.Spec.Pool.PoolSyncPeriod != "" {
1280-
period, err := time.ParseDuration(node.Spec.Pool.PoolSyncPeriod)
1281-
if err != nil {
1282-
l.Error(err, "parse pool sync period failed, use default config")
1283-
} else {
1284-
gcPeriod = period
1285-
}
1286-
}
1283+
gcPeriod := n.poolSyncPeriod(node.Spec.Pool.PoolSyncPeriod)
12871284

12881285
if MetaCtx(ctx).LastGCTime.Add(gcPeriod).After(time.Now()) {
12891286
l.V(4).Info("skip adjustPool", "lastGCTime", MetaCtx(ctx).LastGCTime)
@@ -2022,3 +2019,40 @@ func (n *ReconcileNode) checkWarmUpCompletion(node *networkv1beta1.Node) {
20222019
node.Status.WarmUpCompleted = true
20232020
}
20242021
}
2022+
2023+
func (n *ReconcileNode) poolSyncPeriod(user string) time.Duration {
2024+
system := n.gcPeriod
2025+
if user != "" {
2026+
period, err := time.ParseDuration(user)
2027+
if err == nil {
2028+
system = period
2029+
}
2030+
}
2031+
return system
2032+
}
2033+
2034+
func (n *ReconcileNode) requeueAfter(node *networkv1beta1.Node) time.Duration {
2035+
2036+
poolPeriod := n.poolSyncPeriod(node.Spec.Pool.PoolSyncPeriod)
2037+
2038+
now := time.Now()
2039+
result := poolPeriod
2040+
2041+
if !node.Status.NextSyncOpenAPITime.IsZero() {
2042+
nextSyncTime := node.Status.NextSyncOpenAPITime.Time
2043+
if nextSyncTime.After(now) {
2044+
nextSyncDuration := nextSyncTime.Sub(now)
2045+
// Take the minimum of poolPeriod and nextSyncDuration (priority processing principle)
2046+
if nextSyncDuration < poolPeriod {
2047+
result = nextSyncDuration
2048+
}
2049+
}
2050+
}
2051+
2052+
// Ensure minimum 1s
2053+
if result < 1*time.Second {
2054+
return 1 * time.Second
2055+
}
2056+
2057+
return result
2058+
}

‎pkg/controller/multi-ip/node/pool_test.go‎

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3777,3 +3777,219 @@ func TestCheckWarmUpCompletion(t *testing.T) {
37773777
})
37783778
}
37793779
}
3780+
3781+
func TestReconcileNode_poolSyncPeriod(t *testing.T) {
3782+
tests := []struct {
3783+
name string
3784+
user string
3785+
gcPeriod time.Duration
3786+
want time.Duration
3787+
}{
3788+
{
3789+
name: "empty user string, use system default",
3790+
user: "",
3791+
gcPeriod: 30 * time.Second,
3792+
want: 30 * time.Second,
3793+
},
3794+
{
3795+
name: "valid user duration",
3796+
user: "60s",
3797+
gcPeriod: 30 * time.Second,
3798+
want: 60 * time.Second,
3799+
},
3800+
{
3801+
name: "valid user duration with minutes",
3802+
user: "5m",
3803+
gcPeriod: 30 * time.Second,
3804+
want: 5 * time.Minute,
3805+
},
3806+
{
3807+
name: "invalid user duration, use system default",
3808+
user: "invalid",
3809+
gcPeriod: 30 * time.Second,
3810+
want: 30 * time.Second,
3811+
},
3812+
{
3813+
name: "user duration overrides system default",
3814+
user: "2m",
3815+
gcPeriod: 1 * time.Minute,
3816+
want: 2 * time.Minute,
3817+
},
3818+
}
3819+
3820+
for _, tt := range tests {
3821+
t.Run(tt.name, func(t *testing.T) {
3822+
n := &ReconcileNode{
3823+
gcPeriod: tt.gcPeriod,
3824+
}
3825+
got := n.poolSyncPeriod(tt.user)
3826+
assert.Equal(t, tt.want, got)
3827+
})
3828+
}
3829+
}
3830+
3831+
func TestReconcileNode_requeueAfter(t *testing.T) {
3832+
now := time.Now()
3833+
3834+
tests := []struct {
3835+
name string
3836+
node *networkv1beta1.Node
3837+
gcPeriod time.Duration
3838+
want time.Duration
3839+
wantMin time.Duration
3840+
wantMax time.Duration
3841+
description string
3842+
}{
3843+
{
3844+
name: "zero NextSyncOpenAPITime, use poolPeriod",
3845+
node: &networkv1beta1.Node{
3846+
Spec: networkv1beta1.NodeSpec{
3847+
Pool: &networkv1beta1.PoolSpec{
3848+
PoolSyncPeriod: "",
3849+
},
3850+
},
3851+
Status: networkv1beta1.NodeStatus{
3852+
NextSyncOpenAPITime: metav1.Time{},
3853+
},
3854+
},
3855+
gcPeriod: 30 * time.Second,
3856+
want: 30 * time.Second,
3857+
description: "When NextSyncOpenAPITime is zero, should use poolPeriod",
3858+
},
3859+
{
3860+
name: "NextSyncOpenAPITime in future, less than poolPeriod",
3861+
node: &networkv1beta1.Node{
3862+
Spec: networkv1beta1.NodeSpec{
3863+
Pool: &networkv1beta1.PoolSpec{
3864+
PoolSyncPeriod: "60s",
3865+
},
3866+
},
3867+
Status: networkv1beta1.NodeStatus{
3868+
NextSyncOpenAPITime: metav1.NewTime(now.Add(20 * time.Second)),
3869+
},
3870+
},
3871+
gcPeriod: 30 * time.Second,
3872+
wantMin: 19 * time.Second,
3873+
wantMax: 21 * time.Second,
3874+
description: "When NextSyncOpenAPITime is in future and less than poolPeriod, should use NextSyncOpenAPITime duration",
3875+
},
3876+
{
3877+
name: "NextSyncOpenAPITime in future, greater than poolPeriod",
3878+
node: &networkv1beta1.Node{
3879+
Spec: networkv1beta1.NodeSpec{
3880+
Pool: &networkv1beta1.PoolSpec{
3881+
PoolSyncPeriod: "30s",
3882+
},
3883+
},
3884+
Status: networkv1beta1.NodeStatus{
3885+
NextSyncOpenAPITime: metav1.NewTime(now.Add(60 * time.Second)),
3886+
},
3887+
},
3888+
gcPeriod: 30 * time.Second,
3889+
want: 30 * time.Second,
3890+
description: "When NextSyncOpenAPITime is in future but greater than poolPeriod, should use poolPeriod",
3891+
},
3892+
{
3893+
name: "NextSyncOpenAPITime in past",
3894+
node: &networkv1beta1.Node{
3895+
Spec: networkv1beta1.NodeSpec{
3896+
Pool: &networkv1beta1.PoolSpec{
3897+
PoolSyncPeriod: "",
3898+
},
3899+
},
3900+
Status: networkv1beta1.NodeStatus{
3901+
NextSyncOpenAPITime: metav1.NewTime(now.Add(-10 * time.Second)),
3902+
},
3903+
},
3904+
gcPeriod: 30 * time.Second,
3905+
want: 30 * time.Second,
3906+
description: "When NextSyncOpenAPITime is in past, should use poolPeriod",
3907+
},
3908+
{
3909+
name: "poolPeriod less than 1s, should enforce minimum 1s",
3910+
node: &networkv1beta1.Node{
3911+
Spec: networkv1beta1.NodeSpec{
3912+
Pool: &networkv1beta1.PoolSpec{
3913+
PoolSyncPeriod: "500ms",
3914+
},
3915+
},
3916+
Status: networkv1beta1.NodeStatus{
3917+
NextSyncOpenAPITime: metav1.Time{},
3918+
},
3919+
},
3920+
gcPeriod: 500 * time.Millisecond,
3921+
want: 1 * time.Second,
3922+
description: "When poolPeriod is less than 1s, should enforce minimum 1s",
3923+
},
3924+
{
3925+
name: "NextSyncOpenAPITime duration less than 1s, should enforce minimum 1s",
3926+
node: &networkv1beta1.Node{
3927+
Spec: networkv1beta1.NodeSpec{
3928+
Pool: &networkv1beta1.PoolSpec{
3929+
PoolSyncPeriod: "60s",
3930+
},
3931+
},
3932+
Status: networkv1beta1.NodeStatus{
3933+
NextSyncOpenAPITime: metav1.NewTime(now.Add(500 * time.Millisecond)),
3934+
},
3935+
},
3936+
gcPeriod: 30 * time.Second,
3937+
want: 1 * time.Second,
3938+
description: "When NextSyncOpenAPITime duration is less than 1s, should enforce minimum 1s",
3939+
},
3940+
{
3941+
name: "custom poolSyncPeriod from user config",
3942+
node: &networkv1beta1.Node{
3943+
Spec: networkv1beta1.NodeSpec{
3944+
Pool: &networkv1beta1.PoolSpec{
3945+
PoolSyncPeriod: "2m",
3946+
},
3947+
},
3948+
Status: networkv1beta1.NodeStatus{
3949+
NextSyncOpenAPITime: metav1.NewTime(now.Add(90 * time.Second)),
3950+
},
3951+
},
3952+
gcPeriod: 30 * time.Second,
3953+
wantMin: 89 * time.Second,
3954+
wantMax: 91 * time.Second,
3955+
description: "When user provides custom poolSyncPeriod, should use it and compare with NextSyncOpenAPITime",
3956+
},
3957+
{
3958+
name: "NextSyncOpenAPITime exactly 1s in future",
3959+
node: &networkv1beta1.Node{
3960+
Spec: networkv1beta1.NodeSpec{
3961+
Pool: &networkv1beta1.PoolSpec{
3962+
PoolSyncPeriod: "60s",
3963+
},
3964+
},
3965+
Status: networkv1beta1.NodeStatus{
3966+
NextSyncOpenAPITime: metav1.NewTime(now.Add(1 * time.Second)),
3967+
},
3968+
},
3969+
gcPeriod: 30 * time.Second,
3970+
wantMin: 999 * time.Millisecond,
3971+
wantMax: 1001 * time.Millisecond,
3972+
description: "When NextSyncOpenAPITime is exactly 1s in future, should return approximately 1s",
3973+
},
3974+
}
3975+
3976+
for _, tt := range tests {
3977+
t.Run(tt.name, func(t *testing.T) {
3978+
n := &ReconcileNode{
3979+
gcPeriod: tt.gcPeriod,
3980+
}
3981+
got := n.requeueAfter(tt.node)
3982+
3983+
if tt.want != 0 {
3984+
assert.Equal(t, tt.want, got, tt.description)
3985+
} else {
3986+
if tt.wantMin != 0 && tt.wantMax != 0 {
3987+
assert.GreaterOrEqual(t, got, tt.wantMin, tt.description)
3988+
assert.LessOrEqual(t, got, tt.wantMax, tt.description)
3989+
} else {
3990+
assert.GreaterOrEqual(t, got, 1*time.Second, "result should be at least 1s")
3991+
}
3992+
}
3993+
})
3994+
}
3995+
}

0 commit comments

Comments
 (0)