Merge pull request #134 from kradalby/loop-97

app.go

@@ -172,16 +172,18 @@ func (h *Headscale) Serve() error {
 	r.GET("/apple/:platform", h.ApplePlatformConfig)
 	var err error
 
-	timeout := 30 * time.Second
-
 	go h.watchForKVUpdates(5000)
 	go h.expireEphemeralNodes(5000)
 
 	s := &http.Server{
-		Addr:         h.cfg.Addr,
+		Addr:        h.cfg.Addr,
-		Handler:      r,
+		Handler:     r,
-		ReadTimeout:  timeout,
+		ReadTimeout: 30 * time.Second,
-		WriteTimeout: timeout,
+		// Go does not handle timeouts in HTTP very well, and there is
+		// no good way to handle streaming timeouts, therefore we need to
+		// keep this at unlimited and be careful to clean up connections
+		// https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/#aboutstreaming
+		WriteTimeout: 0,
 	}
 
 	if h.cfg.TLSLetsEncryptHostname != "" {
@@ -194,13 +196,9 @@ func (h *Headscale) Serve() error {
 			HostPolicy: autocert.HostWhitelist(h.cfg.TLSLetsEncryptHostname),
 			Cache:      autocert.DirCache(h.cfg.TLSLetsEncryptCacheDir),
 		}
-		s := &http.Server{
+
-			Addr:         h.cfg.Addr,
+		s.TLSConfig = m.TLSConfig()
-			TLSConfig:    m.TLSConfig(),
+
-			Handler:      r,
-			ReadTimeout:  timeout,
-			WriteTimeout: timeout,
-		}
 		if h.cfg.TLSLetsEncryptChallengeType == "TLS-ALPN-01" {
 			// Configuration via autocert with TLS-ALPN-01 (https://tools.ietf.org/html/rfc8737)
 			// The RFC requires that the validation is done on port 443; in other words, headscale
@@ -211,7 +209,6 @@ func (h *Headscale) Serve() error {
 			// port 80 for the certificate validation in addition to the headscale
 			// service, which can be configured to run on any other port.
 			go func() {
-
 				log.Fatal().
 					Err(http.ListenAndServe(h.cfg.TLSLetsEncryptListen, m.HTTPHandler(http.HandlerFunc(h.redirect)))).
 					Msg("failed to set up a HTTP server")

integration_test.go

@@ -433,7 +433,7 @@ func (s *IntegrationTestSuite) TestPingAllPeers() {
 						command := []string{
 							"tailscale", "ping",
 							"--timeout=1s",
-							"--c=20",
+							"--c=10",
 							"--until-direct=true",
 							ip.String(),
 						}

machine.go

@@ -308,7 +308,8 @@ func (h *Headscale) notifyChangesToPeers(m *Machine) {
 				Str("func", "notifyChangesToPeers").
 				Str("machine", m.Name).
 				Str("peer", p.Name).
-				Msgf("Peer %s does not appear to be polling", p.Name)
+				Msgf("Peer %s does not have an open update client, skipping.", p.Name)
+			continue
 		}
 		log.Trace().
 			Str("func", "notifyChangesToPeers").
@@ -379,11 +380,12 @@ func (h *Headscale) sendRequestOnUpdateChannel(m *tailcfg.Node) error {
 				Msgf("Notified machine %s", m.Name)
 		}
 	} else {
+		err := errors.New("machine does not have an open update channel")
 		log.Info().
 			Str("func", "requestUpdate").
 			Str("machine", m.Name).
-			Msgf("Machine %s does not appear to be polling", m.Name)
+			Msgf("Machine %s does not have an open update channel", m.Name)
-		return errors.New("machine does not seem to be polling")
+		return err
 	}
 	return nil
 }

poll.go

@@ -230,6 +230,7 @@ func (h *Headscale) PollNetMapStream(
 					Str("channel", "pollData").
 					Err(err).
 					Msg("Cannot write data")
+				return false
 			}
 			log.Trace().
 				Str("handler", "PollNetMapStream").
@@ -237,7 +238,7 @@ func (h *Headscale) PollNetMapStream(
 				Str("channel", "pollData").
 				Int("bytes", len(data)).
 				Msg("Data from pollData channel written successfully")
-				// TODO: Abstract away all the database calls, this can cause race conditions
+				// TODO(kradalby): Abstract away all the database calls, this can cause race conditions
 				// when an outdated machine object is kept alive, e.g. db is update from
 				// command line, but then overwritten.
 			err = h.UpdateMachine(&m)
@@ -258,7 +259,7 @@ func (h *Headscale) PollNetMapStream(
 				Str("machine", m.Name).
 				Str("channel", "pollData").
 				Int("bytes", len(data)).
-				Msg("Machine updated successfully after sending pollData")
+				Msg("Machine entry in database updated successfully after sending pollData")
 			return true
 
 		case data := <-keepAliveChan:
@@ -276,6 +277,7 @@ func (h *Headscale) PollNetMapStream(
 					Str("channel", "keepAlive").
 					Err(err).
 					Msg("Cannot write keep alive message")
+				return false
 			}
 			log.Trace().
 				Str("handler", "PollNetMapStream").
@@ -283,7 +285,7 @@ func (h *Headscale) PollNetMapStream(
 				Str("channel", "keepAlive").
 				Int("bytes", len(data)).
 				Msg("Keep alive sent successfully")
-				// TODO: Abstract away all the database calls, this can cause race conditions
+				// TODO(kradalby): Abstract away all the database calls, this can cause race conditions
 				// when an outdated machine object is kept alive, e.g. db is update from
 				// command line, but then overwritten.
 			err = h.UpdateMachine(&m)
@@ -336,6 +338,7 @@ func (h *Headscale) PollNetMapStream(
 						Str("channel", "update").
 						Err(err).
 						Msg("Could not write the map response")
+					return false
 				}
 				log.Trace().
 					Str("handler", "PollNetMapStream").
@@ -347,7 +350,7 @@ func (h *Headscale) PollNetMapStream(
 					// we sometimes end in a state were the update
 					// is not picked up by a client and we use this
 					// to determine if we should "force" an update.
-					// TODO: Abstract away all the database calls, this can cause race conditions
+					// TODO(kradalby): Abstract away all the database calls, this can cause race conditions
 					// when an outdated machine object is kept alive, e.g. db is update from
 					// command line, but then overwritten.
 				err = h.UpdateMachine(&m)
@@ -393,13 +396,33 @@ func (h *Headscale) PollNetMapStream(
 			m.LastSeen = &now
 			h.db.Save(&m)
 
+			log.Trace().
+				Str("handler", "PollNetMapStream").
+				Str("machine", m.Name).
+				Str("channel", "Done").
+				Msg("Cancelling keepAlive channel")
 			cancelKeepAlive <- struct{}{}
 
+			log.Trace().
+				Str("handler", "PollNetMapStream").
+				Str("machine", m.Name).
+				Str("channel", "Done").
+				Msg("Closing update channel")
 			h.closeUpdateChannel(&m)
 
 			close(pollDataChan)
+			log.Trace().
+				Str("handler", "PollNetMapStream").
+				Str("machine", m.Name).
+				Str("channel", "Done").
+				Msg("Closing pollData channel")
 
 			close(keepAliveChan)
+			log.Trace().
+				Str("handler", "PollNetMapStream").
+				Str("machine", m.Name).
+				Str("channel", "Done").
+				Msg("Closing keepAliveChan channel")
 
 			return false
 		}