fix(fallback): do not skip config update when gateways out of sync (#6271) (#6274)

(cherry picked from commit 3b547a9)

Author: czeslavo

CHANGELOG.md

@@ -7,6 +7,7 @@ Adding a new version? You'll need three changes:
 * Add the diff link, like "[2.7.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v1.2.2...v1.2.3".
   This is all the way at the bottom. It's the thing we always forget.
 --->
+ - [3.2.2](#322)
  - [3.2.1](#321)
  - [3.2.0](#320)
  - [3.1.6](#316)
@@ -90,9 +91,21 @@ Adding a new version? You'll need three changes:
  - [0.0.5](#005)
  - [0.0.4 and prior](#004-and-prior)
 
+## 3.2.2
+
+> Release date: 2024-07-01
+
+### Fixed
+
+- Fixed an issue where new gateways were not being populated with the current configuration when
+  `FallbackConfiguration` feature gate was turned on. Previously, configuration updates were skipped
+  if the Kubernetes config cache did not change, leading to inconsistencies. Now, the system ensures
+  that all gateways are populated with the latest configuration regardless of cache changes.
+  [#6271](https://github.com/Kong/kubernetes-ingress-controller/pull/6271)
+
 ## 3.2.1
 
-> Release date: 2024-06-28 
+> Release date: 2024-06-28
 
 ### Fixed
 
@@ -3559,6 +3572,7 @@ Please read the changelog and test in your environment.
  - The initial versions  were rapildy iterated to deliver
    a working ingress controller.
 
+[3.2.2]: https://github.com/kong/kubernetes-ingress-controller/compare/v3.2.1...v3.2.2
 [3.2.1]: https://github.com/kong/kubernetes-ingress-controller/compare/v3.2.0...v3.2.1
 [3.2.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v3.1.6...v3.2.0
 [3.1.6]: https://github.com/kong/kubernetes-ingress-controller/compare/v3.1.5...v3.1.6

internal/adminapi/client.go

@@ -10,6 +10,7 @@ import (
 	"github.com/samber/lo"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util/clock"
 )
@@ -25,6 +26,7 @@ type Client struct {
 	isKonnect           bool
 	konnectControlPlane string
 	lastConfigSHA       []byte
+	lastCacheStoresHash store.SnapshotHash
 
 	// podRef (optional) describes the Pod that the Client communicates with.
 	podRef *k8stypes.NamespacedName
@@ -154,6 +156,16 @@ func (c *Client) KonnectControlPlane() string {
 	return c.konnectControlPlane
 }
 
+// SetLastCacheStoresHash overrides last cache stores hash.
+func (c *Client) SetLastCacheStoresHash(s store.SnapshotHash) {
+	c.lastCacheStoresHash = s
+}
+
+// LastCacheStoresHash returns a checksum of the last successful cache stores push.
+func (c *Client) LastCacheStoresHash() store.SnapshotHash {
+	return c.lastCacheStoresHash
+}
+
 // SetLastConfigSHA overrides last config SHA.
 func (c *Client) SetLastConfigSHA(s []byte) {
 	c.lastConfigSHA = s

internal/dataplane/kong_client.go

@@ -196,15 +196,15 @@ func NewKongClient(
 	configChangeDetector sendconfig.ConfigurationChangeDetector,
 	kongConfigFetcher configfetcher.LastValidConfigFetcher,
 	kongConfigBuilder KongConfigBuilder,
-	cacheStores store.CacheStores,
+	cacheStores *store.CacheStores,
 	fallbackConfigGenerator FallbackConfigGenerator,
 ) (*KongClient, error) {
 	c := &KongClient{
 		logger:                  logger,
 		requestTimeout:          timeout,
 		diagnostic:              diagnostic,
 		prometheusMetrics:       metrics.NewCtrlFuncMetrics(),
-		cache:                   &cacheStores,
+		cache:                   cacheStores,
 		kongConfig:              kongConfig,
 		eventRecorder:           eventRecorder,
 		dbmode:                  dbMode,
@@ -444,23 +444,30 @@ func (c *KongClient) Update(ctx context.Context) error {
 	if c.kongConfig.FallbackConfiguration {
 		var newSnapshotHash store.SnapshotHash
 		var err error
+		// Empty snapshot hash means that the cache hasn't changed since the last snapshot was taken. That optimization can be used
+		// in main code path to avoid unnecessary processing. TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/6095
 		cacheSnapshot, newSnapshotHash, err = c.cache.TakeSnapshotIfChanged(c.lastProcessedSnapshotHash)
 		if err != nil {
 			return fmt.Errorf("failed to take snapshot of cache: %w", err)
 		}
-		// Empty snapshot hash means that the cache hasn't changed since the last snapshot was taken. That optimization can be used
-		// in main code path to avoid unnecessary processing. TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/6095
-		// TODO: We should short-circuit here only if all the Gateways were successfully synced with the current configuration.
-		// https://github.com/Kong/kubernetes-ingress-controller/issues/6219
-		if newSnapshotHash == store.SnapshotHashEmpty {
+		hasNewSnapshotToBeProcessed := newSnapshotHash != store.SnapshotHashEmpty
+		if !hasNewSnapshotToBeProcessed {
 			c.prometheusMetrics.RecordProcessedConfigSnapshotCacheHit()
-			c.logger.V(util.DebugLevel).Info("No configuration change; pushing config to gateway is not necessary, skipping")
-			return nil
+		} else {
+			c.prometheusMetrics.RecordProcessedConfigSnapshotCacheMiss()
+		}
+		if hasNewSnapshotToBeProcessed {
+			c.logger.V(util.DebugLevel).Info("New configuration snapshot detected", "hash", newSnapshotHash)
+			c.lastProcessedSnapshotHash = newSnapshotHash
+			c.kongConfigBuilder.UpdateCache(cacheSnapshot)
 		}
 
-		c.prometheusMetrics.RecordProcessedConfigSnapshotCacheMiss()
-		c.lastProcessedSnapshotHash = newSnapshotHash
-		c.kongConfigBuilder.UpdateCache(cacheSnapshot)
+		if allGatewaysAreInSync := lo.EveryBy(c.clientsProvider.GatewayClientsToConfigure(), func(cl *adminapi.Client) bool {
+			return cl.LastCacheStoresHash() == c.lastProcessedSnapshotHash
+		}); allGatewaysAreInSync {
+			c.logger.V(util.DebugLevel).Info("All gateways are in sync; pushing config is not necessary, skipping")
+			return nil
+		}
 	}
 
 	c.logger.V(util.DebugLevel).Info("Parsing kubernetes objects into data-plane configuration")
@@ -700,6 +707,12 @@ func (c *KongClient) sendOutToGatewayClients(
 		len(gatewayClients) > 1 {
 		for _, client := range gatewayClients {
 			client.SetLastConfigSHA([]byte(shas[0]))
+
+			// If the last processed snapshot hash is not empty, we should set it to the clients as well.
+			// It can be empty when FallbackConfiguration feature gate is off.
+			if c.lastProcessedSnapshotHash != "" {
+				client.SetLastCacheStoresHash(c.lastProcessedSnapshotHash)
+			}
 		}
 	}
 
@@ -840,7 +853,7 @@ func (c *KongClient) sendToClient(
 	sendDiagnostic(diagnostics.DumpMeta{Failed: false, Hash: string(newConfigSHA)}, nil) // No error occurred.
 	// update the lastConfigSHA with the new updated checksum
 	client.SetLastConfigSHA(newConfigSHA)
-
+	client.SetLastCacheStoresHash(c.lastProcessedSnapshotHash)
 	return string(newConfigSHA), nil
 }
 

internal/dataplane/kong_client_golden_test.go

@@ -307,7 +307,7 @@ func runKongClientGoldenTest(t *testing.T, tc kongClientGoldenTestCase) {
 		sendconfig.NewDefaultConfigurationChangeDetector(logger),
 		lastValidConfigFetcher,
 		p,
-		cacheStores,
+		&cacheStores,
 		fallbackConfigGenerator,
 	)
 	require.NoError(t, err)