Skip to content

Commit e39b52c

Browse files
committed
add more tests
Signed-off-by: odubajDT <[email protected]>
1 parent 8f19cd5 commit e39b52c

File tree

2 files changed

+74
-10
lines changed

2 files changed

+74
-10
lines changed

exporter/loadbalancingexporter/resolver_k8s.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -173,16 +173,14 @@ func (r *k8sResolver) start(_ context.Context) error {
173173
},
174174
}
175175

176-
if r.epsListWatcher != nil {
177-
r.logger.Debug("creating and starting endpoints informer")
178-
epsInformer := cache.NewSharedInformer(r.epsListWatcher, &discoveryv1.EndpointSlice{}, 0)
179-
if _, err := epsInformer.AddEventHandler(r.handler); err != nil {
180-
r.logger.Error("unable to start watching for changes to the specified service names", zap.Error(err))
181-
}
182-
go epsInformer.Run(r.stopCh)
183-
if !cache.WaitForCacheSync(r.stopCh, epsInformer.HasSynced) {
184-
initErr = errors.New("endpoints informer not sync")
185-
}
176+
r.logger.Debug("creating and starting endpoints informer")
177+
epsInformer := cache.NewSharedInformer(r.epsListWatcher, &discoveryv1.EndpointSlice{}, 0)
178+
if _, err := epsInformer.AddEventHandler(r.handler); err != nil {
179+
r.logger.Error("unable to start watching for changes to the specified service names", zap.Error(err))
180+
}
181+
go epsInformer.Run(r.stopCh)
182+
if !cache.WaitForCacheSync(r.stopCh, epsInformer.HasSynced) {
183+
initErr = errors.New("endpoints informer not sync")
186184
}
187185
})
188186
if initErr != nil {

exporter/loadbalancingexporter/resolver_k8s_validation_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"go.opentelemetry.io/collector/config/configoptional"
1313
"go.opentelemetry.io/collector/exporter/exportertest"
1414
"go.uber.org/zap"
15+
"k8s.io/client-go/kubernetes/fake"
1516

1617
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/loadbalancingexporter/internal/metadata"
1718
)
@@ -98,6 +99,71 @@ func TestK8sResolverStartCreatesClient(t *testing.T) {
9899
_ = resolver.shutdown(t.Context())
99100
}
100101

102+
// TestK8sResolverStartErrorPropagation verifies that errors from newInClusterClient() are properly returned from start()
103+
func TestK8sResolverStartErrorPropagation(t *testing.T) {
104+
_, tb := getTelemetryAssets(t)
105+
106+
// Create resolver with nil client - it will attempt to create a real k8s client on start()
107+
resolver, err := newK8sResolver(
108+
nil,
109+
zap.NewNop(),
110+
"test-service",
111+
[]int32{4317},
112+
defaultListWatchTimeout,
113+
false,
114+
tb,
115+
)
116+
require.NoError(t, err)
117+
require.Nil(t, resolver.client, "client should be nil before start")
118+
119+
// When running outside a k8s cluster, start() should fail with an error from newInClusterClient()
120+
// This test verifies that the error is properly propagated
121+
err = resolver.start(context.Background())
122+
123+
// Outside a k8s cluster, we expect an error from newInClusterClient()
124+
// The error should be returned from start(), not silently ignored
125+
// The actual error may vary (e.g., "unable to load in-cluster configuration"), but it should not be nil
126+
if err != nil {
127+
// This is the expected behavior outside a k8s cluster
128+
t.Logf("Expected error from start() outside k8s cluster: %v", err)
129+
} else {
130+
// If we're running inside a k8s cluster, start() might succeed
131+
t.Log("start() succeeded - likely running inside a k8s cluster")
132+
require.NotNil(t, resolver.client, "client must be created when start() succeeds")
133+
// Clean up
134+
_ = resolver.shutdown(context.Background())
135+
}
136+
}
137+
138+
// TestK8sResolverStartWithClientDoesNotCreateNew verifies that when a client is provided, start() does not create a new one
139+
func TestK8sResolverStartWithClientDoesNotCreateNew(t *testing.T) {
140+
_, tb := getTelemetryAssets(t)
141+
142+
// Create a fake client
143+
fakeClient := fake.NewSimpleClientset()
144+
145+
resolver, err := newK8sResolver(
146+
fakeClient,
147+
zap.NewNop(),
148+
"test-service",
149+
[]int32{4317},
150+
defaultListWatchTimeout,
151+
false,
152+
tb,
153+
)
154+
require.NoError(t, err)
155+
require.NotNil(t, resolver.client, "client should be set")
156+
require.Equal(t, fakeClient, resolver.client, "client should be the one provided")
157+
158+
// Start with an existing client should not attempt to create a new one
159+
err = resolver.start(context.Background())
160+
require.NoError(t, err)
161+
require.Equal(t, fakeClient, resolver.client, "client should still be the one provided, not replaced")
162+
163+
// Clean up
164+
_ = resolver.shutdown(context.Background())
165+
}
166+
101167
// TestLoadBalancerWithK8sResolverCreation tests that loadbalancer can be created with k8s resolver
102168
func TestLoadBalancerWithK8sResolverCreation(t *testing.T) {
103169
ts, tb := getTelemetryAssets(t)

0 commit comments

Comments
 (0)