From 007ca4f69d7cddd06c120a7f8f3b50463e2f97c7 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 27 Apr 2022 10:05:09 -0400 Subject: [PATCH 1/7] Use t.Run() consistently in reconciler unit tests --- .../reconcilers/instancecount_test.go | 166 +++++++++--------- pkg/controlplane/reconcilers/lease_test.go | 74 ++++---- 2 files changed, 122 insertions(+), 118 deletions(-) diff --git a/pkg/controlplane/reconcilers/instancecount_test.go b/pkg/controlplane/reconcilers/instancecount_test.go index a17a61613ad..8970031afad 100644 --- a/pkg/controlplane/reconcilers/instancecount_test.go +++ b/pkg/controlplane/reconcilers/instancecount_test.go @@ -393,53 +393,54 @@ func TestMasterCountEndpointReconciler(t *testing.T) { }, } for _, test := range reconcileTests { - fakeClient := fake.NewSimpleClientset() - if test.endpoints != nil { - fakeClient = fake.NewSimpleClientset(test.endpoints) - } - epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) - reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) - err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, true) - if err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) - } + t.Run(test.testName, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset() + if test.endpoints != nil { + fakeClient = fake.NewSimpleClientset(test.endpoints) + } + epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) + 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) + } - updates := []core.UpdateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "update" { - continue + updates := []core.UpdateAction{} + for _, action := range fakeClient.Actions() { + if action.GetVerb() != "update" { + continue + } + updates = append(updates, action.(core.UpdateAction)) } - updates = append(updates, action.(core.UpdateAction)) - } - if test.expectUpdate != nil { - if len(updates) != 1 { - t.Errorf("case %q: unexpected updates: %v", test.testName, updates) - } else if e, a := test.expectUpdate, updates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a) + if test.expectUpdate != nil { + if len(updates) != 1 { + t.Errorf("unexpected updates: %v", updates) + } else if e, a := test.expectUpdate, 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) } - } - if test.expectUpdate == nil && len(updates) > 0 { - t.Errorf("case %q: no update expected, yet saw: %v", test.testName, updates) - } - creates := []core.CreateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "create" { - continue + creates := []core.CreateAction{} + for _, action := range fakeClient.Actions() { + if action.GetVerb() != "create" { + continue + } + creates = append(creates, action.(core.CreateAction)) } - creates = append(creates, action.(core.CreateAction)) - } - if test.expectCreate != nil { - if len(creates) != 1 { - t.Errorf("case %q: unexpected creates: %v", test.testName, creates) - } else if e, a := test.expectCreate, creates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("case %q: expected create:\n%#v\ngot:\n%#v\n", test.testName, e, a) + if test.expectCreate != nil { + if len(creates) != 1 { + t.Errorf("unexpected creates: %v", creates) + } else if e, a := test.expectCreate, 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("case %q: no create expected, yet saw: %v", test.testName, creates) - } - + if test.expectCreate == nil && len(creates) > 0 { + t.Errorf("no create expected, yet saw: %v", creates) + } + }) } nonReconcileTests := []struct { @@ -512,53 +513,54 @@ func TestMasterCountEndpointReconciler(t *testing.T) { }, } for _, test := range nonReconcileTests { - fakeClient := fake.NewSimpleClientset() - if test.endpoints != nil { - fakeClient = fake.NewSimpleClientset(test.endpoints) - } - epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) - reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) - err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, false) - if err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) - } + t.Run(test.testName, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset() + if test.endpoints != nil { + fakeClient = fake.NewSimpleClientset(test.endpoints) + } + epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) + 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) + } - updates := []core.UpdateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "update" { - continue + updates := []core.UpdateAction{} + for _, action := range fakeClient.Actions() { + if action.GetVerb() != "update" { + continue + } + updates = append(updates, action.(core.UpdateAction)) } - updates = append(updates, action.(core.UpdateAction)) - } - if test.expectUpdate != nil { - if len(updates) != 1 { - t.Errorf("case %q: unexpected updates: %v", test.testName, updates) - } else if e, a := test.expectUpdate, updates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a) + if test.expectUpdate != nil { + if len(updates) != 1 { + t.Errorf("unexpected updates: %v", updates) + } else if e, a := test.expectUpdate, 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) } - } - if test.expectUpdate == nil && len(updates) > 0 { - t.Errorf("case %q: no update expected, yet saw: %v", test.testName, updates) - } - creates := []core.CreateAction{} - for _, action := range fakeClient.Actions() { - if action.GetVerb() != "create" { - continue + creates := []core.CreateAction{} + for _, action := range fakeClient.Actions() { + if action.GetVerb() != "create" { + continue + } + creates = append(creates, action.(core.CreateAction)) } - creates = append(creates, action.(core.CreateAction)) - } - if test.expectCreate != nil { - if len(creates) != 1 { - t.Errorf("case %q: unexpected creates: %v", test.testName, creates) - } else if e, a := test.expectCreate, creates[0].GetObject(); !reflect.DeepEqual(e, a) { - t.Errorf("case %q: expected create:\n%#v\ngot:\n%#v\n", test.testName, e, a) + if test.expectCreate != nil { + if len(creates) != 1 { + t.Errorf("unexpected creates: %v", creates) + } else if e, a := test.expectCreate, 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("case %q: no create expected, yet saw: %v", test.testName, creates) - } - + if test.expectCreate == nil && len(creates) > 0 { + t.Errorf("no create expected, yet saw: %v", creates) + } + }) } } diff --git a/pkg/controlplane/reconcilers/lease_test.go b/pkg/controlplane/reconcilers/lease_test.go index 3f99897f222..fb58fa0faaa 100644 --- a/pkg/controlplane/reconcilers/lease_test.go +++ b/pkg/controlplane/reconcilers/lease_test.go @@ -448,36 +448,38 @@ func TestLeaseEndpointReconciler(t *testing.T) { }, } for _, test := range reconcileTests { - fakeLeases := newFakeLeases() - fakeLeases.SetKeys(test.endpointKeys) - clientset := fake.NewSimpleClientset() - if test.endpoints != nil { - for _, ep := range test.endpoints.Items { - if _, err := clientset.CoreV1().Endpoints(ep.Namespace).Create(context.TODO(), &ep, metav1.CreateOptions{}); err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) - continue + t.Run(test.testName, func(t *testing.T) { + fakeLeases := newFakeLeases() + fakeLeases.SetKeys(test.endpointKeys) + clientset := fake.NewSimpleClientset() + if test.endpoints != nil { + for _, ep := range test.endpoints.Items { + if _, err := clientset.CoreV1().Endpoints(ep.Namespace).Create(context.TODO(), &ep, metav1.CreateOptions{}); err != nil { + t.Errorf("unexpected error: %v", err) + continue + } } } - } - epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} - r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) - err := r.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, true) - if err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) - } - actualEndpoints, err := clientset.CoreV1().Endpoints(corev1.NamespaceDefault).Get(context.TODO(), test.serviceName, metav1.GetOptions{}) - if err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) - } - if test.expectUpdate != nil { - if e, a := test.expectUpdate, actualEndpoints; !reflect.DeepEqual(e, a) { - t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a) + epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} + 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) } - } - if updatedKeys := fakeLeases.GetUpdatedKeys(); len(updatedKeys) != 1 || updatedKeys[0] != test.ip { - t.Errorf("case %q: expected the master's IP to be refreshed, but the following IPs were refreshed instead: %v", test.testName, updatedKeys) - } + actualEndpoints, err := clientset.CoreV1().Endpoints(corev1.NamespaceDefault).Get(context.TODO(), test.serviceName, metav1.GetOptions{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if test.expectUpdate != nil { + if e, a := test.expectUpdate, actualEndpoints; !reflect.DeepEqual(e, a) { + t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) + } + } + 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) + } + }) } nonReconcileTests := []struct { @@ -556,7 +558,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { if test.endpoints != nil { for _, ep := range test.endpoints.Items { if _, err := clientset.CoreV1().Endpoints(ep.Namespace).Create(context.TODO(), &ep, metav1.CreateOptions{}); err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) + t.Errorf("unexpected error: %v", err) continue } } @@ -565,19 +567,19 @@ 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("case %q: unexpected error: %v", test.testName, err) + t.Errorf("unexpected error: %v", err) } actualEndpoints, err := clientset.CoreV1().Endpoints(corev1.NamespaceDefault).Get(context.TODO(), test.serviceName, metav1.GetOptions{}) if err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) + t.Errorf("unexpected error: %v", err) } if test.expectUpdate != nil { if e, a := test.expectUpdate, actualEndpoints; !reflect.DeepEqual(e, a) { - t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a) + t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) } } if updatedKeys := fakeLeases.GetUpdatedKeys(); len(updatedKeys) != 1 || updatedKeys[0] != test.ip { - t.Errorf("case %q: expected the master's IP to be refreshed, but the following IPs were refreshed instead: %v", test.testName, updatedKeys) + t.Errorf("expected the master's IP to be refreshed, but the following IPs were refreshed instead: %v", updatedKeys) } }) } @@ -677,7 +679,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { clientset := fake.NewSimpleClientset() for _, ep := range test.endpoints.Items { if _, err := clientset.CoreV1().Endpoints(ep.Namespace).Create(context.TODO(), &ep, metav1.CreateOptions{}); err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) + t.Errorf("unexpected error: %v", err) continue } } @@ -685,20 +687,20 @@ 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("case %q: unexpected error: %v", test.testName, err) + t.Errorf("unexpected error: %v", err) } actualEndpoints, err := clientset.CoreV1().Endpoints(corev1.NamespaceDefault).Get(context.TODO(), test.serviceName, metav1.GetOptions{}) if err != nil { - t.Errorf("case %q: unexpected error: %v", test.testName, err) + t.Errorf("unexpected error: %v", err) } if test.expectUpdate != nil { if e, a := test.expectUpdate, actualEndpoints; !reflect.DeepEqual(e, a) { - t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a) + t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) } } for _, key := range fakeLeases.GetUpdatedKeys() { if key == test.ip { - t.Errorf("case %q: Found ip %s in leases but shouldn't be there", test.testName, key) + t.Errorf("Found ip %s in leases but shouldn't be there", key) } } }) From b07fe3a974d46713d2c3ad914ab115c9e7654db9 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 27 Apr 2022 09:22:58 -0400 Subject: [PATCH 2/7] Simplify reconciler unit test setup Pass initial state objects to fake.NewSimpleClientSet() rather than calling Create() by hand. (This will make it easier to have an initial state that is a mix of Endpoints and EndpointSlices later on.) --- .../reconcilers/endpointsadapter_test.go | 79 +++------ .../reconcilers/instancecount_test.go | 116 +++++++------ pkg/controlplane/reconcilers/lease_test.go | 153 ++++++++---------- 3 files changed, 146 insertions(+), 202 deletions(-) diff --git a/pkg/controlplane/reconcilers/endpointsadapter_test.go b/pkg/controlplane/reconcilers/endpointsadapter_test.go index 23a28e391cc..1839faf418f 100644 --- a/pkg/controlplane/reconcilers/endpointsadapter_test.go +++ b/pkg/controlplane/reconcilers/endpointsadapter_test.go @@ -26,6 +26,7 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/fake" ) @@ -37,7 +38,7 @@ func TestEndpointsAdapterGet(t *testing.T) { endpointSlicesEnabled bool expectedError error expectedEndpoints *corev1.Endpoints - endpoints []*corev1.Endpoints + initialState []runtime.Object namespaceParam string nameParam string }{ @@ -45,7 +46,7 @@ func TestEndpointsAdapterGet(t *testing.T) { endpointSlicesEnabled: false, expectedError: nil, expectedEndpoints: endpoints1, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", nameParam: "foo", }, @@ -53,7 +54,7 @@ func TestEndpointsAdapterGet(t *testing.T) { endpointSlicesEnabled: true, expectedError: nil, expectedEndpoints: endpoints1, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", nameParam: "foo", }, @@ -61,7 +62,7 @@ func TestEndpointsAdapterGet(t *testing.T) { endpointSlicesEnabled: false, expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), expectedEndpoints: nil, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: "foo", nameParam: "foo", }, @@ -69,7 +70,7 @@ func TestEndpointsAdapterGet(t *testing.T) { endpointSlicesEnabled: false, expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), expectedEndpoints: nil, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", nameParam: "bar", }, @@ -77,19 +78,12 @@ func TestEndpointsAdapterGet(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { - client := fake.NewSimpleClientset() + client := fake.NewSimpleClientset(testCase.initialState...) epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} if testCase.endpointSlicesEnabled { epAdapter.endpointSliceClient = client.DiscoveryV1() } - for _, endpoints := range testCase.endpoints { - _, err := client.CoreV1().Endpoints(endpoints.Namespace).Create(context.TODO(), endpoints, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("Error creating Endpoints: %v", err) - } - } - endpoints, err := epAdapter.Get(testCase.namespaceParam, testCase.nameParam, metav1.GetOptions{}) if !apiequality.Semantic.DeepEqual(testCase.expectedError, err) { @@ -121,7 +115,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { expectedError error expectedEndpoints *corev1.Endpoints expectedEndpointSlice *discovery.EndpointSlice - endpoints []*corev1.Endpoints + initialState []runtime.Object endpointSlices []*discovery.EndpointSlice namespaceParam string endpointsParam *corev1.Endpoints @@ -131,7 +125,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { expectedError: nil, expectedEndpoints: endpoints1, expectedEndpointSlice: epSlice1, - endpoints: []*corev1.Endpoints{}, + initialState: []runtime.Object{}, namespaceParam: endpoints1.Namespace, endpointsParam: endpoints1, }, @@ -140,7 +134,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { expectedError: nil, expectedEndpoints: endpoints2, expectedEndpointSlice: epSlice2, - endpoints: []*corev1.Endpoints{}, + initialState: []runtime.Object{}, namespaceParam: endpoints2.Namespace, endpointsParam: endpoints2, }, @@ -149,7 +143,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { expectedError: nil, expectedEndpoints: endpoints3, expectedEndpointSlice: epSlice3, - endpoints: []*corev1.Endpoints{}, + initialState: []runtime.Object{}, namespaceParam: endpoints3.Namespace, endpointsParam: endpoints3, }, @@ -158,7 +152,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { expectedError: nil, expectedEndpoints: endpoints1, expectedEndpointSlice: nil, - endpoints: []*corev1.Endpoints{}, + initialState: []runtime.Object{}, namespaceParam: endpoints1.Namespace, endpointsParam: endpoints1, }, @@ -167,7 +161,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { expectedError: errors.NewAlreadyExists(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), expectedEndpoints: nil, expectedEndpointSlice: nil, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: endpoints1.Namespace, endpointsParam: endpoints1, }, @@ -175,19 +169,12 @@ func TestEndpointsAdapterCreate(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { - client := fake.NewSimpleClientset() + client := fake.NewSimpleClientset(testCase.initialState...) epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} if testCase.endpointSlicesEnabled { epAdapter.endpointSliceClient = client.DiscoveryV1() } - for _, endpoints := range testCase.endpoints { - _, err := client.CoreV1().Endpoints(endpoints.Namespace).Create(context.TODO(), endpoints, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("Error creating Endpoints: %v", err) - } - } - endpoints, err := epAdapter.Create(testCase.namespaceParam, testCase.endpointsParam) if !apiequality.Semantic.DeepEqual(testCase.expectedError, err) { @@ -241,7 +228,7 @@ func TestEndpointsAdapterUpdate(t *testing.T) { expectedError error expectedEndpoints *corev1.Endpoints expectedEndpointSlice *discovery.EndpointSlice - endpoints []*corev1.Endpoints + initialState []runtime.Object endpointSlices []*discovery.EndpointSlice namespaceParam string endpointsParam *corev1.Endpoints @@ -251,7 +238,7 @@ func TestEndpointsAdapterUpdate(t *testing.T) { expectedError: nil, expectedEndpoints: endpoints1, expectedEndpointSlice: nil, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", endpointsParam: endpoints1, }, @@ -260,7 +247,7 @@ func TestEndpointsAdapterUpdate(t *testing.T) { expectedError: nil, expectedEndpoints: endpoints4, expectedEndpointSlice: epSlice4IPv4, - endpoints: []*corev1.Endpoints{endpoints4}, + initialState: []runtime.Object{endpoints4}, endpointSlices: []*discovery.EndpointSlice{epSlice4IP}, namespaceParam: "testing", endpointsParam: endpoints4, @@ -270,7 +257,7 @@ func TestEndpointsAdapterUpdate(t *testing.T) { expectedError: nil, expectedEndpoints: endpoints2, expectedEndpointSlice: epSlice2, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", endpointsParam: endpoints2, }, @@ -279,7 +266,7 @@ func TestEndpointsAdapterUpdate(t *testing.T) { expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), expectedEndpoints: nil, expectedEndpointSlice: nil, - endpoints: []*corev1.Endpoints{endpoints1}, + initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", endpointsParam: endpoints3, }, @@ -287,19 +274,12 @@ func TestEndpointsAdapterUpdate(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { - client := fake.NewSimpleClientset() + client := fake.NewSimpleClientset(testCase.initialState...) epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} if testCase.endpointSlicesEnabled { epAdapter.endpointSliceClient = client.DiscoveryV1() } - for _, endpoints := range testCase.endpoints { - _, err := client.CoreV1().Endpoints(endpoints.Namespace).Create(context.TODO(), endpoints, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("Error creating Endpoints: %v", err) - } - } - endpoints, err := epAdapter.Update(testCase.namespaceParam, testCase.endpointsParam) if !apiequality.Semantic.DeepEqual(testCase.expectedError, err) { @@ -389,7 +369,7 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointSlicesEnabled bool expectedError error expectedEndpointSlice *discovery.EndpointSlice - endpointSlices []*discovery.EndpointSlice + initialState []runtime.Object namespaceParam string endpointsParam *corev1.Endpoints }{ @@ -397,7 +377,7 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointSlicesEnabled: true, expectedError: nil, expectedEndpointSlice: epSlice1, - endpointSlices: []*discovery.EndpointSlice{epSlice1}, + initialState: []runtime.Object{epSlice1}, namespaceParam: "testing", endpointsParam: endpoints1, }, @@ -405,7 +385,7 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointSlicesEnabled: true, expectedError: nil, expectedEndpointSlice: epSlice2, - endpointSlices: []*discovery.EndpointSlice{epSlice1}, + initialState: []runtime.Object{epSlice1}, namespaceParam: "testing", endpointsParam: endpoints2, }, @@ -413,7 +393,7 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointSlicesEnabled: true, expectedError: nil, expectedEndpointSlice: epSlice1, - endpointSlices: []*discovery.EndpointSlice{}, + initialState: []runtime.Object{}, namespaceParam: "testing", endpointsParam: endpoints1, }, @@ -421,7 +401,7 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointSlicesEnabled: false, expectedError: nil, expectedEndpointSlice: nil, - endpointSlices: []*discovery.EndpointSlice{}, + initialState: []runtime.Object{}, namespaceParam: "testing", endpointsParam: endpoints1, }, @@ -429,19 +409,12 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { - client := fake.NewSimpleClientset() + client := fake.NewSimpleClientset(testCase.initialState...) epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} if testCase.endpointSlicesEnabled { epAdapter.endpointSliceClient = client.DiscoveryV1() } - for _, endpointSlice := range testCase.endpointSlices { - _, err := client.DiscoveryV1().EndpointSlices(endpointSlice.Namespace).Create(context.TODO(), endpointSlice, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("Error creating EndpointSlice: %v", err) - } - } - err := epAdapter.EnsureEndpointSliceFromEndpoints(testCase.namespaceParam, testCase.endpointsParam) if !apiequality.Semantic.DeepEqual(testCase.expectedError, err) { t.Errorf("Expected error: %v, got: %v", testCase.expectedError, err) diff --git a/pkg/controlplane/reconcilers/instancecount_test.go b/pkg/controlplane/reconcilers/instancecount_test.go index 8970031afad..32ba9da6a33 100644 --- a/pkg/controlplane/reconcilers/instancecount_test.go +++ b/pkg/controlplane/reconcilers/instancecount_test.go @@ -23,6 +23,7 @@ import ( corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" netutils "k8s.io/utils/net" @@ -45,7 +46,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip string endpointPorts []corev1.EndpointPort additionalMasters int - endpoints *corev1.EndpointsList + initialState []runtime.Object expectUpdate *corev1.Endpoints // nil means none expected expectCreate *corev1.Endpoints // nil means none expected }{ @@ -54,7 +55,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: nil, + initialState: nil, expectCreate: &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ @@ -68,14 +69,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, }, { @@ -83,14 +84,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}, {IP: "4.3.2.1"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -106,8 +107,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -119,7 +120,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -140,8 +141,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "4.3.2.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -153,7 +154,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -174,8 +175,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "4.3.2.2", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -184,7 +185,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: nil, }, @@ -194,8 +195,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "4.3.2.2", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -203,7 +204,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -221,14 +222,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("bar", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectCreate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -243,14 +244,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -265,14 +266,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 9090, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -287,14 +288,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "UDP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -309,14 +310,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -335,8 +336,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "bar", Port: 1000, Protocol: "TCP"}, {Name: "baz", Port: 1010, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, @@ -346,7 +347,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "baz", Port: 1010, Protocol: "TCP"}, }, }}, - }}, + }, }, }, { @@ -357,14 +358,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -382,7 +383,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "boo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "boo", Port: 7777, Protocol: "SCTP"}}, - endpoints: nil, + initialState: nil, expectCreate: &corev1.Endpoints{ ObjectMeta: om("boo", true), Subsets: []corev1.EndpointSubset{{ @@ -394,10 +395,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { } for _, test := range reconcileTests { t.Run(test.testName, func(t *testing.T) { - fakeClient := fake.NewSimpleClientset() - if test.endpoints != nil { - fakeClient = fake.NewSimpleClientset(test.endpoints) - } + fakeClient := fake.NewSimpleClientset(test.initialState...) epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, true) @@ -449,7 +447,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip string endpointPorts []corev1.EndpointPort additionalMasters int - endpoints *corev1.EndpointsList + initialState []runtime.Object expectUpdate *corev1.Endpoints // nil means none expected expectCreate *corev1.Endpoints // nil means none expected }{ @@ -461,14 +459,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: nil, }, @@ -480,14 +478,14 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -502,7 +500,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: nil, + initialState: nil, expectCreate: &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ @@ -514,10 +512,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { } for _, test := range nonReconcileTests { t.Run(test.testName, func(t *testing.T) { - fakeClient := fake.NewSimpleClientset() - if test.endpoints != nil { - fakeClient = fake.NewSimpleClientset(test.endpoints) - } + fakeClient := fake.NewSimpleClientset(test.initialState...) epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, false) @@ -576,10 +571,7 @@ func TestEmptySubsets(t *testing.T) { Subsets: nil, }}, } - fakeClient := fake.NewSimpleClientset() - if endpoints != nil { - fakeClient = fake.NewSimpleClientset(endpoints) - } + fakeClient := fake.NewSimpleClientset(endpoints) epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) reconciler := NewMasterCountEndpointReconciler(1, epAdapter) endpointPorts := []corev1.EndpointPort{ diff --git a/pkg/controlplane/reconcilers/lease_test.go b/pkg/controlplane/reconcilers/lease_test.go index fb58fa0faaa..749e5111c74 100644 --- a/pkg/controlplane/reconcilers/lease_test.go +++ b/pkg/controlplane/reconcilers/lease_test.go @@ -29,6 +29,7 @@ import ( corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" netutils "k8s.io/utils/net" ) @@ -97,7 +98,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip string endpointPorts []corev1.EndpointPort endpointKeys []string - endpoints *corev1.EndpointsList + initialState []runtime.Object expectUpdate *corev1.Endpoints // nil means none expected }{ { @@ -105,7 +106,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: nil, + initialState: nil, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ @@ -119,14 +120,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, }, { @@ -135,14 +136,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, endpointKeys: []string{"1.2.3.4"}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, }, { @@ -150,14 +151,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}, {IP: "4.3.2.1"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -173,8 +174,8 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", 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"}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -186,7 +187,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -207,8 +208,8 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "4.3.2.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, endpointKeys: []string{"4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -220,7 +221,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -241,8 +242,8 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "4.3.2.2", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, endpointKeys: []string{"4.3.2.1"}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -250,7 +251,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -268,14 +269,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("bar", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -290,14 +291,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -312,14 +313,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 9090, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -334,14 +335,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "UDP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -356,14 +357,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -378,14 +379,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", false), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -404,8 +405,8 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "bar", Port: 1000, Protocol: "TCP"}, {Name: "baz", Port: 1010, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, @@ -415,7 +416,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "baz", Port: 1010, Protocol: "TCP"}, }, }}, - }}, + }, }, }, { @@ -426,14 +427,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -451,15 +452,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { t.Run(test.testName, func(t *testing.T) { fakeLeases := newFakeLeases() fakeLeases.SetKeys(test.endpointKeys) - clientset := fake.NewSimpleClientset() - if test.endpoints != nil { - for _, ep := range test.endpoints.Items { - if _, err := clientset.CoreV1().Endpoints(ep.Namespace).Create(context.TODO(), &ep, metav1.CreateOptions{}); err != nil { - t.Errorf("unexpected error: %v", err) - continue - } - } - } + clientset := fake.NewSimpleClientset(test.initialState...) epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) @@ -488,7 +481,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip string endpointPorts []corev1.EndpointPort endpointKeys []string - endpoints *corev1.EndpointsList + initialState []runtime.Object expectUpdate *corev1.Endpoints // nil means none expected }{ { @@ -499,14 +492,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: nil, }, @@ -518,14 +511,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -540,7 +533,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - endpoints: nil, + initialState: nil, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ @@ -554,15 +547,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { t.Run(test.testName, func(t *testing.T) { fakeLeases := newFakeLeases() fakeLeases.SetKeys(test.endpointKeys) - clientset := fake.NewSimpleClientset() - if test.endpoints != nil { - for _, ep := range test.endpoints.Items { - if _, err := clientset.CoreV1().Endpoints(ep.Namespace).Create(context.TODO(), &ep, metav1.CreateOptions{}); err != nil { - t.Errorf("unexpected error: %v", err) - continue - } - } - } + clientset := fake.NewSimpleClientset(test.initialState...) epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, false) @@ -602,7 +587,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { ip string endpointPorts []corev1.EndpointPort endpointKeys []string - endpoints *corev1.EndpointsList + initialState []runtime.Object expectUpdate *corev1.Endpoints // nil means none expected }{ { @@ -611,8 +596,8 @@ func TestLeaseRemoveEndpoints(t *testing.T) { ip: "1.2.3.4", 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"}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -623,7 +608,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, expectUpdate: &corev1.Endpoints{ ObjectMeta: om("foo", true), @@ -643,8 +628,8 @@ func TestLeaseRemoveEndpoints(t *testing.T) { ip: "5.6.7.8", 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"}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{ @@ -655,7 +640,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { }, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, - }}, + }, }, }, { @@ -664,11 +649,11 @@ func TestLeaseRemoveEndpoints(t *testing.T) { ip: "5.6.7.8", 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"}, - endpoints: &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ + initialState: []runtime.Object{ + &corev1.Endpoints{ ObjectMeta: om("foo", true), Subsets: nil, - }}, + }, }, }, } @@ -676,13 +661,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { t.Run(test.testName, func(t *testing.T) { fakeLeases := newFakeLeases() fakeLeases.SetKeys(test.endpointKeys) - clientset := fake.NewSimpleClientset() - for _, ep := range test.endpoints.Items { - if _, err := clientset.CoreV1().Endpoints(ep.Namespace).Create(context.TODO(), &ep, metav1.CreateOptions{}); err != nil { - t.Errorf("unexpected error: %v", err) - continue - } - } + clientset := fake.NewSimpleClientset(test.initialState...) epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.RemoveEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts) From 4033de2034ed59dcb38fdb1ce3e91373653fc9fd Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 27 Apr 2022 08:27:41 -0400 Subject: [PATCH 3/7] Simplify endpoint creation in reconciler unit tests Also make the expectCreate / expectUpdate fields into arrays while we're rewriting their values anyway, to avoid additional churn in the next commit. --- pkg/controlplane/reconcilers/helpers_test.go | 52 +++ .../reconcilers/instancecount_test.go | 347 +++------------- pkg/controlplane/reconcilers/lease_test.go | 392 +++--------------- 3 files changed, 154 insertions(+), 637 deletions(-) create mode 100644 pkg/controlplane/reconcilers/helpers_test.go diff --git a/pkg/controlplane/reconcilers/helpers_test.go b/pkg/controlplane/reconcilers/helpers_test.go new file mode 100644 index 00000000000..551094fd2cd --- /dev/null +++ b/pkg/controlplane/reconcilers/helpers_test.go @@ -0,0 +1,52 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconcilers + +import ( + corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func makeEndpointsArray(name string, ips []string, ports []corev1.EndpointPort) []runtime.Object { + return []runtime.Object{ + makeEndpoints(name, ips, ports), + } +} + +func makeEndpoints(name string, ips []string, ports []corev1.EndpointPort) *corev1.Endpoints { + endpoints := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: name, + Labels: map[string]string{ + discoveryv1.LabelSkipMirror: "true", + }, + }, + } + if len(ips) > 0 || len(ports) > 0 { + endpoints.Subsets = []corev1.EndpointSubset{{ + Addresses: make([]corev1.EndpointAddress, len(ips)), + Ports: ports, + }} + for i := range ips { + endpoints.Subsets[0].Addresses[i].IP = ips[i] + } + } + return endpoints +} diff --git a/pkg/controlplane/reconcilers/instancecount_test.go b/pkg/controlplane/reconcilers/instancecount_test.go index 32ba9da6a33..27b901d788b 100644 --- a/pkg/controlplane/reconcilers/instancecount_test.go +++ b/pkg/controlplane/reconcilers/instancecount_test.go @@ -21,8 +21,6 @@ import ( "testing" corev1 "k8s.io/api/core/v1" - discoveryv1 "k8s.io/api/discovery/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" @@ -30,16 +28,6 @@ import ( ) func TestMasterCountEndpointReconciler(t *testing.T) { - ns := metav1.NamespaceDefault - om := func(name string, skipMirrorLabel bool) metav1.ObjectMeta { - o := metav1.ObjectMeta{Namespace: ns, Name: name} - if skipMirrorLabel { - o.Labels = map[string]string{ - discoveryv1.LabelSkipMirror: "true", - } - } - return o - } reconcileTests := []struct { testName string serviceName string @@ -47,8 +35,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { endpointPorts []corev1.EndpointPort additionalMasters int initialState []runtime.Object - expectUpdate *corev1.Endpoints // nil means none expected - expectCreate *corev1.Endpoints // nil means none expected + expectUpdate []runtime.Object + expectCreate []runtime.Object }{ { testName: "no existing endpoints", @@ -56,50 +44,22 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: nil, - expectCreate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []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", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy but too many", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}, {IP: "4.3.2.1"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4", "4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy but too many + extra masters", @@ -107,33 +67,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4", "4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + 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"}}), }, { testName: "existing endpoints satisfy but too many + extra masters + delete first", @@ -141,33 +76,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "4.3.2.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4", "4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy and endpoint addresses length less than master count", @@ -175,19 +85,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "4.3.2.2", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: nil, + initialState: makeEndpointsArray("foo", []string{"4.3.2.1", "4.3.2.2"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: nil, }, { testName: "existing endpoints current IP missing and address length less than master count", @@ -195,137 +94,48 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "4.3.2.2", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, additionalMasters: 3, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.1"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"4.3.2.1", "4.3.2.2"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong name", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("bar", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectCreate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("bar", []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", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong port", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 9090, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 9090, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong protocol", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "UDP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "UDP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong port name", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints extra service ports satisfy", @@ -336,19 +146,13 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "bar", Port: 1000, Protocol: "TCP"}, {Name: "baz", Port: 1010, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{ - {Name: "foo", Port: 8080, Protocol: "TCP"}, - {Name: "bar", Port: 1000, Protocol: "TCP"}, - {Name: "baz", Port: 1010, Protocol: "TCP"}, - }, - }}, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, + []corev1.EndpointPort{ + {Name: "foo", Port: 8080, Protocol: "TCP"}, + {Name: "bar", Port: 1000, Protocol: "TCP"}, + {Name: "baz", Port: 1010, Protocol: "TCP"}, }, - }, + ), }, { testName: "existing endpoints extra service ports missing port", @@ -358,25 +162,13 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, + initialState: makeEndpointsArray("foo", []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"}, + {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{ - {Name: "foo", Port: 8080, Protocol: "TCP"}, - {Name: "bar", Port: 1000, Protocol: "TCP"}, - }, - }}, - }, + ), }, { testName: "no existing sctp endpoints", @@ -384,13 +176,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "boo", Port: 7777, Protocol: "SCTP"}}, initialState: nil, - expectCreate: &corev1.Endpoints{ - ObjectMeta: om("boo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "boo", Port: 7777, Protocol: "SCTP"}}, - }}, - }, + expectCreate: makeEndpointsArray("boo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "boo", Port: 7777, Protocol: "SCTP"}}), }, } for _, test := range reconcileTests { @@ -413,7 +199,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { if test.expectUpdate != nil { if len(updates) != 1 { t.Errorf("unexpected updates: %v", updates) - } else if e, a := test.expectUpdate, updates[0].GetObject(); !reflect.DeepEqual(e, a) { + } 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) } } @@ -431,7 +217,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { if test.expectCreate != nil { if len(creates) != 1 { t.Errorf("unexpected creates: %v", creates) - } else if e, a := test.expectCreate, creates[0].GetObject(); !reflect.DeepEqual(e, a) { + } 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) } } @@ -448,8 +234,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { endpointPorts []corev1.EndpointPort additionalMasters int initialState []runtime.Object - expectUpdate *corev1.Endpoints // nil means none expected - expectCreate *corev1.Endpoints // nil means none expected + expectUpdate []runtime.Object + expectCreate []runtime.Object }{ { testName: "existing endpoints extra service ports missing port no update", @@ -459,15 +245,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), expectUpdate: nil, }, { @@ -478,22 +256,8 @@ func TestMasterCountEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "no existing endpoints", @@ -501,13 +265,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: nil, - expectCreate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []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 { @@ -530,7 +288,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { if test.expectUpdate != nil { if len(updates) != 1 { t.Errorf("unexpected updates: %v", updates) - } else if e, a := test.expectUpdate, updates[0].GetObject(); !reflect.DeepEqual(e, a) { + } 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) } } @@ -548,7 +306,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { if test.expectCreate != nil { if len(creates) != 1 { t.Errorf("unexpected creates: %v", creates) - } else if e, a := test.expectCreate, creates[0].GetObject(); !reflect.DeepEqual(e, a) { + } 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) } } @@ -561,16 +319,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { } func TestEmptySubsets(t *testing.T) { - ns := metav1.NamespaceDefault - om := func(name string) metav1.ObjectMeta { - return metav1.ObjectMeta{Namespace: ns, Name: name} - } - endpoints := &corev1.EndpointsList{ - Items: []corev1.Endpoints{{ - ObjectMeta: om("foo"), - Subsets: nil, - }}, - } + endpoints := makeEndpoints("foo", nil, nil) fakeClient := fake.NewSimpleClientset(endpoints) epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) reconciler := NewMasterCountEndpointReconciler(1, epAdapter) diff --git a/pkg/controlplane/reconcilers/lease_test.go b/pkg/controlplane/reconcilers/lease_test.go index 749e5111c74..aa1c911ab3f 100644 --- a/pkg/controlplane/reconcilers/lease_test.go +++ b/pkg/controlplane/reconcilers/lease_test.go @@ -27,7 +27,6 @@ import ( "testing" corev1 "k8s.io/api/core/v1" - discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" @@ -82,16 +81,6 @@ func (f *fakeLeases) Destroy() { } func TestLeaseEndpointReconciler(t *testing.T) { - ns := corev1.NamespaceDefault - om := func(name string, skipMirrorLabel bool) metav1.ObjectMeta { - o := metav1.ObjectMeta{Namespace: ns, Name: name} - if skipMirrorLabel { - o.Labels = map[string]string{ - discoveryv1.LabelSkipMirror: "true", - } - } - return o - } reconcileTests := []struct { testName string serviceName string @@ -99,7 +88,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { endpointPorts []corev1.EndpointPort endpointKeys []string initialState []runtime.Object - expectUpdate *corev1.Endpoints // nil means none expected + expectUpdate []runtime.Object }{ { testName: "no existing endpoints", @@ -107,28 +96,14 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: nil, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy + refresh existing key", @@ -136,37 +111,15 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, endpointKeys: []string{"1.2.3.4"}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy but too many", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}, {IP: "4.3.2.1"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4", "4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints satisfy but too many + extra masters", @@ -174,33 +127,8 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", 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: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4", "4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + 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"}}), }, { testName: "existing endpoints satisfy but too many + extra masters + delete first", @@ -208,33 +136,8 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "4.3.2.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, endpointKeys: []string{"4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4", "4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"4.3.2.1", "4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints current IP missing", @@ -242,137 +145,48 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "4.3.2.2", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, endpointKeys: []string{"4.3.2.1"}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.1"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.1"}, - {IP: "4.3.2.2"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"4.3.2.1", "4.3.2.2"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong name", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("bar", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []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"}}), }, { testName: "existing endpoints wrong IP", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong port", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 9090, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 9090, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong protocol", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "UDP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "UDP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints wrong port name", serviceName: "foo", ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "baz", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints without skip mirror label", @@ -380,21 +194,20 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: []runtime.Object{ + // can't use makeEndpointsArray() here because we don't want the + // skip-mirror label &corev1.Endpoints{ - ObjectMeta: om("foo", false), + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: "foo", + }, Subsets: []corev1.EndpointSubset{{ Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, }, }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints extra service ports satisfy", @@ -405,19 +218,13 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "bar", Port: 1000, Protocol: "TCP"}, {Name: "baz", Port: 1010, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{ - {Name: "foo", Port: 8080, Protocol: "TCP"}, - {Name: "bar", Port: 1000, Protocol: "TCP"}, - {Name: "baz", Port: 1010, Protocol: "TCP"}, - }, - }}, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, + []corev1.EndpointPort{ + {Name: "foo", Port: 8080, Protocol: "TCP"}, + {Name: "bar", Port: 1000, Protocol: "TCP"}, + {Name: "baz", Port: 1010, Protocol: "TCP"}, }, - }, + ), }, { testName: "existing endpoints extra service ports missing port", @@ -427,25 +234,13 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, + initialState: makeEndpointsArray("foo", []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"}, + {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{ - {Name: "foo", Port: 8080, Protocol: "TCP"}, - {Name: "bar", Port: 1000, Protocol: "TCP"}, - }, - }}, - }, + ), }, } for _, test := range reconcileTests { @@ -465,7 +260,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { t.Errorf("unexpected error: %v", err) } if test.expectUpdate != nil { - if e, a := test.expectUpdate, actualEndpoints; !reflect.DeepEqual(e, a) { + if e, a := test.expectUpdate[0], actualEndpoints; !reflect.DeepEqual(e, a) { t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) } } @@ -482,7 +277,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { endpointPorts []corev1.EndpointPort endpointKeys []string initialState []runtime.Object - expectUpdate *corev1.Endpoints // nil means none expected + expectUpdate []runtime.Object }{ { testName: "existing endpoints extra service ports missing port no update", @@ -492,15 +287,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, + initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), expectUpdate: nil, }, { @@ -511,22 +298,8 @@ func TestLeaseEndpointReconciler(t *testing.T) { {Name: "foo", Port: 8080, Protocol: "TCP"}, {Name: "bar", Port: 1000, Protocol: "TCP"}, }, - initialState: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "4.3.2.1"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: makeEndpointsArray("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "no existing endpoints", @@ -534,13 +307,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { ip: "1.2.3.4", endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: nil, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}}, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, } for _, test := range nonReconcileTests { @@ -559,7 +326,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { t.Errorf("unexpected error: %v", err) } if test.expectUpdate != nil { - if e, a := test.expectUpdate, actualEndpoints; !reflect.DeepEqual(e, a) { + if e, a := test.expectUpdate[0], actualEndpoints; !reflect.DeepEqual(e, a) { t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) } } @@ -571,16 +338,6 @@ func TestLeaseEndpointReconciler(t *testing.T) { } func TestLeaseRemoveEndpoints(t *testing.T) { - ns := corev1.NamespaceDefault - om := func(name string, skipMirrorLabel bool) metav1.ObjectMeta { - o := metav1.ObjectMeta{Namespace: ns, Name: name} - if skipMirrorLabel { - o.Labels = map[string]string{ - discoveryv1.LabelSkipMirror: "true", - } - } - return o - } stopTests := []struct { testName string serviceName string @@ -588,7 +345,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { endpointPorts []corev1.EndpointPort endpointKeys []string initialState []runtime.Object - expectUpdate *corev1.Endpoints // nil means none expected + expectUpdate []runtime.Object }{ { testName: "successful stop reconciling", @@ -596,31 +353,8 @@ func TestLeaseRemoveEndpoints(t *testing.T) { ip: "1.2.3.4", 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: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, - expectUpdate: &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, + initialState: 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"}}), + expectUpdate: makeEndpointsArray("foo", []string{"4.3.2.2", "4.3.2.3", "4.3.2.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "stop reconciling with ip not in endpoint ip list", @@ -628,20 +362,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { ip: "5.6.7.8", 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: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{ - {IP: "1.2.3.4"}, - {IP: "4.3.2.2"}, - {IP: "4.3.2.3"}, - {IP: "4.3.2.4"}, - }, - Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, - }}, - }, - }, + initialState: 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"}}), }, { testName: "endpoint with no subset", @@ -649,12 +370,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { ip: "5.6.7.8", 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: []runtime.Object{ - &corev1.Endpoints{ - ObjectMeta: om("foo", true), - Subsets: nil, - }, - }, + initialState: makeEndpointsArray("foo", nil, nil), }, } for _, test := range stopTests { @@ -673,7 +389,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { t.Errorf("unexpected error: %v", err) } if test.expectUpdate != nil { - if e, a := test.expectUpdate, actualEndpoints; !reflect.DeepEqual(e, a) { + if e, a := test.expectUpdate[0], actualEndpoints; !reflect.DeepEqual(e, a) { t.Errorf("expected update:\n%#v\ngot:\n%#v\n", e, a) } } From f543e7434a88a7390ed566fa7db724c80970136e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 27 Apr 2022 10:38:00 -0400 Subject: [PATCH 4/7] 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) From 91338c13dfe06b11950cf11ebf57054651c5d4a5 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 27 Apr 2022 14:04:23 -0400 Subject: [PATCH 5/7] Use EndpointSlices in all reconciler unit tests EndpointSlice is always enabled now, so make it non-optional in the EndpointsAdapter, make all of the test cases pass an EndpointSlice client, and remove the "EndpointSlices disabled"-specific tests. By changing makeEndpointsArray() to return both an Endpoints and an EndpointsSlice, the "initialObjects" and "expectCreate"/"expectUpdate" fields of (almost) all of the existing unit tests are automatically switched to be EndpointSlice-aware; instead of having an initial state with just Endpoints (or nothing), and testing that just the Endpoints is created/updated correctly, they now have an initial state with both Endpoints and EndpointSlice (or nothing), and test that both objects are created/updated correctly. The handful of existing test cases that used inconsistent Endpoints and EndpointSlice state have been modified to create the objects separately. --- .../reconcilers/endpointsadapter.go | 15 ++---- .../reconcilers/endpointsadapter_test.go | 53 +++---------------- pkg/controlplane/reconcilers/helpers_test.go | 27 ++++++++++ .../reconcilers/instancecount_test.go | 10 ++-- pkg/controlplane/reconcilers/lease_test.go | 12 +++-- 5 files changed, 53 insertions(+), 64 deletions(-) diff --git a/pkg/controlplane/reconcilers/endpointsadapter.go b/pkg/controlplane/reconcilers/endpointsadapter.go index 5781ec995c1..e1350fac213 100644 --- a/pkg/controlplane/reconcilers/endpointsadapter.go +++ b/pkg/controlplane/reconcilers/endpointsadapter.go @@ -57,8 +57,7 @@ func (adapter *EndpointsAdapter) Get(namespace, name string, getOpts metav1.GetO } // Create accepts a namespace and Endpoints object and creates the Endpoints -// object. If an endpointSliceClient exists, a matching EndpointSlice will also -// be created or updated. The created Endpoints object or an error will be +// object and matching EndpointSlice. The created Endpoints object or an error will be // returned. func (adapter *EndpointsAdapter) Create(namespace string, endpoints *corev1.Endpoints) (*corev1.Endpoints, error) { endpoints, err := adapter.endpointClient.Endpoints(namespace).Create(context.TODO(), endpoints, metav1.CreateOptions{}) @@ -68,9 +67,8 @@ func (adapter *EndpointsAdapter) Create(namespace string, endpoints *corev1.Endp return endpoints, err } -// Update accepts a namespace and Endpoints object and updates it. If an -// endpointSliceClient exists, a matching EndpointSlice will also be created or -// updated. The updated Endpoints object or an error will be returned. +// Update accepts a namespace and Endpoints object and updates it and its +// matching EndpointSlice. The updated Endpoints object or an error will be returned. func (adapter *EndpointsAdapter) Update(namespace string, endpoints *corev1.Endpoints) (*corev1.Endpoints, error) { endpoints, err := adapter.endpointClient.Endpoints(namespace).Update(context.TODO(), endpoints, metav1.UpdateOptions{}) if err == nil { @@ -80,12 +78,9 @@ func (adapter *EndpointsAdapter) Update(namespace string, endpoints *corev1.Endp } // EnsureEndpointSliceFromEndpoints accepts a namespace and Endpoints resource -// and creates or updates a corresponding EndpointSlice if an endpointSliceClient -// exists. An error will be returned if it fails to sync the EndpointSlice. +// and creates or updates a corresponding EndpointSlice. An error will be returned +// if it fails to sync the EndpointSlice. func (adapter *EndpointsAdapter) EnsureEndpointSliceFromEndpoints(namespace string, endpoints *corev1.Endpoints) error { - if adapter.endpointSliceClient == nil { - return nil - } endpointSlice := endpointSliceFromEndpoints(endpoints) currentEndpointSlice, err := adapter.endpointSliceClient.EndpointSlices(namespace).Get(context.TODO(), endpointSlice.Name, metav1.GetOptions{}) diff --git a/pkg/controlplane/reconcilers/endpointsadapter_test.go b/pkg/controlplane/reconcilers/endpointsadapter_test.go index 35364634c68..391818db64d 100644 --- a/pkg/controlplane/reconcilers/endpointsadapter_test.go +++ b/pkg/controlplane/reconcilers/endpointsadapter_test.go @@ -43,14 +43,6 @@ func TestEndpointsAdapterGet(t *testing.T) { nameParam string }{ "single-existing-endpoints": { - endpointSlicesEnabled: false, - expectedError: nil, - expectedEndpoints: endpoints1, - initialState: []runtime.Object{endpoints1}, - namespaceParam: "testing", - nameParam: "foo", - }, - "single-existing-endpoints-slices-enabled": { endpointSlicesEnabled: true, expectedError: nil, expectedEndpoints: endpoints1, @@ -59,7 +51,7 @@ func TestEndpointsAdapterGet(t *testing.T) { nameParam: "foo", }, "wrong-namespace": { - endpointSlicesEnabled: false, + endpointSlicesEnabled: true, expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), expectedEndpoints: nil, initialState: []runtime.Object{endpoints1}, @@ -67,7 +59,7 @@ func TestEndpointsAdapterGet(t *testing.T) { nameParam: "foo", }, "wrong-name": { - endpointSlicesEnabled: false, + endpointSlicesEnabled: true, expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), expectedEndpoints: nil, initialState: []runtime.Object{endpoints1}, @@ -79,10 +71,7 @@ func TestEndpointsAdapterGet(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { client := fake.NewSimpleClientset(testCase.initialState...) - epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} - if testCase.endpointSlicesEnabled { - epAdapter.endpointSliceClient = client.DiscoveryV1() - } + epAdapter := NewEndpointsAdapter(client.CoreV1(), client.DiscoveryV1()) endpoints, err := epAdapter.Get(testCase.namespaceParam, testCase.nameParam, metav1.GetOptions{}) @@ -147,15 +136,6 @@ func TestEndpointsAdapterCreate(t *testing.T) { namespaceParam: endpoints3.Namespace, endpointsParam: endpoints3, }, - "single-endpoint-no-slices": { - endpointSlicesEnabled: false, - expectedError: nil, - expectedResult: endpoints1, - expectCreate: []runtime.Object{endpoints1}, - initialState: []runtime.Object{}, - namespaceParam: endpoints1.Namespace, - endpointsParam: endpoints1, - }, "existing-endpoint": { endpointSlicesEnabled: true, expectedError: errors.NewAlreadyExists(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), @@ -172,10 +152,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { client := fake.NewSimpleClientset(testCase.initialState...) - epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} - if testCase.endpointSlicesEnabled { - epAdapter.endpointSliceClient = client.DiscoveryV1() - } + epAdapter := NewEndpointsAdapter(client.CoreV1(), client.DiscoveryV1()) endpoints, err := epAdapter.Create(testCase.namespaceParam, testCase.endpointsParam) @@ -219,10 +196,10 @@ func TestEndpointsAdapterUpdate(t *testing.T) { endpointsParam *corev1.Endpoints }{ "single-existing-endpoints-no-change": { - endpointSlicesEnabled: false, + endpointSlicesEnabled: true, expectedError: nil, expectedResult: endpoints1, - initialState: []runtime.Object{endpoints1}, + initialState: []runtime.Object{endpoints1, epSlice1}, namespaceParam: "testing", endpointsParam: endpoints1, @@ -268,10 +245,7 @@ func TestEndpointsAdapterUpdate(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { client := fake.NewSimpleClientset(testCase.initialState...) - epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} - if testCase.endpointSlicesEnabled { - epAdapter.endpointSliceClient = client.DiscoveryV1() - } + epAdapter := NewEndpointsAdapter(client.CoreV1(), client.DiscoveryV1()) endpoints, err := epAdapter.Update(testCase.namespaceParam, testCase.endpointsParam) @@ -373,23 +347,12 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { namespaceParam: "testing", endpointsParam: endpoints1, }, - "endpointslices-disabled": { - endpointSlicesEnabled: false, - expectedError: nil, - expectedEndpointSlice: nil, - initialState: []runtime.Object{}, - namespaceParam: "testing", - endpointsParam: endpoints1, - }, } for name, testCase := range testCases { t.Run(name, func(t *testing.T) { client := fake.NewSimpleClientset(testCase.initialState...) - epAdapter := EndpointsAdapter{endpointClient: client.CoreV1()} - if testCase.endpointSlicesEnabled { - epAdapter.endpointSliceClient = client.DiscoveryV1() - } + epAdapter := NewEndpointsAdapter(client.CoreV1(), client.DiscoveryV1()) err := epAdapter.EnsureEndpointSliceFromEndpoints(testCase.namespaceParam, testCase.endpointsParam) if !apiequality.Semantic.DeepEqual(testCase.expectedError, err) { diff --git a/pkg/controlplane/reconcilers/helpers_test.go b/pkg/controlplane/reconcilers/helpers_test.go index 502ac978ad5..3f1c458478b 100644 --- a/pkg/controlplane/reconcilers/helpers_test.go +++ b/pkg/controlplane/reconcilers/helpers_test.go @@ -32,6 +32,7 @@ import ( func makeEndpointsArray(name string, ips []string, ports []corev1.EndpointPort) []runtime.Object { return []runtime.Object{ makeEndpoints(name, ips, ports), + makeEndpointSlice(name, ips, ports), } } @@ -57,6 +58,32 @@ func makeEndpoints(name string, ips []string, ports []corev1.EndpointPort) *core return endpoints } +func makeEndpointSlice(name string, ips []string, ports []corev1.EndpointPort) *discoveryv1.EndpointSlice { + slice := &discoveryv1.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: name, + Labels: map[string]string{ + discoveryv1.LabelServiceName: name, + }, + }, + AddressType: discoveryv1.AddressTypeIPv4, + Endpoints: make([]discoveryv1.Endpoint, len(ips)), + Ports: make([]discoveryv1.EndpointPort, len(ports)), + } + ready := true + for i := range ips { + slice.Endpoints[i].Addresses = []string{ips[i]} + slice.Endpoints[i].Conditions.Ready = &ready + } + for i := range ports { + slice.Ports[i].Name = &ports[i].Name + slice.Ports[i].Protocol = &ports[i].Protocol + slice.Ports[i].Port = &ports[i].Port + } + return slice +} + func verifyCreatesAndUpdates(fakeClient *fake.Clientset, expectedCreates, expectedUpdates []runtime.Object) error { errors := []error{} diff --git a/pkg/controlplane/reconcilers/instancecount_test.go b/pkg/controlplane/reconcilers/instancecount_test.go index bec99ed0053..105f816f671 100644 --- a/pkg/controlplane/reconcilers/instancecount_test.go +++ b/pkg/controlplane/reconcilers/instancecount_test.go @@ -180,7 +180,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { for _, test := range reconcileTests { t.Run(test.testName, func(t *testing.T) { fakeClient := fake.NewSimpleClientset(test.initialState...) - epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) + epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), fakeClient.DiscoveryV1()) reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, true) if err != nil { @@ -238,7 +238,7 @@ func TestMasterCountEndpointReconciler(t *testing.T) { for _, test := range nonReconcileTests { t.Run(test.testName, func(t *testing.T) { fakeClient := fake.NewSimpleClientset(test.initialState...) - epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) + epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), fakeClient.DiscoveryV1()) reconciler := NewMasterCountEndpointReconciler(test.additionalMasters+1, epAdapter) err := reconciler.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, false) if err != nil { @@ -254,9 +254,9 @@ func TestMasterCountEndpointReconciler(t *testing.T) { } func TestEmptySubsets(t *testing.T) { - endpoints := makeEndpoints("foo", nil, nil) - fakeClient := fake.NewSimpleClientset(endpoints) - epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), nil) + endpoints := makeEndpointsArray("foo", nil, nil) + fakeClient := fake.NewSimpleClientset(endpoints...) + epAdapter := NewEndpointsAdapter(fakeClient.CoreV1(), fakeClient.DiscoveryV1()) reconciler := NewMasterCountEndpointReconciler(1, epAdapter) endpointPorts := []corev1.EndpointPort{ {Name: "foo", Port: 8080, Protocol: "TCP"}, diff --git a/pkg/controlplane/reconcilers/lease_test.go b/pkg/controlplane/reconcilers/lease_test.go index 37a851d6c48..c1c8aa7b8e6 100644 --- a/pkg/controlplane/reconcilers/lease_test.go +++ b/pkg/controlplane/reconcilers/lease_test.go @@ -205,8 +205,12 @@ func TestLeaseEndpointReconciler(t *testing.T) { Ports: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, }}, }, + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectUpdate: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + // EndpointSlice does not get updated because it was already correct }, - expectUpdate: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, { testName: "existing endpoints extra service ports satisfy", @@ -248,7 +252,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { fakeLeases.SetKeys(test.endpointKeys) clientset := fake.NewSimpleClientset(test.initialState...) - epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} + epAdapter := NewEndpointsAdapter(clientset.CoreV1(), clientset.DiscoveryV1()) r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, true) if err != nil { @@ -312,7 +316,7 @@ func TestLeaseEndpointReconciler(t *testing.T) { fakeLeases := newFakeLeases() fakeLeases.SetKeys(test.endpointKeys) clientset := fake.NewSimpleClientset(test.initialState...) - epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} + epAdapter := NewEndpointsAdapter(clientset.CoreV1(), clientset.DiscoveryV1()) r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.ReconcileEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts, false) if err != nil { @@ -373,7 +377,7 @@ func TestLeaseRemoveEndpoints(t *testing.T) { fakeLeases := newFakeLeases() fakeLeases.SetKeys(test.endpointKeys) clientset := fake.NewSimpleClientset(test.initialState...) - epAdapter := EndpointsAdapter{endpointClient: clientset.CoreV1()} + epAdapter := NewEndpointsAdapter(clientset.CoreV1(), clientset.DiscoveryV1()) r := NewLeaseEndpointReconciler(epAdapter, fakeLeases) err := r.RemoveEndpoints(test.serviceName, netutils.ParseIPSloppy(test.ip), test.endpointPorts) if err != nil { From 07de59ab60413de2e2a2455e36c6fae50a7a1971 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 27 Apr 2022 14:10:51 -0400 Subject: [PATCH 6/7] Remove unused endpointSlicesEnabled fields --- .../reconcilers/endpointsadapter_test.go | 174 ++++++++---------- 1 file changed, 78 insertions(+), 96 deletions(-) diff --git a/pkg/controlplane/reconcilers/endpointsadapter_test.go b/pkg/controlplane/reconcilers/endpointsadapter_test.go index 391818db64d..cef0ccc459b 100644 --- a/pkg/controlplane/reconcilers/endpointsadapter_test.go +++ b/pkg/controlplane/reconcilers/endpointsadapter_test.go @@ -35,36 +35,32 @@ func TestEndpointsAdapterGet(t *testing.T) { endpoints1, _ := generateEndpointsAndSlice("foo", "testing", []int{80, 443}, []string{"10.1.2.3", "10.1.2.4"}) testCases := map[string]struct { - endpointSlicesEnabled bool - expectedError error - expectedEndpoints *corev1.Endpoints - initialState []runtime.Object - namespaceParam string - nameParam string + expectedError error + expectedEndpoints *corev1.Endpoints + initialState []runtime.Object + namespaceParam string + nameParam string }{ "single-existing-endpoints": { - endpointSlicesEnabled: true, - expectedError: nil, - expectedEndpoints: endpoints1, - initialState: []runtime.Object{endpoints1}, - namespaceParam: "testing", - nameParam: "foo", + expectedError: nil, + expectedEndpoints: endpoints1, + initialState: []runtime.Object{endpoints1}, + namespaceParam: "testing", + nameParam: "foo", }, "wrong-namespace": { - endpointSlicesEnabled: true, - expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), - expectedEndpoints: nil, - initialState: []runtime.Object{endpoints1}, - namespaceParam: "foo", - nameParam: "foo", + expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), + expectedEndpoints: nil, + initialState: []runtime.Object{endpoints1}, + namespaceParam: "foo", + nameParam: "foo", }, "wrong-name": { - endpointSlicesEnabled: true, - expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), - expectedEndpoints: nil, - initialState: []runtime.Object{endpoints1}, - namespaceParam: "testing", - nameParam: "bar", + expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), + expectedEndpoints: nil, + initialState: []runtime.Object{endpoints1}, + namespaceParam: "testing", + nameParam: "bar", }, } @@ -100,49 +96,44 @@ func TestEndpointsAdapterCreate(t *testing.T) { epSlice3.AddressType = discovery.AddressTypeIPv6 testCases := map[string]struct { - endpointSlicesEnabled bool - expectedError error - expectedResult *corev1.Endpoints - expectCreate []runtime.Object - expectUpdate []runtime.Object - initialState []runtime.Object - namespaceParam string - endpointsParam *corev1.Endpoints + expectedError error + expectedResult *corev1.Endpoints + expectCreate []runtime.Object + expectUpdate []runtime.Object + initialState []runtime.Object + namespaceParam string + endpointsParam *corev1.Endpoints }{ "single-endpoint": { - endpointSlicesEnabled: true, - expectedError: nil, - expectedResult: endpoints1, - expectCreate: []runtime.Object{endpoints1, epSlice1}, - initialState: []runtime.Object{}, - namespaceParam: endpoints1.Namespace, - endpointsParam: endpoints1, + expectedError: nil, + expectedResult: endpoints1, + expectCreate: []runtime.Object{endpoints1, epSlice1}, + initialState: []runtime.Object{}, + namespaceParam: endpoints1.Namespace, + endpointsParam: endpoints1, }, "single-endpoint-partial-ipv6": { - endpointSlicesEnabled: true, - expectedError: nil, - expectedResult: endpoints2, - expectCreate: []runtime.Object{endpoints2, epSlice2}, - initialState: []runtime.Object{}, - namespaceParam: endpoints2.Namespace, - endpointsParam: endpoints2, + expectedError: nil, + expectedResult: endpoints2, + expectCreate: []runtime.Object{endpoints2, epSlice2}, + initialState: []runtime.Object{}, + namespaceParam: endpoints2.Namespace, + endpointsParam: endpoints2, }, "single-endpoint-full-ipv6": { - endpointSlicesEnabled: true, - expectedError: nil, - expectedResult: endpoints3, - expectCreate: []runtime.Object{endpoints3, epSlice3}, - initialState: []runtime.Object{}, - namespaceParam: endpoints3.Namespace, - endpointsParam: endpoints3, + expectedError: nil, + expectedResult: endpoints3, + expectCreate: []runtime.Object{endpoints3, epSlice3}, + initialState: []runtime.Object{}, + namespaceParam: endpoints3.Namespace, + endpointsParam: endpoints3, }, "existing-endpoint": { - endpointSlicesEnabled: true, - expectedError: errors.NewAlreadyExists(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), - expectedResult: nil, - initialState: []runtime.Object{endpoints1, epSlice1}, - namespaceParam: endpoints1.Namespace, - endpointsParam: endpoints1, + expectedError: errors.NewAlreadyExists(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), + 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}, @@ -186,34 +177,31 @@ func TestEndpointsAdapterUpdate(t *testing.T) { _, epSlice4IPv4 := generateEndpointsAndSlice("foo", "testing", []int{80}, []string{"10.1.2.7", "10.1.2.8"}) testCases := map[string]struct { - endpointSlicesEnabled bool - expectedError error - expectedResult *corev1.Endpoints - expectCreate []runtime.Object - expectUpdate []runtime.Object - initialState []runtime.Object - namespaceParam string - endpointsParam *corev1.Endpoints + expectedError error + expectedResult *corev1.Endpoints + expectCreate []runtime.Object + expectUpdate []runtime.Object + initialState []runtime.Object + namespaceParam string + endpointsParam *corev1.Endpoints }{ "single-existing-endpoints-no-change": { - endpointSlicesEnabled: true, - expectedError: nil, - expectedResult: endpoints1, - initialState: []runtime.Object{endpoints1, epSlice1}, - namespaceParam: "testing", - endpointsParam: endpoints1, + expectedError: nil, + expectedResult: endpoints1, + initialState: []runtime.Object{endpoints1, epSlice1}, + 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, - expectedResult: endpoints4, - initialState: []runtime.Object{endpoints4, epSlice4IP}, - namespaceParam: "testing", - endpointsParam: endpoints4, + expectedError: nil, + 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. @@ -221,21 +209,19 @@ func TestEndpointsAdapterUpdate(t *testing.T) { expectCreate: []runtime.Object{epSlice4IPv4}, }, "add-ports-and-ips": { - endpointSlicesEnabled: true, - expectedError: nil, - expectedResult: endpoints2, - expectUpdate: []runtime.Object{endpoints2, epSlice2}, - initialState: []runtime.Object{endpoints1, epSlice1}, - namespaceParam: "testing", - endpointsParam: endpoints2, + expectedError: nil, + 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"), - expectedResult: nil, - initialState: []runtime.Object{endpoints1, epSlice1}, - namespaceParam: "testing", - endpointsParam: endpoints3, + expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), + 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}, @@ -316,7 +302,6 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpoints2, epSlice2 := generateEndpointsAndSlice("foo", "testing", []int{80, 443}, []string{"10.1.2.3", "10.1.2.4", "10.1.2.5"}) testCases := map[string]struct { - endpointSlicesEnabled bool expectedError error expectedEndpointSlice *discovery.EndpointSlice initialState []runtime.Object @@ -324,7 +309,6 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointsParam *corev1.Endpoints }{ "existing-endpointslice-no-change": { - endpointSlicesEnabled: true, expectedError: nil, expectedEndpointSlice: epSlice1, initialState: []runtime.Object{epSlice1}, @@ -332,7 +316,6 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointsParam: endpoints1, }, "existing-endpointslice-change": { - endpointSlicesEnabled: true, expectedError: nil, expectedEndpointSlice: epSlice2, initialState: []runtime.Object{epSlice1}, @@ -340,7 +323,6 @@ func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { endpointsParam: endpoints2, }, "missing-endpointslice": { - endpointSlicesEnabled: true, expectedError: nil, expectedEndpointSlice: epSlice1, initialState: []runtime.Object{}, From 80e9d948e30dc8e85cc20d27a447ec630739c787 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 28 Jan 2022 16:44:05 -0500 Subject: [PATCH 7/7] Add more Endpoints vs EndpointsSlices reconciler tests --- .../reconcilers/endpointsadapter_test.go | 98 +++++++++++++++++-- .../reconcilers/instancecount_test.go | 50 ++++++++++ pkg/controlplane/reconcilers/lease_test.go | 50 ++++++++++ 3 files changed, 188 insertions(+), 10 deletions(-) diff --git a/pkg/controlplane/reconcilers/endpointsadapter_test.go b/pkg/controlplane/reconcilers/endpointsadapter_test.go index cef0ccc459b..a8d5489fb83 100644 --- a/pkg/controlplane/reconcilers/endpointsadapter_test.go +++ b/pkg/controlplane/reconcilers/endpointsadapter_test.go @@ -32,7 +32,7 @@ import ( ) func TestEndpointsAdapterGet(t *testing.T) { - endpoints1, _ := generateEndpointsAndSlice("foo", "testing", []int{80, 443}, []string{"10.1.2.3", "10.1.2.4"}) + endpoints1, epSlice1 := generateEndpointsAndSlice("foo", "testing", []int{80, 443}, []string{"10.1.2.3", "10.1.2.4"}) testCases := map[string]struct { expectedError error @@ -42,23 +42,37 @@ func TestEndpointsAdapterGet(t *testing.T) { nameParam string }{ "single-existing-endpoints": { + expectedError: nil, + expectedEndpoints: endpoints1, + initialState: []runtime.Object{endpoints1, epSlice1}, + namespaceParam: "testing", + nameParam: "foo", + }, + "endpoints exists, endpointslice does not": { expectedError: nil, expectedEndpoints: endpoints1, initialState: []runtime.Object{endpoints1}, namespaceParam: "testing", nameParam: "foo", }, + "endpointslice exists, endpoints does not": { + expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), + expectedEndpoints: nil, + initialState: []runtime.Object{epSlice1}, + namespaceParam: "testing", + nameParam: "foo", + }, "wrong-namespace": { expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), expectedEndpoints: nil, - initialState: []runtime.Object{endpoints1}, + initialState: []runtime.Object{endpoints1, epSlice1}, namespaceParam: "foo", nameParam: "foo", }, "wrong-name": { expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), expectedEndpoints: nil, - initialState: []runtime.Object{endpoints1}, + initialState: []runtime.Object{endpoints1, epSlice1}, namespaceParam: "testing", nameParam: "bar", }, @@ -128,7 +142,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { namespaceParam: endpoints3.Namespace, endpointsParam: endpoints3, }, - "existing-endpoint": { + "existing-endpoints": { expectedError: errors.NewAlreadyExists(schema.GroupResource{Group: "", Resource: "endpoints"}, "foo"), expectedResult: nil, initialState: []runtime.Object{endpoints1, epSlice1}, @@ -138,6 +152,27 @@ func TestEndpointsAdapterCreate(t *testing.T) { // We expect the create to be attempted, we just also expect it to fail expectCreate: []runtime.Object{endpoints1}, }, + "existing-endpointslice-incorrect": { + // No error when we need to create the Endpoints but the correct + // EndpointSlice already exists + expectedError: nil, + expectedResult: endpoints1, + expectCreate: []runtime.Object{endpoints1}, + initialState: []runtime.Object{epSlice1}, + namespaceParam: endpoints1.Namespace, + endpointsParam: endpoints1, + }, + "existing-endpointslice-correct": { + // No error when we need to create the Endpoints but an incorrect + // EndpointSlice already exists + expectedError: nil, + expectedResult: endpoints2, + expectCreate: []runtime.Object{endpoints2}, + expectUpdate: []runtime.Object{epSlice2}, + initialState: []runtime.Object{epSlice1}, + namespaceParam: endpoints2.Namespace, + endpointsParam: endpoints2, + }, } for name, testCase := range testCases { @@ -155,7 +190,7 @@ func TestEndpointsAdapterCreate(t *testing.T) { t.Errorf("Expected endpoints: %v, got: %v", testCase.expectedResult, endpoints) } - err = verifyCreatesAndUpdates(client, testCase.expectCreate, nil) + err = verifyCreatesAndUpdates(client, testCase.expectCreate, testCase.expectUpdate) if err != nil { t.Errorf("unexpected error in side effects: %v", err) } @@ -216,6 +251,30 @@ func TestEndpointsAdapterUpdate(t *testing.T) { namespaceParam: "testing", endpointsParam: endpoints2, }, + "endpoints-correct-endpointslice-wrong": { + expectedError: nil, + expectedResult: endpoints2, + expectUpdate: []runtime.Object{endpoints2, epSlice2}, + initialState: []runtime.Object{endpoints2, epSlice1}, + namespaceParam: "testing", + endpointsParam: endpoints2, + }, + "endpointslice-correct-endpoints-wrong": { + expectedError: nil, + expectedResult: endpoints2, + expectUpdate: []runtime.Object{endpoints2}, + initialState: []runtime.Object{endpoints1, epSlice2}, + namespaceParam: "testing", + endpointsParam: endpoints2, + }, + "wrong-endpoints": { + expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), + expectedResult: nil, + expectUpdate: []runtime.Object{endpoints3}, + initialState: []runtime.Object{endpoints1, epSlice1}, + namespaceParam: "testing", + endpointsParam: endpoints3, + }, "missing-endpoints": { expectedError: errors.NewNotFound(schema.GroupResource{Group: "", Resource: "endpoints"}, "bar"), expectedResult: nil, @@ -226,6 +285,17 @@ func TestEndpointsAdapterUpdate(t *testing.T) { // We expect the update to be attempted, we just also expect it to fail expectUpdate: []runtime.Object{endpoints3}, }, + "missing-endpointslice": { + // No error when we need to update the Endpoints but the + // EndpointSlice doesn't exist + expectedError: nil, + expectedResult: endpoints1, + expectUpdate: []runtime.Object{endpoints1}, + expectCreate: []runtime.Object{epSlice1}, + initialState: []runtime.Object{endpoints2}, + namespaceParam: "testing", + endpointsParam: endpoints1, + }, } for name, testCase := range testCases { @@ -252,10 +322,12 @@ func TestEndpointsAdapterUpdate(t *testing.T) { } func generateEndpointsAndSlice(name, namespace string, ports []int, addresses []string) (*corev1.Endpoints, *discovery.EndpointSlice) { - objectMeta := metav1.ObjectMeta{Name: name, Namespace: namespace} trueBool := true - epSlice := &discovery.EndpointSlice{ObjectMeta: objectMeta, AddressType: discovery.AddressTypeIPv4} + epSlice := &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + AddressType: discovery.AddressTypeIPv4, + } epSlice.Labels = map[string]string{discovery.LabelServiceName: name} subset := corev1.EndpointSubset{} @@ -292,12 +364,18 @@ func generateEndpointsAndSlice(name, namespace string, ports []int, addresses [] } return &corev1.Endpoints{ - ObjectMeta: objectMeta, - Subsets: []corev1.EndpointSubset{subset}, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + discovery.LabelSkipMirror: "true", + }, + }, + Subsets: []corev1.EndpointSubset{subset}, }, epSlice } -func TestEndpointsAdapterEnsureEndpointSliceFromEndpoints(t *testing.T) { +func TestEndpointManagerEnsureEndpointSliceFromEndpoints(t *testing.T) { endpoints1, epSlice1 := generateEndpointsAndSlice("foo", "testing", []int{80, 443}, []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"}) diff --git a/pkg/controlplane/reconcilers/instancecount_test.go b/pkg/controlplane/reconcilers/instancecount_test.go index 105f816f671..91449e7a4c7 100644 --- a/pkg/controlplane/reconcilers/instancecount_test.go +++ b/pkg/controlplane/reconcilers/instancecount_test.go @@ -51,6 +51,56 @@ func TestMasterCountEndpointReconciler(t *testing.T) { endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, + { + testName: "existing endpoints satisfy, no endpointslice", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectCreate: []runtime.Object{ + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, + { + testName: "existing endpointslice satisfies, no endpoints", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectCreate: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, + { + testName: "existing endpoints satisfy, endpointslice is wrong", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + makeEndpointSlice("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectUpdate: []runtime.Object{ + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, + { + testName: "existing endpointslice satisfies, endpoints is wrong", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpoints("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectUpdate: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, { testName: "existing endpoints satisfy but too many", serviceName: "foo", diff --git a/pkg/controlplane/reconcilers/lease_test.go b/pkg/controlplane/reconcilers/lease_test.go index c1c8aa7b8e6..c80078426bc 100644 --- a/pkg/controlplane/reconcilers/lease_test.go +++ b/pkg/controlplane/reconcilers/lease_test.go @@ -104,6 +104,56 @@ func TestLeaseEndpointReconciler(t *testing.T) { endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, initialState: makeEndpointsArray("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), }, + { + testName: "existing endpoints satisfy, no endpointslice", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectCreate: []runtime.Object{ + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, + { + testName: "existing endpointslice satisfies, no endpoints", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectCreate: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, + { + testName: "existing endpoints satisfy, endpointslice is wrong", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + makeEndpointSlice("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectUpdate: []runtime.Object{ + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, + { + testName: "existing endpointslice satisfies, endpoints is wrong", + serviceName: "foo", + ip: "1.2.3.4", + endpointPorts: []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}, + initialState: []runtime.Object{ + makeEndpoints("foo", []string{"4.3.2.1"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + makeEndpointSlice("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + expectUpdate: []runtime.Object{ + makeEndpoints("foo", []string{"1.2.3.4"}, []corev1.EndpointPort{{Name: "foo", Port: 8080, Protocol: "TCP"}}), + }, + }, { testName: "existing endpoints satisfy + refresh existing key", serviceName: "foo",