From f543e7434a88a7390ed566fa7db724c80970136e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 27 Apr 2022 10:38:00 -0400 Subject: [PATCH] Improve reconciler test result checking Consistently verify creates/updates based on the fake client's action tracking, not based on the return values of the reconciler functions. (This will also let us check that both Endpoints and EndpointSlices were created/updated correctly.) --- .../reconcilers/endpointsadapter.go | 1 + .../reconcilers/endpointsadapter_test.go | 120 +++++++----------- pkg/controlplane/reconcilers/helpers_test.go | 54 ++++++++ .../reconcilers/instancecount_test.go | 81 ++---------- pkg/controlplane/reconcilers/lease_test.go | 50 +++----- 5 files changed, 132 insertions(+), 174 deletions(-) diff --git a/pkg/controlplane/reconcilers/endpointsadapter.go b/pkg/controlplane/reconcilers/endpointsadapter.go index 25e69d010a4..5781ec995c1 100644 --- a/pkg/controlplane/reconcilers/endpointsadapter.go +++ b/pkg/controlplane/reconcilers/endpointsadapter.go @@ -123,6 +123,7 @@ func (adapter *EndpointsAdapter) EnsureEndpointSliceFromEndpoints(namespace stri func endpointSliceFromEndpoints(endpoints *corev1.Endpoints) *discovery.EndpointSlice { endpointSlice := &discovery.EndpointSlice{} endpointSlice.Name = endpoints.Name + endpointSlice.Namespace = endpoints.Namespace endpointSlice.Labels = map[string]string{discovery.LabelServiceName: endpoints.Name} // TODO: Add support for dual stack here (and in the rest of diff --git a/pkg/controlplane/reconcilers/endpointsadapter_test.go b/pkg/controlplane/reconcilers/endpointsadapter_test.go index 1839faf418f..35364634c68 100644 --- a/pkg/controlplane/reconcilers/endpointsadapter_test.go +++ b/pkg/controlplane/reconcilers/endpointsadapter_test.go @@ -113,18 +113,18 @@ func TestEndpointsAdapterCreate(t *testing.T) { testCases := map[string]struct { endpointSlicesEnabled bool expectedError error - expectedEndpoints *corev1.Endpoints - expectedEndpointSlice *discovery.EndpointSlice + expectedResult *corev1.Endpoints + expectCreate []runtime.Object + expectUpdate []runtime.Object initialState []runtime.Object - endpointSlices []*discovery.EndpointSlice namespaceParam string endpointsParam *corev1.Endpoints }{ "single-endpoint": { endpointSlicesEnabled: true, expectedError: nil, - expectedEndpoints: endpoints1, - expectedEndpointSlice: epSlice1, + expectedResult: endpoints1, + expectCreate: []runtime.Object{endpoints1, epSlice1}, initialState: []runtime.Object{}, namespaceParam: endpoints1.Namespace, endpointsParam: endpoints1, @@ -132,8 +132,8 @@ func TestEndpointsAdapterCreate(t *testing.T) { "single-endpoint-partial-ipv6": { endpointSlicesEnabled: true, expectedError: nil, - expectedEndpoints: endpoints2, - expectedEndpointSlice: epSlice2, + expectedResult: endpoints2, + expectCreate: []runtime.Object{endpoints2, epSlice2}, initialState: []runtime.Object{}, namespaceParam: endpoints2.Namespace, endpointsParam: endpoints2, @@ -141,8 +141,8 @@ func TestEndpointsAdapterCreate(t *testing.T) { "single-endpoint-full-ipv6": { endpointSlicesEnabled: true, expectedError: nil, - expectedEndpoints: endpoints3, - expectedEndpointSlice: epSlice3, + expectedResult: endpoints3, + expectCreate: []runtime.Object{endpoints3, epSlice3}, initialState: []runtime.Object{}, namespaceParam: endpoints3.Namespace, endpointsParam: endpoints3, @@ -150,8 +150,8 @@ func TestEndpointsAdapterCreate(t *testing.T) { "single-endpoint-no-slices": { endpointSlicesEnabled: false, expectedError: nil, - expectedEndpoints: endpoints1, - expectedEndpointSlice: nil, + expectedResult: endpoints1, + expectCreate: []runtime.Object{endpoints1}, initialState: []runtime.Object{}, namespaceParam: endpoints1.Namespace, endpointsParam: endpoints1, @@ -159,11 +159,13 @@ func TestEndpointsAdapterCreate(t *testing.T) { "existing-endpoint": { endpointSlicesEnabled: true, expectedError: errors.NewAlreadyExists(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), - expectedEndpoints: nil, - expectedEndpointSlice: nil, - initialState: []runtime.Object{endpoints1}, + expectedResult: nil, + initialState: []runtime.Object{endpoints1, epSlice1}, namespaceParam: endpoints1.Namespace, endpointsParam: endpoints1, + + // We expect the create to be attempted, we just also expect it to fail + expectCreate: []runtime.Object{endpoints1}, }, } @@ -181,37 +183,20 @@ func TestEndpointsAdapterCreate(t *testing.T) { t.Errorf("Expected error: %v, got: %v", testCase.expectedError, err) } - if !apiequality.Semantic.DeepEqual(endpoints, testCase.expectedEndpoints) { - t.Errorf("Expected endpoints: %v, got: %v", testCase.expectedEndpoints, endpoints) + if !apiequality.Semantic.DeepEqual(endpoints, testCase.expectedResult) { + t.Errorf("Expected endpoints: %v, got: %v", testCase.expectedResult, endpoints) } - epSliceList, err := client.DiscoveryV1().EndpointSlices(testCase.namespaceParam).List(context.TODO(), metav1.ListOptions{}) + err = verifyCreatesAndUpdates(client, testCase.expectCreate, nil) if err != nil { - t.Fatalf("Error listing Endpoint Slices: %v", err) - } - - if testCase.expectedEndpointSlice == nil { - if len(epSliceList.Items) != 0 { - t.Fatalf("Expected no Endpoint Slices, got: %v", epSliceList.Items) - } - } else { - if len(epSliceList.Items) == 0 { - t.Fatalf("No Endpoint Slices found, expected: %v", testCase.expectedEndpointSlice) - } - if len(epSliceList.Items) > 1 { - t.Errorf("Only 1 Endpoint Slice expected, got: %v", testCase.expectedEndpointSlice) - } - if !apiequality.Semantic.DeepEqual(*testCase.expectedEndpointSlice, epSliceList.Items[0]) { - t.Errorf("Expected Endpoint Slice: %v, got: %v", testCase.expectedEndpointSlice, epSliceList.Items[0]) - - } + t.Errorf("unexpected error in side effects: %v", err) } }) } } func TestEndpointsAdapterUpdate(t *testing.T) { - endpoints1, _ := generateEndpointsAndSlice("foo", "testing", []int{80}, []string{"10.1.2.3", "10.1.2.4"}) + endpoints1, epSlice1 := generateEndpointsAndSlice("foo", "testing", []int{80}, []string{"10.1.2.3", "10.1.2.4"}) endpoints2, epSlice2 := generateEndpointsAndSlice("foo", "testing", []int{80, 443}, []string{"10.1.2.3", "10.1.2.4", "10.1.2.5"}) endpoints3, _ := generateEndpointsAndSlice("bar", "testing", []int{80, 443}, []string{"10.1.2.3", "10.1.2.4", "10.1.2.5"}) @@ -226,49 +211,57 @@ func TestEndpointsAdapterUpdate(t *testing.T) { testCases := map[string]struct { endpointSlicesEnabled bool expectedError error - expectedEndpoints *corev1.Endpoints - expectedEndpointSlice *discovery.EndpointSlice + expectedResult *corev1.Endpoints + expectCreate []runtime.Object + expectUpdate []runtime.Object initialState []runtime.Object - endpointSlices []*discovery.EndpointSlice namespaceParam string endpointsParam *corev1.Endpoints }{ "single-existing-endpoints-no-change": { endpointSlicesEnabled: false, expectedError: nil, - expectedEndpoints: endpoints1, - expectedEndpointSlice: nil, + expectedResult: endpoints1, initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", endpointsParam: endpoints1, + + // Even though there's no change, we still expect Update() to be + // called, because this unit test ALWAYS calls Update(). + expectUpdate: []runtime.Object{endpoints1}, }, "existing-endpointslice-replaced-with-updated-ipv4-address-type": { endpointSlicesEnabled: true, expectedError: nil, - expectedEndpoints: endpoints4, - expectedEndpointSlice: epSlice4IPv4, - initialState: []runtime.Object{endpoints4}, - endpointSlices: []*discovery.EndpointSlice{epSlice4IP}, + expectedResult: endpoints4, + initialState: []runtime.Object{endpoints4, epSlice4IP}, namespaceParam: "testing", endpointsParam: endpoints4, + + // When AddressType changes, we Delete+Create the EndpointSlice, + // so that shows up in expectCreate, not expectUpdate. + expectUpdate: []runtime.Object{endpoints4}, + expectCreate: []runtime.Object{epSlice4IPv4}, }, "add-ports-and-ips": { endpointSlicesEnabled: true, expectedError: nil, - expectedEndpoints: endpoints2, - expectedEndpointSlice: epSlice2, - initialState: []runtime.Object{endpoints1}, + expectedResult: endpoints2, + expectUpdate: []runtime.Object{endpoints2, epSlice2}, + initialState: []runtime.Object{endpoints1, epSlice1}, namespaceParam: "testing", endpointsParam: endpoints2, }, "missing-endpoints": { endpointSlicesEnabled: true, expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), - expectedEndpoints: nil, - expectedEndpointSlice: nil, - initialState: []runtime.Object{endpoints1}, + expectedResult: nil, + initialState: []runtime.Object{endpoints1, epSlice1}, namespaceParam: "testing", endpointsParam: endpoints3, + + // We expect the update to be attempted, we just also expect it to fail + expectUpdate: []runtime.Object{endpoints3}, }, } @@ -286,30 +279,13 @@ func TestEndpointsAdapterUpdate(t *testing.T) { t.Errorf("Expected error: %v, got: %v", testCase.expectedError, err) } - if !apiequality.Semantic.DeepEqual(endpoints, testCase.expectedEndpoints) { - t.Errorf("Expected endpoints: %v, got: %v", testCase.expectedEndpoints, endpoints) + if !apiequality.Semantic.DeepEqual(endpoints, testCase.expectedResult) { + t.Errorf("Expected endpoints: %v, got: %v", testCase.expectedResult, endpoints) } - epSliceList, err := client.DiscoveryV1().EndpointSlices(testCase.namespaceParam).List(context.TODO(), metav1.ListOptions{}) + err = verifyCreatesAndUpdates(client, testCase.expectCreate, testCase.expectUpdate) if err != nil { - t.Fatalf("Error listing Endpoint Slices: %v", err) - } - - if testCase.expectedEndpointSlice == nil { - if len(epSliceList.Items) != 0 { - t.Fatalf("Expected no Endpoint Slices, got: %v", epSliceList.Items) - } - } else { - if len(epSliceList.Items) == 0 { - t.Fatalf("No Endpoint Slices found, expected: %v", testCase.expectedEndpointSlice) - } - if len(epSliceList.Items) > 1 { - t.Errorf("Only 1 Endpoint Slice expected, got: %v", testCase.expectedEndpointSlice) - } - if !apiequality.Semantic.DeepEqual(*testCase.expectedEndpointSlice, epSliceList.Items[0]) { - t.Errorf("Expected Endpoint Slice: %v, got: %v", testCase.expectedEndpointSlice, epSliceList.Items[0]) - - } + t.Errorf("unexpected error in side effects: %v", err) } }) } diff --git a/pkg/controlplane/reconcilers/helpers_test.go b/pkg/controlplane/reconcilers/helpers_test.go index 551094fd2cd..502ac978ad5 100644 --- a/pkg/controlplane/reconcilers/helpers_test.go +++ b/pkg/controlplane/reconcilers/helpers_test.go @@ -17,10 +17,16 @@ limitations under the License. package reconcilers import ( + "fmt" + corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" ) func makeEndpointsArray(name string, ips []string, ports []corev1.EndpointPort) []runtime.Object { @@ -50,3 +56,51 @@ func makeEndpoints(name string, ips []string, ports []corev1.EndpointPort) *core } return endpoints } + +func verifyCreatesAndUpdates(fakeClient *fake.Clientset, expectedCreates, expectedUpdates []runtime.Object) error { + errors := []error{} + + updates := []k8stesting.UpdateAction{} + creates := []k8stesting.CreateAction{} + for _, action := range fakeClient.Actions() { + if action.GetVerb() == "update" { + updates = append(updates, action.(k8stesting.UpdateAction)) + } else if action.GetVerb() == "create" { + creates = append(creates, action.(k8stesting.CreateAction)) + } + } + + if len(creates) != len(expectedCreates) { + errors = append(errors, fmt.Errorf("expected %d creates got %d", len(expectedCreates), len(creates))) + } + for i := 0; i < len(creates) || i < len(expectedCreates); i++ { + var expected, actual runtime.Object + if i < len(creates) { + actual = creates[i].GetObject() + } + if i < len(expectedCreates) { + expected = expectedCreates[i] + } + if !apiequality.Semantic.DeepEqual(expected, actual) { + errors = append(errors, fmt.Errorf("expected create %d to be:\n%#v\ngot:\n%#v\n", i, expected, actual)) + } + } + + if len(updates) != len(expectedUpdates) { + errors = append(errors, fmt.Errorf("expected %d updates got %d", len(expectedUpdates), len(updates))) + } + for i := 0; i < len(updates) || i < len(expectedUpdates); i++ { + var expected, actual runtime.Object + if i < len(updates) { + actual = updates[i].GetObject() + } + if i < len(expectedUpdates) { + expected = expectedUpdates[i] + } + if !apiequality.Semantic.DeepEqual(expected, actual) { + errors = append(errors, fmt.Errorf("expected update %d to be:\n%#v\ngot:\n%#v\n", i, expected, actual)) + } + } + + return utilerrors.NewAggregate(errors) +} diff --git a/pkg/controlplane/reconcilers/instancecount_test.go b/pkg/controlplane/reconcilers/instancecount_test.go index 27b901d788b..bec99ed0053 100644 --- a/pkg/controlplane/reconcilers/instancecount_test.go +++ b/pkg/controlplane/reconcilers/instancecount_test.go @@ -17,13 +17,11 @@ limitations under the License. package reconcilers import ( - "reflect" "testing" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" - core "k8s.io/client-go/testing" netutils "k8s.io/utils/net" ) @@ -186,43 +184,12 @@ func TestMasterCountEndpointReconciler(t *testing.T) { reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, true) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("unexpected error reconciling: %v", err) } - updates := []core.UpdateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "update" { - continue - } - updates = append(updates, action.(core.UpdateAction)) - } - if test.expectUpdate != nil { - if len(updates) != 1 { - t.Errorf("unexpected updates: %v", updates) - } else if e, a := test.expectUpdate[0], updates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) - } - } - if test.expectUpdate == nil && len(updates) > 0 { - t.Errorf("no update expected, yet saw: %v", updates) - } - - creates := []core.CreateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "create" { - continue - } - creates = append(creates, action.(core.CreateAction)) - } - if test.expectCreate != nil { - if len(creates) != 1 { - t.Errorf("unexpected creates: %v", creates) - } else if e, a := test.expectCreate[0], creates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("expected create:\n%#v\ngot:\n%#v\n", e, a) - } - } - if test.expectCreate == nil && len(creates) > 0 { - t.Errorf("no create expected, yet saw: %v", creates) + err = verifyCreatesAndUpdates(fakeClient, test.expectCreate, test.expectUpdate) + if err != nil { + t.Errorf("unexpected error in side effects: %v", err) } }) } @@ -275,47 +242,15 @@ func TestMasterCountEndpointReconciler(t *testing.T) { reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, false) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("unexpected error reconciling: %v", err) } - updates := []core.UpdateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "update" { - continue - } - updates = append(updates, action.(core.UpdateAction)) - } - if test.expectUpdate != nil { - if len(updates) != 1 { - t.Errorf("unexpected updates: %v", updates) - } else if e, a := test.expectUpdate[0], updates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) - } - } - if test.expectUpdate == nil && len(updates) > 0 { - t.Errorf("no update expected, yet saw: %v", updates) - } - - creates := []core.CreateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "create" { - continue - } - creates = append(creates, action.(core.CreateAction)) - } - if test.expectCreate != nil { - if len(creates) != 1 { - t.Errorf("unexpected creates: %v", creates) - } else if e, a := test.expectCreate[0], creates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("expected create:\n%#v\ngot:\n%#v\n", e, a) - } - } - if test.expectCreate == nil && len(creates) > 0 { - t.Errorf("no create expected, yet saw: %v", creates) + err = verifyCreatesAndUpdates(fakeClient, test.expectCreate, test.expectUpdate) + if err != nil { + t.Errorf("unexpected error in side effects: %v", err) } }) } - } func TestEmptySubsets(t *testing.T) { diff --git a/pkg/controlplane/reconcilers/lease_test.go b/pkg/controlplane/reconcilers/lease_test.go index aa1c911ab3f..37a851d6c48 100644 --- a/pkg/controlplane/reconcilers/lease_test.go +++ b/pkg/controlplane/reconcilers/lease_test.go @@ -22,8 +22,6 @@ https://github.com/openshift/origin/blob/bb340c5dd5ff72718be86fb194dedc0faed7f4c */ import ( - "context" - "reflect" "testing" corev1 "k8s.io/api/core/v1" @@ -89,6 +87,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { endpointKeys []string initialState []runtime.Object expectUpdate []runtime.Object + expectCreate []runtime.Object }{ { testName: "no existing endpoints", @@ -96,7 +95,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: nil, - expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectCreate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy", @@ -154,7 +153,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: makeEndpointsArray("bar", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), - expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectCreate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong IP", @@ -253,17 +252,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, true) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("unexpected error reconciling: %v", err) } - actualEndpoints, err := clientset.CoreV1().Endpoints(corev1.NamespaceDefault).Get(context.TODO(), test.serviceName, metav1.GetOptions{}) + + err = verifyCreatesAndUpdates(clientset, test.expectCreate, test.expectUpdate) if err != nil { - t.Errorf("unexpected error: %v", err) - } - if test.expectUpdate != nil { - if e, a := test.expectUpdate[0], actualEndpoints; !reflect.DeepEqual(e, a) { - t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) - } + t.Errorf("unexpected error in side effects: %v", err) } + if updatedKeys := fakeLeases.GetUpdatedKeys(); len(updatedKeys) != 1 || updatedKeys[0] != test.ip { t.Errorf("expected the master's IP to be refreshed, but the following IPs were refreshed instead: %v", updatedKeys) } @@ -278,6 +274,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { endpointKeys []string initialState []runtime.Object expectUpdate []runtime.Object + expectCreate []runtime.Object }{ { testName: "existing endpoints extra service ports missing port no update", @@ -307,7 +304,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: nil, - expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectCreate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, } for _, test := range nonReconcileTests { @@ -319,17 +316,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, false) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("unexpected error reconciling: %v", err) } - actualEndpoints, err := clientset.CoreV1().Endpoints(corev1.NamespaceDefault).Get(context.TODO(), test.serviceName, metav1.GetOptions{}) + + err = verifyCreatesAndUpdates(clientset, test.expectCreate, test.expectUpdate) if err != nil { - t.Errorf("unexpected error: %v", err) - } - if test.expectUpdate != nil { - if e, a := test.expectUpdate[0], actualEndpoints; !reflect.DeepEqual(e, a) { - t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) - } + t.Errorf("unexpected error in side effects: %v", err) } + if updatedKeys := fakeLeases.GetUpdatedKeys(); len(updatedKeys) != 1 || updatedKeys[0] != test.ip { t.Errorf("expected the master's IP to be refreshed, but the following IPs were refreshed instead: %v", updatedKeys) } @@ -371,6 +365,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, endpointKeys: []string{"1.2.3.4", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, initialState: makeEndpointsArray("foo", nil, nil), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, } for _, test := range stopTests { @@ -382,17 +377,14 @@ func TestLeaseRemoveEndpoints(t *testing.T) { r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.RemoveEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("unexpected error reconciling: %v", err) } - actualEndpoints, err := clientset.CoreV1().Endpoints(corev1.NamespaceDefault).Get(context.TODO(), test.serviceName, metav1.GetOptions{}) + + err = verifyCreatesAndUpdates(clientset, nil, test.expectUpdate) if err != nil { - t.Errorf("unexpected error: %v", err) - } - if test.expectUpdate != nil { - if e, a := test.expectUpdate[0], actualEndpoints; !reflect.DeepEqual(e, a) { - t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) - } + t.Errorf("unexpected error in side effects: %v", err) } + for _, key := range fakeLeases.GetUpdatedKeys() { if key == test.ip { t.Errorf("Found ip %s in leases but shouldn't be there", key)