Sendconfig update improvements

CHANGELOG.md

@@ -61,6 +61,11 @@ released and the release notes may change significantly before then.
 
 #### Under the hood
 
+- the controller manager will no longer log multiple entries for `nil` updates
+  to the Kong Admin API. The result is that operators will no longer see multiple
+  "no configuration change, skipping sync to kong" entries for any single update,
+  instead it will only report this `nil` update scenario the first time it is
+  encountered for any particular SHA derived from the configuration contents.
 - the `--sync-rate-limit` is now deprecated in favor of `--sync-time-seconds`.
   This functionality no longer blocks goroutines until the provided number of
   seconds has passed to enforce rate limiting, now instead it configures a

pkg/sendconfig/common_workflows.go

@@ -11,6 +11,10 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/pkg/store"
 )
 
+// -----------------------------------------------------------------------------
+// Sendconfig - Workflow Functions
+// -----------------------------------------------------------------------------
+
 // UpdateKongAdminSimple is a helper function for the most common usage of PerformUpdate() with only minimal
 // upfront configuration required. This function is specialized and highly opinionated.
 //

pkg/sendconfig/sendconfig.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 	"reflect"
+	"sync"
 
 	"github.com/kong/deck/diff"
 	"github.com/kong/deck/dump"
@@ -33,7 +34,6 @@ func PerformUpdate(ctx context.Context,
 	customEntities []byte,
 	oldSHA []byte,
 ) ([]byte, error) {
-
 	newSHA, err := deckgen.GenerateSHA(targetContent, customEntities)
 	if err != nil {
 		return oldSHA, err
@@ -42,7 +42,9 @@ func PerformUpdate(ctx context.Context,
 	if !reverseSync {
 		// use the previous SHA to determine whether or not to perform an update
 		if equalSHA(oldSHA, newSHA) {
-			log.Info("no configuration change, skipping sync to kong")
+			if !hasSHAUpdateAlreadyBeenReported(newSHA) {
+				log.Info("no configuration change, skipping sync to kong")
+			}
 			return oldSHA, nil
 		}
 	}
@@ -186,3 +188,33 @@ func onUpdateDBMode(ctx context.Context,
 	}
 	return nil
 }
+
+// -----------------------------------------------------------------------------
+// Sendconfig - Logging & Reporting Helper Functions
+// -----------------------------------------------------------------------------
+
+var (
+	latestReportedSHA []byte
+	shaLock           sync.RWMutex
+)
+
+// hasSHAUpdateAlreadyBeenReported is a helper function to allow
+// sendconfig internals to be aware of the last logged/reported
+// update to the Kong Admin API. Given the most recent update SHA,
+// it will return true/false whether or not that SHA has previously
+// been reported (logged, e.t.c.) so that the caller can make
+// decisions (such as staggering or stifling duplicate log lines).
+//
+// TODO: This is a bit of a hack for now to keep backwards compat,
+//       but in the future we might configure rolling this into
+//       some object/interface which has this functionality as an
+//       inherent behavior.
+func hasSHAUpdateAlreadyBeenReported(latestUpdateSHA []byte) bool {
+	shaLock.Lock()
+	defer shaLock.Unlock()
+	if equalSHA(latestReportedSHA, latestUpdateSHA) {
+		return true
+	}
+	latestReportedSHA = latestUpdateSHA
+	return false
+}

pkg/sendconfig/sendconfig_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/kong/deck/file"
 	"github.com/kong/go-kong/kong"
 	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
 )
 
 func Test_renderConfigWithCustomEntities(t *testing.T) {
@@ -116,3 +117,14 @@ func Test_renderConfigWithCustomEntities(t *testing.T) {
 		})
 	}
 }
+
+func Test_updateReportingUtilities(t *testing.T) {
+	assert.False(t, hasSHAUpdateAlreadyBeenReported([]byte("fake-sha")))
+	assert.True(t, hasSHAUpdateAlreadyBeenReported([]byte("fake-sha")))
+	assert.False(t, hasSHAUpdateAlreadyBeenReported([]byte("another-fake-sha")))
+	assert.True(t, hasSHAUpdateAlreadyBeenReported([]byte("another-fake-sha")))
+	assert.False(t, hasSHAUpdateAlreadyBeenReported([]byte("yet-another-fake-sha")))
+	assert.True(t, hasSHAUpdateAlreadyBeenReported([]byte("yet-another-fake-sha")))
+	assert.True(t, hasSHAUpdateAlreadyBeenReported([]byte("yet-another-fake-sha")))
+	assert.True(t, hasSHAUpdateAlreadyBeenReported([]byte("yet-another-fake-sha")))
+}

railgun/internal/proxy/proxy.go

@@ -17,9 +17,13 @@ import (
 const (
 	// DefaultSyncSeconds indicates the time.Duration (minimum) that will occur between
 	// updates to the Kong Proxy Admin API when using the NewProxy() constructor.
+	// this 1s default was based on local testing wherein it appeared sub-second updates
+	// to the Admin API could be problematic (or at least operate differently) based on
+	// which storage backend was in use (i.e. "dbless", "postgres"). This is a workaround
+	// for improvements we still need to investigate upstream.
 	//
-	// NOTE: this default was originally inherited from KIC v1.x.
-	DefaultSyncSeconds float32 = 0.3
+	// See Also: https://github.com/Kong/kubernetes-ingress-controller/issues/1398
+	DefaultSyncSeconds float32 = 1.0
 
 	// DefaultObjectBufferSize is the number of client.Objects that the server will buffer
 	// before it starts rejecting new objects while it processes the originals.

railgun/test/integration/suite_test.go

@@ -40,7 +40,7 @@ const (
 	waitTick = time.Second * 1
 
 	// ingressWait is the default amount of time to wait for any particular ingress resource to be provisioned.
-	ingressWait = time.Minute * 5
+	ingressWait = time.Minute * 3
 
 	// httpcTimeout is the default client timeout for HTTP clients used in tests.
 	httpcTimeout = time.Second * 3