Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(konnect): account for config sync failures in reported node status #4029

Merged
merged 2 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
53 changes: 44 additions & 9 deletions internal/clients/config_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
77 changes: 74 additions & 3 deletions internal/clients/config_status_test.go
Original file line number Diff line number Diff line change
@@ -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++ {
Expand All @@ -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)
})
}
}
42 changes: 23 additions & 19 deletions internal/dataplane/kong_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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(
Expand Down