Skip to content

Commit f36d64b

Browse files
committed
- address feedback
1 parent bca4b83 commit f36d64b

4 files changed

Lines changed: 81 additions & 168 deletions

File tree

dataclients/kubernetes/ingressv1.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,8 @@ func convertPathRuleV1(
153153
traffic.apply(r)
154154
return r, nil
155155
}
156-
var r *eskip.Route
157156

157+
var r *eskip.Route
158158
r = &eskip.Route{
159159
Id: routeID(ns, name, host, prule.Path, svcName),
160160
BackendType: eskip.LBBackend,
@@ -370,6 +370,7 @@ func (ing *ingress) convertDefaultBackendV1(
370370
svcPort = i.Spec.DefaultBackend.Service.Port
371371
)
372372
zoneTarget := false
373+
dataclientZone := ing.zone
373374

374375
svc, err := state.getService(ns, svcName)
375376
if err != nil {
@@ -398,7 +399,7 @@ func (ing *ingress) convertDefaultBackendV1(
398399
}
399400

400401
eps, zoneTarget = state.GetEndpointsByService(
401-
ing.zone,
402+
dataclientZone,
402403
ns,
403404
svcName,
404405
protocol,
@@ -424,7 +425,6 @@ func (ing *ingress) convertDefaultBackendV1(
424425
}
425426

426427
var r *eskip.Route
427-
428428
r = &eskip.Route{
429429
Id: routeID(ns, name, "", "", ""),
430430
BackendType: eskip.LBBackend,
@@ -434,7 +434,7 @@ func (ing *ingress) convertDefaultBackendV1(
434434
if zoneTarget {
435435
var lbeps []*eskip.LBEndpoint
436436
for _, ep := range eps {
437-
lbeps = append(lbeps, &eskip.LBEndpoint{Address: ep, Zone: ing.zone})
437+
lbeps = append(lbeps, &eskip.LBEndpoint{Address: ep, Zone: dataclientZone})
438438
}
439439
r.LBEndpoints = lbeps
440440
} else {

dataclients/kubernetes/routegroup.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,10 @@ func applyServiceBackend(ctx *routeGroupContext, backend *definitions.SkipperBac
188188
return targetPortNotFound(backend.ServiceName, backend.ServicePort)
189189
}
190190

191+
dataclientZone := ctx.zone
192+
191193
eps, zoneTarget := ctx.state.GetEndpointsByTarget(
192-
ctx.zone,
194+
dataclientZone,
193195
namespaceString(ctx.routeGroup.Metadata.Namespace),
194196
s.Meta.Name,
195197
"TCP",
@@ -213,7 +215,7 @@ func applyServiceBackend(ctx *routeGroupContext, backend *definitions.SkipperBac
213215
r.BackendType = eskip.LBBackend
214216
if zoneTarget {
215217
for _, ep := range eps {
216-
r.LBEndpoints = append(r.LBEndpoints, &eskip.LBEndpoint{Address: ep, Zone: ctx.zone})
218+
r.LBEndpoints = append(r.LBEndpoints, &eskip.LBEndpoint{Address: ep, Zone: dataclientZone})
217219
}
218220
} else {
219221
r.LBEndpoints = eskip.NewLBEndpoints(eps)

routesrv/routesrv.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,9 @@ func New(opts skipper.Options) (*RouteServer, error) {
7878
b: b,
7979
}
8080
mux := http.NewServeMux()
81-
if opts.KubernetesTopologyZone != "" {
82-
mux.Handle("/routes/{zone}", b)
83-
} else {
84-
mux.Handle("/routes", b)
85-
}
81+
82+
mux.Handle("/routes", b)
83+
mux.Handle("/routes/{zone}", b)
8684

8785
mux.Handle("/health", bs)
8886
supportHandler := http.NewServeMux()

routesrv/routesrv_test.go

Lines changed: 70 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -907,162 +907,75 @@ func TestESkipBytesHandlerWithNoUpdate(t *testing.T) {
907907
}
908908
}
909909

910-
func TestRoutesWithZoneTwoAddrPerZone(t *testing.T) {
911-
defer tl.Reset()
912-
ks, _ := newKubeServer(t, loadKubeYAML(t, "testdata/zone-aware-traffic/all-zones-2-addr.yaml"))
913-
ks.Start()
914-
defer ks.Close()
915-
rs := newRouteServerWithOptions(t, skipper.Options{
916-
SourcePollTimeout: pollInterval,
917-
Kubernetes: true,
918-
KubernetesURL: ks.URL,
919-
KubernetesTopologyZone: "eu-central-1a",
920-
KubernetesEnableEndpointslices: true,
921-
})
922-
923-
rs.StartUpdates()
924-
defer rs.StopUpdates()
925-
926-
if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
927-
t.Fatalf("routes not initialized: %v", err)
928-
}
929-
w := getZoneAwareRoutes(rs, "eu-central-1a")
930-
931-
want := parseEskipFixture(t, "testdata/zone-aware-traffic/all-zones-2-addr.eskip")
932-
got, err := eskip.Parse(w.Body.String())
933-
if err != nil {
934-
t.Fatalf("served routes are not valid eskip: %s", w.Body)
935-
}
936-
if !eskip.EqLists(got, want) {
937-
t.Errorf("served routes do not reflect kubernetes resources: %s", cmp.Diff(got, want))
938-
}
939-
wantHTTPCode(t, w, http.StatusOK)
940-
}
941-
942-
func TestRoutesWithZoneThreeAddrPerZone(t *testing.T) {
943-
defer tl.Reset()
944-
ks, _ := newKubeServer(t, loadKubeYAML(t, "testdata/zone-aware-traffic/all-zones-3-addr.yaml"))
945-
ks.Start()
946-
defer ks.Close()
947-
rs := newRouteServerWithOptions(t, skipper.Options{
948-
SourcePollTimeout: pollInterval,
949-
Kubernetes: true,
950-
KubernetesURL: ks.URL,
951-
KubernetesTopologyZone: "eu-central-1a",
952-
KubernetesEnableEndpointslices: true,
953-
})
954-
955-
rs.StartUpdates()
956-
defer rs.StopUpdates()
957-
958-
if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
959-
t.Fatalf("routes not initialized: %v", err)
960-
}
961-
w := getZoneAwareRoutes(rs, "eu-central-1a")
962-
963-
want := parseEskipFixture(t, "testdata/zone-aware-traffic/all-zones-3-addr.eskip")
964-
got, err := eskip.Parse(w.Body.String())
965-
if err != nil {
966-
t.Fatalf("served routes are not valid eskip: %s", w.Body)
967-
}
968-
if !eskip.EqLists(got, want) {
969-
t.Errorf("served routes do not reflect kubernetes resources: %s", cmp.Diff(got, want))
970-
}
971-
wantHTTPCode(t, w, http.StatusOK)
972-
}
973-
974-
func TestRoutesWithAllZonesExceptZoneA(t *testing.T) {
975-
defer tl.Reset()
976-
ks, _ := newKubeServer(t, loadKubeYAML(t, "testdata/zone-aware-traffic/all-zones-except-zone-a.yaml"))
977-
ks.Start()
978-
defer ks.Close()
979-
rs := newRouteServerWithOptions(t, skipper.Options{
980-
SourcePollTimeout: pollInterval,
981-
Kubernetes: true,
982-
KubernetesURL: ks.URL,
983-
KubernetesTopologyZone: "eu-central-1a",
984-
KubernetesEnableEndpointslices: true,
985-
})
986-
987-
rs.StartUpdates()
988-
defer rs.StopUpdates()
989-
990-
if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
991-
t.Fatalf("routes not initialized: %v", err)
992-
}
993-
w := getZoneAwareRoutes(rs, "eu-central-1a")
994-
995-
want := parseEskipFixture(t, "testdata/zone-aware-traffic/all-zones-except-zone-a.eskip")
996-
got, err := eskip.Parse(w.Body.String())
997-
if err != nil {
998-
t.Fatalf("served routes are not valid eskip: %s", w.Body)
999-
}
1000-
if !eskip.EqLists(got, want) {
1001-
t.Errorf("served routes do not reflect kubernetes resources: %s", cmp.Diff(got, want))
1002-
}
1003-
wantHTTPCode(t, w, http.StatusOK)
1004-
}
1005-
1006-
func TestRoutesWithAllZonesTopologySetToZoneB(t *testing.T) {
1007-
defer tl.Reset()
1008-
ks, _ := newKubeServer(t, loadKubeYAML(t, "testdata/zone-aware-traffic/all-zones-topology-zone-b.yaml"))
1009-
ks.Start()
1010-
defer ks.Close()
1011-
rs := newRouteServerWithOptions(t, skipper.Options{
1012-
SourcePollTimeout: pollInterval,
1013-
Kubernetes: true,
1014-
KubernetesURL: ks.URL,
1015-
KubernetesTopologyZone: "eu-central-1b",
1016-
KubernetesEnableEndpointslices: true,
1017-
})
1018-
1019-
rs.StartUpdates()
1020-
defer rs.StopUpdates()
1021-
1022-
if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
1023-
t.Fatalf("routes not initialized: %v", err)
1024-
}
1025-
w := getZoneAwareRoutes(rs, "eu-central-1b")
1026-
1027-
want := parseEskipFixture(t, "testdata/zone-aware-traffic/all-zones-topology-zone-b.eskip")
1028-
got, err := eskip.Parse(w.Body.String())
1029-
if err != nil {
1030-
t.Fatalf("served routes are not valid eskip: %s", w.Body)
1031-
}
1032-
if !eskip.EqLists(got, want) {
1033-
t.Errorf("served routes do not reflect kubernetes resources: %s", cmp.Diff(got, want))
1034-
}
1035-
wantHTTPCode(t, w, http.StatusOK)
1036-
}
1037-
1038-
func TestRoutesWithOnlyZoneA(t *testing.T) {
1039-
defer tl.Reset()
1040-
ks, _ := newKubeServer(t, loadKubeYAML(t, "testdata/zone-aware-traffic/only-zone-a.yaml"))
1041-
ks.Start()
1042-
defer ks.Close()
1043-
rs := newRouteServerWithOptions(t, skipper.Options{
1044-
SourcePollTimeout: pollInterval,
1045-
Kubernetes: true,
1046-
KubernetesURL: ks.URL,
1047-
KubernetesTopologyZone: "eu-central-1b",
1048-
KubernetesEnableEndpointslices: true,
1049-
})
1050-
1051-
rs.StartUpdates()
1052-
defer rs.StopUpdates()
1053-
1054-
if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
1055-
t.Fatalf("routes not initialized: %v", err)
1056-
}
1057-
w := getZoneAwareRoutes(rs, "eu-central-1b")
1058-
1059-
want := parseEskipFixture(t, "testdata/zone-aware-traffic/only-zone-a.eskip")
1060-
got, err := eskip.Parse(w.Body.String())
1061-
if err != nil {
1062-
t.Fatalf("served routes are not valid eskip: %s", w.Body)
1063-
}
1064-
if !eskip.EqLists(got, want) {
1065-
t.Errorf("served routes do not reflect kubernetes resources: %s", cmp.Diff(got, want))
910+
func TestRoutesWithZone(t *testing.T) {
911+
912+
for _, tc := range []struct {
913+
name string
914+
zone string
915+
ing string
916+
eskip string
917+
}{
918+
{
919+
name: "TwoAddrPerZone",
920+
zone: "eu-central-1a",
921+
ing: "testdata/zone-aware-traffic/all-zones-2-addr.yaml",
922+
eskip: "testdata/zone-aware-traffic/all-zones-2-addr.eskip",
923+
},
924+
{
925+
name: "ThreeAddrPerZone",
926+
zone: "eu-central-1a",
927+
ing: "testdata/zone-aware-traffic/all-zones-3-addr.yaml",
928+
eskip: "testdata/zone-aware-traffic/all-zones-3-addr.eskip",
929+
},
930+
{
931+
name: "AllZonesExceptZoneA",
932+
zone: "eu-central-1a",
933+
ing: "testdata/zone-aware-traffic/all-zones-except-zone-a.yaml",
934+
eskip: "testdata/zone-aware-traffic/all-zones-except-zone-a.eskip",
935+
},
936+
{
937+
name: "AllZonesTopologySetToZoneB",
938+
zone: "eu-central-1b",
939+
ing: "testdata/zone-aware-traffic/all-zones-topology-zone-b.yaml",
940+
eskip: "testdata/zone-aware-traffic/all-zones-topology-zone-b.eskip",
941+
},
942+
{
943+
name: "OnlyZoneA",
944+
zone: "eu-central-1a",
945+
ing: "testdata/zone-aware-traffic/only-zone-a.yaml",
946+
eskip: "testdata/zone-aware-traffic/only-zone-a.eskip",
947+
},
948+
} {
949+
t.Run(tc.name, func(t *testing.T) {
950+
defer tl.Reset()
951+
ks, _ := newKubeServer(t, loadKubeYAML(t, tc.ing))
952+
ks.Start()
953+
defer ks.Close()
954+
rs := newRouteServerWithOptions(t, skipper.Options{
955+
SourcePollTimeout: pollInterval,
956+
Kubernetes: true,
957+
KubernetesURL: ks.URL,
958+
KubernetesTopologyZone: tc.zone,
959+
KubernetesEnableEndpointslices: true,
960+
})
961+
962+
rs.StartUpdates()
963+
defer rs.StopUpdates()
964+
965+
if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
966+
t.Fatalf("routes not initialized: %v", err)
967+
}
968+
w := getZoneAwareRoutes(rs, tc.zone)
969+
970+
want := parseEskipFixture(t, tc.eskip)
971+
got, err := eskip.Parse(w.Body.String())
972+
if err != nil {
973+
t.Fatalf("served routes are not valid eskip: %s", w.Body)
974+
}
975+
if !eskip.EqLists(got, want) {
976+
t.Errorf("served routes do not reflect kubernetes resources: %s", cmp.Diff(got, want))
977+
}
978+
wantHTTPCode(t, w, http.StatusOK)
979+
})
1066980
}
1067-
wantHTTPCode(t, w, http.StatusOK)
1068981
}

0 commit comments

Comments
 (0)