feat(konnect): account for konnect config sync failures in reported n…

…ode status It extends the config sync status reported to Konnect's Node API with values that take into account failures of syncing configuration with Konnect RG, effectively extending it with three new values (on the API surface level): - INGRESS_CONTROLLER_STATE_OPERATIONAL_KONNECT_OUT_OF_SYNC - INGRESS_CONTROLLER_STATE_PARTIAL_CONFIG_FAIL_KONNECT_OUT_OF_SYNC - INGRESS_CONTROLLER_STATE_INOPERABLE_KONNECT_OUT_OF_SYNC There's no strict contract for allowed values on the API level, however, there's an implicit contract between KIC and Konnect UI that directly consumes the Node object, therefore we're keeping the old values unchanged to make them backward-compatible.
Kong · May 18, 2023 · 667eff3 · 667eff3
1 parent 6ce0245
commit 667eff3
Show file tree

Hide file tree

Showing 8 changed files with 410 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -114,11 +114,15 @@ Adding a new version? You'll need three changes:
   during the start-up phase. From now on, they will be dynamically started in runtime
   once their installation is detected, making restarting the process unnecessary.
   [#3996](https://github.com/Kong/kubernetes-ingress-controller/pull/3996)
-- Disable translation of unspported kubernetes objects when translating to
-  expression based routes is enabled (`ExpressionRoutes` feature enabled AND 
+- Disable translation of unsupported Kubernetes objects when translating to
+  expression based routes is enabled (`ExpressionRoutes` feature enabled AND
   kong using router flavor `expressions`), and generate a translation failure
   event attached to each of the unsupported objects.
   [#4022](https://github.com/Kong/kubernetes-ingress-controller/pull/4022)
+- Controller's configuration synchronization status reported to Konnect's Node API
+  now accounts for potential failures in synchronizing configuration with Konnect's
+  Runtime Group Admin API.
+  [#4029](https://github.com/Kong/kubernetes-ingress-controller/pull/4029)
 
 ### Changed
 

diff --git a/internal/clients/config_status.go b/internal/clients/config_status.go
@@ -7,19 +7,54 @@ import (
 	"github.com/go-logr/logr"
 )
 
-type ConfigStatus int
+// ConfigStatus is an enumerated type that represents the status of the configuration synchronisation.
+// Look at CalculateConfigStatus for more details.
+type ConfigStatus string
 
 const (
-	// ConfigStatusOK: no error happens in translation from k8s objects to kong configuration
-	// and succeeded to apply kong configuration to kong gateway.
-	ConfigStatusOK ConfigStatus = iota
-	// ConfigStatusTranslationErrorHappened: error happened in translation of k8s objects
-	// but succeeded to apply kong configuration for remaining objects.
-	ConfigStatusTranslationErrorHappened
-	// ConfigStatusApplyFailed: failed to apply kong configurations.
-	ConfigStatusApplyFailed
+	ConfigStatusOK                                         ConfigStatus = "OK"
+	ConfigStatusTranslationErrorHappened                   ConfigStatus = "TranslationErrorHappened"
+	ConfigStatusApplyFailed                                ConfigStatus = "ApplyFailed"
+	ConfigStatusOKKonnectApplyFailed                       ConfigStatus = "OKKonnectApplyFailed"
+	ConfigStatusTranslationErrorHappenedKonnectApplyFailed ConfigStatus = "TranslationErrorHappenedKonnectApplyFailed"
+	ConfigStatusApplyFailedKonnectApplyFailed              ConfigStatus = "ApplyFailedKonnectApplyFailed"
+	ConfigStatusUnknown                                    ConfigStatus = "Unknown"
 )
 
+// CalculateConfigStatusInput aggregates the input to CalculateConfigStatus.
+type CalculateConfigStatusInput struct {
+	// Any error occurred when syncing with Gateways.
+	GatewaysFailed bool
+
+	// Any error occurred when syncing with Konnect,
+	KonnectFailed bool
+
+	// Translation of some of Kubernetes objects failed.
+	TranslationFailuresOccurred bool
+}
+
+// CalculateConfigStatus calculates a clients.ConfigStatus that sums up the configuration synchronisation result as
+// a single enumerated value.
+func CalculateConfigStatus(i CalculateConfigStatusInput) ConfigStatus {
+	switch {
+	case !i.GatewaysFailed && !i.KonnectFailed && !i.TranslationFailuresOccurred:
+		return ConfigStatusOK
+	case !i.GatewaysFailed && !i.KonnectFailed && i.TranslationFailuresOccurred:
+		return ConfigStatusTranslationErrorHappened
+	case i.GatewaysFailed && !i.KonnectFailed: // We don't care about translation failures if we can't apply to gateways.
+		return ConfigStatusApplyFailed
+	case !i.GatewaysFailed && i.KonnectFailed && !i.TranslationFailuresOccurred:
+		return ConfigStatusOKKonnectApplyFailed
+	case !i.GatewaysFailed && i.KonnectFailed && i.TranslationFailuresOccurred:
+		return ConfigStatusTranslationErrorHappenedKonnectApplyFailed
+	case i.GatewaysFailed && i.KonnectFailed: // We don't care about translation failures if we can't apply to gateways.
+		return ConfigStatusApplyFailedKonnectApplyFailed
+	}
+
+	// Shouldn't happen.
+	return ConfigStatusUnknown
+}
+
 type ConfigStatusNotifier interface {
 	NotifyConfigStatus(context.Context, ConfigStatus)
 }

diff --git a/internal/clients/config_status_test.go b/internal/clients/config_status_test.go
@@ -1,24 +1,27 @@
-package clients
+package clients_test
 
 import (
 	"context"
 	"testing"
 	"time"
 
 	"github.com/go-logr/logr/testr"
+	"github.com/stretchr/testify/require"
+
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/clients"
 )
 
 func TestChannelConfigNotifier(t *testing.T) {
 	logger := testr.New(t)
-	n := NewChannelConfigNotifier(logger)
+	n := clients.NewChannelConfigNotifier(logger)
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 
 	ch := n.SubscribeConfigStatus()
 
 	// Call NotifyConfigStatus 5 times to make sure that the method is non-blocking.
 	for i := 0; i < 5; i++ {
-		n.NotifyConfigStatus(ctx, ConfigStatusOK)
+		n.NotifyConfigStatus(ctx, clients.ConfigStatusOK)
 	}
 
 	for i := 0; i < 5; i++ {
@@ -29,3 +32,71 @@ func TestChannelConfigNotifier(t *testing.T) {
 		}
 	}
 }
+
+func TestCalculateConfigStatus(t *testing.T) {
+	testCases := []struct {
+		name string
+
+		gatewayFailure      bool
+		konnectFailure      bool
+		translationFailures bool
+
+		expectedConfigStatus clients.ConfigStatus
+	}{
+		{
+			name:                 "success",
+			expectedConfigStatus: clients.ConfigStatusOK,
+		},
+		{
+			name:                 "gateway failure",
+			gatewayFailure:       true,
+			expectedConfigStatus: clients.ConfigStatusApplyFailed,
+		},
+		{
+			name:                 "translation failures",
+			translationFailures:  true,
+			expectedConfigStatus: clients.ConfigStatusTranslationErrorHappened,
+		},
+		{
+			name:                 "konnect failure",
+			konnectFailure:       true,
+			expectedConfigStatus: clients.ConfigStatusOKKonnectApplyFailed,
+		},
+		{
+			name:                 "both gateway and konnect failure",
+			gatewayFailure:       true,
+			konnectFailure:       true,
+			expectedConfigStatus: clients.ConfigStatusApplyFailedKonnectApplyFailed,
+		},
+		{
+			name:                 "translation failures and konnect failure",
+			translationFailures:  true,
+			konnectFailure:       true,
+			expectedConfigStatus: clients.ConfigStatusTranslationErrorHappenedKonnectApplyFailed,
+		},
+		{
+			name:                 "gateway failure with translation failures",
+			gatewayFailure:       true,
+			translationFailures:  true,
+			expectedConfigStatus: clients.ConfigStatusApplyFailed,
+		},
+		{
+			name:                 "both gateway and konnect failure with translation failures",
+			gatewayFailure:       true,
+			konnectFailure:       true,
+			translationFailures:  true,
+			expectedConfigStatus: clients.ConfigStatusApplyFailedKonnectApplyFailed,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := clients.CalculateConfigStatus(clients.CalculateConfigStatusInput{
+				GatewaysFailed:              tc.gatewayFailure,
+				KonnectFailed:               tc.konnectFailure,
+				TranslationFailuresOccurred: tc.translationFailures,
+			})
+			require.Equal(t, tc.expectedConfigStatus, result)
+		})
+	}
+}
diff --git a/internal/dataplane/kong_client.go b/internal/dataplane/kong_client.go
@@ -368,21 +368,22 @@ func (c *KongClient) Update(ctx context.Context) error {
 		c.logger.Debug("successfully built data-plane configuration")
 	}
 
-	shas, err := c.sendOutToGatewayClients(ctx, parsingResult.KongState, c.kongConfig)
-	if err != nil {
-		c.configStatusNotifier.NotifyConfigStatus(ctx, clients.ConfigStatusApplyFailed)
-		return err
-	}
-
-	c.trySendOutToKonnectClient(ctx, parsingResult.KongState, c.kongConfig)
-
-	// succeeded to apply configuration to Kong gateway.
-	// notify the receiver of config status that translation error happened when there are translation errors,
-	// otherwise notify that config status is OK.
-	if len(parsingResult.TranslationFailures) > 0 {
-		c.configStatusNotifier.NotifyConfigStatus(ctx, clients.ConfigStatusTranslationErrorHappened)
-	} else {
-		c.configStatusNotifier.NotifyConfigStatus(ctx, clients.ConfigStatusOK)
+	shas, gatewaysSyncErr := c.sendOutToGatewayClients(ctx, parsingResult.KongState, c.kongConfig)
+	konnectSyncErr := c.maybeSendOutToKonnectClient(ctx, parsingResult.KongState, c.kongConfig)
+
+	// Taking into account the results of syncing configuration with Gateways and Konnect, and potential translation
+	// failures, calculate the config status and report it.
+	c.configStatusNotifier.NotifyConfigStatus(ctx, clients.CalculateConfigStatus(
+		clients.CalculateConfigStatusInput{
+			GatewaysFailed:              gatewaysSyncErr != nil,
+			KonnectFailed:               konnectSyncErr != nil,
+			TranslationFailuresOccurred: len(parsingResult.TranslationFailures) > 0,
+		},
+	))
+
+	// In case of a failure in syncing configuration with Gateways, propagate the error.
+	if gatewaysSyncErr != nil {
+		return gatewaysSyncErr
 	}
 
 	// report on configured Kubernetes objects if enabled
@@ -420,12 +421,13 @@ func (c *KongClient) sendOutToGatewayClients(
 	return previousSHAs, nil
 }
 
-// It will try to send ignore errors that are returned from Konnect client.
-func (c *KongClient) trySendOutToKonnectClient(ctx context.Context, s *kongstate.KongState, config sendconfig.Config) {
+// maybeSendOutToKonnectClient sends out the configuration to Konnect when KonnectClient is provided.
+// It's a noop when Konnect integration is not enabled.
+func (c *KongClient) maybeSendOutToKonnectClient(ctx context.Context, s *kongstate.KongState, config sendconfig.Config) error {
 	konnectClient := c.clientsProvider.KonnectClient()
 	// There's no KonnectClient configured, that's totally fine.
 	if konnectClient == nil {
-		return
+		return nil
 	}
 
 	if _, err := c.sendToClient(ctx, konnectClient, s, config); err != nil {
@@ -434,11 +436,13 @@ func (c *KongClient) trySendOutToKonnectClient(ctx context.Context, s *kongstate
 
 		if errors.Is(err, sendconfig.ErrUpdateSkippedDueToBackoffStrategy{}) {
 			c.logger.WithError(err).Warn("Skipped pushing configuration to Konnect")
-			return
 		}
 
 		c.logger.WithError(err).Warn("Failed pushing configuration to Konnect")
+		return err
 	}
+
+	return nil
 }
 
 func (c *KongClient) sendToClient(