Skip to content

Commit b465ebb

Browse files
committed
feat(datalayer): extend file-discovery to support cluster endpoints
Signed-off-by: Liav Weiss <lweiss@redhat.com>
1 parent 0b4ddab commit b465ebb

2 files changed

Lines changed: 170 additions & 14 deletions

File tree

pkg/epp/framework/plugins/datalayer/discovery/file/plugin.go

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@ import (
2828
"os"
2929
"sort"
3030
"strconv"
31+
"strings"
3132
"sync"
3233

3334
"github.com/fsnotify/fsnotify"
3435
"k8s.io/apimachinery/pkg/types"
36+
"k8s.io/apimachinery/pkg/util/validation"
3537
"sigs.k8s.io/controller-runtime/pkg/log"
3638
"sigs.k8s.io/yaml"
3739

@@ -43,11 +45,12 @@ const PluginType = "file-discovery"
4345

4446
// EndpointEntry is the YAML/JSON representation of a single endpoint.
4547
type EndpointEntry struct {
46-
Name string `json:"name" yaml:"name"`
47-
Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"`
48-
Address string `json:"address" yaml:"address"`
49-
Port string `json:"port" yaml:"port"`
50-
Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"`
48+
Name string `json:"name" yaml:"name"`
49+
Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"`
50+
Address string `json:"address" yaml:"address"`
51+
Port string `json:"port" yaml:"port"`
52+
MetricsPort string `json:"metricsPort,omitempty" yaml:"metricsPort,omitempty"`
53+
Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"`
5154
}
5255

5356
// EndpointsFile is the top-level structure of the endpoints YAML/JSON file.
@@ -238,9 +241,12 @@ func (f *FileDiscovery) load(notifier fwkdl.DiscoveryNotifier) error {
238241
incoming := make(map[types.NamespacedName]struct{}, len(ef.Endpoints))
239242
var errs []error
240243
for _, e := range ef.Endpoints {
241-
if ip := net.ParseIP(e.Address); ip == nil || ip.To4() == nil {
242-
errs = append(errs, fmt.Errorf("endpoint %q: invalid IPv4 address %q", e.Name, e.Address))
243-
continue
244+
ip := net.ParseIP(e.Address)
245+
if ip == nil || ip.To4() == nil {
246+
if len(validation.IsDNS1123Subdomain(strings.ToLower(e.Address))) > 0 {
247+
errs = append(errs, fmt.Errorf("endpoint %q: address %q is not a valid IPv4 or hostname", e.Name, e.Address))
248+
continue
249+
}
244250
}
245251
if portNum, err := strconv.Atoi(e.Port); err != nil || portNum < 1 || portNum > 65535 {
246252
errs = append(errs, fmt.Errorf("endpoint %q: invalid port %q", e.Name, e.Port))
@@ -250,12 +256,20 @@ func (f *FileDiscovery) load(notifier fwkdl.DiscoveryNotifier) error {
250256
if ns == "" {
251257
ns = "default"
252258
}
259+
podName := e.Name
260+
if ip == nil {
261+
podName = ""
262+
}
263+
mPort := e.Port
264+
if e.MetricsPort != "" {
265+
mPort = e.MetricsPort
266+
}
253267
meta := &fwkdl.EndpointMetadata{
254268
NamespacedName: types.NamespacedName{Name: e.Name, Namespace: ns},
255-
PodName: e.Name,
269+
PodName: podName,
256270
Address: e.Address,
257271
Port: e.Port,
258-
MetricsHost: net.JoinHostPort(e.Address, e.Port),
272+
MetricsHost: net.JoinHostPort(e.Address, mPort),
259273
Labels: e.Labels,
260274
}
261275
incoming[meta.NamespacedName] = struct{}{}

pkg/epp/framework/plugins/datalayer/discovery/file/plugin_test.go

Lines changed: 146 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,15 +189,39 @@ func TestStart_MissingFile(t *testing.T) {
189189
assert.ErrorContains(t, err, "initial load failed")
190190
}
191191

192-
func TestStart_InvalidIP(t *testing.T) {
192+
func TestStart_EmptyAddress(t *testing.T) {
193193
path := writeTemp(t, `
194194
endpoints:
195195
- name: ep1
196-
address: "not-an-ip"
196+
address: ""
197197
port: "8000"
198198
`)
199199
err := newFD(path, false).Start(context.Background(), &recordingNotifier{})
200-
assert.ErrorContains(t, err, "invalid IPv4 address")
200+
assert.ErrorContains(t, err, "is not a valid IPv4 or hostname")
201+
}
202+
203+
func TestStart_InvalidAddressFormat(t *testing.T) {
204+
tests := []struct {
205+
name string
206+
address string
207+
}{
208+
{"spaces", "not a valid host"},
209+
{"special chars", "!!!garbage!!!"},
210+
{"trailing dot label", "host-.example.com"},
211+
{"leading hyphen", "-host.example.com"},
212+
}
213+
for _, tt := range tests {
214+
t.Run(tt.name, func(t *testing.T) {
215+
path := writeTemp(t, fmt.Sprintf(`
216+
endpoints:
217+
- name: ep1
218+
address: %q
219+
port: "8000"
220+
`, tt.address))
221+
err := newFD(path, false).Start(context.Background(), &recordingNotifier{})
222+
assert.ErrorContains(t, err, "is not a valid IPv4 or hostname")
223+
})
224+
}
201225
}
202226

203227
func TestStart_RejectsIPv6(t *testing.T) {
@@ -208,7 +232,125 @@ endpoints:
208232
port: "8000"
209233
`)
210234
err := newFD(path, false).Start(context.Background(), &recordingNotifier{})
211-
assert.ErrorContains(t, err, "invalid IPv4 address")
235+
assert.ErrorContains(t, err, "is not a valid IPv4 or hostname")
236+
}
237+
238+
func TestStart_HostnameAddress(t *testing.T) {
239+
path := writeTemp(t, `
240+
endpoints:
241+
- name: cluster-us-east
242+
namespace: clusters
243+
address: "spoke-us-east.example.com"
244+
port: "443"
245+
labels:
246+
region: us-east
247+
model: llama-7b
248+
- name: cluster-eu-west
249+
namespace: clusters
250+
address: "spoke-eu-west.example.com"
251+
port: "443"
252+
labels:
253+
region: eu-west
254+
model: llama-7b
255+
`)
256+
notifier := &recordingNotifier{}
257+
ctx, cancel := context.WithCancel(context.Background())
258+
cancel()
259+
260+
require.NoError(t, newFD(path, false).Start(ctx, notifier))
261+
require.Len(t, notifier.upserted, 2)
262+
263+
for _, m := range notifier.upserted {
264+
assert.Empty(t, m.PodName, "hostname address should produce empty PodName")
265+
}
266+
assert.Equal(t, "spoke-us-east.example.com:443", notifier.upserted[0].MetricsHost)
267+
assert.Equal(t, "spoke-eu-west.example.com:443", notifier.upserted[1].MetricsHost)
268+
assert.ElementsMatch(t, []string{"clusters/cluster-us-east", "clusters/cluster-eu-west"}, notifier.upsertedNames())
269+
}
270+
271+
func TestStart_IPv4PodNameIsSet(t *testing.T) {
272+
path := writeTemp(t, `
273+
endpoints:
274+
- name: vllm-pod-1
275+
address: "10.0.0.5"
276+
port: "8000"
277+
`)
278+
notifier := &recordingNotifier{}
279+
ctx, cancel := context.WithCancel(context.Background())
280+
cancel()
281+
282+
require.NoError(t, newFD(path, false).Start(ctx, notifier))
283+
require.Len(t, notifier.upserted, 1)
284+
assert.Equal(t, "vllm-pod-1", notifier.upserted[0].PodName, "IPv4 address should preserve PodName")
285+
assert.Equal(t, "10.0.0.5:8000", notifier.upserted[0].MetricsHost)
286+
}
287+
288+
func TestStart_MixedIPv4AndHostname(t *testing.T) {
289+
path := writeTemp(t, `
290+
endpoints:
291+
- name: vllm-pod-1
292+
address: "10.0.0.5"
293+
port: "8000"
294+
- name: cluster-us-east
295+
address: "spoke-us-east.example.com"
296+
port: "443"
297+
`)
298+
notifier := &recordingNotifier{}
299+
ctx, cancel := context.WithCancel(context.Background())
300+
cancel()
301+
302+
require.NoError(t, newFD(path, false).Start(ctx, notifier))
303+
require.Len(t, notifier.upserted, 2)
304+
305+
var pod, cluster *fwkdl.EndpointMetadata
306+
for _, m := range notifier.upserted {
307+
if m.NamespacedName.Name == "vllm-pod-1" {
308+
pod = m
309+
} else {
310+
cluster = m
311+
}
312+
}
313+
require.NotNil(t, pod)
314+
require.NotNil(t, cluster)
315+
assert.Equal(t, "vllm-pod-1", pod.PodName, "IPv4 endpoint should have PodName set")
316+
assert.Empty(t, cluster.PodName, "hostname endpoint should have empty PodName")
317+
}
318+
319+
func TestStart_MetricsPortOverride(t *testing.T) {
320+
path := writeTemp(t, `
321+
endpoints:
322+
- name: cluster-us-east
323+
address: "spoke-gateway.example.com"
324+
port: "443"
325+
metricsPort: "9090"
326+
`)
327+
notifier := &recordingNotifier{}
328+
ctx, cancel := context.WithCancel(context.Background())
329+
cancel()
330+
331+
require.NoError(t, newFD(path, false).Start(ctx, notifier))
332+
require.Len(t, notifier.upserted, 1)
333+
334+
m := notifier.upserted[0]
335+
assert.Equal(t, "spoke-gateway.example.com", m.Address, "Address should be unchanged")
336+
assert.Equal(t, "443", m.Port, "Port should be unchanged")
337+
assert.Equal(t, "spoke-gateway.example.com:9090", m.MetricsHost, "MetricsHost should use address with metricsPort")
338+
}
339+
340+
func TestStart_MetricsFieldsNotSetFallsBack(t *testing.T) {
341+
path := writeTemp(t, `
342+
endpoints:
343+
- name: ep1
344+
address: "10.0.0.1"
345+
port: "8000"
346+
`)
347+
notifier := &recordingNotifier{}
348+
ctx, cancel := context.WithCancel(context.Background())
349+
cancel()
350+
351+
require.NoError(t, newFD(path, false).Start(ctx, notifier))
352+
require.Len(t, notifier.upserted, 1)
353+
assert.Equal(t, "10.0.0.1:8000", notifier.upserted[0].MetricsHost, "MetricsHost should fall back to Address:Port")
212354
}
213355

214356
func TestStart_InvalidPort(t *testing.T) {

0 commit comments

Comments
 (0)