From 9a0d435d5e70136d51e2ae0d621055fac981720d Mon Sep 17 00:00:00 2001
From: "Z.J. van de Weg" <git@zjvandeweg.nl>
Date: Fri, 24 Mar 2017 10:32:06 +0100
Subject: [PATCH] Block serving requests until first full update

Given that this might lead to 404 responses eventhough the request is
valid, we should wait until the first full update cycle is done.
---
 app.go          |  6 ++++--
 domains.go      | 55 +++++++++++++++++++++++++------------------------
 domains_test.go |  3 ++-
 helpers_test.go |  3 +--
 4 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/app.go b/app.go
index 351441d09..66621ee69 100644
--- a/app.go
+++ b/app.go
@@ -94,6 +94,9 @@ func (a *theApp) UpdateDomains(domains domains) {
 func (a *theApp) Run() {
 	var wg sync.WaitGroup
 
+	lastUpdate := []byte("first-update")
+	updateDomains(a.Domain, a.UpdateDomains, &lastUpdate)
+
 	// Listen for HTTP
 	for _, fd := range a.ListenHTTP {
 		wg.Add(1)
@@ -143,8 +146,7 @@ func (a *theApp) Run() {
 			}
 		}(a.ListenMetrics)
 	}
-
-	go watchDomains(a.Domain, a.UpdateDomains, time.Second)
+	go watchDomains(a.Domain, a.UpdateDomains, time.Second, &lastUpdate)
 
 	wg.Wait()
 }
diff --git a/domains.go b/domains.go
index d298b63b6..78f13ccfd 100644
--- a/domains.go
+++ b/domains.go
@@ -124,38 +124,39 @@ func (d domains) ReadGroups(rootDomain string) error {
 	return nil
 }
 
-func watchDomains(rootDomain string, updater domainsUpdater, interval time.Duration) {
-	lastUpdate := []byte("no-update")
-
+// We should block other goroutines until the first update has been completed
+func watchDomains(rootDomain string, updater domainsUpdater, interval time.Duration, lastUpdate *[]byte) {
 	for {
-		// Read the update file
-		update, err := ioutil.ReadFile(".update")
-		if err != nil && !os.IsNotExist(err) {
-			log.Println("Failed to read update timestamp:", err)
-		}
+		time.Sleep(interval)
+		updateDomains(rootDomain, updater, lastUpdate)
+	}
+}
 
-		// If it's the same ignore
-		if bytes.Equal(lastUpdate, update) {
-			time.Sleep(interval)
-			continue
-		}
-		lastUpdate = update
+func updateDomains(rootDomain string, updater domainsUpdater, lastUpdate *[]byte) {
+	// Read the update file
+	update, err := ioutil.ReadFile(".update")
+	if err != nil && !os.IsNotExist(err) {
+		log.Println("Failed to read update timestamp:", err)
+	}
 
-		started := time.Now()
-		domains := make(domains)
-		domains.ReadGroups(rootDomain)
-		duration := time.Since(started)
-		log.Println("Updated", len(domains), "domains in", duration, "Hash:", update)
+	if bytes.Equal(*lastUpdate, update) {
+		return
+	}
 
-		if updater != nil {
-			updater(domains)
-		}
+	*lastUpdate = update
 
-		// Update prometheus metrics
-		metrics.DomainLastUpdateTime.Set(float64(time.Now().UTC().Unix()))
-		metrics.DomainsServed.Set(float64(len(domains)))
-		metrics.DomainUpdates.Inc()
+	started := time.Now()
+	domains := make(domains)
+	domains.ReadGroups(rootDomain)
+	duration := time.Since(started)
+	log.Println("Updated", len(domains), "domains in", duration, "Hash:", update)
 
-		time.Sleep(interval)
+	if updater != nil {
+		updater(domains)
 	}
+
+	// Update prometheus metrics
+	metrics.DomainLastUpdateTime.Set(float64(time.Now().UTC().Unix()))
+	metrics.DomainsServed.Set(float64(len(domains)))
+	metrics.DomainUpdates.Inc()
 }
diff --git a/domains_test.go b/domains_test.go
index 5a2882d69..8feb1f73b 100644
--- a/domains_test.go
+++ b/domains_test.go
@@ -52,9 +52,10 @@ func TestWatchDomains(t *testing.T) {
 	setUpTests()
 
 	update := make(chan domains)
+	lastUpdate := []byte("no-update")
 	go watchDomains("gitlab.io", func(domains domains) {
 		update <- domains
-	}, time.Microsecond*50)
+	}, time.Microsecond*50, &lastUpdate)
 
 	defer os.Remove(updateFile)
 
diff --git a/helpers_test.go b/helpers_test.go
index 60d043045..559f6fe33 100644
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -47,6 +47,7 @@ var InsecureHTTPSClient = &http.Client{
 	Transport: &http.Transport{
 		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 	},
+	Timeout: 10 * time.Second,
 }
 
 var CertificateFixture = `-----BEGIN CERTIFICATE-----
@@ -153,11 +154,9 @@ func RunPagesProcess(t *testing.T, pagesPath string, listeners []ListenSpec, pro
 	// for now. Without it, intermittent failures occur.
 	//
 	// TODO: replace this with explicit status from the pages binary
-	// TODO: fix the first-request race
 	for _, spec := range listeners {
 		spec.WaitUntilListening()
 	}
-	time.Sleep(500 * time.Millisecond)
 
 	return func() {
 		cmd.Process.Signal(os.Interrupt)
-- 
GitLab