feat(mgr) implement config-based readiness (#1720)

* feat(mgr) implement config-based readiness

Only mark the controller ready after it can assume that the proxy has
configuration available, either immediately if the proxy has a database
or after the first config push if the proxy has no database.

* fix(doc) correct tags in changelog

Several tags were incorrect in the changelog footer links. Align tags
with their actual names and the appropriate comparison versions.

Co-authored-by: Jimin <[email protected]>
Co-authored-by: Michał Flendrich <[email protected]>

CHANGELOG.md

@@ -1,4 +1,6 @@
 # Table of Contents
+
+ - [2.0.0](#200---tba)
  - [2.0.0-beta.2](#200-beta2---202108016)
  - [2.0.0-beta.1](#200-beta1---20210806)
  - [2.0.0-alpha.3](#200-alpha3---20210802)
@@ -34,6 +36,15 @@
  - [0.0.5](#005---20180602)
  - [0.0.4 and prior](#004-and-prior)
 
+## [2.0.0] - TBA
+
+#### Added
+
+- In DB-less mode, the controller only marks itself ready once it has
+  successfully applied configuration at least once. This ensures that proxies
+  do not start handling traffic until they are configured.
+  [#1720](https://github.com/Kong/kubernetes-ingress-controller/issues/1720)
+
 ## [2.0.0-beta.2] - 2021/08/16
 
 #### Added
@@ -1284,11 +1295,12 @@ Please read the changelog and test in your environment.
  - The initial versions  were rapildy iterated to deliver
    a working ingress controller.
 
-[2.0.0-beta.2]: https://github.com/kong/kubernetes-ingress-controller/compare/2.0.0-beta.1...2.0.0-beta.2
-[2.0.0-beta.1]: https://github.com/kong/kubernetes-ingress-controller/compare/2.0.0-alpha.3...2.0.0-beta.1
-[2.0.0-alpha.3]: https://github.com/kong/kubernetes-ingress-controller/compare/2.0.0-alpha.2...2.0.0-alpha.3
+[2.0.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.0.0-beta.2...v2.0.0
+[2.0.0-beta.2]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.0.0-beta.1...v2.0.0-beta.2
+[2.0.0-beta.1]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.0.0-alpha.3...v2.0.0-beta.1
+[2.0.0-alpha.3]: https://github.com/kong/kubernetes-ingress-controller/compare/2.0.0-alpha.2...v2.0.0-alpha.3
 [2.0.0-alpha.2]: https://github.com/kong/kubernetes-ingress-controller/compare/2.0.0-alpha.1...2.0.0-alpha.2
-[2.0.0-alpha.1]: https://github.com/kong/kubernetes-ingress-controller/compare/1.2.0...2.0.0-alpha.1
+[2.0.0-alpha.1]: https://github.com/kong/kubernetes-ingress-controller/compare/1.3.0...2.0.0-alpha.1
 [1.3.2]: https://github.com/kong/kubernetes-ingress-controller/compare/1.3.1...1.3.2
 [1.3.1]: https://github.com/kong/kubernetes-ingress-controller/compare/1.3.0...1.3.1
 [1.3.0]: https://github.com/kong/kubernetes-ingress-controller/compare/1.2.0...1.3.0

internal/manager/run.go

@@ -3,7 +3,9 @@ package manager
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"net/http"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -84,7 +86,12 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic) e
 	if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil {
 		return fmt.Errorf("unable to setup healthz: %w", err)
 	}
-	if err := mgr.AddReadyzCheck("check", healthz.Ping); err != nil {
+	if err := mgr.AddReadyzCheck("check", func(_ *http.Request) error {
+		if !proxy.IsReady() {
+			return errors.New("proxy not yet configured")
+		}
+		return nil
+	}); err != nil {
 		return fmt.Errorf("unable to setup readyz: %w", err)
 	}
 

internal/proxy/clientgo_cached_proxy_resolver.go

@@ -3,6 +3,7 @@ package proxy
 import (
 	"context"
 	"fmt"
+	"sync"
 	"time"
 
 	"github.com/blang/semver/v4"
@@ -75,6 +76,8 @@ func NewCacheBasedProxyWithStagger(ctx context.Context,
 		stagger:             stagger,
 		proxyRequestTimeout: proxyRequestTimeout,
 		syncTicker:          time.NewTicker(stagger),
+
+		configApplied: false,
 	}
 
 	// initialize the proxy which validates connectivity with the Admin API and
@@ -107,6 +110,10 @@ type clientgoCachedProxyResolver struct {
 	// updated in the Kong Proxy and is used to avoid making unnecessary updates.
 	lastConfigSHA []byte
 
+	// configApplied is true if config has been applied at least once
+	configApplied      bool
+	configAppliedMutex sync.RWMutex
+
 	// kong configuration
 	kongConfig        sendconfig.Kong
 	enableReverseSync bool
@@ -155,13 +162,27 @@ func (p *clientgoCachedProxyResolver) ObjectExists(obj client.Object) (bool, err
 	return exists, err
 }
 
+func (p *clientgoCachedProxyResolver) IsReady() bool {
+	// If the proxy is has no database, it is only ready after a successful sync
+	// Otherwise, it has no configuration loaded
+	if p.dbmode == "off" {
+		p.configAppliedMutex.RLock()
+		defer p.configAppliedMutex.RUnlock()
+		return p.configApplied
+	}
+	// If the proxy has a database, it is ready immediately
+	// It will load existing configuration from the database
+	return true
+}
+
 // -----------------------------------------------------------------------------
 // Client Go Cached Proxy Resolver - Private Methods - Servers
 // -----------------------------------------------------------------------------
 
 // startProxyUpdateServer runs a server in a background goroutine that is responsible for
 // updating the kong proxy backend at regular intervals.
 func (p *clientgoCachedProxyResolver) startProxyUpdateServer() {
+	var initialConfig sync.Once
 	for {
 		select {
 		case <-p.ctx.Done():
@@ -179,6 +200,7 @@ func (p *clientgoCachedProxyResolver) startProxyUpdateServer() {
 				break
 			}
 			p.lastConfigSHA = updateConfigSHA
+			initialConfig.Do(p.markConfigApplied)
 		}
 	}
 }
@@ -243,6 +265,13 @@ func (p *clientgoCachedProxyResolver) kongRootWithTimeout() (map[string]interfac
 	return p.kongConfig.Client.Root(ctx)
 }
 
+// markConfigApplied marks that config has been applied
+func (p *clientgoCachedProxyResolver) markConfigApplied() {
+	p.configAppliedMutex.Lock()
+	defer p.configAppliedMutex.Unlock()
+	p.configApplied = true
+}
+
 // -----------------------------------------------------------------------------
 // Private Helper Functions
 // -----------------------------------------------------------------------------

internal/proxy/clientgo_cached_proxy_resolver_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/kong/kubernetes-testing-framework/pkg/utils/kong"
 	"github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators"
 
+	"github.com/kong/kubernetes-ingress-controller/internal/sendconfig"
 	"github.com/kong/kubernetes-ingress-controller/internal/store"
 	"github.com/kong/kubernetes-ingress-controller/internal/util"
 )
@@ -99,6 +100,29 @@ func Test_FetchCustomEntities(t *testing.T) {
 	}
 }
 
+func TestIsReady(t *testing.T) {
+	fakePostgresKong := sendconfig.Kong{InMemory: false}
+	fakeDblessKong := sendconfig.Kong{InMemory: true}
+	postgresProxy := clientgoCachedProxyResolver{
+		kongConfig: fakePostgresKong,
+		dbmode:     "postgres",
+	}
+	dblessProxy := clientgoCachedProxyResolver{
+		kongConfig: fakeDblessKong,
+		dbmode:     "off",
+	}
+
+	t.Log("checking initial readiness state")
+	assert.True(t, postgresProxy.IsReady())
+	assert.False(t, dblessProxy.IsReady())
+
+	t.Log("marking config applied and checking readiness after")
+	postgresProxy.markConfigApplied()
+	dblessProxy.markConfigApplied()
+	assert.True(t, postgresProxy.IsReady())
+	assert.True(t, dblessProxy.IsReady())
+}
+
 func TestCaching(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()

internal/proxy/proxy.go

@@ -59,6 +59,10 @@ type Proxy interface {
 
 	// ObjectExists indicates whether or not any version of the provided object is already present in the proxy.
 	ObjectExists(obj client.Object) (bool, error)
+
+	// IsReady returns true if the proxy is considered ready.
+	// A ready proxy has configuration available and can handle traffic.
+	IsReady() bool
 }
 
 // KongUpdater is a type of function that describes how to provide updates to the Kong Admin API

test/integration/controller_test.go

@@ -1,4 +1,5 @@
-//+build integration_tests
+//go:build integration_tests
+// +build integration_tests
 
 package integration
 
@@ -28,6 +29,20 @@ func TestHealthEndpoint(t *testing.T) {
 	}, ingressWait, waitTick)
 }
 
+func TestReadyEndpoint(t *testing.T) {
+	t.Parallel()
+	assert.Eventually(t, func() bool {
+		readyzURL := fmt.Sprintf("http://localhost:%v/readyz", manager.HealthzPort)
+		resp, err := httpc.Get(readyzURL)
+		if err != nil {
+			t.Logf("WARNING: error while waiting for %s: %v", readyzURL, err)
+			return false
+		}
+		defer resp.Body.Close()
+		return resp.StatusCode == http.StatusOK
+	}, ingressWait, waitTick)
+}
+
 func TestMetricsEndpoint(t *testing.T) {
 	t.Parallel()
 	assert.Eventually(t, func() bool {