feat: store and push last valid config

CHANGELOG.md

@@ -82,6 +82,12 @@ 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)
+- 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
+  if a configuration error prevents the controller from sending the latest
+  configuration.
+  [#4205](https://github.com/Kong/kubernetes-ingress-controller/pull/4205)
 
 ### Changed
 

internal/dataplane/kong_client.go

@@ -117,6 +117,11 @@ type KongClient struct {
 	// SHAs is a slice is configuration hashes send in last batch send.
 	SHAs []string
 
+	// lastValidKongState contains the last valid configuration pushed
+	// to the gateways. It is used as a fallback in case a newer config version is
+	// somehow broken.
+	lastValidKongState *kongstate.KongState
+
 	// clientsProvider allows retrieving the most recent set of clients.
 	clientsProvider clients.AdminAPIClientsProvider
 
@@ -148,6 +153,7 @@ func NewKongClient(
 	configChangeDetector sendconfig.ConfigurationChangeDetector,
 	parser KongConfigBuilder,
 	cacheStores store.CacheStores,
+	lastValidKongState *kongstate.KongState,
 ) (*KongClient, error) {
 	c := &KongClient{
 		logger:                 logger,
@@ -164,6 +170,7 @@ func NewKongClient(
 		updateStrategyResolver: updateStrategyResolver,
 		configChangeDetector:   configChangeDetector,
 		kongConfigBuilder:      parser,
+		lastValidKongState:     lastValidKongState,
 	}
 
 	return c, nil
@@ -388,6 +395,13 @@ func (c *KongClient) Update(ctx context.Context) error {
 
 	// In case of a failure in syncing configuration with Gateways, propagate the error.
 	if gatewaysSyncErr != nil {
+		if c.lastValidKongState != nil {
+			_, fallbackSyncErr := c.sendOutToGatewayClients(ctx, c.lastValidKongState, c.kongConfig)
+			if fallbackSyncErr != nil {
+				return errors.Join(gatewaysSyncErr, fallbackSyncErr)
+			}
+			c.logger.Debug("due to errors in the current config, the last valid config has been pushed to Gateways")
+		}
 		return gatewaysSyncErr
 	}
 
@@ -423,6 +437,8 @@ func (c *KongClient) sendOutToGatewayClients(
 	sort.Strings(shas)
 	c.SHAs = shas
 
+	c.lastValidKongState = s
+
 	return previousSHAs, nil
 }
 

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,22 +491,131 @@ 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,
 	updateStrategyResolver *mockUpdateStrategyResolver,
 	clientsProvider mockGatewayClientsProvider,
 	configChangeDetector sendconfig.ConfigurationChangeDetector,
 	configBuilder *mockKongConfigBuilder,
-) *dataplane.KongClient {
+	lastKongState *kongstate.KongState,
+) *KongClient {
 	logger := logrus.New()
 	timeout := time.Second
 	ingressClass := "kong"
 	diagnostic := util.ConfigDumpDiagnostic{}
 	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

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)

test/e2e/all_in_one_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/stretchr/testify/require"
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	corev1 "k8s.io/api/core/v1"
+	netv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 
@@ -305,7 +306,7 @@ func TestDeployAllInOneDBLESS(t *testing.T) {
 	deployments := getManifestDeployments(manifestFilePath)
 
 	t.Log("running ingress tests to verify all-in-one deployed ingress controller and proxy are functional")
-	deployIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends)
+	ingress := deployIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends)
 	verifyIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends)
 	ensureAllProxyReplicasAreConfigured(ctx, t, env, deployments.ProxyNN)
 
@@ -325,6 +326,19 @@ func TestDeployAllInOneDBLESS(t *testing.T) {
 	t.Log("scale proxy to 3 replicas and wait for all instances to be ready")
 	scaleDeployment(ctx, t, env, deployments.ProxyNN, 3)
 	ensureAllProxyReplicasAreConfigured(ctx, t, env, deployments.ProxyNN)
+
+	t.Log("scale proxy to 1 replica")
+	scaleDeployment(ctx, t, env, deployments.ProxyNN, 1)
+
+	t.Log("misconfigure the ingress")
+	reconfigureExistingIngress(ctx, t, env, ingress, func(i *netv1.Ingress) {
+		ingress.Spec.Rules[0].HTTP.Paths[0].Path = badEchoPath
+	})
+
+	t.Log("scale proxy to 3 replicas and verify that the new replicas get the old good configuration")
+	scaleDeployment(ctx, t, env, deployments.ProxyNN, 3)
+	// verify all the proxy replicas have the last good configuration
+	ensureAllProxyReplicasAreConfigured(ctx, t, env, deployments.ProxyNN)
 }
 
 func ensureAllProxyReplicasAreConfigured(ctx context.Context, t *testing.T, env environments.Environment, proxyDeploymentNN k8stypes.NamespacedName) {