Repository: kubernetes-sigs/gateway-api-inference-extension

Issue:
Pod Metrics test flaked
I got a test run failure: https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_gateway-api-inference-extension/683/pull-gateway-api-inference-extension-test-unit-main/1914386503648153600

In a purely doc focused PR: https://github.com/kubernetes-sigs/gateway-api-inference-extension/pull/683

We should look into this flake


Discussion:
@kfswain: @LukeAVanDrie yo! Wanna assign this to yourself since you tackled this? Thanks btw!!@LukeAVanDrie: I don't have permissions to do that, unless there is another process I am unaware of to set that field on the issue.

edit: looks like I figured it out@LukeAVanDrie: /assign @LukeAVanDrie 

Solution patch:
diff --git a/pkg/epp/backend/metrics/pod_metrics.go b/pkg/epp/backend/metrics/pod_metrics.go
index f885dbf..70ac38a 100644
--- a/pkg/epp/backend/metrics/pod_metrics.go
+++ b/pkg/epp/backend/metrics/pod_metrics.go
@@ -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
 }
@@ -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)
@@ -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)
+	})
 }
diff --git a/pkg/epp/backend/metrics/types.go b/pkg/epp/backend/metrics/types.go
index 92478db..bb78c2b 100644
--- a/pkg/epp/backend/metrics/types.go
+++ b/pkg/epp/backend/metrics/types.go
@@ -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())



Test patch:
diff --git a/pkg/epp/backend/metrics/pod_metrics_test.go b/pkg/epp/backend/metrics/pod_metrics_test.go
index c654d06..796b636 100644
--- a/pkg/epp/backend/metrics/pod_metrics_test.go
+++ b/pkg/epp/backend/metrics/pod_metrics_test.go
@@ -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)