Skip to content

Commit 123fb5a

Browse files
committed
Addressing comments
Signed-off-by: Dyanngg <[email protected]>
1 parent 25eb037 commit 123fb5a

File tree

2 files changed

+34
-13
lines changed

2 files changed

+34
-13
lines changed

pkg/apiserver/apiserver.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,6 @@ func installHandlers(c *ExtraConfig, s *genericapiserver.GenericAPIServer) {
310310

311311
// Get new NetworkPolicyValidator
312312
v := controllernetworkpolicy.NewNetworkPolicyValidator(c.networkPolicyController)
313-
// Set up Tier event handlers to notify validator when Tiers are actually created/deleted
314-
c.networkPolicyController.SetupTierEventHandlersForValidator(v)
315313
// Install handlers for NetworkPolicy related validation
316314
s.Handler.NonGoRestfulMux.HandleFunc("/validate/tier", webhook.HandlerForValidateFunc(v.Validate))
317315
s.Handler.NonGoRestfulMux.HandleFunc("/validate/acnp", webhook.HandlerForValidateFunc(v.Validate))
@@ -331,6 +329,12 @@ func installHandlers(c *ExtraConfig, s *genericapiserver.GenericAPIServer) {
331329
}()
332330
return nil
333331
})
332+
333+
// Start the NetworkPolicyValidator background routines
334+
s.AddPostStartHook("start-validator-routines", func(context genericapiserver.PostStartHookContext) error {
335+
go v.Run(context.Done())
336+
return nil
337+
})
334338
}
335339

336340
if features.DefaultFeatureGate.Enabled(features.Egress) || features.DefaultFeatureGate.Enabled(features.ServiceExternalIP) {

pkg/controller/networkpolicy/validate.go

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ type priorityReservation struct {
104104
func newTierPriorityTracker() *tierPriorityTracker {
105105
return &tierPriorityTracker{
106106
pendingPriorities: make(map[int32]*priorityReservation),
107-
validationTimeout: 30 * time.Second, // timeout for waiting for other validations
107+
validationTimeout: 5 * time.Second, // timeout for waiting for other validations
108108
creationTimeout: 60 * time.Second, // timeout for waiting for actual K8s creation
109109
}
110110
}
@@ -120,7 +120,7 @@ func (t *tierPriorityTracker) reservePriorityForValidation(priority int32, tierN
120120
if existing, exists := t.pendingPriorities[priority]; exists {
121121
// Check if the existing reservation has timed out
122122
if time.Since(existing.createdAt) > t.creationTimeout {
123-
klog.Warningf("Tier priority %d reservation for %s has timed out, allowing new reservation", priority, existing.tierName)
123+
klog.V(2).InfoS("Tier priority reservation for has timed out, allowing new reservation", "priority", priority, "tier", existing.tierName)
124124
close(existing.waitChan)
125125
delete(t.pendingPriorities, priority)
126126
} else {
@@ -159,7 +159,7 @@ func (t *tierPriorityTracker) waitForPriorityAvailable(priority int32) bool {
159159
case <-waitChan:
160160
return true
161161
case <-time.After(t.validationTimeout):
162-
klog.Warningf("Timeout waiting for Tier priority %d to become available", priority)
162+
klog.InfoS("Timeout waiting for Tier priority to become available", "priority", priority)
163163
return false
164164
}
165165
}
@@ -176,7 +176,7 @@ func (t *tierPriorityTracker) releasePriorityReservation(priority int32, tierNam
176176
delete(t.pendingPriorities, priority)
177177
klog.V(4).InfoS("Released priority reservation for Tier", "priority", priority, "tier", tierName)
178178
} else {
179-
klog.Warningf("Attempted to release priority %d for Tier %s, but it's reserved by %s", priority, tierName, existing.tierName)
179+
klog.InfoS("Attempted to release priority for a different Tier", "priority", priority, "tier", tierName, "existingTier", existing.tierName)
180180
}
181181
}
182182
}
@@ -190,7 +190,8 @@ func (t *tierPriorityTracker) cleanupExpiredReservations() {
190190
now := time.Now()
191191
for priority, reservation := range t.pendingPriorities {
192192
if now.Sub(reservation.createdAt) > t.creationTimeout {
193-
klog.Warningf("Cleaning up expired priority %d reservation for Tier %s (age: %v)", priority, reservation.tierName, now.Sub(reservation.createdAt))
193+
klog.V(4).InfoS("Cleaning up expired priority reservation for Tier",
194+
"priority", priority, "tier", reservation.tierName, "age", now.Sub(reservation.createdAt))
194195
close(reservation.waitChan)
195196
delete(t.pendingPriorities, priority)
196197
}
@@ -281,12 +282,22 @@ func NewNetworkPolicyValidator(networkPolicyController *NetworkPolicyController)
281282
vr.RegisterGroupValidator(&gv)
282283
vr.RegisterAdminNetworkPolicyValidator(&av)
283284

284-
// Start cleanup routine for expired reservations from the tierValidator
285-
go tv.startCleanupRoutine()
286-
285+
// Set up Tier event handlers to notify validator when Tiers are actually created/deleted
286+
networkPolicyController.SetupTierEventHandlersForValidator(&vr)
287287
return &vr
288288
}
289289

290+
// Run starts the background routines for the NetworkPolicyValidator.
291+
func (v *NetworkPolicyValidator) Run(stopCh <-chan struct{}) {
292+
// Start cleanup routine for expired reservations from the tierValidator
293+
for _, val := range v.tierValidators {
294+
if tv, ok := val.(*tierValidator); ok {
295+
go tv.startCleanupRoutine(stopCh)
296+
break
297+
}
298+
}
299+
}
300+
290301
// Validate function validates a Group, ClusterGroup, Tier or Antrea Policy object
291302
func (v *NetworkPolicyValidator) Validate(ar *admv1.AdmissionReview) *admv1.AdmissionResponse {
292303
var result *metav1.Status
@@ -627,7 +638,7 @@ func (v *NetworkPolicyValidator) validateTier(curTier, oldTier *crdv1beta1.Tier,
627638
}
628639

629640
// startCleanupRoutine starts a background routine to clean up expired priority reservations
630-
func (t *tierValidator) startCleanupRoutine() {
641+
func (t *tierValidator) startCleanupRoutine(stopCh <-chan struct{}) {
631642
if t.priorityTracker == nil {
632643
klog.ErrorS(nil, "Priority tracker is nil, cannot start cleanup routine")
633644
return
@@ -636,8 +647,14 @@ func (t *tierValidator) startCleanupRoutine() {
636647
ticker := time.NewTicker(30 * time.Second) // Check every 30 seconds
637648
defer ticker.Stop()
638649

639-
for range ticker.C {
640-
t.priorityTracker.cleanupExpiredReservations()
650+
for {
651+
select {
652+
case <-ticker.C:
653+
t.priorityTracker.cleanupExpiredReservations()
654+
case <-stopCh:
655+
klog.InfoS("Stopping Tier priority tracker cleanup routine")
656+
return
657+
}
641658
}
642659
}
643660

0 commit comments

Comments
 (0)