Skip to content

Commit 47c712a

Browse files
authored
[exporter/loadbalancing] small refactor of k8s resolver tests (open-telemetry#41244)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description I've simplified the k8s resolver tests to improve reliability and clarity. I replaced the `verifyFn` with `expectedEndpoints` for simpler assertions. I also replaced the `assert.Eventually` polling with a `waitForCondition` helper. I had a tough time reproducing these flaky tests, but I think this refactor should help. I've also increased the condition wait time by 200 milliseconds. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#40966 Fixes open-telemetry#40778 Fixes open-telemetry#40087 --------- Signed-off-by: Robbie Lankford <[email protected]>
1 parent 1aa68d5 commit 47c712a

File tree

1 file changed

+46
-64
lines changed

1 file changed

+46
-64
lines changed

exporter/loadbalancingexporter/resolver_k8s_test.go

Lines changed: 46 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package loadbalancingexporter
66
import (
77
"context"
88
"fmt"
9+
"slices"
910
"testing"
1011
"time"
1112

@@ -15,6 +16,7 @@ import (
1516
corev1 "k8s.io/api/core/v1"
1617
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1718
"k8s.io/apimachinery/pkg/types"
19+
"k8s.io/apimachinery/pkg/util/wait"
1820
"k8s.io/client-go/kubernetes/fake"
1921
"sigs.k8s.io/controller-runtime/pkg/client"
2022
)
@@ -82,11 +84,11 @@ func TestK8sResolve(t *testing.T) {
8284
}
8385
}
8486
tests := []struct {
85-
name string
86-
args args
87-
simulateFn func(*suiteContext, args) error
88-
onChangeFn func([]string)
89-
verifyFn func(*suiteContext, args) error
87+
name string
88+
args args
89+
simulateFn func(*suiteContext, args) error
90+
onChangeFn func([]string)
91+
expectedEndpoints []string
9092
}{
9193
{
9294
name: "add new IP to existing backends",
@@ -110,19 +112,11 @@ func TestK8sResolve(t *testing.T) {
110112
Patch(context.TODO(), args.service, types.MergePatchType, data, metav1.PatchOptions{})
111113
return err
112114
},
113-
verifyFn: func(ctx *suiteContext, _ args) error {
114-
if _, err := ctx.resolver.resolve(context.Background()); err != nil {
115-
return err
116-
}
117-
118-
assert.Equal(t, []string{
119-
"10.10.0.11:8080",
120-
"10.10.0.11:9090",
121-
"192.168.10.100:8080",
122-
"192.168.10.100:9090",
123-
}, ctx.resolver.Endpoints(), "resolver failed, endpoints not equal")
124-
125-
return nil
115+
expectedEndpoints: []string{
116+
"10.10.0.11:8080",
117+
"10.10.0.11:9090",
118+
"192.168.10.100:8080",
119+
"192.168.10.100:9090",
126120
},
127121
},
128122
{
@@ -147,17 +141,9 @@ func TestK8sResolve(t *testing.T) {
147141
onChangeFn: func([]string) {
148142
assert.Fail(t, "should not call onChange")
149143
},
150-
verifyFn: func(ctx *suiteContext, _ args) error {
151-
if _, err := ctx.resolver.resolve(context.Background()); err != nil {
152-
return err
153-
}
154-
155-
assert.Equal(t, []string{
156-
"192.168.10.100:8080",
157-
"192.168.10.100:9090",
158-
}, ctx.resolver.Endpoints(), "resolver failed, endpoints not equal")
159-
160-
return nil
144+
expectedEndpoints: []string{
145+
"192.168.10.100:8080",
146+
"192.168.10.100:9090",
161147
},
162148
},
163149
{
@@ -183,19 +169,11 @@ func TestK8sResolve(t *testing.T) {
183169
Patch(context.TODO(), args.service, types.MergePatchType, data, metav1.PatchOptions{})
184170
return err
185171
},
186-
verifyFn: func(ctx *suiteContext, _ args) error {
187-
if _, err := ctx.resolver.resolve(context.Background()); err != nil {
188-
return err
189-
}
190-
191-
assert.Equal(t, []string{
192-
"pod-0.lb.default:8080",
193-
"pod-0.lb.default:9090",
194-
"pod-1.lb.default:8080",
195-
"pod-1.lb.default:9090",
196-
}, ctx.resolver.Endpoints(), "resolver failed, endpoints not equal")
197-
198-
return nil
172+
expectedEndpoints: []string{
173+
"pod-0.lb.default:8080",
174+
"pod-0.lb.default:9090",
175+
"pod-1.lb.default:8080",
176+
"pod-1.lb.default:9090",
199177
},
200178
},
201179
{
@@ -220,16 +198,8 @@ func TestK8sResolve(t *testing.T) {
220198
Patch(context.TODO(), args.service, types.MergePatchType, data, metav1.PatchOptions{})
221199
return err
222200
},
223-
verifyFn: func(ctx *suiteContext, _ args) error {
224-
if _, err := ctx.resolver.resolve(context.Background()); err != nil {
225-
return err
226-
}
227-
228-
assert.Equal(t, []string{
229-
"10.10.0.11:4317",
230-
}, ctx.resolver.Endpoints(), "resolver failed, endpoints not equal")
231-
232-
return nil
201+
expectedEndpoints: []string{
202+
"10.10.0.11:4317",
233203
},
234204
},
235205
{
@@ -244,13 +214,7 @@ func TestK8sResolve(t *testing.T) {
244214
return suiteCtx.clientset.CoreV1().Endpoints(args.namespace).
245215
Delete(context.TODO(), args.service, metav1.DeleteOptions{})
246216
},
247-
verifyFn: func(suiteCtx *suiteContext, _ args) error {
248-
if _, err := suiteCtx.resolver.resolve(context.Background()); err != nil {
249-
return err
250-
}
251-
assert.Empty(t, suiteCtx.resolver.Endpoints(), "resolver failed, endpoints should empty")
252-
return nil
253-
},
217+
expectedEndpoints: nil,
254218
},
255219
}
256220

@@ -266,15 +230,33 @@ func TestK8sResolve(t *testing.T) {
266230
err := tt.simulateFn(suiteCtx, tt.args)
267231
assert.NoError(t, err)
268232

269-
assert.Eventually(t, func() bool {
270-
err := tt.verifyFn(suiteCtx, tt.args)
271-
assert.NoError(t, err)
272-
return true
273-
}, time.Second, 20*time.Millisecond)
233+
slices.Sort(tt.expectedEndpoints)
234+
235+
cErr := waitForCondition(t, 1200*time.Millisecond, 20*time.Millisecond, func(ctx context.Context) (bool, error) {
236+
if _, err := suiteCtx.resolver.resolve(ctx); err != nil {
237+
return false, err
238+
}
239+
got := suiteCtx.resolver.Endpoints()
240+
return slices.Equal(tt.expectedEndpoints, got), nil
241+
})
242+
if cErr != nil {
243+
t.Logf("waitForCondition: timed out waiting for resolver endpoints to match expected: %v", cErr)
244+
}
245+
assert.Equal(t, tt.expectedEndpoints, suiteCtx.resolver.Endpoints(), "resolver returned unexpected endpoints after update")
274246
})
275247
}
276248
}
277249

250+
// waitForCondition will poll the condition function until it returns true or times out.
251+
// Any errors returned from the condition are treated as test failures.
252+
func waitForCondition(t *testing.T, timeout, interval time.Duration, condition func(context.Context) (bool, error)) error {
253+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
254+
defer cancel()
255+
t.Helper()
256+
257+
return wait.PollUntilContextTimeout(ctx, interval, timeout, true, condition)
258+
}
259+
278260
func Test_newK8sResolver(t *testing.T) {
279261
type args struct {
280262
logger *zap.Logger

0 commit comments

Comments
 (0)