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 {