From 07ea0220295ebea49067365cc23fb4f92ea9d4a1 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Tue, 27 Jun 2023 16:39:16 +0200 Subject: [PATCH] test(unit): last valid config unit test added Signed-off-by: Mattia Lavacca --- CHANGELOG.md | 5 - internal/dataplane/kong_client.go | 2 + internal/dataplane/kong_client_test.go | 142 ++++++++++++++++++++++--- internal/manager/run.go | 1 + 4 files changed, 133 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 570c5b6563..6464735e8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,11 +82,6 @@ Adding a new version? You'll need three changes: - Prometheus metrics now include counts of resources that the controller cannot send to the proxy instances and the last successful configuration push time. [#4181](https://github.com/Kong/kubernetes-ingress-controller/pull/4181) -- `--konnect-initial-license-polling-period` and `--konnect-license-polling-period` - CLI flags were added to allow configuring periods at which KIC polls license - from Konnect. The initial period will be used until a valid license is retrieved. - The default values are 1m and 12h respectively. - [#4178](https://github.com/Kong/kubernetes-ingress-controller/pull/4178) - Store the last known good configuration. If Kong rejects the latest configuration, send the last good configuration to Kong instances with no configuration. This allows newly-started Kong instances to serve traffic even diff --git a/internal/dataplane/kong_client.go b/internal/dataplane/kong_client.go index ebab9d8be8..07b9b8cf07 100644 --- a/internal/dataplane/kong_client.go +++ b/internal/dataplane/kong_client.go @@ -153,6 +153,7 @@ func NewKongClient( configChangeDetector sendconfig.ConfigurationChangeDetector, parser KongConfigBuilder, cacheStores store.CacheStores, + lastValidKongState *kongstate.KongState, ) (*KongClient, error) { c := &KongClient{ logger: logger, @@ -169,6 +170,7 @@ func NewKongClient( updateStrategyResolver: updateStrategyResolver, configChangeDetector: configChangeDetector, kongConfigBuilder: parser, + lastValidKongState: lastValidKongState, } return c, nil diff --git a/internal/dataplane/kong_client_test.go b/internal/dataplane/kong_client_test.go index 78da91f61f..d1a175d874 100644 --- a/internal/dataplane/kong_client_test.go +++ b/internal/dataplane/kong_client_test.go @@ -1,10 +1,11 @@ -package dataplane_test +package dataplane import ( "context" "errors" "fmt" "net/http" + "strings" "sync" "testing" "time" @@ -14,6 +15,7 @@ import ( gokong "github.com/kong/go-kong/kong" "github.com/samber/lo" "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" netv1 "k8s.io/api/networking/v1" @@ -24,7 +26,6 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi" "github.com/kong/kubernetes-ingress-controller/v2/internal/clients" - "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/failures" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser" @@ -111,7 +112,7 @@ func TestUniqueObjects(t *testing.T) { require.NoError(t, err) translationFailures = append(translationFailures, translationFailure) } - uniqueObjs := dataplane.UniqueObjects(tc.reportedObjs, translationFailures) + uniqueObjs := UniqueObjects(tc.reportedObjs, translationFailures) require.Len(t, uniqueObjs, len(tc.uniqueObjs)) require.ElementsMatch(t, tc.uniqueObjs, uniqueObjs) }) @@ -147,6 +148,7 @@ type mockUpdateStrategyResolver struct { shouldReturnErrorOnUpdate map[string]struct{} t *testing.T lock sync.RWMutex + singleError bool } func newMockUpdateStrategyResolver(t *testing.T) *mockUpdateStrategyResolver { @@ -161,7 +163,7 @@ func (f *mockUpdateStrategyResolver) ResolveUpdateStrategy(c sendconfig.UpdateCl defer f.lock.Unlock() url := c.AdminAPIClient().BaseRootURL() - return &mockUpdateStrategy{onUpdate: f.updateCalledForURLCallback(url)} + return &mockUpdateStrategy{onUpdate: f.updateCalledForURLCallback(url, f.singleError)} } // returnErrorOnUpdate will cause the mockUpdateStrategy with a given Admin API URL to return an error on Update(). @@ -178,13 +180,16 @@ func (f *mockUpdateStrategyResolver) returnErrorOnUpdate(url string, shouldRetur // updateCalledForURLCallback returns a function that will be called when the mockUpdateStrategy is called. // That enables us to track which URLs were called. -func (f *mockUpdateStrategyResolver) updateCalledForURLCallback(url string) func() error { +func (f *mockUpdateStrategyResolver) updateCalledForURLCallback(url string, singleError bool) func() error { return func() error { f.lock.Lock() defer f.lock.Unlock() f.updateCalledForURLs = append(f.updateCalledForURLs, url) if _, ok := f.shouldReturnErrorOnUpdate[url]; ok { + if singleError { + delete(f.shouldReturnErrorOnUpdate, url) + } return errors.New("error on update") } return nil @@ -316,7 +321,7 @@ func TestKongClientUpdate_AllExpectedClientsAreCalledAndErrorIsPropagated(t *tes configChangeDetector := mockConfigurationChangeDetector{hasConfigurationChanged: true} configBuilder := newMockKongConfigBuilder() - kongClient := setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder) + kongClient := setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder, nil) err := kongClient.Update(ctx) if tc.expectError { @@ -345,7 +350,7 @@ func TestKongClientUpdate_WhenNoChangeInConfigNoClientGetsCalled(t *testing.T) { configChangeDetector := mockConfigurationChangeDetector{hasConfigurationChanged: false} configBuilder := newMockKongConfigBuilder() - kongClient := setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder) + kongClient := setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder, nil) ctx := context.Background() err := kongClient.Update(ctx) @@ -372,15 +377,18 @@ func (m *mockConfigStatusQueue) WasNotified() bool { type mockKongConfigBuilder struct { translationFailuresToReturn []failures.ResourceFailure + kongState *kongstate.KongState } func newMockKongConfigBuilder() *mockKongConfigBuilder { - return &mockKongConfigBuilder{} + return &mockKongConfigBuilder{ + kongState: &kongstate.KongState{}, + } } func (p *mockKongConfigBuilder) BuildKongConfig() parser.KongConfigBuildingResult { return parser.KongConfigBuildingResult{ - KongState: &kongstate.KongState{}, + KongState: p.kongState, TranslationFailures: p.translationFailuresToReturn, } } @@ -420,7 +428,7 @@ func TestKongClientUpdate_ConfigStatusIsAlwaysNotified(t *testing.T) { updateStrategyResolver = newMockUpdateStrategyResolver(t) configChangeDetector = mockConfigurationChangeDetector{hasConfigurationChanged: true} configBuilder = newMockKongConfigBuilder() - kongClient = setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder) + kongClient = setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder, nil) ) testCases := []struct { @@ -483,6 +491,114 @@ func TestKongClientUpdate_ConfigStatusIsAlwaysNotified(t *testing.T) { } } +func TestKongClientUpdate_PushLastValidConfig(t *testing.T) { + var ( + ctx = context.Background() + testGatewayClient = mustSampleGatewayClient(t) + + clientsProvider = mockGatewayClientsProvider{ + gatewayClients: []*adminapi.Client{testGatewayClient}, + } + + updateStrategyResolver = newMockUpdateStrategyResolver(t) + configChangeDetector = mockConfigurationChangeDetector{hasConfigurationChanged: true} + lastKongState = &kongstate.KongState{ + Services: []kongstate.Service{ + { + Service: gokong.Service{ + Name: gokong.String("last_service"), + }, + Namespace: "last_namespace", + Routes: []kongstate.Route{ + { + Route: gokong.Route{ + Name: gokong.String("last_route"), + }, + }, + }, + }, + }, + } + newKongState = &kongstate.KongState{ + Services: []kongstate.Service{ + { + Service: gokong.Service{ + Name: gokong.String("new_service"), + }, + Namespace: "new_namespace", + Routes: []kongstate.Route{ + { + Route: gokong.Route{ + Name: gokong.String("new_route"), + }, + }, + }, + }, + }, + } + configBuilder = newMockKongConfigBuilder() + ) + configBuilder.kongState = newKongState + + testCases := []struct { + name string + gatewayFailure bool + translationFailures bool + singleError bool + lastValidKongState *kongstate.KongState + expectedLastKongState *kongstate.KongState + errorsSize int + }{ + { + name: "success, new fallback set", + lastValidKongState: lastKongState, + expectedLastKongState: newKongState, + }, + { + name: "failure, no fallback", + gatewayFailure: true, + singleError: true, + lastValidKongState: nil, + expectedLastKongState: nil, + errorsSize: 1, + }, + { + name: "failure, fallback pushed with success", + gatewayFailure: true, + singleError: true, + lastValidKongState: lastKongState, + expectedLastKongState: lastKongState, + errorsSize: 1, + }, + { + name: "failure, fallback pushed with failure", + gatewayFailure: true, + singleError: false, + lastValidKongState: lastKongState, + expectedLastKongState: lastKongState, + errorsSize: 2, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + updateStrategyResolver.returnErrorOnUpdate(testGatewayClient.BaseRootURL(), tc.gatewayFailure) + updateStrategyResolver.singleError = tc.singleError + kongClient := setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder, tc.lastValidKongState) + + err := kongClient.Update(ctx) + if tc.errorsSize > 0 { + // check if the error is joined with other errors. When there are multiple errors, + // they are separated by \n, hence we count the number of \n. + assert.Equal(t, strings.Count(err.Error(), "\n"), tc.errorsSize-1) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expectedLastKongState, kongClient.lastValidKongState) + }) + } +} + // setupTestKongClient creates a KongClient with mocked dependencies. func setupTestKongClient( t *testing.T, @@ -490,7 +606,8 @@ func setupTestKongClient( clientsProvider mockGatewayClientsProvider, configChangeDetector sendconfig.ConfigurationChangeDetector, configBuilder *mockKongConfigBuilder, -) *dataplane.KongClient { + lastKongState *kongstate.KongState, +) *KongClient { logger := logrus.New() timeout := time.Second ingressClass := "kong" @@ -498,7 +615,7 @@ func setupTestKongClient( config := sendconfig.Config{} dbMode := "off" - kongClient, err := dataplane.NewKongClient( + kongClient, err := NewKongClient( logger, timeout, ingressClass, @@ -511,6 +628,7 @@ func setupTestKongClient( configChangeDetector, configBuilder, store.NewCacheStores(), + lastKongState, ) require.NoError(t, err) return kongClient diff --git a/internal/manager/run.go b/internal/manager/run.go index 6df4627d47..e5793e5454 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -171,6 +171,7 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d configurationChangeDetector, configParser, cache, + nil, ) if err != nil { return fmt.Errorf("failed to initialize kong data-plane client: %w", err)