Skip to content

Commit

Permalink
fix: fix paging in GetAdminAPIsForService
Browse files Browse the repository at this point in the history
  • Loading branch information
pmalek committed Apr 4, 2023
1 parent 36f95b4 commit a2c08a4
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 2 deletions.
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ Adding a new version? You'll need three changes:

## Unreleased

### Fixed

- Fix paging in `GetAdminAPIsForService` which might have caused the controller
to only return the head of the list of Endpoints for Admin API service.
[#3846](https://github.com/Kong/kubernetes-ingress-controller/pull/3846)

### Deprecated

- Removed support for extensions/v1beta1 Ingress which was removed in kubernetes 1.22.
Expand All @@ -88,8 +94,8 @@ Adding a new version? You'll need three changes:

> Release date: 2023-03-29
This release was intended to include a fix for a deadlock in `AdminAPIClientsManager`
([#3816](https://github.com/Kong/kubernetes-ingress-controller/pull/3816)), but it wasn't
This release was intended to include a fix for a deadlock in `AdminAPIClientsManager`
([#3816](https://github.com/Kong/kubernetes-ingress-controller/pull/3816)), but it wasn't
backported properly. It is included in the next patch release.

## [2.9.0]
Expand Down
1 change: 1 addition & 0 deletions internal/adminapi/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func GetAdminAPIsForService(
if endpointsList.Continue == "" {
break
}
continueToken = endpointsList.Continue
}
return addresses, nil
}
Expand Down
115 changes: 115 additions & 0 deletions internal/adminapi/endpoints_envtest_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
//go:build envtest
// +build envtest

package adminapi_test

import (
"context"
"fmt"
"testing"

"github.com/google/uuid"
"github.com/samber/lo"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
discoveryv1 "k8s.io/api/discovery/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes/scheme"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v2/test/envtest"
)

func TestGetAdminAPIsForServiceReturnsAllAddressesCorrectlyPagingThroughResults(t *testing.T) {
t.Parallel()

var client ctrlclient.Client
{
cfg := envtest.Setup(t, scheme.Scheme)
var err error
client, err = ctrlclient.New(cfg, ctrlclient.Options{
Scheme: scheme.Scheme,
})
require.NoError(t, err)
}

// In tests below we use a deferred cancel to stop the manager and not wait
// for its timeout.

testcases := []struct {
subnetC int
subnetD int
}{
{subnetC: 1, subnetD: 100},
{subnetC: 1, subnetD: 101},
{subnetC: 1, subnetD: 250},
{subnetC: 2, subnetD: 250},
{subnetC: 5, subnetD: 250},
}

for _, tc := range testcases {
tc := tc
t.Run(fmt.Sprintf("%dx%d", tc.subnetC, tc.subnetD), func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var (
ns = envtest.CreateNamespace(ctx, t, client)
serviceName = uuid.NewString()
service = types.NamespacedName{
Namespace: ns.Name,
Name: serviceName,
}
)

for i := 0; i < tc.subnetC; i++ {
for j := 0; j < tc.subnetD; j++ {
es := discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Name: uuid.NewString(),
Namespace: ns.Name,
Labels: map[string]string{
"kubernetes.io/service-name": serviceName,
},
},
AddressType: discoveryv1.AddressTypeIPv4,
Endpoints: []discoveryv1.Endpoint{
{
Addresses: []string{fmt.Sprintf("10.0.%d.%d", i, j)},
Conditions: discoveryv1.EndpointConditions{
Ready: lo.ToPtr(true),
Terminating: lo.ToPtr(false),
},
TargetRef: testPodReference("pod-1", ns.Name),
},
},
Ports: []discoveryv1.EndpointPort{
{
Name: lo.ToPtr("admin"),
Port: lo.ToPtr(int32(8444)),
},
},
}
require.NoError(t, client.Create(ctx, &es))
}
}

got, err := adminapi.GetAdminAPIsForService(context.Background(), client, service, sets.New("admin"))
require.NoError(t, err)
require.Len(t, got, tc.subnetD*tc.subnetC, "GetAdminAPIsForService should return all valid addresses")
})
}
}

func testPodReference(name, ns string) *corev1.ObjectReference {
return &corev1.ObjectReference{
Kind: "Pod",
Namespace: ns,
Name: name,
}
}

0 comments on commit a2c08a4

Please sign in to comment.