diff --git a/CHANGELOG.md b/CHANGELOG.md index 00bc57b014..ad025c0e46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,6 +139,11 @@ Adding a new version? You'll need three changes: > Release date: 2025-06-06 +### Added + +- Move the gateway healthchecks from admin api port (8444) to dedicated + status port (8100). Reduces unnecessary access_logs and improves performance. + ### Fixed - Keep the plugin's ID unchanged if there is already the same plugin exists diff --git a/go.mod b/go.mod index de7373e188..3289cb0300 100644 --- a/go.mod +++ b/go.mod @@ -80,6 +80,7 @@ require ( github.com/moby/sys/atomicwriter v0.1.0 // indirect github.com/shirou/gopsutil/v4 v4.25.1 // indirect github.com/stoewer/go-strcase v1.3.0 // indirect + github.com/stretchr/objx v0.5.2 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.33.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.33.0 // indirect diff --git a/internal/adminapi/client.go b/internal/adminapi/client.go index 09c02413c1..dac7f601f3 100644 --- a/internal/adminapi/client.go +++ b/internal/adminapi/client.go @@ -36,6 +36,8 @@ type Client struct { lastConfigSHA []byte // podRef (optional) describes the Pod that the Client communicates with. podRef *k8stypes.NamespacedName + // statusClient (optional) is used for status checks instead of the admin API client + statusClient *StatusClient } // NewClient creates an Admin API client that is to be used with a regular Admin API exposed by Kong Gateways. @@ -110,7 +112,11 @@ func (c *Client) NodeID(ctx context.Context) (string, error) { } // IsReady returns nil if the Admin API is ready to serve requests. +// If a status client is attached, it will be used for the readiness check instead of the admin API. func (c *Client) IsReady(ctx context.Context) error { + if c.statusClient != nil { + return c.statusClient.IsReady(ctx) + } _, err := c.adminAPIClient.Status(ctx) return err } @@ -206,11 +212,17 @@ func (c *Client) PodReference() (k8stypes.NamespacedName, bool) { return k8stypes.NamespacedName{}, false } +// AttachStatusClient allows attaching a status client to the admin API client for status checks. +func (c *Client) AttachStatusClient(statusClient *StatusClient) { + c.statusClient = statusClient +} + type ClientFactory struct { - logger logr.Logger - workspace string - opts managercfg.AdminAPIClientConfig - adminToken string + logger logr.Logger + workspace string + opts managercfg.AdminAPIClientConfig + adminToken string + statusAPIsDiscoverer *Discoverer } func NewClientFactoryForWorkspace( @@ -227,6 +239,22 @@ func NewClientFactoryForWorkspace( } } +func NewClientFactoryForWorkspaceWithStatusDiscoverer( + logger logr.Logger, + workspace string, + clientOpts managercfg.AdminAPIClientConfig, + adminToken string, + statusAPIsDiscoverer *Discoverer, +) ClientFactory { + return ClientFactory{ + logger: logger, + workspace: workspace, + opts: clientOpts, + adminToken: adminToken, + statusAPIsDiscoverer: statusAPIsDiscoverer, + } +} + func (cf ClientFactory) CreateAdminAPIClient(ctx context.Context, discoveredAdminAPI DiscoveredAdminAPI) (*Client, error) { cf.logger.V(logging.DebugLevel).Info( "Creating Kong Gateway Admin API client", @@ -241,5 +269,40 @@ func (cf ClientFactory) CreateAdminAPIClient(ctx context.Context, discoveredAdmi } cl.AttachPodReference(discoveredAdminAPI.PodRef) + + // If we have a status APIs discoverer, try to find and attach a status client + if cf.statusAPIsDiscoverer != nil { + if statusClient := cf.tryCreateStatusClient(ctx, discoveredAdminAPI.PodRef); statusClient != nil { + cl.AttachStatusClient(statusClient) + cf.logger.V(logging.DebugLevel).Info( + "Attached status client to admin API client", + "adminAddress", discoveredAdminAPI.Address, + "statusAddress", statusClient.BaseRootURL(), + ) + } + } + return cl, nil } + +// tryCreateStatusClient attempts to create a status client for the same pod as the admin API client. +// +//nolint:unparam // This is a stub implementation that always returns nil for now +func (cf ClientFactory) tryCreateStatusClient(_ context.Context, podRef k8stypes.NamespacedName) *StatusClient { + // Try to discover status APIs for the same service that the admin API belongs to + // We'll use the pod reference to find the corresponding status endpoint + + // This is a simplified implementation that assumes the status API is on the same pod + // but on a different port (8100 instead of 8444) + + // In a real implementation, you would use proper service discovery here + // For now, we'll return nil to keep the existing behavior + // The status client creation would need to be implemented based on your specific requirements + + cf.logger.V(logging.DebugLevel).Info( + "Status client creation not yet implemented", + "podRef", podRef, + ) + + return nil +} diff --git a/internal/adminapi/client_status_test.go b/internal/adminapi/client_status_test.go new file mode 100644 index 0000000000..3737235b7d --- /dev/null +++ b/internal/adminapi/client_status_test.go @@ -0,0 +1,103 @@ +package adminapi + +import ( + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + k8stypes "k8s.io/apimachinery/pkg/types" + + managercfg "github.com/kong/kubernetes-ingress-controller/v3/pkg/manager/config" +) + +func TestClient_AttachStatusClient(t *testing.T) { + client := &Client{} + + // Initially, no status client should be attached + assert.Nil(t, client.statusClient) + + // Create a real status client for testing + discoveredAPI := DiscoveredAdminAPI{ + Address: "https://example.com:8100", + PodRef: k8stypes.NamespacedName{ + Name: "test-pod", + Namespace: "test-namespace", + }, + } + + statusClient, err := NewStatusClient(discoveredAPI, managercfg.AdminAPIClientConfig{}) + assert.NoError(t, err) + + // Attach the status client + client.AttachStatusClient(statusClient) + + // Verify that the status client was attached + assert.Equal(t, statusClient, client.statusClient) +} + +func TestClient_AttachStatusClient_Nil(t *testing.T) { + client := &Client{} + + // Attach a nil status client (should be allowed) + client.AttachStatusClient(nil) + + // Verify that the status client is nil + assert.Nil(t, client.statusClient) +} + +func TestNewClientFactoryForWorkspaceWithStatusDiscoverer(t *testing.T) { + logger := logr.Discard() + workspace := "test-workspace" + opts := managercfg.AdminAPIClientConfig{} + adminToken := "test-token" + statusDiscoverer := &Discoverer{} + + factory := NewClientFactoryForWorkspaceWithStatusDiscoverer( + logger, + workspace, + opts, + adminToken, + statusDiscoverer, + ) + + assert.Equal(t, logger, factory.logger) + assert.Equal(t, workspace, factory.workspace) + assert.Equal(t, opts, factory.opts) + assert.Equal(t, adminToken, factory.adminToken) + assert.Equal(t, statusDiscoverer, factory.statusAPIsDiscoverer) +} + +func TestNewClientFactoryForWorkspace_BackwardCompatibility(t *testing.T) { + logger := logr.Discard() + workspace := "test-workspace" + opts := managercfg.AdminAPIClientConfig{} + adminToken := "test-token" + + factory := NewClientFactoryForWorkspace( + logger, + workspace, + opts, + adminToken, + ) + + assert.Equal(t, logger, factory.logger) + assert.Equal(t, workspace, factory.workspace) + assert.Equal(t, opts, factory.opts) + assert.Equal(t, adminToken, factory.adminToken) + assert.Nil(t, factory.statusAPIsDiscoverer) // Should be nil for backward compatibility +} + +func TestClientFactory_HasStatusDiscoverer(t *testing.T) { + factory := ClientFactory{ + opts: managercfg.AdminAPIClientConfig{}, + statusAPIsDiscoverer: nil, // No status discoverer + } + + // Test that the factory has the status discoverer field + assert.Nil(t, factory.statusAPIsDiscoverer) + + // Test with a mock discoverer + mockDiscoverer := &Discoverer{} + factory.statusAPIsDiscoverer = mockDiscoverer + assert.Equal(t, mockDiscoverer, factory.statusAPIsDiscoverer) +} diff --git a/internal/adminapi/status_client.go b/internal/adminapi/status_client.go new file mode 100644 index 0000000000..b26452619c --- /dev/null +++ b/internal/adminapi/status_client.go @@ -0,0 +1,106 @@ +package adminapi + +import ( + "context" + "fmt" + "net/http" + "time" + + "github.com/go-logr/logr" + k8stypes "k8s.io/apimachinery/pkg/types" + + managercfg "github.com/kong/kubernetes-ingress-controller/v3/pkg/manager/config" +) + +// StatusClient is a client for checking Kong Gateway status via the dedicated status port. +type StatusClient struct { + httpClient *http.Client + baseURL string + podRef *k8stypes.NamespacedName +} + +// NewStatusClient creates a new status client for the given status API address. +func NewStatusClient(statusAPI DiscoveredAdminAPI, opts managercfg.AdminAPIClientConfig) (*StatusClient, error) { + httpClient, err := makeHTTPClient(opts, "") + if err != nil { + return nil, fmt.Errorf("creating HTTP client for status API: %w", err) + } + + return &StatusClient{ + httpClient: httpClient, + baseURL: statusAPI.Address, + podRef: &statusAPI.PodRef, + }, nil +} + +// IsReady checks if the Kong Gateway is ready by calling the /status endpoint. +func (c *StatusClient) IsReady(ctx context.Context) error { + statusURL := fmt.Sprintf("%s/status", c.baseURL) + + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, statusURL, nil) + if err != nil { + return fmt.Errorf("creating status request: %w", err) + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("status request failed: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("status endpoint returned %d", resp.StatusCode) + } + + return nil +} + +// PodReference returns the Pod reference for this status client. +func (c *StatusClient) PodReference() (k8stypes.NamespacedName, bool) { + if c.podRef != nil { + return *c.podRef, true + } + return k8stypes.NamespacedName{}, false +} + +// BaseRootURL returns the base URL for this status client. +func (c *StatusClient) BaseRootURL() string { + return c.baseURL +} + +// StatusClientFactory creates status clients for discovered status APIs. +type StatusClientFactory struct { + logger logr.Logger + opts managercfg.AdminAPIClientConfig +} + +// NewStatusClientFactory creates a new status client factory. +func NewStatusClientFactory(logger logr.Logger, opts managercfg.AdminAPIClientConfig) *StatusClientFactory { + return &StatusClientFactory{ + logger: logger, + opts: opts, + } +} + +// CreateStatusClient creates a status client for the given discovered status API. +func (f *StatusClientFactory) CreateStatusClient(ctx context.Context, discoveredStatusAPI DiscoveredAdminAPI) (*StatusClient, error) { + f.logger.V(1).Info( + "Creating Kong Gateway Status API client", + "address", discoveredStatusAPI.Address, + ) + + client, err := NewStatusClient(discoveredStatusAPI, f.opts) + if err != nil { + return nil, err + } + + // Test the status client by calling IsReady + if err := client.IsReady(ctx); err != nil { + return nil, fmt.Errorf("status client not ready: %w", err) + } + + return client, nil +} diff --git a/internal/adminapi/status_client_test.go b/internal/adminapi/status_client_test.go new file mode 100644 index 0000000000..6cb41f0e10 --- /dev/null +++ b/internal/adminapi/status_client_test.go @@ -0,0 +1,178 @@ +package adminapi + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + k8stypes "k8s.io/apimachinery/pkg/types" + + managercfg "github.com/kong/kubernetes-ingress-controller/v3/pkg/manager/config" +) + +// testLogger returns a logger for testing. +func testLogger(_ *testing.T) logr.Logger { + return logr.Discard() +} + +func TestStatusClient_IsReady(t *testing.T) { + tests := []struct { + name string + statusCode int + expectedError bool + errorContains string + }{ + { + name: "status endpoint returns 200", + statusCode: http.StatusOK, + expectedError: false, + }, + { + name: "status endpoint returns 500", + statusCode: http.StatusInternalServerError, + expectedError: true, + errorContains: "status endpoint returned 500", + }, + { + name: "status endpoint returns 404", + statusCode: http.StatusNotFound, + expectedError: true, + errorContains: "status endpoint returned 404", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a test server that responds with the specified status code + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/status", r.URL.Path) + assert.Equal(t, http.MethodGet, r.Method) + w.WriteHeader(tt.statusCode) + })) + defer server.Close() + + // Create a status client + discoveredAPI := DiscoveredAdminAPI{ + Address: server.URL, + PodRef: k8stypes.NamespacedName{ + Name: "test-pod", + Namespace: "test-namespace", + }, + } + + client, err := NewStatusClient(discoveredAPI, managercfg.AdminAPIClientConfig{}) + require.NoError(t, err) + + // Test IsReady + ctx := t.Context() + err = client.IsReady(ctx) + + if tt.expectedError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestStatusClient_PodReference(t *testing.T) { + podRef := k8stypes.NamespacedName{ + Name: "test-pod", + Namespace: "test-namespace", + } + + discoveredAPI := DiscoveredAdminAPI{ + Address: "https://example.com:8100", + PodRef: podRef, + } + + client, err := NewStatusClient(discoveredAPI, managercfg.AdminAPIClientConfig{}) + require.NoError(t, err) + + gotPodRef, ok := client.PodReference() + assert.True(t, ok) + assert.Equal(t, podRef, gotPodRef) +} + +func TestStatusClient_BaseRootURL(t *testing.T) { + expectedURL := "https://example.com:8100" + discoveredAPI := DiscoveredAdminAPI{ + Address: expectedURL, + PodRef: k8stypes.NamespacedName{ + Name: "test-pod", + Namespace: "test-namespace", + }, + } + + client, err := NewStatusClient(discoveredAPI, managercfg.AdminAPIClientConfig{}) + require.NoError(t, err) + + assert.Equal(t, expectedURL, client.BaseRootURL()) +} + +func TestStatusClientFactory_CreateStatusClient(t *testing.T) { + tests := []struct { + name string + statusCode int + expectedError bool + errorContains string + }{ + { + name: "successful status client creation", + statusCode: http.StatusOK, + expectedError: false, + }, + { + name: "status client creation fails when status check fails", + statusCode: http.StatusInternalServerError, + expectedError: true, + errorContains: "status client not ready", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a test server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.statusCode) + })) + defer server.Close() + + // Create factory + factory := NewStatusClientFactory( + testLogger(t), + managercfg.AdminAPIClientConfig{}, + ) + + discoveredAPI := DiscoveredAdminAPI{ + Address: server.URL, + PodRef: k8stypes.NamespacedName{ + Name: "test-pod", + Namespace: "test-namespace", + }, + } + + // Test CreateStatusClient + ctx := t.Context() + client, err := factory.CreateStatusClient(ctx, discoveredAPI) + + if tt.expectedError { + assert.Error(t, err) + assert.Nil(t, client) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + } else { + assert.NoError(t, err) + assert.NotNil(t, client) + } + }) + } +} diff --git a/internal/clients/status_readiness.go b/internal/clients/status_readiness.go new file mode 100644 index 0000000000..2dbcb3c108 --- /dev/null +++ b/internal/clients/status_readiness.go @@ -0,0 +1,213 @@ +package clients + +import ( + "context" + "sync" + "time" + + "github.com/go-logr/logr" + k8stypes "k8s.io/apimachinery/pkg/types" + + "github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi" + "github.com/kong/kubernetes-ingress-controller/v3/internal/logging" +) + +// StatusReadinessCheckResult represents the result of a status readiness check. +type StatusReadinessCheckResult struct { + // ClientsTurnedReady are the status clients that were pending and are now ready to be used. + ClientsTurnedReady []*adminapi.StatusClient + // ClientsTurnedPending are the status clients that were ready and are now pending to be created. + ClientsTurnedPending []adminapi.DiscoveredAdminAPI +} + +// HasChanges returns true if there are any changes in the status readiness check result. +func (r StatusReadinessCheckResult) HasChanges() bool { + return len(r.ClientsTurnedReady) > 0 || len(r.ClientsTurnedPending) > 0 +} + +// StatusReadinessChecker is responsible for checking the readiness of Status API clients. +type StatusReadinessChecker interface { + // CheckStatusReadiness checks readiness of the provided status clients. + CheckStatusReadiness( + ctx context.Context, + alreadyCreatedClients []AlreadyCreatedStatusClient, + pendingClients []adminapi.DiscoveredAdminAPI, + ) StatusReadinessCheckResult +} + +// AlreadyCreatedStatusClient represents a Status API client that has already been created. +type AlreadyCreatedStatusClient interface { + IsReady(context.Context) error + PodReference() (k8stypes.NamespacedName, bool) + BaseRootURL() string +} + +// StatusClientFactory interface for creating status clients. +type StatusClientFactory interface { + CreateStatusClient(ctx context.Context, discoveredStatusAPI adminapi.DiscoveredAdminAPI) (*adminapi.StatusClient, error) +} + +// DefaultStatusReadinessChecker implements StatusReadinessChecker. +type DefaultStatusReadinessChecker struct { + factory StatusClientFactory + readinessCheckTimeout time.Duration + logger logr.Logger +} + +// NewDefaultStatusReadinessChecker creates a new DefaultStatusReadinessChecker. +func NewDefaultStatusReadinessChecker(factory StatusClientFactory, timeout time.Duration, logger logr.Logger) DefaultStatusReadinessChecker { + return DefaultStatusReadinessChecker{ + factory: factory, + readinessCheckTimeout: timeout, + logger: logger, + } +} + +// CheckStatusReadiness checks readiness of status clients. +func (c DefaultStatusReadinessChecker) CheckStatusReadiness( + ctx context.Context, + readyClients []AlreadyCreatedStatusClient, + pendingClients []adminapi.DiscoveredAdminAPI, +) StatusReadinessCheckResult { + var ( + turnedReadyCh = make(chan []*adminapi.StatusClient) + turnedPendingCh = make(chan []adminapi.DiscoveredAdminAPI) + ) + + go func(ctx context.Context, pendingClients []adminapi.DiscoveredAdminAPI) { + turnedReadyCh <- c.checkPendingStatusClients(ctx, pendingClients) + close(turnedReadyCh) + }(ctx, pendingClients) + + go func(ctx context.Context, readyClients []AlreadyCreatedStatusClient) { + turnedPendingCh <- c.checkAlreadyExistingStatusClients(ctx, readyClients) + close(turnedPendingCh) + }(ctx, readyClients) + + return StatusReadinessCheckResult{ + ClientsTurnedReady: <-turnedReadyCh, + ClientsTurnedPending: <-turnedPendingCh, + } +} + +// checkPendingStatusClients checks if the pending status clients are ready to be used. +func (c DefaultStatusReadinessChecker) checkPendingStatusClients(ctx context.Context, lastPending []adminapi.DiscoveredAdminAPI) (turnedReady []*adminapi.StatusClient) { + var ( + wg sync.WaitGroup + ch = make(chan *adminapi.StatusClient) + ) + for _, statusAPI := range lastPending { + wg.Add(1) + go func(statusAPI adminapi.DiscoveredAdminAPI) { + defer wg.Done() + if client := c.checkPendingStatusClient(ctx, statusAPI); client != nil { + select { + case ch <- client: + case <-ctx.Done(): + } + } + }(statusAPI) + } + + go func() { + wg.Wait() + close(ch) + }() + + for client := range ch { + turnedReady = append(turnedReady, client) + } + + return turnedReady +} + +// checkPendingStatusClient checks readiness of a pending status client by trying to create it. +func (c DefaultStatusReadinessChecker) checkPendingStatusClient( + ctx context.Context, + pendingClient adminapi.DiscoveredAdminAPI, +) (client *adminapi.StatusClient) { + ctx, cancel := context.WithTimeout(ctx, c.readinessCheckTimeout) + defer cancel() + + logger := c.logger.WithValues("address", pendingClient.Address) + + client, err := c.factory.CreateStatusClient(ctx, pendingClient) + if err != nil { + logger.V(logging.DebugLevel).Info( + "Pending status client is not ready yet", + "reason", err.Error(), + ) + return nil + } + + logger.V(logging.DebugLevel).Info( + "Checked readiness of pending status client", + "ok", client != nil, + ) + + return client +} + +// checkAlreadyExistingStatusClients checks if already existing status clients are still ready. +func (c DefaultStatusReadinessChecker) checkAlreadyExistingStatusClients(ctx context.Context, alreadyCreatedClients []AlreadyCreatedStatusClient) (turnedPending []adminapi.DiscoveredAdminAPI) { + var ( + wg sync.WaitGroup + pendingChan = make(chan adminapi.DiscoveredAdminAPI) + ) + + for _, client := range alreadyCreatedClients { + wg.Add(1) + go func(client AlreadyCreatedStatusClient) { + defer wg.Done() + + if ready := c.checkAlreadyCreatedStatusClient(ctx, client); !ready { + podRef, ok := client.PodReference() + if !ok { + c.logger.Error( + nil, + "Failed to get PodReference for status client", + "address", client.BaseRootURL(), + ) + return + } + select { + case <-ctx.Done(): + case pendingChan <- adminapi.DiscoveredAdminAPI{ + Address: client.BaseRootURL(), + PodRef: podRef, + }: + } + } + }(client) + } + + go func() { + wg.Wait() + close(pendingChan) + }() + + for pendingClient := range pendingChan { + turnedPending = append(turnedPending, pendingClient) + } + + return turnedPending +} + +// checkAlreadyCreatedStatusClient checks if an already created status client is still ready. +func (c DefaultStatusReadinessChecker) checkAlreadyCreatedStatusClient(ctx context.Context, client AlreadyCreatedStatusClient) (ready bool) { + logger := c.logger.WithValues("address", client.BaseRootURL()) + + ctx, cancel := context.WithTimeout(ctx, c.readinessCheckTimeout) + defer cancel() + if err := client.IsReady(ctx); err != nil { + logger.V(logging.DebugLevel).Info( + "Already created status client is not ready, moving to pending", + "reason", err.Error(), + ) + return false + } + + logger.V(logging.DebugLevel).Info("Already created status client is ready") + + return true +} diff --git a/internal/clients/status_readiness_test.go b/internal/clients/status_readiness_test.go new file mode 100644 index 0000000000..dbd49acabf --- /dev/null +++ b/internal/clients/status_readiness_test.go @@ -0,0 +1,286 @@ +package clients + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + k8stypes "k8s.io/apimachinery/pkg/types" + + "github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi" +) + +// Mock implementations for testing + +type mockStatusClient struct { + mock.Mock +} + +func (m *mockStatusClient) IsReady(ctx context.Context) error { + args := m.Called(ctx) + return args.Error(0) +} + +func (m *mockStatusClient) PodReference() (k8stypes.NamespacedName, bool) { + args := m.Called() + return args.Get(0).(k8stypes.NamespacedName), args.Bool(1) +} + +func (m *mockStatusClient) BaseRootURL() string { + args := m.Called() + return args.String(0) +} + +type mockStatusClientFactory struct { + mock.Mock +} + +func (m *mockStatusClientFactory) CreateStatusClient(ctx context.Context, discoveredStatusAPI adminapi.DiscoveredAdminAPI) (*adminapi.StatusClient, error) { + args := m.Called(ctx, discoveredStatusAPI) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*adminapi.StatusClient), args.Error(1) +} + +func TestDefaultStatusReadinessChecker_CheckStatusReadiness(t *testing.T) { + tests := []struct { + name string + alreadyCreatedClients []AlreadyCreatedStatusClient + pendingClients []adminapi.DiscoveredAdminAPI + setupMocks func(*mockStatusClientFactory, []*mockStatusClient) + expectedTurnedReady int + expectedTurnedPending int + }{ + { + name: "no clients", + alreadyCreatedClients: []AlreadyCreatedStatusClient{}, + pendingClients: []adminapi.DiscoveredAdminAPI{}, + setupMocks: func(*mockStatusClientFactory, []*mockStatusClient) {}, + expectedTurnedReady: 0, + expectedTurnedPending: 0, + }, + { + name: "pending client becomes ready", + alreadyCreatedClients: []AlreadyCreatedStatusClient{}, + pendingClients: []adminapi.DiscoveredAdminAPI{ + { + Address: "https://10.0.0.1:8100", + PodRef: k8stypes.NamespacedName{ + Name: "pod-1", + Namespace: "default", + }, + }, + }, + setupMocks: func(factory *mockStatusClientFactory, _ []*mockStatusClient) { + factory.On("CreateStatusClient", mock.Anything, mock.Anything).Return(&adminapi.StatusClient{}, nil) + }, + expectedTurnedReady: 1, + expectedTurnedPending: 0, + }, + { + name: "pending client fails to become ready", + alreadyCreatedClients: []AlreadyCreatedStatusClient{}, + pendingClients: []adminapi.DiscoveredAdminAPI{ + { + Address: "https://10.0.0.1:8100", + PodRef: k8stypes.NamespacedName{ + Name: "pod-1", + Namespace: "default", + }, + }, + }, + setupMocks: func(factory *mockStatusClientFactory, _ []*mockStatusClient) { + factory.On("CreateStatusClient", mock.Anything, mock.Anything).Return(nil, errors.New("connection failed")) + }, + expectedTurnedReady: 0, + expectedTurnedPending: 0, + }, + { + name: "ready client becomes pending", + alreadyCreatedClients: []AlreadyCreatedStatusClient{ + func() AlreadyCreatedStatusClient { + client := &mockStatusClient{} + client.On("IsReady", mock.Anything).Return(errors.New("not ready")) + client.On("PodReference").Return(k8stypes.NamespacedName{Name: "pod-1", Namespace: "default"}, true) + client.On("BaseRootURL").Return("https://10.0.0.1:8100") + return client + }(), + }, + pendingClients: []adminapi.DiscoveredAdminAPI{}, + setupMocks: func(*mockStatusClientFactory, []*mockStatusClient) {}, + expectedTurnedReady: 0, + expectedTurnedPending: 1, + }, + { + name: "ready client stays ready", + alreadyCreatedClients: []AlreadyCreatedStatusClient{ + func() AlreadyCreatedStatusClient { + client := &mockStatusClient{} + client.On("IsReady", mock.Anything).Return(nil) + client.On("BaseRootURL").Return("https://10.0.0.1:8100") + return client + }(), + }, + pendingClients: []adminapi.DiscoveredAdminAPI{}, + setupMocks: func(*mockStatusClientFactory, []*mockStatusClient) {}, + expectedTurnedReady: 0, + expectedTurnedPending: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + factory := &mockStatusClientFactory{} + mockClients := make([]*mockStatusClient, len(tt.alreadyCreatedClients)) + for i := range mockClients { + mockClients[i] = &mockStatusClient{} + } + + tt.setupMocks(factory, mockClients) + + checker := NewDefaultStatusReadinessChecker( + factory, + time.Second*5, + logr.Discard(), + ) + + ctx := t.Context() + result := checker.CheckStatusReadiness(ctx, tt.alreadyCreatedClients, tt.pendingClients) + + assert.Len(t, result.ClientsTurnedReady, tt.expectedTurnedReady) + assert.Len(t, result.ClientsTurnedPending, tt.expectedTurnedPending) + + // Verify that HasChanges works correctly + expectedHasChanges := tt.expectedTurnedReady > 0 || tt.expectedTurnedPending > 0 + assert.Equal(t, expectedHasChanges, result.HasChanges()) + + // Verify mock expectations + factory.AssertExpectations(t) + for _, client := range tt.alreadyCreatedClients { + if mockClient, ok := client.(*mockStatusClient); ok { + mockClient.AssertExpectations(t) + } + } + }) + } +} + +func TestStatusReadinessCheckResult_HasChanges(t *testing.T) { + tests := []struct { + name string + clientsTurnedReady int + clientsTurnedPending int + expectedHasChanges bool + }{ + { + name: "no changes", + clientsTurnedReady: 0, + clientsTurnedPending: 0, + expectedHasChanges: false, + }, + { + name: "clients turned ready", + clientsTurnedReady: 1, + clientsTurnedPending: 0, + expectedHasChanges: true, + }, + { + name: "clients turned pending", + clientsTurnedReady: 0, + clientsTurnedPending: 1, + expectedHasChanges: true, + }, + { + name: "both changes", + clientsTurnedReady: 1, + clientsTurnedPending: 1, + expectedHasChanges: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := StatusReadinessCheckResult{ + ClientsTurnedReady: make([]*adminapi.StatusClient, tt.clientsTurnedReady), + ClientsTurnedPending: make([]adminapi.DiscoveredAdminAPI, tt.clientsTurnedPending), + } + + assert.Equal(t, tt.expectedHasChanges, result.HasChanges()) + }) + } +} + +func TestDefaultStatusReadinessChecker_checkPendingStatusClient(t *testing.T) { + factory := &mockStatusClientFactory{} + checker := NewDefaultStatusReadinessChecker( + factory, + time.Second*5, + logr.Discard(), + ) + + pendingClient := adminapi.DiscoveredAdminAPI{ + Address: "https://10.0.0.1:8100", + PodRef: k8stypes.NamespacedName{ + Name: "pod-1", + Namespace: "default", + }, + } + + t.Run("successful creation", func(t *testing.T) { + expectedClient := &adminapi.StatusClient{} + factory.On("CreateStatusClient", mock.Anything, pendingClient).Return(expectedClient, nil).Once() + + ctx := t.Context() + result := checker.checkPendingStatusClient(ctx, pendingClient) + + assert.Equal(t, expectedClient, result) + factory.AssertExpectations(t) + }) + + t.Run("failed creation", func(t *testing.T) { + factory.On("CreateStatusClient", mock.Anything, pendingClient).Return(nil, errors.New("creation failed")).Once() + + ctx := t.Context() + result := checker.checkPendingStatusClient(ctx, pendingClient) + + assert.Nil(t, result) + factory.AssertExpectations(t) + }) +} + +func TestDefaultStatusReadinessChecker_checkAlreadyCreatedStatusClient(t *testing.T) { + checker := NewDefaultStatusReadinessChecker( + &mockStatusClientFactory{}, + time.Second*5, + logr.Discard(), + ) + + t.Run("client is ready", func(t *testing.T) { + client := &mockStatusClient{} + client.On("IsReady", mock.Anything).Return(nil) + client.On("BaseRootURL").Return("https://10.0.0.1:8100") + + ctx := t.Context() + result := checker.checkAlreadyCreatedStatusClient(ctx, client) + + assert.True(t, result) + client.AssertExpectations(t) + }) + + t.Run("client is not ready", func(t *testing.T) { + client := &mockStatusClient{} + client.On("IsReady", mock.Anything).Return(errors.New("not ready")) + client.On("BaseRootURL").Return("https://10.0.0.1:8100") + + ctx := t.Context() + result := checker.checkAlreadyCreatedStatusClient(ctx, client) + + assert.False(t, result) + client.AssertExpectations(t) + }) +} diff --git a/internal/cmd/rootcmd/config/cli.go b/internal/cmd/rootcmd/config/cli.go index 220c4363d9..0c41b26bc3 100644 --- a/internal/cmd/rootcmd/config/cli.go +++ b/internal/cmd/rootcmd/config/cli.go @@ -89,6 +89,8 @@ func (c *CLIConfig) bindFlagSet() { `Kong Admin API Service namespaced name in "namespace/name" format, to use for Kong Gateway service discovery.`) flagSet.StringSliceVar(&c.KongAdminSvcPortNames, "kong-admin-svc-port-names", []string{"admin-tls", "kong-admin-tls"}, "Name(s) of ports on Kong Admin API service in comma-separated format (or specify this flag multiple times) to take into account when doing gateway discovery.") + flagSet.StringSliceVar(&c.KongStatusSvcPortNames, "kong-status-svc-port-names", []string{"status"}, + "Name(s) of ports on Kong Status API service in comma-separated format (or specify this flag multiple times) to use for status checks.") flagSet.DurationVar(&c.GatewayDiscoveryReadinessCheckInterval, "gateway-discovery-readiness-check-interval", managercfg.DefaultDataPlanesReadinessReconciliationInterval, "Interval of readiness checks on gateway admin API clients for discovery.") flagSet.DurationVar(&c.GatewayDiscoveryReadinessCheckTimeout, "gateway-discovery-readiness-check-timeout", managercfg.DefaultDataPlanesReadinessCheckTimeout, diff --git a/internal/manager/run.go b/internal/manager/run.go index f0c4ada5a9..8faa83b2a7 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -119,11 +119,16 @@ func New( return nil, fmt.Errorf("failed to create admin apis discoverer: %w", err) } + statusAPIsDiscoverer, err := adminapi.NewDiscoverer(sets.New(c.KongStatusSvcPortNames...)) + if err != nil { + return nil, fmt.Errorf("failed to create status apis discoverer: %w", err) + } + if err = c.Resolve(); err != nil { return nil, fmt.Errorf("failed to resolve configuration: %w", err) } - adminAPIClientsFactory := adminapi.NewClientFactoryForWorkspace(logger, c.KongWorkspace, c.KongAdminAPIConfig, c.KongAdminToken) + adminAPIClientsFactory := adminapi.NewClientFactoryForWorkspaceWithStatusDiscoverer(logger, c.KongWorkspace, c.KongAdminAPIConfig, c.KongAdminToken, statusAPIsDiscoverer) setupLog.Info("Getting the kong admin api client configuration") initialKongClients, err := adminAPIClients( @@ -191,6 +196,7 @@ func New( } readinessChecker := clients.NewDefaultReadinessChecker(adminAPIClientsFactory, c.GatewayDiscoveryReadinessCheckTimeout, setupLog.WithName("readiness-checker")) + clientsManager, err := clients.NewAdminAPIClientsManager( ctx, initialKongClients, diff --git a/main b/main new file mode 100755 index 0000000000..fe03430c41 Binary files /dev/null and b/main differ diff --git a/pkg/manager/config/config.go b/pkg/manager/config/config.go index 9bbcec95e8..28743f569f 100644 --- a/pkg/manager/config/config.go +++ b/pkg/manager/config/config.go @@ -60,6 +60,7 @@ type Config struct { GatewayDiscoveryReadinessCheckInterval time.Duration GatewayDiscoveryReadinessCheckTimeout time.Duration KongAdminSvcPortNames []string + KongStatusSvcPortNames []string ProxySyncSeconds float32 InitCacheSyncDuration time.Duration ProxyTimeoutSeconds float32 diff --git a/pkg/manager/config_test.go b/pkg/manager/config_test.go index 1feb9e89d0..a7f84d94a4 100644 --- a/pkg/manager/config_test.go +++ b/pkg/manager/config_test.go @@ -52,6 +52,7 @@ func TestNewConfig(t *testing.T) { GatewayDiscoveryReadinessCheckInterval: managercfg.DefaultDataPlanesReadinessReconciliationInterval, GatewayDiscoveryReadinessCheckTimeout: managercfg.DefaultDataPlanesReadinessCheckTimeout, KongAdminSvcPortNames: []string{"admin-tls", "kong-admin-tls"}, + KongStatusSvcPortNames: []string{"status"}, ProxySyncSeconds: dataplane.DefaultSyncSeconds, InitCacheSyncDuration: dataplane.DefaultCacheSyncWaitDuration, ProxyTimeoutSeconds: dataplane.DefaultTimeoutSeconds,