Merge pull request #51 from neiljerram/datastore-v-liveness

Don't let datastore connectivity block liveness reporting

pkg/daemon/daemon.go

@@ -201,17 +201,28 @@ configRetry:
 	t.BuildInfoLogCxt.WithField("config", configParams).Info(
 		"Successfully loaded configuration.")
 
-	// Make sure the datastore is initialized, otherwise the Syncer may spin, looking for
-	// non-existent resources.
-	for {
-		err := t.DatastoreClient.EnsureInitialized()
-		if err != nil {
-			log.WithError(err).Error("Failed to ensure datastore was initialized")
-			time.Sleep(1 * time.Second)
-			continue
+	// Ensure that, as soon as we are able to connect to the datastore at all, it is
+	// initialized; otherwise the Syncer may spin, looking for non-existent resources.
+	//
+	// But, do this in a background goroutine so as not to block Typha overall; specifically, we
+	// want Typha to report itself as live even if there is an initial problem connecting to the
+	// datastore.
+	//
+	// Note that Typha should cope with intermittent loss of connectivity to the datastore,
+	// because - apart from this EnsureInitialized call here - all of its interaction with the
+	// datastore is via a Syncer, and the Syncer is designed to handle HTTP request errors by
+	// reconnecting.
+	go func() {
+		for {
+			err := t.DatastoreClient.EnsureInitialized()
+			if err != nil {
+				log.WithError(err).Error("Failed to ensure datastore was initialized")
+				time.Sleep(1 * time.Second)
+				continue
+			}
+			break
 		}
-		break
-	}
+	}()
 	t.ConfigParams = configParams
 }
 

pkg/daemon/daemon_test.go

@@ -15,6 +15,11 @@
 package daemon_test
 
 import (
+	"errors"
+	"strconv"
+	"sync"
+	"time"
+
 	. "github.com/projectcalico/typha/pkg/daemon"
 
 	"context"
@@ -94,6 +99,8 @@ var _ = Describe("Daemon", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			d.ParseCommandLineArgs([]string{"-c", configFile.Name()})
+		})
+		JustBeforeEach(func() {
 			d.LoadConfiguration()
 			Expect(loggingConfigured).To(BeTrue())
 		})
@@ -102,8 +109,46 @@ var _ = Describe("Daemon", func() {
 			Expect(err).NotTo(HaveOccurred())
 		})
 
+		const (
+			downSecs  = 2
+			checkTime = "2s"
+		)
+		downSecsStr := strconv.Itoa(downSecs)
+
 		It("should load the configuration and connect to the datastore", func() {
-			Expect(backend.initCalled).To(BeTrue())
+			Eventually(backend.getNumInitCalls).Should(Equal(1))
+			Consistently(backend.getNumInitCalls, checkTime, "1s").Should(Equal(1))
+		})
+
+		Describe("with datastore down for "+downSecsStr+"s", func() {
+			BeforeEach(func() {
+				backend.mutex.Lock()
+				defer backend.mutex.Unlock()
+				backend.failInit = true
+			})
+			JustBeforeEach(func() {
+				time.Sleep(downSecs * time.Second)
+			})
+
+			It("should try >="+downSecsStr+" times to initialize the datastore", func() {
+				Eventually(backend.getNumInitCalls).Should(BeNumerically(">=", downSecs))
+			})
+
+			Describe("with datastore now available", func() {
+				var numFailedInitCalls int
+
+				JustBeforeEach(func() {
+					backend.mutex.Lock()
+					defer backend.mutex.Unlock()
+					backend.failInit = false
+					numFailedInitCalls = backend.initCalled
+				})
+
+				It("should initialize the datastore", func() {
+					Eventually(backend.getNumInitCalls).Should(Equal(numFailedInitCalls + 1))
+					Consistently(backend.getNumInitCalls, checkTime, "1s").Should(Equal(numFailedInitCalls + 1))
+				})
+			})
 		})
 
 		It("should create the server components", func() {
@@ -155,20 +200,35 @@ var _ = Describe("Daemon", func() {
 })
 
 type mockBackend struct {
+	mutex        sync.Mutex
 	syncerCalled bool
-	initCalled   bool
+	initCalled   int
+	failInit     bool
 }
 
 func (b *mockBackend) Syncer(callbacks bapi.SyncerCallbacks) bapi.Syncer {
+	b.mutex.Lock()
+	defer b.mutex.Unlock()
 	b.syncerCalled = true
 	return &dummySyncer{}
 }
 
 func (b *mockBackend) EnsureInitialized() error {
-	b.initCalled = true
+	b.mutex.Lock()
+	defer b.mutex.Unlock()
+	b.initCalled++
+	if b.failInit {
+		return errors.New("Failure simulated by test code")
+	}
 	return nil
 }
 
+func (b *mockBackend) getNumInitCalls() int {
+	b.mutex.Lock()
+	defer b.mutex.Unlock()
+	return b.initCalled
+}
+
 type dummySyncer struct {
 }