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.
This commit is contained in:
Dan Winship 2022-04-27 14:04:23 -04:00
parent f543e7434a
commit 91338c13df
5 changed files with 53 additions and 64 deletions

View File

@ -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{})

View File

@ -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) {

View File

@ -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{}

View File

@ -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"},

View File

@ -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 {