Skip to content

Fix Test Flakiness by adding short sleep in TestMetricsRefresh #824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pkg/epp/backend/metrics/pod_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ type podMetrics struct {
ds Datastore
interval time.Duration

once sync.Once // ensure the StartRefreshLoop is only called once.
done chan struct{}
startOnce sync.Once // ensures the refresh loop goroutine is started only once
stopOnce sync.Once // ensures the done channel is closed only once
done chan struct{}

logger logr.Logger
}
Expand Down Expand Up @@ -86,7 +87,7 @@ func toInternalPod(pod *corev1.Pod) *backend.Pod {
// start starts a goroutine exactly once to periodically update metrics. The goroutine will be
// stopped either when stop() is called, or the given ctx is cancelled.
func (pm *podMetrics) startRefreshLoop(ctx context.Context) {
pm.once.Do(func() {
pm.startOnce.Do(func() {
go func() {
pm.logger.V(logutil.DEFAULT).Info("Starting refresher", "pod", pm.GetPod())
ticker := time.NewTicker(pm.interval)
Expand Down Expand Up @@ -138,5 +139,7 @@ func (pm *podMetrics) refreshMetrics() error {

func (pm *podMetrics) StopRefreshLoop() {
pm.logger.V(logutil.DEFAULT).Info("Stopping refresher", "pod", pm.GetPod())
close(pm.done)
pm.stopOnce.Do(func() {
close(pm.done)
})
}
1 change: 1 addition & 0 deletions pkg/epp/backend/metrics/pod_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func TestMetricsRefresh(t *testing.T) {
// Stop the loop, and simulate metric update again, this time the PodMetrics won't get the
// new update.
pm.StopRefreshLoop()
time.Sleep(pmf.refreshMetricsInterval * 2 /* small buffer for robustness */)
pmc.SetRes(map[types.NamespacedName]*MetricsState{namespacedName: updated})
// Still expect the same condition (no metrics update).
assert.EventuallyWithT(t, condition, time.Second, time.Millisecond)
Expand Down
13 changes: 7 additions & 6 deletions pkg/epp/backend/metrics/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,13 @@ type PodMetricsFactory struct {
func (f *PodMetricsFactory) NewPodMetrics(parentCtx context.Context, in *corev1.Pod, ds Datastore) PodMetrics {
pod := toInternalPod(in)
pm := &podMetrics{
pmc: f.pmc,
ds: ds,
interval: f.refreshMetricsInterval,
once: sync.Once{},
done: make(chan struct{}),
logger: log.FromContext(parentCtx).WithValues("pod", pod.NamespacedName),
pmc: f.pmc,
ds: ds,
interval: f.refreshMetricsInterval,
startOnce: sync.Once{},
stopOnce: sync.Once{},
done: make(chan struct{}),
logger: log.FromContext(parentCtx).WithValues("pod", pod.NamespacedName),
}
pm.pod.Store(pod)
pm.metrics.Store(newMetricsState())
Expand Down