Skip to content

Commit

Permalink
feat(config) make timeout configurable for the kong client (#1401)
Browse files Browse the repository at this point in the history
* feat(config) make timeout configurable for the kong client

Signed-off-by: Tharun <[email protected]>

* feat(config) update changelog and add test for timeout

Signed-off-by: Tharun <[email protected]>

Co-authored-by: Shane Utt <[email protected]>
  • Loading branch information
tharun208 and shaneutt authored Jun 11, 2021
1 parent 54c8000 commit 93833d7
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ released and the release notes may change significantly before then.
`--watch-namespace "namespaceA,namespaceB"`).
- support for the `konghq.com/host-aliases` annotation.
[#1016](https:/Kong/kubernetes-ingress-controller/pull/1016/)
- Added `--proxy-timeout-seconds` flag to configure the kong client api timeout.
[#1401](https:/Kong/kubernetes-ingress-controller/pull/1401)

[kong-udp]:https://konghq.com/blog/kong-gateway-2-2-released/#UDP-Support

Expand Down
10 changes: 7 additions & 3 deletions railgun/internal/proxy/clientgo_cached_proxy_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ func NewCacheBasedProxy(ctx context.Context,
ingressClassName string,
enableReverseSync bool,
kongUpdater KongUpdater,
timeout time.Duration,
) (Proxy, error) {
stagger, err := time.ParseDuration(fmt.Sprintf("%gs", DefaultSyncSeconds))
if err != nil {
return nil, err
}
return NewCacheBasedProxyWithStagger(ctx, logger, k8s, kongConfig, ingressClassName, enableReverseSync, stagger, kongUpdater)
return NewCacheBasedProxyWithStagger(ctx, logger, k8s, kongConfig, ingressClassName, enableReverseSync, stagger, timeout, kongUpdater)
}

// NewCacheBasedProxy will provide a new Proxy object. Note that this starts some background goroutines and the caller
Expand All @@ -49,6 +50,7 @@ func NewCacheBasedProxyWithStagger(ctx context.Context,
ingressClassName string,
enableReverseSync bool,
stagger time.Duration,
timeout time.Duration,
kongUpdater KongUpdater,
) (Proxy, error) {
// configure the cachestores and the proxy instance
Expand All @@ -70,6 +72,7 @@ func NewCacheBasedProxyWithStagger(ctx context.Context,
update: make(chan *cachedObject, DefaultObjectBufferSize),
del: make(chan *cachedObject, DefaultObjectBufferSize),
stagger: stagger,
timeout: timeout,
syncTicker: time.NewTicker(stagger),
}

Expand Down Expand Up @@ -124,6 +127,7 @@ type clientgoCachedProxyResolver struct {
ingressClassName string
ctx context.Context
stagger time.Duration
timeout time.Duration
syncTicker *time.Ticker
stopCh chan struct{}

Expand Down Expand Up @@ -295,10 +299,10 @@ func (p *clientgoCachedProxyResolver) cacheDelete(cobj *cachedObject) error {
return cobj.err
}

// kongRootWithTimeout provides the root configuration from Kong, but uses a default timeout to avoid long waits if the Admin API
// kongRootWithTimeout provides the root configuration from Kong, but uses a configurable timeout to avoid long waits if the Admin API
// is not yet ready to respond. If a timeout error occurs, the caller is responsible for providing a retry mechanism.
func (p *clientgoCachedProxyResolver) kongRootWithTimeout() (map[string]interface{}, error) {
ctx, cancel := context.WithTimeout(p.ctx, 3*time.Second)
ctx, cancel := context.WithTimeout(p.ctx, p.timeout)
defer cancel()
return p.kongConfig.Client.Root(ctx)
}
Expand Down
25 changes: 24 additions & 1 deletion railgun/internal/proxy/clientgo_cached_proxy_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package proxy

import (
"context"
"net/http"
"testing"
"time"

"github.com/google/uuid"
"github.com/kong/kubernetes-ingress-controller/pkg/store"
"github.com/kong/kubernetes-testing-framework/pkg/generators/k8s"
"github.com/kong/kubernetes-testing-framework/pkg/kong"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -97,7 +99,7 @@ func TestCaching(t *testing.T) {
defer cancel()

t.Log("configuring and starting a new proxy server")
proxyInterface, err := NewCacheBasedProxy(ctx, logger, fakeK8sClient, fakeKongConfig, "kongtests", false, mockKongAdmin)
proxyInterface, err := NewCacheBasedProxy(ctx, logger, fakeK8sClient, fakeKongConfig, "kongtests", false, mockKongAdmin, time.Millisecond*300)
assert.NoError(t, err)

t.Log("ensuring the integrity of the proxy server")
Expand Down Expand Up @@ -134,3 +136,24 @@ func TestCaching(t *testing.T) {
proxy.syncTicker.Reset(time.Millisecond * 200)
assert.Eventually(t, func() bool { return fakeKongAdminUpdateCount() == previousUpdateCount+1 }, time.Second*10, time.Millisecond*200)
}

func TestProxyTimeout(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
t.Log("configuring and starting a new proxy server")

// mock the next Admin API response (which will be / to get the root config) to ensure
// that it takes longer than the timeout we will set, in order to trigger the timeout.
fakeKongAdminAPI.MockNextResponse(kong.AdminAPIResponse{
Status: http.StatusGatewayTimeout,
Body: []byte{},
Callback: func() { time.Sleep(time.Millisecond * 30) },
})

// the timeout is shorter than the wait time for the http response, we should expect
// to see the the context deadline for the http response triggered.
timeout := time.Millisecond * 10

_, err := NewCacheBasedProxy(ctx, logger, fakeK8sClient, fakeKongConfig, "kongtests", false, mockKongAdmin, timeout)
assert.Contains(t, err.Error(), "context deadline exceeded")
}
8 changes: 8 additions & 0 deletions railgun/manager/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ func Run(ctx context.Context, c *config.Config) error {
return err
}

// determine the proxy timeout
timeoutDuration, err := time.ParseDuration(fmt.Sprintf("%gs", c.ProxyTimeoutSeconds))
if err != nil {
setupLog.Error(err, "%s is not a valid number of seconds to the timeout config for the kong client")
return err
}

// start the proxy cache server
prx, err := proxy.NewCacheBasedProxyWithStagger(ctx,
// NOTE: logr-based loggers use the "logger" field instead of "subsystem". When replacing logrus with logr, replace
Expand All @@ -117,6 +124,7 @@ func Run(ctx context.Context, c *config.Config) error {
c.IngressClassName,
c.EnableReverseSync,
syncTickDuration,
timeoutDuration,
sendconfig.UpdateKongAdminSimple,
)
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions railgun/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type Config struct {
ProbeAddr string
KongAdminURL string
ProxySyncSeconds float32
ProxyTimeoutSeconds float32
KongCustomEntitiesSecret string

// Kubernetes configurations
Expand Down Expand Up @@ -120,6 +121,11 @@ func (c *Config) FlagSet() *pflag.FlagSet {
"Define the rate (in seconds) in which configuration updates will be applied to the Kong Admin API. (default: %g seconds)",
proxy.DefaultSyncSeconds,
))
flagSet.Float32Var(&c.ProxyTimeoutSeconds, "proxy-timeout-seconds", proxy.DefaultSyncSeconds,
fmt.Sprintf(
"Define the rate (in seconds) in which the timeout configuration will be applied to the Kong client. (default: %g seconds)",
proxy.DefaultSyncSeconds,
))
flagSet.StringVar(&c.KongCustomEntitiesSecret, "kong-custom-entities-secret", "", `A Secret containing custom entities for DB-less mode, in "namespace/name" format`)

// Kubernetes configurations
Expand Down

0 comments on commit 93833d7

Please sign in to comment.