Skip to content

Commit

Permalink
Fix unidling with EndpointSlices
Browse files Browse the repository at this point in the history 
The code was mistakenly assuming that an EndpointSlice would have the
same name as its Service, rather than having a name *based on* its
Service's name.

Also, when HybridProxier gets OnEndpointSlicesSynced, it needs to send
OnEndpointsSynced to the unidling proxy, not OnEndpointSlicesSynced.
  • Loading branch information
@danwinship
danwinship committed Jul 31, 2021
1 parent dee11e9 commit 6b3206b
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 67 deletions.
19 changes: 15 additions & 4 deletions pkg/network/proxy/hybridproxier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,16 @@ func (p *HybridProxier) OnEndpointsSynced() {
klog.V(6).Infof("endpoints synced")
}

func endpointSliceServiceName(slice *discoveryv1beta1.EndpointSlice) string {
serviceName := slice.Labels[discoveryv1beta1.LabelServiceName]
if serviceName == "" {
klog.Warningf("EndpointSlice %s/%s has no %q label",
slice.Namespace, slice.Name, discoveryv1beta1.LabelServiceName)
return slice.Name
}
return serviceName
}

func sliceToEndpoints(slice *discoveryv1beta1.EndpointSlice) *corev1.Endpoints {
if slice == nil {
return nil
Expand All @@ -329,6 +339,7 @@ func sliceToEndpoints(slice *discoveryv1beta1.EndpointSlice) *corev1.Endpoints {
},
},
}
endpoints.Name = endpointSliceServiceName(slice)
for _, ep := range slice.Endpoints {
addr := corev1.EndpointAddress{
NodeName: ep.NodeName,
Expand Down Expand Up @@ -369,7 +380,7 @@ func endpointsIfEmptySlice(slice *discoveryv1beta1.EndpointSlice) *corev1.Endpoi
}

func (p *HybridProxier) OnEndpointSliceAdd(slice *discoveryv1beta1.EndpointSlice) {
svcName := types.NamespacedName{Namespace: slice.Namespace, Name: slice.Name}
svcName := types.NamespacedName{Namespace: slice.Namespace, Name: endpointSliceServiceName(slice)}
hsvc := p.getService(svcName)
defer p.releaseService(svcName)

Expand All @@ -384,7 +395,7 @@ func (p *HybridProxier) OnEndpointSliceAdd(slice *discoveryv1beta1.EndpointSlice
}

func (p *HybridProxier) OnEndpointSliceUpdate(oldSlice, slice *discoveryv1beta1.EndpointSlice) {
svcName := types.NamespacedName{Namespace: slice.Namespace, Name: slice.Name}
svcName := types.NamespacedName{Namespace: slice.Namespace, Name: endpointSliceServiceName(slice)}
hsvc := p.getService(svcName)
defer p.releaseService(svcName)

Expand All @@ -401,7 +412,7 @@ func (p *HybridProxier) OnEndpointSliceUpdate(oldSlice, slice *discoveryv1beta1.
}

func (p *HybridProxier) OnEndpointSliceDelete(slice *discoveryv1beta1.EndpointSlice) {
svcName := types.NamespacedName{Namespace: slice.Namespace, Name: slice.Name}
svcName := types.NamespacedName{Namespace: slice.Namespace, Name: endpointSliceServiceName(slice)}
hsvc := p.getService(svcName)
defer p.releaseService(svcName)

Expand All @@ -418,7 +429,7 @@ func (p *HybridProxier) OnEndpointSliceDelete(slice *discoveryv1beta1.EndpointSl

func (p *HybridProxier) OnEndpointSlicesSynced() {
klog.V(6).Infof("hybrid proxy: endpointslices synced")
p.unidlingProxy.OnEndpointSlicesSynced()
p.unidlingProxy.OnEndpointsSynced()
p.mainProxy.OnEndpointSlicesSynced()
}

Expand Down
48 changes: 24 additions & 24 deletions pkg/network/proxy/hybridproxier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestHybridProxy(t *testing.T) {
proxy.OnEndpointSliceAdd(slice1)

err = mainProxy.assertEvents("after creating first endpoints",
"add endpointslice testns/one 1.2.3.4",
"add endpointslice testns/one-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -127,7 +127,7 @@ func TestHybridProxy(t *testing.T) {

err = mainProxy.assertEvents("after idling first service",
"delete service testns/one",
"update endpointslice testns/one -",
"update endpointslice testns/one-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -148,7 +148,7 @@ func TestHybridProxy(t *testing.T) {

err = mainProxy.assertEvents("after unidling first service",
"add service testns/one",
"update endpointslice testns/one 1.2.3.4",
"update endpointslice testns/one-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -167,7 +167,7 @@ func TestHybridProxy(t *testing.T) {
proxy.OnEndpointSliceUpdate(slice1, slice1modified)

err = mainProxy.assertEvents("after modifying first service",
"update endpointslice testns/one 5.6.7.8",
"update endpointslice testns/one-slice1 5.6.7.8",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -191,7 +191,7 @@ func TestHybridProxy(t *testing.T) {
proxy.OnEndpointSliceUpdate(slice1modified, slice1modified2)

mainProxy.assertEvents("after re-modifying first service",
"update endpointslice testns/one 9.10.11.12",
"update endpointslice testns/one-slice1 9.10.11.12",
)
unidlingProxy.assertEvents("after re-modifying first service",
"delete endpoints testns/one 5.6.7.8",
Expand All @@ -201,7 +201,7 @@ func TestHybridProxy(t *testing.T) {
proxy.OnEndpointSliceUpdate(slice1, slice1modified)

mainProxy.assertEvents("after re-re-modifying first service",
"update endpointslice testns/one 5.6.7.8",
"update endpointslice testns/one-slice1 5.6.7.8",
)
unidlingProxy.assertNoEvents("after re-re-modifying first service")

Expand All @@ -212,7 +212,7 @@ func TestHybridProxy(t *testing.T) {
proxy.OnEndpointSliceAdd(slice2)

err = mainProxy.assertEvents("after creating second endpoints",
"add endpointslice testns/two 9.10.11.12",
"add endpointslice testns/two-slice1 9.10.11.12",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -251,7 +251,7 @@ func TestHybridProxy(t *testing.T) {

err = mainProxy.assertEvents("after idling second service",
"delete service testns/two",
"update endpointslice testns/two -",
"update endpointslice testns/two-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -272,7 +272,7 @@ func TestHybridProxy(t *testing.T) {
err = mainProxy.assertEvents("after unidling second service",
"add service testns/two",
"update service testns/two",
"update endpointslice testns/two 9.10.11.12",
"update endpointslice testns/two-slice1 9.10.11.12",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -312,7 +312,7 @@ func TestHybridProxy(t *testing.T) {
proxy.OnEndpointSliceAdd(slice3)

err = mainProxy.assertEvents("after creating third endpoints",
"add endpointslice testns/three 1.2.3.4",
"add endpointslice testns/three-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -332,7 +332,7 @@ func TestHybridProxy(t *testing.T) {
err = mainProxy.assertEvents("after idling third service",
"update service testns/three",
"delete service testns/three",
"update endpointslice testns/three -",
"update endpointslice testns/three-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -367,7 +367,7 @@ func TestHybridProxy(t *testing.T) {
proxy.OnEndpointSliceDelete(slice3idled)

err = mainProxy.assertEvents("after deleting third endpoints",
"delete endpointslice testns/three -",
"delete endpointslice testns/three-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -400,9 +400,9 @@ func TestHybridProxy(t *testing.T) {

err = mainProxy.assertEvents("after cleanup",
"delete service testns/one",
"delete endpointslice testns/one 5.6.7.8",
"delete endpointslice testns/one-slice1 5.6.7.8",
"delete service testns/two",
"delete endpointslice testns/two 9.10.11.12",
"delete endpointslice testns/two-slice1 9.10.11.12",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -435,7 +435,7 @@ func TestHybridProxyPreIdled(t *testing.T) {
proxy.OnServiceAdd(svcpi)

err = mainProxy.assertEvents("after creating pre-idled service",
"add endpointslice testns/pre-idled -",
"add endpointslice testns/pre-idled-slice1 -",
"add service testns/pre-idled",
"delete service testns/pre-idled",
)
Expand All @@ -458,7 +458,7 @@ func TestHybridProxyPreIdled(t *testing.T) {

err = mainProxy.assertEvents("after unidling pre-idled service",
"add service testns/pre-idled",
"update endpointslice testns/pre-idled 1.2.3.4",
"update endpointslice testns/pre-idled-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -481,7 +481,7 @@ func TestHybridProxyPreIdled(t *testing.T) {

err = mainProxy.assertEvents("after deleting pre-idled service",
"delete service testns/pre-idled",
"delete endpointslice testns/pre-idled 1.2.3.4",
"delete endpointslice testns/pre-idled-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -518,7 +518,7 @@ func TestHybridProxyReIdling(t *testing.T) {

err = mainProxy.assertEvents("after creating re-idling service",
"add service testns/re-idle",
"add endpointslice testns/re-idle 1.2.3.4",
"add endpointslice testns/re-idle-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -537,7 +537,7 @@ func TestHybridProxyReIdling(t *testing.T) {

err = mainProxy.assertEvents("after idling re-idle service",
"delete service testns/re-idle",
"update endpointslice testns/re-idle -",
"update endpointslice testns/re-idle-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -555,7 +555,7 @@ func TestHybridProxyReIdling(t *testing.T) {

err = mainProxy.assertEvents("after unidling re-idle service",
"add service testns/re-idle",
"update endpointslice testns/re-idle 1.2.3.4",
"update endpointslice testns/re-idle-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -575,7 +575,7 @@ func TestHybridProxyReIdling(t *testing.T) {

err = mainProxy.assertEvents("after re-idling re-idle service",
"delete service testns/re-idle",
"update endpointslice testns/re-idle -",
"update endpointslice testns/re-idle-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand All @@ -594,7 +594,7 @@ func TestHybridProxyReIdling(t *testing.T) {

err = mainProxy.assertEvents("after unidling re-idle service",
"add service testns/re-idle",
"update endpointslice testns/re-idle 1.2.3.4",
"update endpointslice testns/re-idle-slice1 1.2.3.4",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -622,7 +622,7 @@ func TestHybridProxyReIdling(t *testing.T) {

err = mainProxy.assertEvents("after re-idling re-idle service after time passed",
"delete service testns/re-idle",
"update endpointslice testns/re-idle -",
"update endpointslice testns/re-idle-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -652,7 +652,7 @@ func TestHybridProxyReIdling(t *testing.T) {
proxy.OnEndpointSliceDelete(sliceriIdled)

err = mainProxy.assertEvents("after deleting re-idle service",
"delete endpointslice testns/re-idle -",
"delete endpointslice testns/re-idle-slice1 -",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down

0 comments on commit 6b3206b

Please sign in to comment.